| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <2025072457-quaking-each-ea25@gregkh>
Date: Thu, 24 Jul 2025 08:35:07 +0200
From: Greg KH <gregkh@...uxfoundation.org>
To: Weitao Wang <WeitaoWang-oc@...oxin.com>
Cc: mathias.nyman@...el.com, linux-usb@...r.kernel.org,
linux-kernel@...r.kernel.org, WeitaoWang@...oxin.com,
wwt8723@....com, CobeChen@...oxin.com
Subject: Re: [PATCH] usb:xhci:Fix slot_id resource race conflict
On Thu, Jul 24, 2025 at 08:40:12PM +0800, 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 drivers slot_id resources conflict, let the xhci_free_virt_device
> function call in the interrupt handler when disable slot command success.
>
> Fixes: 7faac1953ed1 ("xhci: avoid race between disable slot command and host runtime suspend")
> Signed-off-by: Weitao Wang <WeitaoWang-oc@...oxin.com>
> ---
> drivers/usb/host/xhci-hub.c | 5 +++--
> drivers/usb/host/xhci-ring.c | 7 +++++--
> drivers/usb/host/xhci.c | 29 ++++++++++++++++++++++-------
> 3 files changed, 30 insertions(+), 11 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..4d219418f9e1 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,10 +4086,11 @@ 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);
> + ret = xhci_disable_slot(xhci, udev->slot_id);
>
> spin_lock_irqsave(&xhci->lock, flags);
> - xhci_free_virt_device(xhci, udev->slot_id);
> + if (ret)
> + 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
>
Hi,
This is the friendly patch-bot of Greg Kroah-Hartman. You have sent him
a patch that has triggered this response. He used to manually respond
to these common problems, but in order to save his sanity (he kept
writing the same thing over and over, yet to different people), I was
created. Hopefully you will not take offence and will fix the problem
in your patch and resubmit it so that it can be accepted into the Linux
kernel tree.
You are receiving this message because of the following common error(s)
as indicated below:
- You have marked a patch with a "Fixes:" tag for a commit that is in an
older released kernel, yet you do not have a cc: stable line in the
signed-off-by area at all, which means that the patch will not be
applied to any older kernel releases. To properly fix this, please
follow the documented rules in the
Documentation/process/stable-kernel-rules.rst file for how to resolve
this.
If you wish to discuss this problem further, or you have questions about
how to resolve this issue, please feel free to respond to this email and
Greg will reply once he has dug out from the pending patches received
from other developers.
thanks,
greg k-h's patch email bot
Powered by blists - more mailing lists