Skip to content

Commit

Permalink
usb: hub: Fix usb enumeration issue due to address0 race
Browse files Browse the repository at this point in the history
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: 638139e ("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>
  • Loading branch information
matnyman authored and gregkh committed Nov 17, 2021
1 parent 3624688 commit 6ae6dc2
Showing 1 changed file with 11 additions and 3 deletions.
14 changes: 11 additions & 3 deletions drivers/usb/core/hub.c
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -5016,7 +5014,6 @@ hub_port_init(struct usb_hub *hub, struct usb_device *udev, int port1,
hub_port_disable(hub, port1, 0);
update_devnum(udev, devnum); /* for disconnect processing */
}
mutex_unlock(hcd->address0_mutex);
return retval;
}

Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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);

Expand Down Expand Up @@ -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);
Expand All @@ -5396,6 +5399,8 @@ static void hub_port_connect(struct usb_hub *hub, int port1, u16 portstatus,
}

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)
Expand Down Expand Up @@ -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.
Expand All @@ -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;
Expand Down

0 comments on commit 6ae6dc2

Please sign in to comment.