netdev - Re: [PATCH 14/52] net: cxgb4vf: remove unnecessary pci_set

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-id: <000001ceaf56$1cdf7d60$569e7820$%han@samsung.com>
Date:	Thu, 12 Sep 2013 10:19:22 +0900
From:	Jingoo Han <jg1.han@...sung.com>
To:	'Casey Leedom' <leedom@...lsio.com>
Cc:	"'David S. Miller'" <davem@...emloft.net>, netdev@...r.kernel.org,
	'Jingoo Han' <jg1.han@...sung.com>
Subject: Re: [PATCH 14/52] net: cxgb4vf: remove unnecessary pci_set_drvdata()

On Thursday, September 12, 2013 2:24 AM, Casey Leedom wrote:
> 
>    I agree that the redundant pci_set_drvdata(pdev, NULL) in
> cxgb4vf_pci_probe() under the err_release_regions: label is unneeded,
> but don't we need to NULL out the PCI Driver Data under the
> err_free_adapter: label and also in cxgb4vf_pci_remove()?  Or is that
> handled automatically in the PCI infrastructure code which calls the
> Device Probe and Remove routines?  Mostly I was just being an
> obsessively clean housewife assuming that we'd want to clean up these
> references ...


No, 'pci_set_drvdata(pdev, NULL) under err_free_adapter label' is not
necessary.

As you know, pci_set_drvdata(pdev, NULL) calls dev_set_drvdata() as below:
pci_set_drvdata(pdev, NULL) is dev_set_drvdata(&pdev->dev, NULL).

./include/linux/pci.h
1504:static inline void pci_set_drvdata(struct pci_dev *pdev, void *data)
1505{
1506	dev_set_drvdata(&pdev->dev, data);
1507}


However, when the driver goes to err_free_adapter label,
The following sequence will be done.
    kfree(adapter) -> .... -> return -ENOMEM;

In this case,
when probe() returns error value such as '-ENOMEM',
really_probe() of driver core automatically calls 'dev_set_drvdata(dev, NULL)'
as below:

./drivers/base/dd.c
303-probe_failed:
304	devres_release_all(dev);
305	driver_sysfs_remove(dev);
306	dev->driver = NULL;
307	dev_set_drvdata(dev, NULL);

Thus, without 'pci_set_drvdata(pdev, NULL) under err_free_adapter label',
dev_set_drvdata(dev, NULL) can be called.

I already tested this with other drivers such as e1000e LAN card driver.

Best regards,
Jingoo Han


--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html