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, 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.
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 --- v2->v3 - When disable slot command completes successfully, only clear some strategic pointers instead of calling xhci_free_virt_device in the interrupt context.
drivers/usb/host/xhci-hub.c | 3 +-- drivers/usb/host/xhci-mem.c | 20 +++++++++----------- drivers/usb/host/xhci-ring.c | 9 +++++++-- drivers/usb/host/xhci.c | 21 ++++++++++++++------- drivers/usb/host/xhci.h | 3 ++- 5 files changed, 33 insertions(+), 23 deletions(-)
diff --git a/drivers/usb/host/xhci-hub.c b/drivers/usb/host/xhci-hub.c index 92bb84f8132a..b3a59ce1b3f4 100644 --- a/drivers/usb/host/xhci-hub.c +++ b/drivers/usb/host/xhci-hub.c @@ -704,8 +704,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 6680afa4f596..962b0c20b883 100644 --- a/drivers/usb/host/xhci-mem.c +++ b/drivers/usb/host/xhci-mem.c @@ -865,21 +865,18 @@ int xhci_alloc_tt_info(struct xhci_hcd *xhci, * 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 (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);
@@ -920,8 +917,9 @@ void xhci_free_virt_device(struct xhci_hcd *xhci, int slot_id) dev->udev->slot_id = 0; if (dev->rhub_port && dev->rhub_port->slot_id == slot_id) dev->rhub_port->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); }
/* @@ -962,7 +960,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 94c9c9271658..7a440ec52ff6 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,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) @@ -1853,7 +1858,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..b1419e3ec7f9 100644 --- a/drivers/usb/host/xhci.c +++ b/drivers/usb/host/xhci.c @@ -3930,8 +3930,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) @@ -4088,7 +4087,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);
} @@ -4137,6 +4136,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. @@ -4243,8 +4252,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; } @@ -4380,8 +4388,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 a20f4e7cd43a..85d5b964bf1e 100644 --- a/drivers/usb/host/xhci.h +++ b/drivers/usb/host/xhci.h @@ -1791,7 +1791,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, @@ -1888,6 +1888,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);
On 30.7.2025 18.27, Weitao Wang wrote:
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, 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.
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
Thanks, added to queue with some minor commit message tuning
-Mathias
Hi Weitao,
kernel test robot noticed the following build warnings:
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/Weitao-Wang/usb-xhci-Fix-slot... base: https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/usb.git usb-testing patch link: https://lore.kernel.org/r/20250730152715.8368-1-WeitaoWang-oc%40zhaoxin.com patch subject: [PATCH v3] usb:xhci:Fix slot_id resource race conflict config: x86_64-randconfig-161-20250801 (https://download.01.org/0day-ci/archive/20250801/202508010850.Bqd6wf47-lkp@i...) compiler: gcc-12 (Debian 12.2.0-14+deb12u1) 12.2.0
If you fix the issue in a separate patch/commit (i.e. not just a new version of the same patch/commit), kindly add following tags | Reported-by: kernel test robot lkp@intel.com | Reported-by: Dan Carpenter dan.carpenter@linaro.org | Closes: https://lore.kernel.org/r/202508010850.Bqd6wf47-lkp@intel.com/
New smatch warnings: drivers/usb/host/xhci-mem.c:913 xhci_free_virt_device() warn: variable dereferenced before check 'dev->out_ctx' (see line 878)
vim +913 drivers/usb/host/xhci-mem.c
0b5ed80150eb59 Weitao Wang 2025-07-30 868 void xhci_free_virt_device(struct xhci_hcd *xhci, struct xhci_virt_device *dev, 0b5ed80150eb59 Weitao Wang 2025-07-30 869 int slot_id) 3ffbba9511b414 Sarah Sharp 2009-04-27 870 { 3ffbba9511b414 Sarah Sharp 2009-04-27 871 int i; 2e27980e6eb781 Sarah Sharp 2011-09-02 872 int old_active_eps = 0; 3ffbba9511b414 Sarah Sharp 2009-04-27 873 3ffbba9511b414 Sarah Sharp 2009-04-27 874 /* Slot ID 0 is reserved */ 0b5ed80150eb59 Weitao Wang 2025-07-30 875 if (slot_id == 0 || !dev) 3ffbba9511b414 Sarah Sharp 2009-04-27 876 return; 3ffbba9511b414 Sarah Sharp 2009-04-27 877 0b5ed80150eb59 Weitao Wang 2025-07-30 @878 if (xhci->dcbaa->dev_context_ptrs[slot_id] == cpu_to_le64(dev->out_ctx->dma)) ^^^^^^^^^^^^ dev->out_ctx dereferenced without checking for NULL
8e595a5d30a5ee Sarah Sharp 2009-07-27 879 xhci->dcbaa->dev_context_ptrs[slot_id] = 0; 3ffbba9511b414 Sarah Sharp 2009-04-27 880 d850c1658328e7 Zhengjun Xing 2018-06-21 881 trace_xhci_free_virt_device(dev); d850c1658328e7 Zhengjun Xing 2018-06-21 882 2e27980e6eb781 Sarah Sharp 2011-09-02 883 if (dev->tt_info) 2e27980e6eb781 Sarah Sharp 2011-09-02 884 old_active_eps = dev->tt_info->active_eps; 2e27980e6eb781 Sarah Sharp 2011-09-02 885 98871e9470a50c Felipe Balbi 2017-01-23 886 for (i = 0; i < 31; i++) { 63a0d9abd18cdc Sarah Sharp 2009-09-04 887 if (dev->eps[i].ring) 63a0d9abd18cdc Sarah Sharp 2009-09-04 888 xhci_ring_free(xhci, dev->eps[i].ring); 8df75f42f8e67e Sarah Sharp 2010-04-02 889 if (dev->eps[i].stream_info) 8df75f42f8e67e Sarah Sharp 2010-04-02 890 xhci_free_stream_info(xhci, 8df75f42f8e67e Sarah Sharp 2010-04-02 891 dev->eps[i].stream_info); 5aed5b7c2430ce Mathias Nyman 2022-10-24 892 /* 5aed5b7c2430ce Mathias Nyman 2022-10-24 893 * Endpoints are normally deleted from the bandwidth list when 5aed5b7c2430ce Mathias Nyman 2022-10-24 894 * endpoints are dropped, before device is freed. 5aed5b7c2430ce Mathias Nyman 2022-10-24 895 * If host is dying or being removed then endpoints aren't 5aed5b7c2430ce Mathias Nyman 2022-10-24 896 * dropped cleanly, so delete the endpoint from list here. 5aed5b7c2430ce Mathias Nyman 2022-10-24 897 * Only applicable for hosts with software bandwidth checking. 2e27980e6eb781 Sarah Sharp 2011-09-02 898 */ 5aed5b7c2430ce Mathias Nyman 2022-10-24 899 5aed5b7c2430ce Mathias Nyman 2022-10-24 900 if (!list_empty(&dev->eps[i].bw_endpoint_list)) { 5aed5b7c2430ce Mathias Nyman 2022-10-24 901 list_del_init(&dev->eps[i].bw_endpoint_list); 5aed5b7c2430ce Mathias Nyman 2022-10-24 902 xhci_dbg(xhci, "Slot %u endpoint %u not removed from BW list!\n", 2e27980e6eb781 Sarah Sharp 2011-09-02 903 slot_id, i); 8df75f42f8e67e Sarah Sharp 2010-04-02 904 } 5aed5b7c2430ce Mathias Nyman 2022-10-24 905 } 839c817ce67178 Sarah Sharp 2011-09-02 906 /* If this is a hub, free the TT(s) from the TT list */ 839c817ce67178 Sarah Sharp 2011-09-02 907 xhci_free_tt_info(xhci, dev, slot_id); 2e27980e6eb781 Sarah Sharp 2011-09-02 908 /* If necessary, update the number of active TTs on this root port */ 2e27980e6eb781 Sarah Sharp 2011-09-02 909 xhci_update_tt_active_eps(xhci, dev, old_active_eps); 3ffbba9511b414 Sarah Sharp 2009-04-27 910 3ffbba9511b414 Sarah Sharp 2009-04-27 911 if (dev->in_ctx) d115b04818e57b John Youn 2009-07-27 912 xhci_free_container_ctx(xhci, dev->in_ctx); 3ffbba9511b414 Sarah Sharp 2009-04-27 @913 if (dev->out_ctx) ^^^^^^^^^^^^ Can dev->out_ctx be NULL?
d115b04818e57b John Youn 2009-07-27 914 xhci_free_container_ctx(xhci, dev->out_ctx); d115b04818e57b John Youn 2009-07-27 915 a400efe455f7b6 Mathias Nyman 2018-03-16 916 if (dev->udev && dev->udev->slot_id) a400efe455f7b6 Mathias Nyman 2018-03-16 917 dev->udev->slot_id = 0; 74151b5349266b Niklas Neronin 2024-02-29 918 if (dev->rhub_port && dev->rhub_port->slot_id == slot_id) 74151b5349266b Niklas Neronin 2024-02-29 919 dev->rhub_port->slot_id = 0; 0b5ed80150eb59 Weitao Wang 2025-07-30 920 if (xhci->devs[slot_id] == dev) 326b4810cc9952 Randy Dunlap 2010-04-19 921 xhci->devs[slot_id] = NULL; 0b5ed80150eb59 Weitao Wang 2025-07-30 922 kfree(dev); 3ffbba9511b414 Sarah Sharp 2009-04-27 923 }
linux-stable-mirror@lists.linaro.org
-
Dan Carpenter -
Mathias Nyman -
Weitao Wang