[PATCH 1/2] usb: gadget: composite: Race between disconnect/unbind and setup

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

 



usb_gadget_remove_driver() runs through a four-step sequence to shut down
the gadget driver.  For the case of a composite gadget + at91 UDC, this
would look like:

    udc->driver->disconnect(udc->gadget);          // composite_disconnect()
    usb_gadget_disconnect(udc->gadget);            // at91_pullup(gadget, 0)
    udc->driver->unbind(udc->gadget);              // composite_unbind()
    usb_gadget_udc_stop(udc->gadget, udc->driver); // at91_stop()

composite_disconnect() says:

    if (cdev->config)
        reset_config(cdev);

reset_config() sets cdev->config to NULL.  composite_unbind() later tests
for this:

    WARN_ON(cdev->config);

But SETUP packets may be sent to the composite driver up until the point
when usb_gadget_disconnect() returns.  A SETUP packet can cause
cdev->config to become non-NULL in between the time when
composite_disconnect() returns, and the time composite_unbind() is called.
That is what was seen when running an overnight test:

    while :; do
        insmod g_ether.ko
        busybox usleep $(($RANDOM * 10))
        rmmod g_ether
    done

There was a Linux host on the other end of the cable.  (Which eventually
locked up, FWIW.)

A quick fix is to just run composite_disconnect() a second time from
composite_unbind(), to reverse the effects of any SET_CONFIGURATION
requests that might have happened before usb_gadget_disconnect() finished
doing its thing.

Signed-off-by: Kevin Cernekee <cernekee@xxxxxxxxx>
---
 drivers/usb/gadget/composite.c | 12 ++++++++----
 1 file changed, 8 insertions(+), 4 deletions(-)

diff --git a/drivers/usb/gadget/composite.c b/drivers/usb/gadget/composite.c
index 3f72110..e8594c9 100644
--- a/drivers/usb/gadget/composite.c
+++ b/drivers/usb/gadget/composite.c
@@ -1383,12 +1383,16 @@ composite_unbind(struct usb_gadget *gadget)
 {
 	struct usb_composite_dev	*cdev = get_gadget_data(gadget);
 
-	/* composite_disconnect() must already have been called
-	 * by the underlying peripheral controller driver!
-	 * so there's no i/o concurrency that could affect the
+	/*
+	 * usb_gadget_disconnect() has already been called by the udc-core
+	 * code, so we shouldn't see any new I/O that could affect the
 	 * state protected by cdev->lock.
+	 *
+	 * composite_disconnect() MAY have already been called, but not
+	 * since we asked the UDC to stop passing traffic.  Invoke it again
+	 * in case something happened since then.
 	 */
-	WARN_ON(cdev->config);
+	composite_disconnect(gadget);
 
 	while (!list_empty(&cdev->configs)) {
 		struct usb_configuration	*c;
-- 
1.7.11.1

--
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