[PATCH] usb: gadget: bcm63xx_udc: fix dequeue of unqueued request

[liuc63]

From: Liu Chao <liuc63@xxxxxxxxxxxx>

bcm63xx_udc_dequeue() only checks whether bep->queue is empty; it never
verifies that the passed request is actually linked on that queue.  If
the request was already completed by the data ISR (which list_del's it)
or belongs to another endpoint, list_del() corrupts an unrelated list
node.  Worse, the out: path unconditionally sets req->status and calls
req->complete() even when rc == -EINVAL, so a request that was never
queued (or was already completed and possibly freed by the gadget
driver) gets completed a second time, leading to a double completion
and use-after-free.

Walk bep->queue to confirm the request is queued before touching it,
and only invoke the completion callback when the request was actually
removed from the queue.  The -EINVAL path now returns without
completing.

Fixes: 613065e53cb1 ("usb: gadget: bcm63xx UDC driver")
Cc: stable@xxxxxxxxxxxxxxx
Reviewed-by: Weibin Liu <liuwb@xxxxxxxxxxxx>
Signed-off-by: Liu Chao <liuc63@xxxxxxxxxxxx>
---
 drivers/usb/gadget/udc/bcm63xx_udc.c | 18 +++++++++++++-----
 1 file changed, 13 insertions(+), 5 deletions(-)

diff --git a/drivers/usb/gadget/udc/bcm63xx_udc.c b/drivers/usb/gadget/udc/bcm63xx_udc.c
index 502612a56..a27dc7df7 100644
--- a/drivers/usb/gadget/udc/bcm63xx_udc.c
+++ b/drivers/usb/gadget/udc/bcm63xx_udc.c
@@ -1209,15 +1209,20 @@ static int bcm63xx_udc_dequeue(struct usb_ep *ep, struct usb_request *req)
 	int rc = 0;
 
 	spin_lock_irqsave(&udc->lock, flags);
-	if (list_empty(&bep->queue)) {
+
+	/* Make sure the request is actually queued on this endpoint */
+	list_for_each_entry(cur, &bep->queue, queue) {
+		if (cur == breq)
+			break;
+	}
+	if (&cur->queue == &bep->queue) {
 		rc = -EINVAL;
 		goto out;
 	}
 
-	cur = list_first_entry(&bep->queue, struct bcm63xx_req, queue);
 	usb_gadget_unmap_request(&udc->gadget, &breq->req, bep->iudma->is_tx);
 
-	if (breq == cur) {
+	if (breq == list_first_entry(&bep->queue, struct bcm63xx_req, queue)) {
 		iudma_reset_channel(udc, bep->iudma);
 		list_del(&breq->queue);
 
@@ -1235,8 +1240,11 @@ static int bcm63xx_udc_dequeue(struct usb_ep *ep, struct usb_request *req)
 out:
 	spin_unlock_irqrestore(&udc->lock, flags);
 
-	req->status = -ESHUTDOWN;
-	req->complete(ep, req);
+	/* Only complete the request if it was actually removed */
+	if (rc == 0) {
+		req->status = -ESHUTDOWN;
+		req->complete(ep, req);
+	}
 
 	return rc;
 }
--
2.50.1