From: Liu Chao 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@vger.kernel.org Reviewed-by: Weibin Liu Signed-off-by: Liu Chao --- 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