On Mon, 3 Sep 2018, Mathias Nyman wrote: > The steps taken by usb core to set a new interface is very different from > what is done on the xHC host side. > > xHC hardware will do everything in one go. One command is used to set up > new endpoints, free old endpoints, check bandwidth, and run the new > endpoints. > > All this is done by xHC when usb core asks the hcd to check for > available bandwidth. At this point usb core has not yet flushed the old > endpoints, which will cause use-after-free issues in xhci driver as > queued URBs are cancelled on a re-allocated endpoint. > > To resolve this add a call to usb_disable_interface() which will flush > the endpoints before calling usb_hcd_alloc_bandwidth() > > Additional checks in xhci driver will also be implemented to gracefully > handle stale URB cancel on freed and re-allocated endpoints > > Cc: <stable@xxxxxxxxxxxxxxx> > Reported-by: Sudip Mukherjee <sudipm.mukherjee@xxxxxxxxx> > Signed-off-by: Mathias Nyman <mathias.nyman@xxxxxxxxxxxxxxx> > --- > v2: update kerneldoc as well > --- > drivers/usb/core/message.c | 11 +++++++++++ > 1 file changed, 11 insertions(+) > > diff --git a/drivers/usb/core/message.c b/drivers/usb/core/message.c > index 228672f..bfa5eda 100644 > --- a/drivers/usb/core/message.c > +++ b/drivers/usb/core/message.c > @@ -1341,6 +1341,11 @@ void usb_enable_interface(struct usb_device *dev, > * is submitted that needs that bandwidth. Some other operating systems > * allocate bandwidth early, when a configuration is chosen. > * > + * xHCI reserves bandwidth and configures the alternate setting in > + * usb_hcd_alloc_bandwidth(). If it fails the original interface altsetting > + * may be disabled. Drivers cannot rely on any particular alternate > + * setting being in effect after a failure. > + * > * This call is synchronous, and may not be used in an interrupt context. > * Also, drivers must not change altsettings while urbs are scheduled for > * endpoints in that interface; all such urbs must first be completed > @@ -1376,6 +1381,12 @@ int usb_set_interface(struct usb_device *dev, int interface, int alternate) > alternate); > return -EINVAL; > } > + /* > + * usb3 hosts configure the interface in usb_hcd_alloc_bandwidth, > + * including freeing dropped endpoint ring buffers. > + * Make sure the interface endpoints are flushed before that > + */ > + usb_disable_interface(dev, iface, false); > > /* Make sure we have enough bandwidth for this alternate interface. > * Remove the current alt setting and add the new alt setting. Acked-by: Alan Stern <stern@xxxxxxxxxxxxxxxxxxx>
