* 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/
This archive was generated by hypermail 2b29 : Mon Jun 23 2003 - 22:00:27 EST