usb: xhci: Fix slot_id resource race conflict

[ Upstream commit 2eb03376151bb8585caa23ed2673583107bb5193 ]

xHC controller may immediately reuse a slot_id after it's disabled,
giving it to a new enumerating device before the xhci driver freed
all resources related to the disabled device.

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 checks that slot_id is still in use (by device-A) in
	     xhci_alloc_virt_device, and fail to enumerate due to 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, clear xhci->devs[slot_id] and
xhci->dcbba->dev_context_ptrs[slot_id] pointers in the interrupt context
when disable slot command completes successfully. Simultaneously, adjust
function xhci_free_virt_device to accurately handle device release.

[minor smatch warning and commit message fix -Mathias]

Cc: stable@vger.kernel.org
Fixes: 7faac1953ed1 ("xhci: avoid race between disable slot command and host runtime suspend")
Signed-off-by: Weitao Wang <WeitaoWang-oc@zhaoxin.com>
Signed-off-by: Mathias Nyman <mathias.nyman@linux.intel.com>
Link: https://lore.kernel.org/r/20250819125844.2042452-2-mathias.nyman@linux.intel.com
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
[ Adjust context ]
Signed-off-by: Sasha Levin <sashal@kernel.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

diff --git a/drivers/usb/host/xhci-hub.c b/drivers/usb/host/xhci-hub.c
index a2b6a922077ee38a92e36643d057f2b9deec1388..d3d535ed00b50bcbd4aa540b1a4b85380d8e1187 100644 (file)
--- a/drivers/usb/host/xhci-hub.c
+++ b/drivers/usb/host/xhci-hub.c
@@ -735,8 +735,7 @@ static int xhci_enter_test_mode(struct xhci_hcd *xhci,
                if (!xhci->devs[i])
                        continue;
 
-               retval = xhci_disable_slot(xhci, i);
-               xhci_free_virt_device(xhci, i);
+               retval = xhci_disable_and_free_slot(xhci, i);
                if (retval)
                        xhci_err(xhci, "Failed to disable slot %d, %d. Enter test mode anyway\n",
                                 i, retval);

diff --git a/drivers/usb/host/xhci-mem.c b/drivers/usb/host/xhci-mem.c
index b51464eeac46ee265b7749e1bdc97480a7c63c8e..04718048b74bd97eee148492c6a6f11587c2f819 100644 (file)
--- a/drivers/usb/host/xhci-mem.c
+++ b/drivers/usb/host/xhci-mem.c
@@ -849,21 +849,20 @@ free_tts:
  * will be manipulated by the configure endpoint, allocate device, or update
  * hub functions while this function is removing the TT entries from the list.
  */
-void xhci_free_virt_device(struct xhci_hcd *xhci, int slot_id)
+void xhci_free_virt_device(struct xhci_hcd *xhci, struct xhci_virt_device *dev,
+               int slot_id)
 {
-       struct xhci_virt_device *dev;
        int i;
        int old_active_eps = 0;
 
        /* Slot ID 0 is reserved */
-       if (slot_id == 0 || !xhci->devs[slot_id])
+       if (slot_id == 0 || !dev)
                return;
 
-       dev = xhci->devs[slot_id];
-
-       xhci->dcbaa->dev_context_ptrs[slot_id] = 0;
-       if (!dev)
-               return;
+       /* If device ctx array still points to _this_ device, clear it */
+       if (dev->out_ctx &&
+           xhci->dcbaa->dev_context_ptrs[slot_id] == cpu_to_le64(dev->out_ctx->dma))
+               xhci->dcbaa->dev_context_ptrs[slot_id] = 0;
 
        trace_xhci_free_virt_device(dev);
 
@@ -902,8 +901,9 @@ void xhci_free_virt_device(struct xhci_hcd *xhci, int slot_id)
 
        if (dev->udev && dev->udev->slot_id)
                dev->udev->slot_id = 0;
-       kfree(xhci->devs[slot_id]);
-       xhci->devs[slot_id] = NULL;
+       if (xhci->devs[slot_id] == dev)
+               xhci->devs[slot_id] = NULL;
+       kfree(dev);
 }
 
 /*
@@ -945,7 +945,7 @@ static void xhci_free_virt_devices_depth_first(struct xhci_hcd *xhci, int slot_i
 out:
        /* we are now at a leaf device */
        xhci_debugfs_remove_slot(xhci, slot_id);
-       xhci_free_virt_device(xhci, slot_id);
+       xhci_free_virt_device(xhci, vdev, slot_id);
 }
 
 int xhci_alloc_virt_device(struct xhci_hcd *xhci, int slot_id,

diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c
index 2a74a47a1c763214c7e9086084f406a20c8aeb1e..a21ac9d80275f712aad281cddf32f7aee9b96e98 100644 (file)
--- a/drivers/usb/host/xhci-ring.c
+++ b/drivers/usb/host/xhci-ring.c
@@ -1587,7 +1587,8 @@ static void xhci_handle_cmd_enable_slot(struct xhci_hcd *xhci, int slot_id,
                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;
@@ -1602,6 +1603,10 @@ 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->dcbaa->dev_context_ptrs[slot_id] = 0;
+               xhci->devs[slot_id] = NULL;
+       }
 }
 
 static void xhci_handle_cmd_config_ep(struct xhci_hcd *xhci, int slot_id,
@@ -1850,7 +1855,7 @@ static void handle_cmd_completion(struct xhci_hcd *xhci,
                xhci_handle_cmd_enable_slot(xhci, 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 f5e170fe5f797d264505a08a3adc8a96765f00f4..8fd88fedbb30ea13e318a8dcf9dfdf6a672727b2 100644 (file)
--- a/drivers/usb/host/xhci.c
+++ b/drivers/usb/host/xhci.c
@@ -3760,8 +3760,7 @@ static int xhci_discover_or_reset_device(struct usb_hcd *hcd,
                 * Obtaining a new device slot to inform the xHCI host that
                 * the USB device has been reset.
                 */
-               ret = xhci_disable_slot(xhci, udev->slot_id);
-               xhci_free_virt_device(xhci, udev->slot_id);
+               ret = xhci_disable_and_free_slot(xhci, udev->slot_id);
                if (!ret) {
                        ret = xhci_alloc_dev(hcd, udev);
                        if (ret == 1)
@@ -3916,7 +3915,7 @@ static void xhci_free_dev(struct usb_hcd *hcd, struct usb_device *udev)
        xhci_disable_slot(xhci, udev->slot_id);
 
        spin_lock_irqsave(&xhci->lock, flags);
-       xhci_free_virt_device(xhci, udev->slot_id);
+       xhci_free_virt_device(xhci, virt_dev, udev->slot_id);
        spin_unlock_irqrestore(&xhci->lock, flags);
 
 }
@@ -3965,6 +3964,16 @@ int xhci_disable_slot(struct xhci_hcd *xhci, u32 slot_id)
        return 0;
 }
 
+int xhci_disable_and_free_slot(struct xhci_hcd *xhci, u32 slot_id)
+{
+       struct xhci_virt_device *vdev = xhci->devs[slot_id];
+       int ret;
+
+       ret = xhci_disable_slot(xhci, slot_id);
+       xhci_free_virt_device(xhci, vdev, slot_id);
+       return ret;
+}
+
 /*
  * Checks if we have enough host controller resources for the default control
  * endpoint.
@@ -4071,8 +4080,7 @@ 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);
+       xhci_disable_and_free_slot(xhci, udev->slot_id);
 
        return 0;
 }
@@ -4208,8 +4216,7 @@ static int xhci_setup_device(struct usb_hcd *hcd, struct usb_device *udev,
                dev_warn(&udev->dev, "Device not responding to setup %s.\n", act);
 
                mutex_unlock(&xhci->mutex);
-               ret = xhci_disable_slot(xhci, udev->slot_id);
-               xhci_free_virt_device(xhci, udev->slot_id);
+               ret = xhci_disable_and_free_slot(xhci, udev->slot_id);
                if (!ret) {
                        if (xhci_alloc_dev(hcd, udev) == 1)
                                xhci_setup_addressable_virt_dev(xhci, udev);

diff --git a/drivers/usb/host/xhci.h b/drivers/usb/host/xhci.h
index 159cdfc71290709c573d262534cf32fe9b31dac7..808f2ee43b9444c15bc8e61945c9b869e2a6a913 100644 (file)
--- a/drivers/usb/host/xhci.h
+++ b/drivers/usb/host/xhci.h
@@ -1798,7 +1798,7 @@ void xhci_dbg_trace(struct xhci_hcd *xhci, void (*trace)(struct va_format *),
 /* xHCI memory management */
 void xhci_mem_cleanup(struct xhci_hcd *xhci);
 int xhci_mem_init(struct xhci_hcd *xhci, gfp_t flags);
-void xhci_free_virt_device(struct xhci_hcd *xhci, int slot_id);
+void xhci_free_virt_device(struct xhci_hcd *xhci, struct xhci_virt_device *dev, int slot_id);
 int xhci_alloc_virt_device(struct xhci_hcd *xhci, int slot_id, struct usb_device *udev, gfp_t flags);
 int xhci_setup_addressable_virt_dev(struct xhci_hcd *xhci, struct usb_device *udev);
 void xhci_copy_ep0_dequeue_into_input_ctx(struct xhci_hcd *xhci,
@@ -1895,6 +1895,7 @@ void xhci_reset_bandwidth(struct usb_hcd *hcd, struct usb_device *udev);
 int xhci_update_hub_device(struct usb_hcd *hcd, struct usb_device *hdev,
                           struct usb_tt *tt, gfp_t mem_flags);
 int xhci_disable_slot(struct xhci_hcd *xhci, u32 slot_id);
+int xhci_disable_and_free_slot(struct xhci_hcd *xhci, u32 slot_id);
 int xhci_ext_cap_init(struct xhci_hcd *xhci);
 
 int xhci_suspend(struct xhci_hcd *xhci, bool do_wakeup);