Re: [PATCH v2] usb: dwc3: gadget: don't error on dequeue of a completed request

[Greg Kroah-Hartman]

On Fri, Sep 04, 2026 at 04:58:57PM +0000, Cole Munz wrote:
> Dequeuing a request that has already been given back logs an error and
> returns -EINVAL:
> 
>   dwc3 23000000.usb: request 00000000ad92f1c4 was not queued to ep0out
> 
> f_fs hits this on every teardown. functionfs_unbind() dequeues ep0req
> unconditionally before freeing it, which
> commit ce405d561b02 ("usb: gadget: f_fs: Ensure ep0req is dequeued
> before free_request") made deliberate to close a use-after-free. By then
> the control transfer has long completed, so dwc3_gadget_ep_dequeue()
> finds the request on none of cancelled_list, pending_list or
> started_list and falls through to the error path.
> 
> Nothing is actually wrong. The request is not queued, which is what the
> caller asked for, and both callers ignore the return value and free the
> request straight after. The only effect is an error line in every gadget
> teardown, which buries real USB errors.
> 
> dwc3 already tracks enough to tell the two cases apart.
> dwc3_gadget_ep_alloc_request() sets DWC3_REQUEST_STATUS_UNKNOWN, both
> __dwc3_gadget_ep_queue() and __dwc3_gadget_ep0_queue() set
> DWC3_REQUEST_STATUS_QUEUED, and dwc3_gadget_giveback() sets
> DWC3_REQUEST_STATUS_COMPLETED. A request that reaches the end of dequeue
> with status COMPLETED was queued to this endpoint and has finished.
> Anything else was never queued here, or the driver lost track of it.
> Keep the error for those, and return success for a completed request.
> 
> A completed request still has to be dequeued on the endpoint it belongs
> to. req->dep is set once at allocation and never changes, and
> __dwc3_gadget_ep_queue() rejects the same mismatch with a WARN, so a
> wrong-endpoint dequeue stays on the error path here as well.
> 
> This is narrower than the cdnsp fix for the same caller,
> commit 34f08eb0ba6e ("usb: cdnsp: Fixes issue with dequeuing not queued
> requests"), which returns 0 whenever usb_request::status is not
> -EINPROGRESS. That also swallows a request that was never queued, since
> status is zero out of allocation. Going by dwc3's own request status
> keeps that case an error, which is what was asked for when a separate
> ep0 dequeue was proposed in 2022.
> 
> Fixes: 72246da40f37 ("usb: Introduce DesignWare USB3 DRD Driver")
> Cc: stable@xxxxxxxxxxxxxxx
> Link: https://lore.kernel.org/linux-usb/20221117054917.30104-1-quic_ugoswami@xxxxxxxxxxx/
> Assisted-by: LLM sparse
> Signed-off-by: Cole Munz <Munzzyy1@xxxxxxxxx>
> ---
> All three were missing, sorry about that.
> 
> v2: add Fixes:, Cc: stable and Assisted-by tags. No code change.
> 
>  drivers/usb/dwc3/gadget.c | 19 ++++++++++++++++---
>  1 file changed, 16 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
> index fa944856f956..f68eb9254afa 100644
> --- a/drivers/usb/dwc3/gadget.c
> +++ b/drivers/usb/dwc3/gadget.c
> @@ -2181,9 +2181,22 @@ static int dwc3_gadget_ep_dequeue(struct usb_ep *ep,
>  		}
>  	}
>  
> -	dev_err(dwc->dev, "request %p was not queued to %s\n",
> -		request, ep->name);
> -	ret = -EINVAL;
> +	/*
> +	 * The request is on none of this endpoint's lists. That is the
> +	 * expected state once it has been given back: a function may dequeue
> +	 * a request before freeing it, and f_fs does so unconditionally for
> +	 * ep0req in functionfs_unbind(). Nothing is queued, which is what the
> +	 * caller asked for, so report success. Any other status means the
> +	 * request was never queued here or the driver lost track of it, and
> +	 * stays an error. So does a completed request handed to the wrong
> +	 * endpoint: req->dep is fixed at allocation, and the queue side
> +	 * rejects the same mismatch.
> +	 */

LLMs love to add long comments like this that make almost no sense.
Please rewrite this in a human voice, being very concise, if you still
feel a comment is needed.

thanks,

greg k-h