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

[sashiko-bot@kernel.org]

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