Messages in this thread | | | Date | Sun, 26 Oct 2008 20:51:25 +0800 | From | Yu Zhao <> | Subject | Re: [PATCH] pci: Fixing drivers/pci/search.c compilation warning. |
| |
Matthew Wilcox wrote: > This patch seems to have been overlooked. It also seems to have had > some kind of midair collision with a patch from Greg that ignored the > real bug I found. > > Here's an updated version. I think it should also be applied to > -stable. > > ---- > > Subject: [PCI] Fix reference counting bug > > pci_get_subsys() will decrement the reference count of the device that > it starts searching from. Unfortunately, the pci_find_device() interface > will already have decremented the reference count of the device earlier, > so the device will end up losing all reference counts and be freed. > > We can fix this by incrementing the reference count of the device to > start searching from before calling pci_get_subsys(). > > Signed-off-by: Matthew Wilcox <willy@linux.intel.com> > > diff --git a/drivers/pci/search.c b/drivers/pci/search.c > index 4edfc47..5af8bd5 100644 > --- a/drivers/pci/search.c > +++ b/drivers/pci/search.c > @@ -166,6 +166,7 @@ struct pci_dev *pci_find_device(unsigned int vendor, unsigned int device, > { > 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; > @@ -270,12 +271,8 @@ static struct pci_dev *pci_get_dev_by_id(const struct pci_device_id *id, > struct pci_dev *pdev = NULL; > > WARN_ON(in_interrupt()); > - if (from) { > - /* FIXME > - * take the cast off, when bus_find_device is made const. > - */ > - dev_start = (struct device *)&from->dev; > - } > + if (from) > + dev_start = &from->dev; > dev = bus_find_device(&pci_bus_type, dev_start, (void *)id, > match_pci_dev_by_id); > if (dev)
This reminds me of other problems of PCI search functions.
The 'dev_start' is passed to bus_find_device(), and its 'knode_bus' reference count is decreased by klist_iter_init_node() in that function. The problem is the reference count may be already decrease to 0 because the PCI device 'from' is hot-plugged off (e.g., pci_remove_bus) when the search goes. A warning is fired when klist_iter_init_node() detects the reference count becomes 0.
Some code uses pci_find_device() in a way that is not safe with the hotplug, because a device may be destroyed after bus_find_device() returns it and before it's held by pci_dev_get() in the next round. Following is an example from a random grep:
for ( ;; ) { if ((dev_netjet = pci_find_device(PCI_VENDOR_ID_TIGERJET, PCI_DEVICE_ID_TIGERJET_300, dev_netjet))) { ret = njs_pci_probe(dev_netjet, cs); ... } ... }
And some others use pci_get_bus_and_slot(), which has similar problem as above.
Thanks, Yu
| |