From 5f7bf07733f8b777300f38da0d263b812dd616bf Mon Sep 17 00:00:00 2001 From: Sriharsha Allenki Date: Wed, 24 Feb 2021 11:51:23 +0530 Subject: [PATCH] usb: dwc3: gadget: Prevent queue on endpoints after bus reset There is a race between ep_queue from function driver and ep_disable during function disable as part of bus reset. This race is leading to a scenario where the endpoint is disabled with an active transfer. And when the endpoint is enabled back, this leads to a stale buffer access by the controller. The race is as below: qdss_disable usb_qdss_write | -----> Waits on qdss_lock | ----> Acquires qdss_lock | | ----> Checks for qdss_connected | | ----> Releases qdss_lock | ------> Acquires qdss_lock | | ------> Clears usb_connected | | ------> Releases qdss_lock | usb_ep_disable | | ------> Acquires dwc->lock | dwc3_remove_reuests | | | dwc3_stop_active_transfer | | | | ------> Releases dwc->lock usb_ep_queue | | ----> Acquires dwc->lock usb_gadget_giveback | | dwc3_gadget_kick_transfer | | ----> Releases dwc->lock | | ----> Acquires dwc->lock | disable_endpoint After this state when the endpoint is enabled back, there is already and active transfer on this endpoint. Any START TRANSFER command on this endpoint would lead to "No Transfer Resource" error from the controller. Fix this by preventing the ep_queue when a reset of the gadget is in progress. This is done by clearing the connected flag as part of dwc3_gadget_reset and checking for this as part of ep_queue. Set this flag once we have received the CONNECTION DONE interrupt from the controller. Change-Id: Idc52a72ed0a965c2043618046a5b0dc32bfd1985 Signed-off-by: Sriharsha Allenki --- drivers/usb/dwc3/gadget.c | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c index b02dadcbab1f..b7f3084c5a94 100644 --- a/drivers/usb/dwc3/gadget.c +++ b/drivers/usb/dwc3/gadget.c @@ -1596,7 +1596,8 @@ static int __dwc3_gadget_ep_queue(struct dwc3_ep *dep, struct dwc3_request *req) { struct dwc3 *dwc = dep->dwc; - if (!dep->endpoint.desc || !dwc->pullups_connected) { + if (!dep->endpoint.desc || !dwc->pullups_connected || + !dwc->connected) { dev_err_ratelimited(dwc->dev, "%s: can't queue to disabled endpoint\n", dep->name); return -ESHUTDOWN; @@ -3355,6 +3356,8 @@ static void dwc3_reset_gadget(struct dwc3 *dwc) { struct usb_gadget_driver *gadget_driver; + dwc->connected = false; + if (!dwc->gadget_driver) return; @@ -3488,8 +3491,6 @@ static void dwc3_gadget_reset_interrupt(struct dwc3 *dwc) { u32 reg; - dwc->connected = true; - /* * WORKAROUND: DWC3 revisions <1.88a have an issue which * would cause a missing Disconnect Event if there's a @@ -3675,6 +3676,8 @@ static void dwc3_gadget_conndone_interrupt(struct dwc3 *dwc) dwc3_writel(dwc->regs, DWC3_DCTL, reg); } + dwc->connected = true; + dep = dwc->eps[0]; ret = __dwc3_gadget_ep_enable(dep, DWC3_DEPCFG_ACTION_MODIFY); if (ret) {