On Fri, 13 Sep 2013, Bjorn Helgaas wrote: > On Fri, Sep 13, 2013 at 12:25 PM, Alan Stern <stern@xxxxxxxxxxxxxxxxxxx> 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 majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
