[<prev] [next>] [thread-next>] [day] [month] [year] [list]
Message-ID: <20250725185101.8375-1-WeitaoWang-oc@zhaoxin.com>
Date: Sat, 26 Jul 2025 02:51:01 +0800
From: Weitao Wang <WeitaoWang-oc@...oxin.com>
To: <gregkh@...uxfoundation.org>, <mathias.nyman@...el.com>,
<linux-usb@...r.kernel.org>, <linux-kernel@...r.kernel.org>
CC: <WeitaoWang@...oxin.com>, <wwt8723@....com>, <CobeChen@...oxin.com>,
<stable@...r.kernel.org>
Subject: [PATCH v2] usb:xhci:Fix slot_id resource race conflict
In such a scenario, device-A with slot_id equal to 1 is disconnecting
while device-B is enumerating, device-B will fail to enumerate in the
follow sequence.
1.[device-A] send disable slot command
2.[device-B] send enable slot command
3.[device-A] disable slot command completed and wakeup waiting thread
4.[device-B] enable slot command completed with slot_id equal to 1 and
wakeup waiting thread
5.[device-B] driver check this slot_id was used by someone(device-A) in
xhci_alloc_virt_device, this device fails to enumerate as this conflict
6.[device-A] xhci->devs[slot_id] set to NULL in xhci_free_virt_device
To fix driver's slot_id resources conflict, let the xhci_free_virt_device
functionm call in the interrupt handler when disable slot command success.
Cc: stable@...r.kernel.org
Fixes: 7faac1953ed1 ("xhci: avoid race between disable slot command and host runtime suspend")
Signed-off-by: Weitao Wang <WeitaoWang-oc@...oxin.com>
---
v1->v2
- Adjust the lock position in the function xhci_free_dev.
drivers/usb/host/xhci-hub.c | 5 +++--
drivers/usb/host/xhci-ring.c | 7 +++++--
drivers/usb/host/xhci.c | 35 +++++++++++++++++++++++++----------
3 files changed, 33 insertions(+), 14 deletions(-)
diff --git a/drivers/usb/host/xhci-hub.c b/drivers/usb/host/xhci-hub.c
index 92bb84f8132a..fd8a64aa5779 100644
--- a/drivers/usb/host/xhci-hub.c
+++ b/drivers/usb/host/xhci-hub.c
@@ -705,10 +705,11 @@ static int xhci_enter_test_mode(struct xhci_hcd *xhci,
continue;
retval = xhci_disable_slot(xhci, i);
- xhci_free_virt_device(xhci, i);
- if (retval)
+ if (retval) {
xhci_err(xhci, "Failed to disable slot %d, %d. Enter test mode anyway\n",
i, retval);
+ xhci_free_virt_device(xhci, i);
+ }
}
spin_lock_irqsave(&xhci->lock, *flags);
/* Put all ports to the Disable state by clear PP */
diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c
index 94c9c9271658..93dc28399c3c 100644
--- a/drivers/usb/host/xhci-ring.c
+++ b/drivers/usb/host/xhci-ring.c
@@ -1589,7 +1589,8 @@ static void xhci_handle_cmd_enable_slot(int slot_id, struct xhci_command *comman
command->slot_id = 0;
}
-static void xhci_handle_cmd_disable_slot(struct xhci_hcd *xhci, int slot_id)
+static void xhci_handle_cmd_disable_slot(struct xhci_hcd *xhci, int slot_id,
+ u32 cmd_comp_code)
{
struct xhci_virt_device *virt_dev;
struct xhci_slot_ctx *slot_ctx;
@@ -1604,6 +1605,8 @@ static void xhci_handle_cmd_disable_slot(struct xhci_hcd *xhci, int slot_id)
if (xhci->quirks & XHCI_EP_LIMIT_QUIRK)
/* Delete default control endpoint resources */
xhci_free_device_endpoint_resources(xhci, virt_dev, true);
+ if (cmd_comp_code == COMP_SUCCESS)
+ xhci_free_virt_device(xhci, slot_id);
}
static void xhci_handle_cmd_config_ep(struct xhci_hcd *xhci, int slot_id)
@@ -1853,7 +1856,7 @@ static void handle_cmd_completion(struct xhci_hcd *xhci,
xhci_handle_cmd_enable_slot(slot_id, cmd, cmd_comp_code);
break;
case TRB_DISABLE_SLOT:
- xhci_handle_cmd_disable_slot(xhci, slot_id);
+ xhci_handle_cmd_disable_slot(xhci, slot_id, cmd_comp_code);
break;
case TRB_CONFIG_EP:
if (!cmd->completion)
diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
index 8a819e853288..6c6f6ebb8953 100644
--- a/drivers/usb/host/xhci.c
+++ b/drivers/usb/host/xhci.c
@@ -3931,13 +3931,14 @@ static int xhci_discover_or_reset_device(struct usb_hcd *hcd,
* the USB device has been reset.
*/
ret = xhci_disable_slot(xhci, udev->slot_id);
- xhci_free_virt_device(xhci, udev->slot_id);
if (!ret) {
ret = xhci_alloc_dev(hcd, udev);
if (ret == 1)
ret = 0;
else
ret = -EINVAL;
+ } else {
+ xhci_free_virt_device(xhci, udev->slot_id);
}
return ret;
}
@@ -4085,11 +4086,12 @@ static void xhci_free_dev(struct usb_hcd *hcd, struct usb_device *udev)
for (i = 0; i < 31; i++)
virt_dev->eps[i].ep_state &= ~EP_STOP_CMD_PENDING;
virt_dev->udev = NULL;
- xhci_disable_slot(xhci, udev->slot_id);
-
- spin_lock_irqsave(&xhci->lock, flags);
- xhci_free_virt_device(xhci, udev->slot_id);
- spin_unlock_irqrestore(&xhci->lock, flags);
+ ret = xhci_disable_slot(xhci, udev->slot_id);
+ if (ret) {
+ spin_lock_irqsave(&xhci->lock, flags);
+ xhci_free_virt_device(xhci, udev->slot_id);
+ spin_unlock_irqrestore(&xhci->lock, flags);
+ }
}
@@ -4128,9 +4130,20 @@ int xhci_disable_slot(struct xhci_hcd *xhci, u32 slot_id)
wait_for_completion(command->completion);
- if (command->status != COMP_SUCCESS)
+ if (command->status != COMP_SUCCESS) {
xhci_warn(xhci, "Unsuccessful disable slot %u command, status %d\n",
slot_id, command->status);
+ switch (command->status) {
+ case COMP_COMMAND_ABORTED:
+ case COMP_COMMAND_RING_STOPPED:
+ xhci_warn(xhci, "Timeout while waiting for disable slot command\n");
+ ret = -ETIME;
+ break;
+ default:
+ ret = -EINVAL;
+ break;
+ }
+ }
xhci_free_command(xhci, command);
@@ -4243,8 +4256,9 @@ int xhci_alloc_dev(struct usb_hcd *hcd, struct usb_device *udev)
return 1;
disable_slot:
- xhci_disable_slot(xhci, udev->slot_id);
- xhci_free_virt_device(xhci, udev->slot_id);
+ ret = xhci_disable_slot(xhci, udev->slot_id);
+ if (ret)
+ xhci_free_virt_device(xhci, udev->slot_id);
return 0;
}
@@ -4381,10 +4395,11 @@ static int xhci_setup_device(struct usb_hcd *hcd, struct usb_device *udev,
mutex_unlock(&xhci->mutex);
ret = xhci_disable_slot(xhci, udev->slot_id);
- xhci_free_virt_device(xhci, udev->slot_id);
if (!ret) {
if (xhci_alloc_dev(hcd, udev) == 1)
xhci_setup_addressable_virt_dev(xhci, udev);
+ } else {
+ xhci_free_virt_device(xhci, udev->slot_id);
}
kfree(command->completion);
kfree(command);
--
2.32.0
Powered by blists - more mailing lists