[PATCH] xhci: give back halted TD when skipping endpoint reset after disconnect

[PATCH] xhci: give back halted TD when skipping endpoint reset after disconnect

[Yiyu Zhou]

Commit 042aad8d0db6 ("xhci: prevent endpoint recovery after roothub
disconnect") makes xhci_handle_halted_endpoint() return early when the
root port is disconnected, and commit 7e0ef4332ed9 ("xhci: avoid xHC
endpoint changes after disconnect or link error.") also sets
EP_DROP_PENDING there, which keeps the doorbell from being rung until
the device is reset.

The callers rely on xhci_handle_halted_endpoint() to take over the
halted TD.  For example, finish_td() returns right after calling it,
expecting the TD to be given back once the Reset Endpoint command
completes.  With the early return, nobody gives the TD back.  Its URB
stays pending until the submitter times out and unlinks it.  Every URB
queued on the endpoint afterwards never starts, because the doorbell is
blocked, and also has to time out.

Devices that disconnect and reconnect themselves during enumeration hit
this.  A PFU HHKB Professional HYBRID Type-S keyboard (04fe:0021) drops
off the bus about 300 ms after its first enumeration, while usbhid is
still probing its third interface.  The probe runs in hub_wq, so the
disconnect is not handled until the probe returns.  On a root port the
HID descriptor request and its four retries in hid_get_class_descriptor()
each wait for the full 5 second USB_CTRL_GET_TIMEOUT:

  [57685.006036] usb 3-2: new full-speed USB device number 24 using xhci_hcd
  ...
  [57685.291304] hid-generic 0003:04FE:0021.0017: input,hidraw1: USB HID v1.11 Keyboard [PFU Limited HHKB-Hybrid] on usb-0000:00:14.0-2/input1
  [57711.221083] usbhid 3-2:1.2: can't add hid device: -110
  [57711.221613] usbhid 3-2:1.2: probe with driver usbhid failed with error -110
  [57711.221842] usb 3-2: USB disconnect, device number 24

The re-enumeration that follows can stall the same way, leaving the
keyboard dead until reboot.  Behind an external hub, where the root
port stays connected, the same keyboard recovers in about 300 ms, as it
did on the root port before these commits.

Keep skipping Reset Endpoint on a disconnected or inactive port, but
give the halted TD back right away, with -EPROTO if no error status was
set yet.  The endpoint is not restarted until the device is reset or
dropped, so the ring dequeue pointer can stay where it is, as
xhci_invalidate_cancelled_tds() already assumes for cancelled TDs on
such endpoints.  Also refuse new URBs on an endpoint with
EP_DROP_PENDING set, like xhci_urb_enqueue() already does for an
inactive link, so they fail right away instead of timing out.

Fixes: 042aad8d0db6 ("xhci: prevent endpoint recovery after roothub disconnect")
Fixes: 7e0ef4332ed9 ("xhci: avoid xHC endpoint changes after disconnect or link error.")
Assisted-by: LLM
Signed-off-by: Yiyu Zhou <yiyu@yiyuzhou.io>
---
Tested on v7.3-rc6 on a System76 Galago Pro (Intel 8086:51ed xHCI)
with the keyboard plugged directly into a root port, including twice
right after resume from suspend.  On every hotplug the keyboard still
dropped off during the probe of its third interface, and it was
re-enumerated with all three interfaces bound about 0.3 s later.
Before this patch, the same sequence hung usbhid for 26 s, and often
left the keyboard dead until reboot.

 drivers/usb/host/xhci-ring.c | 24 ++++++++++++++++++++----
 drivers/usb/host/xhci.c      |  5 +++++
 2 files changed, 25 insertions(+), 4 deletions(-)

diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c
index ec278a9f9540..82190380831d 100644
--- a/drivers/usb/host/xhci-ring.c
+++ b/drivers/usb/host/xhci-ring.c
@@ -979,6 +979,24 @@ static int xhci_reset_halted_ep(struct xhci_hcd *xhci, unsigned int slot_id,
 	return ret;
 }
 
+/*
+ * Mark the endpoint to be dropped instead of reset, and give back the halted
+ * TD now, as nobody else will.  The endpoint is not restarted before device
+ * reset or drop, so there is no need to move the ring dequeue past the TD.
+ */
+static int xhci_drop_halted_endpoint(struct xhci_hcd *xhci,
+				     struct xhci_virt_ep *ep,
+				     struct xhci_td *td)
+{
+	ep->ep_state |= EP_DROP_PENDING;
+
+	if (td)
+		xhci_td_cleanup(xhci, td, xhci_urb_to_transfer_ring(xhci, td->urb),
+				td->status ?: -EPROTO);
+
+	return -ENODEV;
+}
+
 static int xhci_handle_halted_endpoint(struct xhci_hcd *xhci,
 				struct xhci_virt_ep *ep,
 				struct xhci_td *td,
@@ -993,10 +1011,8 @@ static int xhci_handle_halted_endpoint(struct xhci_hcd *xhci,
 	 * Can cause host hang.
 	 * Device will be reset to recover an inactive link, so don't do anything
 	 */
-	if (rhub_port->link_inactive || !rhub_port->connected) {
-		ep->ep_state |= EP_DROP_PENDING;
-		return -ENODEV;
-	}
+	if (rhub_port->link_inactive || !rhub_port->connected)
+		return xhci_drop_halted_endpoint(xhci, ep, td);
 
 	/* add td to cancelled list and let reset ep handler take care of it */
 	if (reset_type == EP_HARD_RESET) {
diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
index a9e47e178c28..491a541b57b0 100644
--- a/drivers/usb/host/xhci.c
+++ b/drivers/usb/host/xhci.c
@@ -1691,6 +1691,11 @@ static int xhci_urb_enqueue(struct usb_hcd *hcd, struct urb *urb, gfp_t mem_flag
 		ret = -EINVAL;
 		goto free_priv;
 	}
+	if (*ep_state & EP_DROP_PENDING) {
+		xhci_dbg(xhci, "Can't queue urb, ep halted after disconnect or link error\n");
+		ret = -ENODEV;
+		goto free_priv;
+	}
 
 	switch (usb_endpoint_type(&urb->ep->desc)) {
 
-- 
2.55.0

Re: [PATCH] xhci: give back halted TD when skipping endpoint reset after disconnect

[sashiko-bot]

> Commit 042aad8d0db6 ("xhci: prevent endpoint recovery after roothub
> disconnect") makes xhci_handle_halted_endpoint() return early when the
> root port is disconnected, and commit 7e0ef4332ed9 ("xhci: avoid xHC
> endpoint changes after disconnect or link error.") also sets
> EP_DROP_PENDING there, which keeps the doorbell from being rung until
> the device is reset.
> 
> The callers rely on xhci_handle_halted_endpoint() to take over the
> halted TD.  For example, finish_td() returns right after calling it,
> expecting the TD to be given back once the Reset Endpoint command
> completes.  With the early return, nobody gives the TD back.  Its URB
> stays pending until the submitter times out and unlinks it.  Every URB
> queued on the endpoint afterwards never starts, because the doorbell is
> blocked, and also has to time out.
> 
> [ ... ]
> 
> Fixes: 042aad8d0db6 ("xhci: prevent endpoint recovery after roothub disconnect")
> Fixes: 7e0ef4332ed9 ("xhci: avoid xHC endpoint changes after disconnect or link error.")
> Assisted-by: LLM
> Signed-off-by: Yiyu Zhou <yiyu@yiyuzhou.io>

Sashiko has reviewed this patch and found no issues. It looks great!

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261010064407.11030-1-yiyu@yiyuzhou.io?part=1

Re: [PATCH] xhci: give back halted TD when skipping endpoint reset after disconnect

[Yiyu Zhou]

This fixes a regression introduced in v7.3-rc1.  Up to v7.2.1 the
keyboard recovered from the same hotplug in well under a second, and
reverting 042aad8d0db6 and 7e0ef4332ed9 on top of v7.3-rc5 also fixed
it.  It would be good to get this into 7.3.

#regzbot introduced: 042aad8d0db6

Re: [PATCH] xhci: give back halted TD when skipping endpoint reset after disconnect

[Michal Pecio]

On Fri,  9 Oct 2026 23:42:19 -0700, Yiyu Zhou wrote:
> Commit 042aad8d0db6 ("xhci: prevent endpoint recovery after roothub
> disconnect") makes xhci_handle_halted_endpoint() return early when the
> root port is disconnected, and commit 7e0ef4332ed9 ("xhci: avoid xHC
> endpoint changes after disconnect or link error.") also sets
> EP_DROP_PENDING there, which keeps the doorbell from being rung until
> the device is reset.
> 
> The callers rely on xhci_handle_halted_endpoint() to take over the
> halted TD.  For example, finish_td() returns right after calling it,
> expecting the TD to be given back once the Reset Endpoint command
> completes.  With the early return, nobody gives the TD back.  Its URB
> stays pending until the submitter times out and unlinks it.  Every URB
> queued on the endpoint afterwards never starts, because the doorbell
> is blocked, and also has to time out.

Before 7.3 there was a similar device flag VDEV_PORT_ERROR which stuck
until the device is reset or deleted. It only applied to USB3 devices.
URBs were left hanging, but nobody cared, because disconnect() handler
of the class driver would unlink them.
 
> Devices that disconnect and reconnect themselves during enumeration hit
> this.  A PFU HHKB Professional HYBRID Type-S keyboard (04fe:0021) drops
> off the bus about 300 ms after its first enumeration, while usbhid is
> still probing its third interface.  The probe runs in hub_wq, so the
> disconnect is not handled until the probe returns.

But probe breaks this scheme, it seems we do have no choice but to stop
being lazy and give back the URB right there.

Regression occurred when the mechanism has been generalized to USB2 by
also looking at the PORTSC.CCS bit, even though the problems it tries
to solve have been unique to USB3 so far.

But this change seems sensible, because a USB3 port may autonomously
exit the Error state to Disconnected on receiver termination loss, see
xHCI 4.19.1.2.6, and PLS changes to RxDetect. It may even reach Enabled
before software intervention; seems we should look at PORTSC.CSC too.

(On a USB2 port, like this keyboard uses, such race doesn't exist).

And since xhci_irq() clears these per-port flags when USB3 link comes
back, there is no guarantee that the permanent EP_DROP_PENDING state
will be latched on the lost device. Whether the HW bugs caused by link
loss will still be happening on the "new" link is unclear; I only know
about Renesas: it doesn't mind and actually recovers on reconnection.
But the device is reset, which may or may not catch some xHC off guard.

Not sure if going back to a sticky per-device bit wouldn't be better.
One downside is that we need to flag all affected devices, which means
a loop in IRQ. Not great, but there is never more than 255 devices, so
maybe a few microseconds at worst. Likely not enough latency to break
anything, and it saves some repetitive checks in other places.

I could cook up a patch like that, I think it would make things easier,
as there is no need for those very similar checks in urb_enqueue() for
example, as seen below. But let's see what Mathias thinks.

> On a root port the HID descriptor request and its four retries in
> hid_get_class_descriptor() each wait for the full 5 second
> USB_CTRL_GET_TIMEOUT:
> 
>   [57685.006036] usb 3-2: new full-speed USB device number 24 using
> xhci_hcd ...
>   [57685.291304] hid-generic 0003:04FE:0021.0017: input,hidraw1: USB
> HID v1.11 Keyboard [PFU Limited HHKB-Hybrid] on
> usb-0000:00:14.0-2/input1 [57711.221083] usbhid 3-2:1.2: can't add
> hid device: -110 [57711.221613] usbhid 3-2:1.2: probe with driver
> usbhid failed with error -110 [57711.221842] usb 3-2: USB disconnect,
> device number 24
> 
> The re-enumeration that follows can stall the same way, leaving the
> keyboard dead until reboot.  Behind an external hub, where the root
> port stays connected, the same keyboard recovers in about 300 ms, as
> it did on the root port before these commits.
> 
> Keep skipping Reset Endpoint on a disconnected or inactive port, but
> give the halted TD back right away, with -EPROTO if no error status
> was set yet.  The endpoint is not restarted until the device is reset
> or dropped, so the ring dequeue pointer can stay where it is, as
> xhci_invalidate_cancelled_tds() already assumes for cancelled TDs on
> such endpoints.  Also refuse new URBs on an endpoint with
> EP_DROP_PENDING set, like xhci_urb_enqueue() already does for an
> inactive link, so they fail right away instead of timing out.
> 
> Fixes: 042aad8d0db6 ("xhci: prevent endpoint recovery after roothub
> disconnect") Fixes: 7e0ef4332ed9 ("xhci: avoid xHC endpoint changes
> after disconnect or link error.") Assisted-by: LLM
> Signed-off-by: Yiyu Zhou <yiyu@yiyuzhou.io>
> ---
> Tested on v7.3-rc6 on a System76 Galago Pro (Intel 8086:51ed xHCI)
> with the keyboard plugged directly into a root port, including twice
> right after resume from suspend.  On every hotplug the keyboard still
> dropped off during the probe of its third interface, and it was
> re-enumerated with all three interfaces bound about 0.3 s later.
> Before this patch, the same sequence hung usbhid for 26 s, and often
> left the keyboard dead until reboot.
> 
>  drivers/usb/host/xhci-ring.c | 24 ++++++++++++++++++++----
>  drivers/usb/host/xhci.c      |  5 +++++
>  2 files changed, 25 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/usb/host/xhci-ring.c
> b/drivers/usb/host/xhci-ring.c index ec278a9f9540..82190380831d 100644
> --- a/drivers/usb/host/xhci-ring.c
> +++ b/drivers/usb/host/xhci-ring.c
> @@ -979,6 +979,24 @@ static int xhci_reset_halted_ep(struct xhci_hcd
> *xhci, unsigned int slot_id, return ret;
>  }
>  
> +/*
> + * Mark the endpoint to be dropped instead of reset, and give back the halted
> + * TD now, as nobody else will.  The endpoint is not restarted before device
> + * reset or drop, so there is no need to move the ring dequeue past the TD.
> + */
> +static int xhci_drop_halted_endpoint(struct xhci_hcd *xhci,
> +				     struct xhci_virt_ep *ep,
> +				     struct xhci_td *td)
> +{
> +	ep->ep_state |= EP_DROP_PENDING;
> +
> +	if (td)
> +		xhci_td_cleanup(xhci, td, xhci_urb_to_transfer_ring(xhci, td->urb),
> +				td->status ?: -EPROTO);
> +
> +	return -ENODEV;
> +}

Not sure if this is big enough to justify a new function, and the
comment partly duplicates existing comment at the call site.

>  static int xhci_handle_halted_endpoint(struct xhci_hcd *xhci,
>  				struct xhci_virt_ep *ep,
>  				struct xhci_td *td,
> @@ -993,10 +1011,8 @@ static int xhci_handle_halted_endpoint(struct xhci_hcd *xhci,
>  	 * Can cause host hang.
>  	 * Device will be reset to recover an inactive link, so don't do anything */
> -	if (rhub_port->link_inactive || !rhub_port->connected) {
> -		ep->ep_state |= EP_DROP_PENDING;
> -		return -ENODEV;
> -	}
> +	if (rhub_port->link_inactive || !rhub_port->connected)
> +		return xhci_drop_halted_endpoint(xhci, ep, td);
>  
>  	/* add td to cancelled list and let reset ep handler take care of it */
>	if (reset_type == EP_HARD_RESET) {
> diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
> index a9e47e178c28..491a541b57b0 100644
> --- a/drivers/usb/host/xhci.c
> +++ b/drivers/usb/host/xhci.c
> @@ -1691,6 +1691,11 @@ static int xhci_urb_enqueue(struct usb_hcd
> *hcd, struct urb *urb, gfp_t mem_flag ret = -EINVAL;
>  		goto free_priv;
>  	}
> +	if (*ep_state & EP_DROP_PENDING) {
> +		xhci_dbg(xhci, "Can't queue urb, ep halted after disconnect or link error\n");
> +		ret = -ENODEV;
> +		goto free_priv;
> +	}

It would probably make more sense to log the conditions which set this
flag then every failed submission attempt after the flag is set.

This is the aforementioned "very similar check", because a few lines
prior we have the related test for rhub_port->link_inactive.

>  
>  	switch (usb_endpoint_type(&urb->ep->desc)) {
>  
> -- 
> 2.55.0
> 

Re: [PATCH] xhci: give back halted TD when skipping endpoint reset after disconnect

[Mathias Nyman]

On 10/12/26 11:41, Michal Pecio wrote:
> On Fri,  9 Oct 2026 23:42:19 -0700, Yiyu Zhou wrote:
>> Commit 042aad8d0db6 ("xhci: prevent endpoint recovery after roothub
>> disconnect") makes xhci_handle_halted_endpoint() return early when the
>> root port is disconnected, and commit 7e0ef4332ed9 ("xhci: avoid xHC
>> endpoint changes after disconnect or link error.") also sets
>> EP_DROP_PENDING there, which keeps the doorbell from being rung until
>> the device is reset.
>>
>> The callers rely on xhci_handle_halted_endpoint() to take over the
>> halted TD.  For example, finish_td() returns right after calling it,
>> expecting the TD to be given back once the Reset Endpoint command
>> completes.  With the early return, nobody gives the TD back.  Its URB
>> stays pending until the submitter times out and unlinks it.  Every URB
>> queued on the endpoint afterwards never starts, because the doorbell
>> is blocked, and also has to time out.
> 
> Before 7.3 there was a similar device flag VDEV_PORT_ERROR which stuck
> until the device is reset or deleted. It only applied to USB3 devices.
> URBs were left hanging, but nobody cared, because disconnect() handler
> of the class driver would unlink them.
>   
>> Devices that disconnect and reconnect themselves during enumeration hit
>> this.  A PFU HHKB Professional HYBRID Type-S keyboard (04fe:0021) drops
>> off the bus about 300 ms after its first enumeration, while usbhid is
>> still probing its third interface.  The probe runs in hub_wq, so the
>> disconnect is not handled until the probe returns.
> 
> But probe breaks this scheme, it seems we do have no choice but to stop
> being lazy and give back the URB right there.
> 
> Regression occurred when the mechanism has been generalized to USB2 by
> also looking at the PORTSC.CCS bit, even though the problems it tries
> to solve have been unique to USB3 so far.
> 
> But this change seems sensible, because a USB3 port may autonomously
> exit the Error state to Disconnected on receiver termination loss, see
> xHCI 4.19.1.2.6, and PLS changes to RxDetect. It may even reach Enabled
> before software intervention; seems we should look at PORTSC.CSC too.
> 
> (On a USB2 port, like this keyboard uses, such race doesn't exist).
> 
> And since xhci_irq() clears these per-port flags when USB3 link comes
> back, there is no guarantee that the permanent EP_DROP_PENDING state
> will be latched on the lost device. Whether the HW bugs caused by link
> loss will still be happening on the "new" link is unclear; I only know
> about Renesas: it doesn't mind and actually recovers on reconnection.
> But the device is reset, which may or may not catch some xHC off guard.
> 
> Not sure if going back to a sticky per-device bit wouldn't be better.
> One downside is that we need to flag all affected devices, which means
> a loop in IRQ. Not great, but there is never more than 255 devices, so
> maybe a few microseconds at worst. Likely not enough latency to break
> anything, and it saves some repetitive checks in other places.
> 
> I could cook up a patch like that, I think it would make things easier,
> as there is no need for those very similar checks in urb_enqueue() for
> example, as seen below. But let's see what Mathias thinks.
> 

The solution in this patch seems reasonable to me.
It's simple enough and should solve this, and similar cases.

We should probably give back all TDs of the halted endpoint,
not just the one we halted on.

Agree with the minor changes you suggested to this patch.

-Mathias

[PATCH v2] xhci: give back all TDs when skipping endpoint reset after disconnect

[Yiyu Zhou]

Commit 042aad8d0db6 ("xhci: prevent endpoint recovery after roothub
disconnect") makes xhci_handle_halted_endpoint() return early when the
root port is disconnected, and commit 7e0ef4332ed9 ("xhci: avoid xHC
endpoint changes after disconnect or link error.") also sets
EP_DROP_PENDING there, which keeps the doorbell from being rung until
the device is reset.

The callers rely on xhci_handle_halted_endpoint() to take over the
halted TD.  For example, finish_td() returns right after calling it,
expecting the TD to be given back once the Reset Endpoint command
completes.  With the early return, nobody gives the TD back.  Its URB
stays pending until the submitter times out and unlinks it.  URBs
queued behind it are stuck the same way, and every URB queued on the
endpoint afterwards never starts, because the doorbell is blocked, and
also has to time out.

Devices that disconnect and reconnect themselves during enumeration hit
this.  A PFU HHKB Professional HYBRID Type-S keyboard (04fe:0021) drops
off the bus about 300 ms after its first enumeration, while usbhid is
still probing its third interface.  The probe runs in hub_wq, so the
disconnect is not handled until the probe returns.  On a root port the
HID descriptor request and its four retries in hid_get_class_descriptor()
each wait for the full 5 second USB_CTRL_GET_TIMEOUT:

  [57685.006036] usb 3-2: new full-speed USB device number 24 using xhci_hcd
  ...
  [57685.291304] hid-generic 0003:04FE:0021.0017: input,hidraw1: USB HID v1.11 Keyboard [PFU Limited HHKB-Hybrid] on usb-0000:00:14.0-2/input1
  [57711.221083] usbhid 3-2:1.2: can't add hid device: -110
  [57711.221613] usbhid 3-2:1.2: probe with driver usbhid failed with error -110
  [57711.221842] usb 3-2: USB disconnect, device number 24

The re-enumeration that follows can stall the same way, leaving the
keyboard dead until reboot.  Behind an external hub, where the root
port stays connected, the same keyboard recovers in about 300 ms, as it
did on the root port before these commits.

Keep skipping Reset Endpoint on a disconnected or inactive port, but
give back the halted TD right away, with -EPROTO if no error status was
set yet, and all other TDs on the endpoint with -ESHUTDOWN.  The
endpoint is not restarted until the device is reset or dropped, so the
ring dequeue pointer can stay where it is, as
xhci_invalidate_cancelled_tds() already assumes for cancelled TDs on
such endpoints.  Also refuse new URBs on an endpoint with
EP_DROP_PENDING set, like xhci_urb_enqueue() already does for an
inactive link, so they fail right away instead of timing out.

Fixes: 042aad8d0db6 ("xhci: prevent endpoint recovery after roothub disconnect")
Fixes: 7e0ef4332ed9 ("xhci: avoid xHC endpoint changes after disconnect or link error.")
Closes: https://lore.kernel.org/all/20261010071811.20483-1-yiyu@yiyuzhou.io/
Assisted-by: LLM
Signed-off-by: Yiyu Zhou <yiyu@yiyuzhou.io>
---
v2:
- Give back all TDs on the halted endpoint, not just the halted one
  (Mathias)
- Open code the helper, and log when the endpoint is flagged instead of
  on every rejected URB (Michal)
- Add a Closes: tag for regzbot

v1: https://lore.kernel.org/linux-usb/20261010064407.11030-1-yiyu@yiyuzhou.io/

Tested on v7.3-rc6 on a System76 Galago Pro (Intel 8086:51ed xHCI).  The
keyboard was plugged directly into two root ports three times, each time
right after resume from suspend.  Each time it dropped off during probe
and was re-enumerated with all three interfaces bound about 0.3 s later,
with no timeouts.  Unplugging a dock with six devices behind one root
port also went through this path on 12 halted endpoints with no errors.

 drivers/usb/host/xhci-ring.c | 10 +++++++++-
 drivers/usb/host/xhci.c      |  4 ++++
 2 files changed, 13 insertions(+), 1 deletion(-)

diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c
index ec278a9f9540..2463d57ff705 100644
--- a/drivers/usb/host/xhci-ring.c
+++ b/drivers/usb/host/xhci-ring.c
@@ -65,6 +65,7 @@
 static int queue_command(struct xhci_hcd *xhci, struct xhci_command *cmd,
 			 u32 field1, u32 field2,
 			 u32 field3, u32 field4, bool command_must_succeed);
+static void xhci_kill_endpoint_urbs(struct xhci_hcd *xhci, int slot_id, int ep_index);
 
 /*
  * Returns zero if the TRB isn't in this segment, otherwise it returns the DMA
@@ -991,10 +992,17 @@ static int xhci_handle_halted_endpoint(struct xhci_hcd *xhci,
 	/*
 	 * Avoid resetting endpoint if link is inactive or device disonnected.
 	 * Can cause host hang.
-	 * Device will be reset to recover an inactive link, so don't do anything
+	 * The endpoint won't restart before the device is reset or the endpoint
+	 * dropped, so give back all its TDs now instead of letting them time out.
 	 */
 	if (rhub_port->link_inactive || !rhub_port->connected) {
+		xhci_dbg(xhci, "Not resetting ep %u slot %u after disconnect or link error\n",
+			 ep->ep_index, slot_id);
 		ep->ep_state |= EP_DROP_PENDING;
+		if (td)
+			xhci_td_cleanup(xhci, td, xhci_urb_to_transfer_ring(xhci, td->urb),
+					td->status ?: -EPROTO);
+		xhci_kill_endpoint_urbs(xhci, slot_id, ep->ep_index);
 		return -ENODEV;
 	}
 
diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
index a9e47e178c28..41ff45f5f909 100644
--- a/drivers/usb/host/xhci.c
+++ b/drivers/usb/host/xhci.c
@@ -1691,6 +1691,10 @@ static int xhci_urb_enqueue(struct usb_hcd *hcd, struct urb *urb, gfp_t mem_flag
 		ret = -EINVAL;
 		goto free_priv;
 	}
+	if (*ep_state & EP_DROP_PENDING) {
+		ret = -ENODEV;
+		goto free_priv;
+	}
 
 	switch (usb_endpoint_type(&urb->ep->desc)) {
 
-- 
2.55.0

Re: [PATCH v2] xhci: give back all TDs when skipping endpoint reset after disconnect

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] usb: xhci: memory leak of bounce buffer DMA mappings on cancelled TDs
--

commit 5b2d9318e9ae6d6a16a3494c205c16c0cd1ca311
Author: Yiyu Zhou <yiyu@yiyuzhou.io>

xhci: give back all TDs when skipping endpoint reset after disconnect

This commit modifies xhci_handle_halted_endpoint() to immediately give back
all TDs on a halted endpoint if the port is disconnected or the link is
inactive. It calls xhci_kill_endpoint_urbs() to prevent pending URBs from
stalling until they time out.

Closes: https://lore.kernel.org/all/20261010071811.20483-1-yiyu@yiyuzhou.io/

> diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c
> index ec278a9f95404..2463d57ff705c 100644
> --- a/drivers/usb/host/xhci-ring.c
> +++ b/drivers/usb/host/xhci-ring.c
[ ... ]
> @@ -991,10 +992,17 @@ static int xhci_handle_halted_endpoint(struct xhci_hcd *xhci,
>  	/*
>  	 * Avoid resetting endpoint if link is inactive or device disonnected.
>  	 * Can cause host hang.
> -	 * Device will be reset to recover an inactive link, so don't do anything
> +	 * The endpoint won't restart before the device is reset or the endpoint
> +	 * dropped, so give back all its TDs now instead of letting them time out.
>  	 */
>  	if (rhub_port->link_inactive || !rhub_port->connected) {
> +		xhci_dbg(xhci, "Not resetting ep %u slot %u after disconnect or link error\n",
> +			 ep->ep_index, slot_id);
>  		ep->ep_state |= EP_DROP_PENDING;
> +		if (td)
> +			xhci_td_cleanup(xhci, td, xhci_urb_to_transfer_ring(xhci, td->urb),
> +					td->status ?: -EPROTO);
> +		xhci_kill_endpoint_urbs(xhci, slot_id, ep->ep_index);

[Severity: Medium]
Can this newly added call to xhci_kill_endpoint_urbs() leak bounce buffer
DMA mappings?

When xhci_kill_endpoint_urbs() iterates over ep->cancelled_td_list, it calls
xhci_giveback_urb_in_irq() without unmapping bounce buffers:

drivers/usb/host/xhci-ring.c:xhci_kill_endpoint_urbs() {
    ...
	list_for_each_entry_safe(cur_td, tmp, &ep->cancelled_td_list,
			cancelled_td_list) {
		list_del_init(&cur_td->cancelled_td_list);
		inc_td_cnt(cur_td->urb);

		if (last_td_in_urb(cur_td))
			xhci_giveback_urb_in_irq(xhci, cur_td, -ESHUTDOWN);
	}
}

TDs that were unlinked by xhci_invalidate_cancelled_tds() are removed from
ring->td_list but remain on ep->cancelled_td_list. If the device disconnects
before they are processed by xhci_giveback_invalidated_tds(), this new call
to xhci_kill_endpoint_urbs() will process them.

Since xhci_kill_ring_urbs() only checks ring->td_list, and this second loop
in xhci_kill_endpoint_urbs() omits the call to xhci_unmap_td_bounce_buffer(),
it seems their bounce buffer DMA mappings will be leaked during normal runtime
device disconnects.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261013073825.17962-1-yiyu@yiyuzhou.io?part=1