usb: dwc3: Fix out of bound memory access for event buffer
The commit 49c45e0b44f4 ("usb: dwc3: gadget: Clear
pending events when stopping controller") added a
race of writing to the GEVNTCOUNT between the run_stop
and the dwc3_check_event_buf. This causes the
GEVNTCOUNT to be decremented below zero by the controller
and is resulting in a huge values(0xFFFC) which is much
larger than the event buffer size(0x1000).
When this happens the next dwc3_interrupt will be accessing
the next page after the event buffer resulting in a
memory abort.
Fix this by discarding any interrupts that are fired
after the run_stop bit is cleared. And also compare
the count value with the event buffer length to
prevent out of bound memory access.
The earlier commit still leaves a window of an event being
generated by the controller between clearing the pending
events and clearing the run_stop bit preventing the controller
from being halted. Fix this by clearing the pending events
after the run_stop bit is cleared.
Change-Id: Ic5244485dc1af728848f40c45f920a6a6f880ac2
Signed-off-by: Sriharsha Allenki <sallenki@codeaurora.org>
diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
index 281fb40..2426ea9 100644
--- a/drivers/usb/dwc3/gadget.c
+++ b/drivers/usb/dwc3/gadget.c
@@ -2098,16 +2098,6 @@ static int dwc3_gadget_run_stop(struct dwc3 *dwc, int is_on, int suspend)
*/
dwc3_stop_active_transfers(dwc);
- /*
- * Clear out any pending events (i.e. End Transfer Command
- * Complete) before clearing run/stop
- */
- reg1 = dwc3_readl(dwc->regs, DWC3_GEVNTCOUNT(0));
- reg1 &= DWC3_GEVNTCOUNT_MASK;
- dbg_log_string("remaining EVNTCOUNT(0)=%d", reg1);
- dwc3_writel(dwc->regs, DWC3_GEVNTCOUNT(0), reg1);
- dwc3_notify_event(dwc, DWC3_GSI_EVT_BUF_CLEAR, 0);
-
reg &= ~DWC3_DCTL_RUN_STOP;
if (dwc->has_hibernation && !suspend)
@@ -2116,6 +2106,19 @@ static int dwc3_gadget_run_stop(struct dwc3 *dwc, int is_on, int suspend)
dwc3_writel(dwc->regs, DWC3_DCTL, reg);
+ /* Controller is not halted until the events are acknowledged */
+ if (!is_on) {
+ /*
+ * Clear out any pending events (i.e. End Transfer Command
+ * Complete).
+ */
+ reg1 = dwc3_readl(dwc->regs, DWC3_GEVNTCOUNT(0));
+ reg1 &= DWC3_GEVNTCOUNT_MASK;
+ dbg_log_string("remaining EVNTCOUNT(0)=%d", reg1);
+ dwc3_writel(dwc->regs, DWC3_GEVNTCOUNT(0), reg1);
+ dwc3_notify_event(dwc, DWC3_GSI_EVT_BUF_CLEAR, 0);
+ }
+
do {
reg = dwc3_readl(dwc->regs, DWC3_DSTS);
reg &= DWC3_DSTS_DEVCTRLHLT;
@@ -3748,6 +3751,12 @@ static irqreturn_t dwc3_check_event_buf(struct dwc3_event_buffer *evt)
if (dwc->err_evt_seen)
return IRQ_HANDLED;
+ /* Controller is being halted, ignore the interrupts */
+ if (!dwc->pullups_connected) {
+ dbg_event(0xFF, "NO_PULLUP", 0);
+ return IRQ_HANDLED;
+ }
+
/*
* With PCIe legacy interrupt, test shows that top-half irq handler can
* be called again after HW interrupt deassertion. Check if bottom-half
@@ -3762,6 +3771,19 @@ static irqreturn_t dwc3_check_event_buf(struct dwc3_event_buffer *evt)
if (!count)
return IRQ_NONE;
+ if (count > evt->length) {
+ dbg_event(0xFF, "HUGE_EVCNT", count);
+ /*
+ * If writes from dwc3_interrupt and run_stop(0) races
+ * with each other, the count can result in a very large
+ * value.In that case setting the evt->lpos here
+ * is a no-op. The value will be reset as part of run_stop(1).
+ */
+ evt->lpos = (evt->lpos + count) % DWC3_EVENT_BUFFERS_SIZE;
+ dwc3_writel(dwc->regs, DWC3_GEVNTCOUNT(0), count);
+ return IRQ_HANDLED;
+ }
+
evt->count = count;
evt->flags |= DWC3_EVENT_PENDING;
