Messages in this thread | ![/](/images/icornerl.gif) | | Date | Wed, 18 Jun 2003 15:33:24 -0700 | From | Chris Wright <> | Subject | Re: [RFC] PCI device list locking - take 2 |
| |
* Greg KH (greg@kroah.com) wrote: > static void *pci_seq_start(struct seq_file *m, loff_t *pos) > { > - struct list_head *p = &pci_devices; > + struct pci_dev *dev = NULL; > loff_t n = *pos; > > - /* XXX: surely we need some locking for traversing the list? */ > + dev = pci_get_device(PCI_ANY_ID, PCI_ANY_ID, dev); > while (n--) { > - p = p->next; > - if (p == &pci_devices) > - return NULL; > + dev = pci_get_device(PCI_ANY_ID, PCI_ANY_ID, dev); > + if (dev == NULL) > + goto exit;
I think this still has the same problem. pci_get_device grabs lock, walks list, gets ref, and drops lock. But the ref doesn't hold it on the list, right?. So some pci_remove_* could do list_del(&dev->global_list), poison the prev/next pointers. Subsequent pci_get_device would do ->next and oops. It seems the lock needs to be held for entire start/next/stop sequence, or the ref needs to keep it on list.
> +struct pci_dev * > +pci_get_subsys(unsigned int vendor, unsigned int device, <snip> > +exit: > + if (from) > + pci_put_dev(from); > + if (dev) > + pci_get_dev(dev);
Heh, the hch in me notes that pci_{put,get}_dev already check NULL device ;-)
thanks, -chris -- Linux Security Modules http://lsm.immunix.org http://lsm.bkbits.net - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
| ![\](/images/icornerr.gif) |