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

[Thinh Nguyen <Thinh.Nguyen@synopsys.com>]

On Fri, Sep 04, 2026, 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@vger.kernel.org
> Link: https://urldefense.com/v3/__https://lore.kernel.org/linux-usb/20221117054917.30104-1-quic_ugoswami@quicinc.com/__;!!A4F2R9G_pg!exBSfbxMvn0256JnM9YGZHRuDHPfgf1X4grkdaDDfYGqfbWTc9oBZslv4KvNpjA_wmWM9R9rXsh508WX1YaBoJs$ 
> Assisted-by: LLM sparse
> Signed-off-by: Cole Munz <Munzzyy1@proton.me>
> ---
> v3: cut the block comment down to one line.
> v2: add Fixes:, Cc: stable and Assisted-by tags.
> 
>  drivers/usb/dwc3/gadget.c | 9 ++++++---
>  1 file changed, 6 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
> index fa944856f956..9ff6a733d3c5 100644
> --- a/drivers/usb/dwc3/gadget.c
> +++ b/drivers/usb/dwc3/gadget.c
> @@ -2181,9 +2181,12 @@ 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;
> +	/* Dequeuing a completed request is a no-op, not an error. */
> +	if (req->status != DWC3_REQUEST_STATUS_COMPLETED || req->dep != dep) {
> +		dev_err(dwc->dev, "request %p was not queued to %s\n",
> +			request, ep->name);
> +		ret = -EINVAL;
> +	}
>  out:
>  	spin_unlock_irqrestore(&dwc->lock, flags);
>  
> -- 
> 2.55.0
> 
> 

NAK.

This is not a fix. This changes the dequeue() behavior. You're breaking
the documented behavior of usb_ep_dequeue():

	If the request is still active on the endpoint, it is dequeued and
	eventually its completion routine is called (with status -ECONNRESET);
	else a negative error code is returned.  This routine is asynchronous,
	that is, it may return before the completion routine runs.

BR,
Thinh