Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
From:   Krishna Kurapati <quic_kriskura@quicinc.com>
To:     Thinh Nguyen <Thinh.Nguyen@synopsys.com>,
        Greg Kroah-Hartman <gregkh@linuxfoundation.org>
CC:     <linux-usb@vger.kernel.org>, <linux-kernel@vger.kernel.org>,
        <quic_ppratap@quicinc.com>, <quic_wcheng@quicinc.com>,
        <quic_jackp@quicinc.com>, <quic_ugoswami@quicinc.com>,
        Krishna Kurapati <quic_kriskura@quicinc.com>
Subject: [RFC] usb: dwc3: gadget: Fix amount of data copied from event buf to cache
Date:   Sun, 21 May 2023 15:33:30 +0530
Message-ID: <20230521100330.22478-1-quic_kriskura@quicinc.com>
MIME-Version: 1.0
Content-Transfer-Encoding: 8bit
Content-Type: text/plain
Precedence: bulk

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 limiting the total data being copied to ev->length to
avoid copying data beyond bounds. Moreover this is logical because if
the controller generated events more than the size of ring buffer,
some of them might have been overwritten by the controller.

Signed-off-by: Krishna Kurapati <quic_kriskura@quicinc.com>
---
Only one instance of this crash was observed so far. As per the
databook:

"The controller always leaves one entry free in each Event Buffer.
When the Event Buffer is almost full, hardware writes the Event
Buffer Overflow event and the USB eventually gets stalled when
endpoints start responding NRDY or the link layer stops returning
credits (in SuperSpeed). This event is an indication to software that
it is not processing events quickly enough. During this time, events
are queued up internally. When software frees up Event Buffer space,
the queued up events are written out and the USB returns to normal
operation"

I didn't see any overflow event in the event buffer after parsing
crash dump. Although this could be some HW issue, I thought we can
include this fix in software as well to avoid such scenario.

 drivers/usb/dwc3/gadget.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
index 5965796bc5d5..cff043ab9017 100644
--- a/drivers/usb/dwc3/gadget.c
+++ b/drivers/usb/dwc3/gadget.c
@@ -4457,6 +4457,9 @@ static irqreturn_t dwc3_check_event_buf(struct dwc3_event_buffer *evt)
 	if (!count)
 		return IRQ_NONE;
 
+	if (count > evt->length)
+		count  = evt->length;
+
 	evt->count = count;
 	evt->flags |= DWC3_EVENT_PENDING;
 
-- 
2.40.0