[PATCH] usb: xhci: fix EP0 context leak on virt device allocation failure

[PATCH] usb: xhci: fix EP0 context leak on virt device allocation failure

[Anfernee Cao]

When XHCI_EP_LIMIT_QUIRK is set, xhci_alloc_dev() reserves an endpoint
context for the default control endpoint before allocating the virt
device:

	ret = xhci_reserve_host_control_ep_resources(xhci);
	...
	if (!xhci_alloc_virt_device(xhci, slot_id, udev, GFP_NOIO))
		goto disable_slot;

The reservation is normally released by xhci_handle_cmd_disable_slot(),
which calls xhci_free_device_endpoint_resources() for the virt device
found at xhci->devs[slot_id].  xhci_alloc_virt_device() does not install
the virt device when it fails, so the Disable Slot completion handler
finds an empty slot, returns early, and the reserved context is never
released.

Each failure leaks one endpoint context.  Once num_active_eps reaches
limit_active_eps, every new device is rejected with "Not enough host
resources" and cannot be enumerated.

Fix this by releasing the reservation when xhci_alloc_virt_device()
fails.

Fixes: 2cf95c18d506 ("Intel xhci: Limit number of active endpoints to 64.")
Cc: stable@vger.kernel.org
Signed-off-by: Anfernee Cao <anferneemelokc5@gmail.com>
---
 drivers/usb/host/xhci.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
index 7f7babe9d681..8b9a6f88cca5 100644
--- a/drivers/usb/host/xhci.c
+++ b/drivers/usb/host/xhci.c
@@ -4278,6 +4278,15 @@ int xhci_alloc_dev(struct usb_hcd *hcd, struct usb_device *udev)
 	 */
 	if (!xhci_alloc_virt_device(xhci, slot_id, udev, GFP_NOIO)) {
 		xhci_warn(xhci, "Could not allocate xHCI USB device data structures\n");
+		/*
+		 * The virt device was not installed, so the Disable Slot
+		 * completion handler cannot drop the EP0 context reservation.
+		 */
+		if (xhci->quirks & XHCI_EP_LIMIT_QUIRK) {
+			spin_lock_irqsave(&xhci->lock, flags);
+			xhci->num_active_eps--;
+			spin_unlock_irqrestore(&xhci->lock, flags);
+		}
 		goto disable_slot;
 	}
 	vdev = xhci->devs[slot_id];
-- 
2.54.0.windows.1

Re: [PATCH] usb: xhci: fix EP0 context leak on virt device allocation failure

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] usb: xhci: wrong slot ID passed on error in xhci_alloc_dev()
--

commit 19636b8ccf2f4295bbc8eb2a4bffefe6e79d41d3
Author: Anfernee Cao <anferneemelokc5@gmail.com>

usb: xhci: fix EP0 context leak on virt device allocation failure

This commit decrements xhci->num_active_eps when xhci_alloc_virt_device()
fails in xhci_alloc_dev() and XHCI_EP_LIMIT_QUIRK is set, since no virt
device is installed for the Disable Slot completion handler to clean up.

> diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
> index 30ac1bc4559b7..ea6f5c413d152 100644
> --- a/drivers/usb/host/xhci.c
> +++ b/drivers/usb/host/xhci.c
> @@ -4278,6 +4278,15 @@ int xhci_alloc_dev(struct usb_hcd *hcd, struct usb_device *udev)
>  	 */
>  	if (!xhci_alloc_virt_device(xhci, slot_id, udev, GFP_NOIO)) {
>  		xhci_warn(xhci, "Could not allocate xHCI USB device data structures\n");
> +		/*
> +		 * The virt device was not installed, so the Disable Slot
> +		 * completion handler cannot drop the EP0 context reservation.
> +		 */
> +		if (xhci->quirks & XHCI_EP_LIMIT_QUIRK) {
> +			spin_lock_irqsave(&xhci->lock, flags);
> +			xhci->num_active_eps--;
> +			spin_unlock_irqrestore(&xhci->lock, flags);
> +		}
>  		goto disable_slot;
>  	}
>  	vdev = xhci->devs[slot_id];

[Severity: High]
Does jumping to disable_slot here actually disable the newly enabled
slot_id?

In drivers/usb/host/xhci.c:xhci_alloc_dev(), the newly enabled hardware
slot is stored in the local variable slot_id, while udev->slot_id is only
updated after xhci_alloc_virt_device() succeeds:

	slot_id = command->slot_id;
	...
	if (!xhci_alloc_virt_device(xhci, slot_id, udev, GFP_NOIO)) {
		...
		goto disable_slot;
	}
	vdev = xhci->devs[slot_id];
	slot_ctx = xhci_get_slot_ctx(xhci, vdev->out_ctx);
	trace_xhci_alloc_dev(slot_ctx);

	udev->slot_id = slot_id;
	...
disable_slot:
	xhci_disable_and_free_slot(xhci, udev->slot_id);

When either xhci_reserve_host_control_ep_resources() or
xhci_alloc_virt_device() fails and jumps to disable_slot, udev->slot_id has
not been set to slot_id yet.

During normal enumeration, udev->slot_id is 0, so
xhci_disable_and_free_slot(xhci, udev->slot_id) queues TRB_DISABLE_SLOT for
slot 0 and leaks the newly enabled hardware slot in slot_id.

Even worse, if xhci_alloc_dev() is called from
xhci_discover_or_reset_device() when virt_dev->udev != udev:

	if (virt_dev->udev != udev) {
		xhci_dbg(xhci, "The device to be reset with slot ID %u does "
				"not match the udev. Re-allocate the device\n",
				slot_id);
		ret = xhci_alloc_dev(hcd, udev);

udev->slot_id still holds an active slot ID belonging to another usb_device.
Jumping to disable_slot will disable and free that other device's active
slot and xhci_virt_device (and xhci_handle_cmd_disable_slot() will also
call xhci_free_device_endpoint_resources() on that other device's non-NULL
virt_dev, decrementing xhci->num_active_eps for its endpoints in addition
to the decrement added here), while still leaking the newly enabled
slot_id.

Should the disable_slot label pass slot_id instead of udev->slot_id to
xhci_disable_and_free_slot()?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261017111114.1135-1-anferneemelokc5@gmail.com?part=1

Re: [PATCH] usb: xhci: fix EP0 context leak on virt device allocation failure

[Anfernee Cao]

> Does jumping to disable_slot here actually disable the newly enabled
> slot_id?

Yes, you're right, and thanks for catching it.  The disable_slot label
should pass the local slot_id.

This is a pre-existing bug, separate from the EP0 context accounting
leak fixed by this patch, and it has already been submitted:

  [PATCH] usb: xhci: disable the correct slot on alloc_dev failure
  https://patchew.org/linux/6C94F1F35FF16E76+20260806084909.2959131-1-raoxu@uniontech.com/
  Fixes: 3ffbba9511b4 ("USB: xhci: Allocate and address USB devices")

I deliberately kept this patch independent of that one.  The decrement
added here releases the endpoint context reservation made by this
invocation, regardless of which slot the current error path ends up
disabling.  Once Xu Rao's fix lands, the error path will disable the
correct slot and the two fixes compose cleanly.

If the maintainers prefer, I can resend this rebased on top of that fix
once it is applied, or send both as a series.