[PATCH v3] usb: gadget: aspeed_udc: avoid past-the-end iterator in dequeue
Maoyi Xie <[email protected]>
| Newsgroups | org.ozlabs.lists.linux-aspeed,org.infradead.lists.linux-arm-kernel,org.kernel.vger.linux-kernel,org.kernel.vger.linux-usb |
|---|---|
| Message-ID | <[email protected]> |
ast_udc_ep_dequeue() declares the loop cursor `req` outside the list_for_each_entry(). After the loop it tests `&req->req != _req` to decide whether the request was found. If the queue holds no match, `req` is past-the-end. It then aliases container_of(&ep->queue, struct ast_udc_request, queue) via offset cancellation. Whether that synthetic address equals `_req` depends on heap layout. The function can return 0 without dequeueing anything. Default `rc` to -EINVAL and set it to 0 only inside the match branch. `req` is no longer read after the loop, so the past-the-end dereference goes away. No extra cursor variable or post-loop test is needed. Suggested-by: Alan Stern <[email protected]> Suggested-by: Andrew Jeffery <[email protected]> Signed-off-by: Maoyi Xie <[email protected]> --- v3: Switch to Andrew Jeffery's shape: default rc to -EINVAL, set rc=0 inside the match branch, drop the post-loop check. Smaller diff, no extra cursor variable, no goto. Same semantic fix as v2. v2: https://lore.kernel.org/linux-usb/[email protected]/ v1: https://lore.kernel.org/linux-usb/[email protected]/ drivers/usb/gadget/udc/aspeed_udc.c | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/drivers/usb/gadget/udc/aspeed_udc.c b/drivers/usb/gadget/udc/aspeed_udc.c index 7fc6696b7694..75f9c831b21a 100644 --- a/drivers/usb/gadget/udc/aspeed_udc.c +++ b/drivers/usb/gadget/udc/aspeed_udc.c @@ -694,7 +694,7 @@ static int ast_udc_ep_dequeue(struct usb_ep *_ep, struct usb_request *_req) struct ast_udc_dev *udc = ep->udc; struct ast_udc_request *req; unsigned long flags; - int rc = 0; + int rc = -EINVAL; spin_lock_irqsave(&udc->lock, flags); @@ -704,14 +704,11 @@ static int ast_udc_ep_dequeue(struct usb_ep *_ep, struct usb_request *_req) list_del_init(&req->queue); ast_udc_done(ep, req, -ESHUTDOWN); _req->status = -ECONNRESET; + rc = 0; break; } } - /* dequeue request not found */ - if (&req->req != _req) - rc = -EINVAL; - spin_unlock_irqrestore(&udc->lock, flags); return rc; -- 2.34.1