Re: [PATCH 2/2] USB: don't hold usb_bus_list_lock for usb_disconnect()

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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




[Index of Archives]     [Linux Media]     [Linux Input]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Old Linux USB Devel Archive]

  Powered by Linux