On Fri, 13 Sep 2013, Bjorn Helgaas wrote:
> On Fri, Sep 13, 2013 at 12:25 PM, Alan Stern <st...@rowland.harvard.edu>
> wrote:
> > On Fri, 13 Sep 2013, Bjorn Helgaas wrote:
> >
> >> usb_bus_list_lock protects the usb_bus_list, and we don't touch that list
> >> in usb_disconnect(), so there's no reason to hold the lock here.
> >
> > The code says:
> >
> > /* used when updating list of hcds */
> > DEFINE_MUTEX(usb_bus_list_lock); /* exported only for usbfs */
> > EXPORT_SYMBOL_GPL (usb_bus_list_lock);
> >
> > Unforunately, the first comment is incomplete. usb_bus_list_lock
> > protects both the list of buses and also the set of root hubs. That's
> > why it isn't needed in hub_quiesce() and hub_port_connect_change();
> > those routines remove only non-root devices.
> >
> > Removing the usages in usb_add_hcd() and usb_remove_hcd() isn't safe.
> > You can see where this usage is important in
> > devices.c:usb_device_read().
>
> I don't know how the set of root hubs is managed, so I don't see the
> conflict between, e.g., usb_remove_hcd() and usb_device_read(). I'm
> not looking to become a USB expert, so I'll just take your word for it
> -- I don't want to waste more of your time on it.
Very briefly, usb_device_read() dereferences bus->root_hub:
usb_lock_device(bus->root_hub);
On the other hand, usb_remove_hcd() calls both
usb_disconnect(&rhdev);
and
usb_put_dev(hcd->self.root_hub);
(note that rhdev == hcd->self.root_hub == bus->root_hub), which
unregisters and deallocates the root_hub structure. Something has to
prevent usb_device_read() from accessing the structure after it has
been unregistered and freed; that's where usb_bus_list_lock comes in.
Alan Stern
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
