On Tue, Sep 28, 2010 at 12:57:32AM -0400, Matthew Wilcox wrote: > > If I unplug a device while the UAS driver is loaded, I get an oops > in usb_free_streams(). This is because usb_unbind_interface() calls > usb_disable_interface() which calls usb_disable_endpoint() which sets > ep_out and ep_in to NULL. Then the UAS driver calls usb_pipe_endpoint() > which returns a NULL pointer and passes an array of NULL pointers to > usb_free_streams(). > > I think the correct fix for this is to check for the NULL pointer > in usb_free_streams() rather than making the driver check for this > situation. My original patch for this checked for dev->state == > USB_STATE_NOTATTACHED, but the call to usb_disable_interface() is > conditional, so not all drivers would want this check. > > [This should probably go into Sarah's xhci tree, not directly into Greg's > usb tree] I finally got around to looking at this patch. Unfortunately, I don't think it's quite the right fix. The bandwidth allocation functions assume that streams are deallocated before the interface is freed. It will leak streams rings if it's called otherwise. So if usb_free_streams() isn't called before the USB core calls usb_disable_interface(), then we'll leak all the streams rings. I think I need to spend some quality time with the UAS device to see exactly what the right fix is. Sarah Sharp > Signed-off-by: Matthew Wilcox <willy@xxxxxxxxxxxxxxx> > > diff --git a/drivers/usb/core/hcd.c b/drivers/usb/core/hcd.c > index 49009b2..39706de 100644 > --- a/drivers/usb/core/hcd.c > +++ b/drivers/usb/core/hcd.c > @@ -1874,7 +1874,7 @@ void usb_free_streams(struct usb_interface *interface, > > /* Streams only apply to bulk endpoints. */ > for (i = 0; i < num_eps; i++) > - if (!usb_endpoint_xfer_bulk(&eps[i]->desc)) > + if (!eps[i] || !usb_endpoint_xfer_bulk(&eps[i]->desc)) > return; > > hcd->driver->free_streams(hcd, dev, eps, num_eps, mem_flags); -- 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
