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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [thread-next>] [day] [month] [year] [list]

Message-ID: <20231221151620.16030-1-quic_kriskura@quicinc.com>
Date: Thu, 21 Dec 2023 20:46:20 +0530
From: Krishna Kurapati <quic_kriskura@...cinc.com>
To: Thinh Nguyen <Thinh.Nguyen@...opsys.com>,
        Greg Kroah-Hartman
	<gregkh@...uxfoundation.org>
CC: <linux-kernel@...r.kernel.org>, <linux-usb@...r.kernel.org>,
        <quic_ppratap@...cinc.com>, <quic_jackp@...cinc.com>,
        Krishna Kurapati
	<quic_kriskura@...cinc.com>
Subject: [PATCH v2] usb: dwc3: gadget: Ignore dwc3 interrupt if GEVNTCOUNT reads corrupt value

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@...cinc.com>
---
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@quicinc.com/

 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