lkml.org 
[lkml]   [2003]   [Jun]   [18]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [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/

\
 
 \ /
  Last update: 2005-03-22 13:36    [from the cache]
©2003-2011 Jasper Spaans