Re: Disconnect race in Gadget core

[Felipe Balbi <balbi@kernel.org>]

[-- Attachment #1: Type: text/plain, Size: 1565 bytes --]


Hi Alan,

Alan Stern <stern@rowland.harvard.edu> writes:
> I just noticed this potential race in the Gadget core, specifically, in 
> usb_gadget_remove_driver.  Here's the relevant code:
>
> 	usb_gadget_disconnect(udc->gadget);
> 	if (udc->gadget->irq)
> 		synchronize_irq(udc->gadget->irq);
> 	udc->driver->unbind(udc->gadget);
> 	usb_gadget_udc_stop(udc);
>
> usb_gadget_disconnect will turn off the D+ pull-up, telling the host (or 
> upstream hub) that the gadget is no longer connected to the bus.  The 
> udc->driver->unbind call then tells the gadget driver that it will no 
> longer receive any callbacks from the UDC driver or the gadget core.
>
> Now suppose that at just this moment, the user unplugs the USB cable.  
> The UDC driver will notice that the Vbus voltage has gone away and will 
> invoke the gadget driver's ->disconnect callback.  After all, it doesn't 
> realize it should avoid making these callbacks until usb_gadget_udc_stop 
> has run.
>
> As a result, the gadget driver's disconnect callback may be invoked 
> _after_ the driver has been unbound from the gadget.

argh, nice catch!

> How should we fix this potential problem?

I think we can just move usb_gadget_udc_stop(). Looking at a few udcs,
they're just either freeing or masking UDC's interrupts which should
prevent the race you describe while also making sure that no further
interrupts will trigger.

Perhaps we could move udc_stop() before synchronize_irq(). Do you
foresee any issues with that for net2272 or dummy?

-- 
balbi

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 511 bytes --]