[PATCH] usb: gadget: lpc32xx_udc: handle interrupts with an empty queue

[PATCH] usb: gadget: lpc32xx_udc: handle interrupts with an empty queue

[Fang Xieyan]

A request can be dequeued after a DMA transfer has completed and filled
both endpoint buffers, while the delayed endpoint or DMA interrupt is
still pending.  lpc32xx_ep_dequeue() removes the request from ep->queue
and gives it back via done() without clearing the pending endpoint or
DMA interrupt status.

When the interrupt handler then runs with an empty queue,
list_entry(ep->queue.next, ...) derives an invalid request pointer from
the list head; list_entry() does not return NULL.  Passing that pointer
to done() in udc_handle_eps(), or dereferencing req->dd_desc_ptr in
udc_handle_dma_ep(), results in invalid memory accesses and can crash
the kernel.

Check whether the request queue is empty before touching the request in
both handlers.  In udc_handle_eps(), do this before sending the deferred
ZLP and disable the endpoint interrupt when there is nothing to
complete.  In udc_handle_dma_ep(), still disable the DMA channel and
clear the pending EOT/NDDRT (and SYSERR) interrupt status so the
interrupt does not keep firing, without dereferencing the missing
request.

Fixes: 24a28e428351 ("USB: gadget driver for LPC32xx")
Cc: stable@vger.kernel.org
Reviewed-by: Guanglei Zhu <zhugl3@xiaopeng.com>
Signed-off-by: Fang Xieyan <fangxy@xiaopeng.com>
---
 drivers/usb/gadget/udc/lpc32xx_udc.c | 25 ++++++++++++++++++++++++-
 1 file changed, 24 insertions(+), 1 deletion(-)

diff --git a/drivers/usb/gadget/udc/lpc32xx_udc.c b/drivers/usb/gadget/udc/lpc32xx_udc.c
index 044c31869cfb..67b0d6f73e43 100644
--- a/drivers/usb/gadget/udc/lpc32xx_udc.c
+++ b/drivers/usb/gadget/udc/lpc32xx_udc.c
@@ -1945,6 +1945,13 @@ static void udc_handle_eps(struct lpc32xx_udc *udc, struct lpc32xx_ep *ep)
 	/* Get endpoint status */
 	epstatus = udc_clearep_getsts(udc, ep->hwep_num);
 
+	/* If there isn't a request waiting, something went wrong */
+	if (list_empty(&ep->queue)) {
+		ep->req_pending = 0;
+		uda_disable_hwepint(udc, ep->hwep_num);
+		return;
+	}
+
 	/*
 	 * This should never happen, but protect against writing to the
 	 * buffer when full.
@@ -1958,7 +1965,6 @@ static void udc_handle_eps(struct lpc32xx_udc *udc, struct lpc32xx_ep *ep)
 	} else
 		return;
 
-	/* If there isn't a request waiting, something went wrong */
 	req = list_entry(ep->queue.next, struct lpc32xx_request, queue);
 
 	done(ep, req, 0);
@@ -1985,6 +1991,23 @@ static void udc_handle_dma_ep(struct lpc32xx_udc *udc, struct lpc32xx_ep *ep)
 	ep->totalints++;
 #endif
 
+	if (list_empty(&ep->queue)) {
+		u32 mask = 1U << ep->hwep_num;
+
+		ep->req_pending = 0;
+		ep_err(ep, "DMA interrupt on no req!\n");
+
+		/* Disable DMA and clear pending interrupts */
+		udc_ep_dma_disable(udc, ep->hwep_num);
+		writel(mask, USBD_EOTINTCLR(udc->udp_baseaddr));
+		writel(mask, USBD_NDDRTINTCLR(udc->udp_baseaddr));
+
+		if (readl(USBD_SYSERRTINTST(udc->udp_baseaddr)) & mask)
+			writel(mask, USBD_SYSERRTINTCLR(udc->udp_baseaddr));
+
+		return;
+	}
+
 	req = list_entry(ep->queue.next, struct lpc32xx_request, queue);
 	dd = req->dd_desc_ptr;
 
-- 
2.50.1

Re: [PATCH] usb: gadget: lpc32xx_udc: handle interrupts with an empty queue

[sashiko-bot]

> A request can be dequeued after a DMA transfer has completed and filled
> both endpoint buffers, while the delayed endpoint or DMA interrupt is
> still pending.  lpc32xx_ep_dequeue() removes the request from ep->queue
> and gives it back via done() without clearing the pending endpoint or
> DMA interrupt status.
> 
> When the interrupt handler then runs with an empty queue,
> list_entry(ep->queue.next, ...) derives an invalid request pointer from
> the list head; list_entry() does not return NULL.  Passing that pointer
> to done() in udc_handle_eps(), or dereferencing req->dd_desc_ptr in
> udc_handle_dma_ep(), results in invalid memory accesses and can crash
> the kernel.
> 
> Check whether the request queue is empty before touching the request in
> both handlers.  In udc_handle_eps(), do this before sending the deferred
> ZLP and disable the endpoint interrupt when there is nothing to
> complete.  In udc_handle_dma_ep(), still disable the DMA channel and
> clear the pending EOT/NDDRT (and SYSERR) interrupt status so the
> interrupt does not keep firing, without dereferencing the missing
> request.
> 
> Fixes: 24a28e428351 ("USB: gadget driver for LPC32xx")
> Cc: stable@vger.kernel.org
> Reviewed-by: Guanglei Zhu <zhugl3@xiaopeng.com>
> Signed-off-by: Fang Xieyan <fangxy@xiaopeng.com>

Sashiko has reviewed this patch and found no issues.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261109121035.34561-1-fangxy@xiaopeng.com?part=1