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

[Cole Munz]

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.

Link: https://lore.kernel.org/linux-usb/20221117054917.30104-1-quic_ugoswami@xxxxxxxxxxx/
Signed-off-by: Cole Munz <Munzzyy1@xxxxxxxxx>
---
 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 1082e9c9afaa..e6c13f29f705 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.
+	 */
+	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);
 

base-commit: 8d3ae59288f1e7d58d76558a6ee96d533bc5019f
-- 
2.55.0