linux-kernel - Re: Can we remove pci_find

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] [day] [month] [year] [list]

Message-Id: <20100120151109.29FA240109@xenon.ts.pxnet.com>
Date:	Wed, 20 Jan 2010 16:11:08 +0100 (CET)
From:	Tilman Schmidt <tilman@...p.cc>
To:	Alan Cox <alan@...rguk.ukuu.org.uk>
Cc:	Andi Kleen <andi@...stfloor.org>, Greg KH <gregkh@...e.de>,
	Stephen Rothwell <sfr@...b.auug.org.au>,
	LKML <linux-kernel@...r.kernel.org>, netdev@...r.kernel.org,
	Karsten Keil <isdn@...ux-pingi.de>,
	isdn4linux@...tserv.isdn4linux.de
Subject: Re: Can we remove pci_find_device() yet?

On Thu, Jan 14, 2010 at 11:02:14AM +0000, Alan Cox wrote:
> > e.g. set some global variable that forbids device removal
> > and warn in the kernel log. In theory this could be also
> > done per device, but I guess that would be more effort.
> 
> There is a simpler way to do that, which is to just leak a reference in
> the hisax_find_pci_device hack. The pci_dev won't be going anywhere then.

Something like this? (On top of my previous patch introducing
hisax_find_pci_device in the first place.)

Subject: [PATCH RFC] isdn/hisax: intentionally leak a reference in hisax_find_pci_device

Take a reference to the returned pci_dev which will never be released.
Until the HiSax drivers are fixed for proper PCI hotplug support,
this appears as the lesser evil compared to the danger of the
structure being freed without the caller knowing it.

Impact: replacing a bug by another, less consequential one
Signed-off-by: Tilman Schmidt <tilman@...p.cc>
---
 drivers/isdn/hisax/hisax.h |   13 +++++--------
 1 files changed, 5 insertions(+), 8 deletions(-)

diff --git a/drivers/isdn/hisax/hisax.h b/drivers/isdn/hisax/hisax.h
index 832a878..e459502 100644
--- a/drivers/isdn/hisax/hisax.h
+++ b/drivers/isdn/hisax/hisax.h
@@ -1328,21 +1328,18 @@ void TeiFree(void);
 
 #include <linux/pci.h>
 
-/* adaptation wrapper for old usage
- * WARNING! This is unfit for use in a PCI hotplug environment,
- * as the returned PCI device can disappear at any moment in time.
+/* This function replaces the former function pci_find_device().
+ * Unlike the latter, it takes a reference on the returned pci_dev,
+ * resulting in a reference leak. This is intentional and serves to
+ * prevent an untimely release of the PCI device structure.
  * Callers should be converted to use pci_get_device() instead.
  */
 static inline struct pci_dev *hisax_find_pci_device(unsigned int vendor,
 						    unsigned int device,
 						    struct pci_dev *from)
 {
-	struct pci_dev *pdev;
-
 	pci_dev_get(from);
-	pdev = pci_get_subsys(vendor, device, PCI_ANY_ID, PCI_ANY_ID, from);
-	pci_dev_put(pdev);
-	return pdev;
+	return pci_get_subsys(vendor, device, PCI_ANY_ID, PCI_ANY_ID, from);
 }
 
 #endif
-- 
1.6.5.3.298.g39add

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/