Re: [PATCH v2] usb: free iso schedules on failed submit

[Alan Stern]

On Tue, Jun 30, 2026 at 03:14:19PM +0800, Dawei Feng wrote:
> EHCI and FOTG210 isochronous submits build an ehci_iso_sched before
> linking the URB to the endpoint queue, and keep the staged schedule in
> urb->hcpriv until iso_stream_schedule() and the link helpers consume it.
> If the controller is no longer accessible, or usb_hcd_link_urb_to_ep()
> fails, submit jumps to done_not_linked before that handoff happens and
> leaks the staged schedule still attached to urb->hcpriv.
> 
> Free the staged schedule from done_not_linked when submit fails before
> the URB is linked and clear urb->hcpriv after the free.
> 
> The bug was first flagged by an experimental analysis tool we are
> developing for kernel memory-management bugs while analyzing
> v6.13-rc1. The tool is still under development and is not yet publicly
> available. Manual inspection confirms that the bug is still
> present in v7.1.1.
> 
> An x86_64 allyesconfig build showed no new warnings. As we do not have an
> EHCI host controller with a USB isochronous device to test with, no
> runtime testing was able to be performed.
> 
> Fixes: 8de98402652c ("[PATCH] USB: Fix USB suspend/resume crasher (#2)")
> Fixes: e9df41c5c589 ("USB: make HCDs responsible for managing endpoint queues")
> Fixes: 7d50195f6c50 ("usb: host: Faraday fotg210-hcd driver")
> Cc: stable@xxxxxxxxxxxxxxx
> Signed-off-by: Dawei Feng <dawei.feng@xxxxxxxxxx>
> ---
> Changes in v2:
> - Move negative iso_stream_schedule() cleanup to the submit failure path.
> - Clear urb->hcpriv after iso_stream_schedule() frees the schedule for an
>   immediately completed EHCI URB.

For the ehci-sched portion:

Reviewed-by: Alan Stern <stern@xxxxxxxxxxxxxxxxxxx>

>  drivers/usb/fotg210/fotg210-hcd.c |  6 ++++--
>  drivers/usb/host/ehci-sched.c     | 11 +++++++++--
>  2 files changed, 13 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/usb/fotg210/fotg210-hcd.c b/drivers/usb/fotg210/fotg210-hcd.c
> index 1a48329a4e08..956be5b56510 100644
> --- a/drivers/usb/fotg210/fotg210-hcd.c
> +++ b/drivers/usb/fotg210/fotg210-hcd.c
> @@ -4267,8 +4267,6 @@ static int iso_stream_schedule(struct fotg210_hcd *fotg210, struct urb *urb,
>  	return 0;
>  
>  fail:
> -	iso_sched_free(stream, sched);
> -	urb->hcpriv = NULL;
>  	return status;
>  }
>  
> @@ -4562,6 +4560,10 @@ static int itd_submit(struct fotg210_hcd *fotg210, struct urb *urb,
>  	else
>  		usb_hcd_unlink_urb_from_ep(fotg210_to_hcd(fotg210), urb);
>  done_not_linked:
> +	if (status < 0) {
> +		iso_sched_free(stream, urb->hcpriv);
> +		urb->hcpriv = NULL;
> +	}
>  	spin_unlock_irqrestore(&fotg210->lock, flags);
>  done:
>  	return status;
> diff --git a/drivers/usb/host/ehci-sched.c b/drivers/usb/host/ehci-sched.c
> index a241337c9af8..57d07d1c2dfa 100644
> --- a/drivers/usb/host/ehci-sched.c
> +++ b/drivers/usb/host/ehci-sched.c
> @@ -1623,6 +1623,7 @@ iso_stream_schedule(
>  			status = 1;	/* and give it back immediately */
>  			iso_sched_free(stream, sched);
>  			sched = NULL;
> +			urb->hcpriv = NULL;
>  		}
>  	}
>  	urb->error_count = skip / period;
> @@ -1653,8 +1654,6 @@ iso_stream_schedule(
>  	return status;
>  
>   fail:
> -	iso_sched_free(stream, sched);
> -	urb->hcpriv = NULL;
>  	return status;
>  }
>  
> @@ -1966,6 +1965,10 @@ static int itd_submit(struct ehci_hcd *ehci, struct urb *urb,
>  		usb_hcd_unlink_urb_from_ep(ehci_to_hcd(ehci), urb);
>  	}
>   done_not_linked:
> +	if (status < 0) {
> +		iso_sched_free(stream, urb->hcpriv);
> +		urb->hcpriv = NULL;
> +	}
>  	spin_unlock_irqrestore(&ehci->lock, flags);
>   done:
>  	return status;
> @@ -2343,6 +2346,10 @@ static int sitd_submit(struct ehci_hcd *ehci, struct urb *urb,
>  		usb_hcd_unlink_urb_from_ep(ehci_to_hcd(ehci), urb);
>  	}
>   done_not_linked:
> +	if (status < 0) {
> +		iso_sched_free(stream, urb->hcpriv);
> +		urb->hcpriv = NULL;
> +	}
>  	spin_unlock_irqrestore(&ehci->lock, flags);
>   done:
>  	return status;
> -- 
> 2.34.1