[PATCH v2] usb: dwc3: gadget: Ignore dwc3 interrupt if GEVNTCOUNT reads corrupt value

From: Krishna Kurapati
Date: Thu Dec 21 2023 - 10:17:00 EST


In the current implementation, the check_event_buf call reads the
GEVNTCOUNT register to determine the amount of event data generated
and copies it from ev->buf to ev->cache after masking interrupt.

During copy if the amount of data to be copied is more than
(length - lpos), we fill the ev->cache till the end of 4096 byte
buffer allocated and then start filling from the top (lpos = 0).

In one instance of SMMU crash it is observed that GEVNTCOUNT register
reads more than 4096 bytes:

dwc3_readl base=0xffffffc0091dc000 offset=50188 value=63488

(offset = 50188 -> 0xC40C) -> reads 63488 bytes

As per crash dump:
dwc->lpos = 2056

and evt->buf is at 0xFFFFFFC009185000 and the crash is at
0xFFFFFFC009186000. The diff which is exactly 0x1000 bytes.

We first memcpy 2040 bytes from (buf + lpos) to (buf + 0x1000).

And then we copy the rest of the data (64388 - 2040) from beginning
of dwc->ev_buf. While doing so we go beyond bounds as we are trying
to memcpy 62348 bytes into a 4K buffer resulting in crash.

Fix this by ignoring the interrupt when GEVNTCOUNT register reads a
value more than the event ring allocated.

Signed-off-by: Krishna Kurapati <quic_kriskura@xxxxxxxxxxx>
---
Changes in v2:
Instead of fixing amount of data being copied from ring, ignored
the interrupt when count is corrupt as per suggestion from Thinh.

Link to v1:
https://lore.kernel.org/all/20230521100330.22478-1-quic_kriskura@xxxxxxxxxxx/

drivers/usb/dwc3/gadget.c | 10 +++++++++-
1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
index 858fe4c299b7..e27933fdcce3 100644
--- a/drivers/usb/dwc3/gadget.c
+++ b/drivers/usb/dwc3/gadget.c
@@ -4451,6 +4451,7 @@ static irqreturn_t dwc3_thread_interrupt(int irq, void *_evt)
static irqreturn_t dwc3_check_event_buf(struct dwc3_event_buffer *evt)
{
struct dwc3 *dwc = evt->dwc;
+ int ret = IRQ_WAKE_THREAD;
u32 amount;
u32 count;

@@ -4480,6 +4481,12 @@ static irqreturn_t dwc3_check_event_buf(struct dwc3_event_buffer *evt)
if (!count)
return IRQ_NONE;

+ if (count > evt->length) {
+ dev_err(dwc->dev, "GEVTCOUNT corrupt\n");
+ ret = IRQ_NONE;
+ goto done;
+ }
+
evt->count = count;
evt->flags |= DWC3_EVENT_PENDING;

@@ -4493,9 +4500,10 @@ static irqreturn_t dwc3_check_event_buf(struct dwc3_event_buffer *evt)
if (amount < count)
memcpy(evt->cache, evt->buf, count - amount);

+done:
dwc3_writel(dwc->regs, DWC3_GEVNTCOUNT(0), count);

- return IRQ_WAKE_THREAD;
+ return ret;
}

static irqreturn_t dwc3_interrupt(int irq, void *_evt)
--
2.42.0