On Tue, 17 Apr 2012, Alexander Shishkin wrote:
> Currently, on gadget removal path, dummy_pullup() gets called after
> gadget's disconnect(). Now, dummy_pullup() decides that it needs to
> disconnect again, resulting in a crash. IOW, rmmod g_zero will crash
> with dummy_hcd.
This doesn't sound right. The gadget removal path is dummy_udc_stop(),
right? That routine sets dum->driver to NULL before calling
dummy_pullup(). Consequently the last test in the "if" statement you
are changing won't succeed, so the gadget's disconnect method won't be
called.
> After aa0747394337e50533badd46e8fb7106bad3311e (usb: gadget: dummy_hcd:
> convert to new-style udc-probe) this problem should be reproducible, so
> from v3.1 on.
>
> I can't pretend to fully comprehend the logic around set_link_state(),
> but I'm fairly sure it shouldn't end up calling disconnect twice.
It shouldn't. If it does, the reason is not what you think.
> Signed-off-by: Alexander Shishkin <alexander.shishkin@xxxxxxxxxxxxxxx>
> ---
> drivers/usb/gadget/dummy_hcd.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/usb/gadget/dummy_hcd.c b/drivers/usb/gadget/dummy_hcd.c
> index a6dfd21..66fce0e 100644
> --- a/drivers/usb/gadget/dummy_hcd.c
> +++ b/drivers/usb/gadget/dummy_hcd.c
> @@ -371,7 +371,7 @@ static void set_link_state(struct dummy_hcd *dum_hcd)
> * We're connected and not reset (reset occurred now),
> * and driver attached - disconnect!
> */
> - if ((dum_hcd->old_status & USB_PORT_STAT_CONNECTION) != 0 &&
> + if ((dum_hcd->port_status & USB_PORT_STAT_CONNECTION) != 0 &&
Definitely should be old_status. The idea is that this code should run
if we were connected the last time we checked but we aren't connected
now.
> (dum_hcd->old_status & USB_PORT_STAT_RESET) == 0 &&
> dum->driver) {
> stop_activity(dum);
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
