usb: hub: Fix usb enumeration issue due to address0 race

xHC hardware can only have one slot in default state with address 0
waiting for a unique address at a time, otherwise "undefined behavior
may occur" according to xhci spec 5.4.3.4

The address0_mutex exists to prevent this across both xhci roothubs.

If hub_port_init() fails, it may unlock the mutex and exit with a xhci
slot in default state. If the other xhci roothub calls hub_port_init()
at this point we end up with two slots in default state.

Make sure the address0_mutex protects the slot default state across
hub_port_init() retries, until slot is addressed or disabled.

Note, one known minor case is not fixed by this patch.
If device needs to be reset during resume, but fails all hub_port_init()
retries in usb_reset_and_verify_device(), then it's possible the slot is
still left in default state when address0_mutex is unlocked.

Cc: <stable@vger.kernel.org>
Fixes: 638139eb95d2 ("usb: hub: allow to process more usb hub events in parallel")
Signed-off-by: Mathias Nyman <mathias.nyman@linux.intel.com>
Link: https://lore.kernel.org/r/20211115221630.871204-1-mathias.nyman@linux.intel.com
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
index 86658a81d28445181198ea9f2876a8a2b44fdee9..00c3506324e47d341dd4c7eb86e77829ca7acdf0 100644 (file)
--- a/drivers/usb/core/hub.c
+++ b/drivers/usb/core/hub.c
@@ -4700,8 +4700,6 @@ hub_port_init(struct usb_hub *hub, struct usb_device *udev, int port1,
        if (oldspeed == USB_SPEED_LOW)
                delay = HUB_LONG_RESET_TIME;
 
-       mutex_lock(hcd->address0_mutex);
-
        /* Reset the device; full speed may morph to high speed */
        /* FIXME a USB 2.0 device may morph into SuperSpeed on reset. */
        retval = hub_port_reset(hub, port1, udev, delay, false);
@@ -5016,7 +5014,6 @@ fail:
                hub_port_disable(hub, port1, 0);
                update_devnum(udev, devnum);    /* for disconnect processing */
        }
-       mutex_unlock(hcd->address0_mutex);
        return retval;
 }
 
@@ -5246,6 +5243,9 @@ static void hub_port_connect(struct usb_hub *hub, int port1, u16 portstatus,
                unit_load = 100;
 
        status = 0;
+
+       mutex_lock(hcd->address0_mutex);
+
        for (i = 0; i < PORT_INIT_TRIES; i++) {
 
                /* reallocate for each attempt, since references
@@ -5282,6 +5282,8 @@ static void hub_port_connect(struct usb_hub *hub, int port1, u16 portstatus,
                if (status < 0)
                        goto loop;
 
+               mutex_unlock(hcd->address0_mutex);
+
                if (udev->quirks & USB_QUIRK_DELAY_INIT)
                        msleep(2000);
 
@@ -5370,6 +5372,7 @@ static void hub_port_connect(struct usb_hub *hub, int port1, u16 portstatus,
 
 loop_disable:
                hub_port_disable(hub, port1, 1);
+               mutex_lock(hcd->address0_mutex);
 loop:
                usb_ep0_reinit(udev);
                release_devnum(udev);
@@ -5396,6 +5399,8 @@ loop:
        }
 
 done:
+       mutex_unlock(hcd->address0_mutex);
+
        hub_port_disable(hub, port1, 1);
        if (hcd->driver->relinquish_port && !hub->hdev->parent) {
                if (status != -ENOTCONN && status != -ENODEV)
@@ -5915,6 +5920,8 @@ static int usb_reset_and_verify_device(struct usb_device *udev)
        bos = udev->bos;
        udev->bos = NULL;
 
+       mutex_lock(hcd->address0_mutex);
+
        for (i = 0; i < PORT_INIT_TRIES; ++i) {
 
                /* ep0 maxpacket size may change; let the HCD know about it.
@@ -5924,6 +5931,7 @@ static int usb_reset_and_verify_device(struct usb_device *udev)
                if (ret >= 0 || ret == -ENOTCONN || ret == -ENODEV)
                        break;
        }
+       mutex_unlock(hcd->address0_mutex);
 
        if (ret < 0)
                goto re_enumerate;