Wesley Cheng wrote: > In the situations where the DWC3 gadget stops active transfers, once > calling the dwc3_gadget_giveback(), there is a chance where a function > driver can queue a new USB request in between the time where the dwc3 > lock has been released and re-aquired. This occurs after we've already > issued an ENDXFER command. When the stop active transfers continues > to remove USB requests from all dep lists, the newly added request will > also be removed, while controller still has an active TRB for it. > This can lead to the controller accessing an unmapped memory address. > > Fix this by ensuring parameters to prevent EP queuing are set before > calling the stop active transfers API. > > Fixes: ae7e86108b12 ("usb: dwc3: Stop active transfers before halting the controller") > Signed-off-by: Wesley Cheng <wcheng@xxxxxxxxxxxxxx> > --- > Changes since V1: > - Added Fixes tag to point to the commit this is addressing > > drivers/usb/dwc3/gadget.c | 10 +++++----- > 1 file changed, 5 insertions(+), 5 deletions(-) > > diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c > index 4780983..4d98fbf 100644 > --- a/drivers/usb/dwc3/gadget.c > +++ b/drivers/usb/dwc3/gadget.c > @@ -783,8 +783,6 @@ static int __dwc3_gadget_ep_disable(struct dwc3_ep *dep) > > trace_dwc3_gadget_ep_disable(dep); > > - dwc3_remove_requests(dwc, dep); > - > /* make sure HW endpoint isn't stalled */ > if (dep->flags & DWC3_EP_STALL) > __dwc3_gadget_ep_set_halt(dep, 0, false); > @@ -803,6 +801,8 @@ static int __dwc3_gadget_ep_disable(struct dwc3_ep *dep) > dep->endpoint.desc = NULL; > } > > + dwc3_remove_requests(dwc, dep); > + > return 0; > } > > @@ -1617,7 +1617,7 @@ 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(dwc->dev, "%s: can't queue to disabled endpoint\n", > dep->name); > return -ESHUTDOWN; > @@ -2247,6 +2247,7 @@ static int dwc3_gadget_pullup(struct usb_gadget *g, int is_on) > if (!is_on) { > u32 count; > > + dwc->connected = false; You want to set this before __dwc3_gadget_stop() right? Then you may want to remove this same setting few lines below this. Otherwise, this looks good. Thanks, Thinh > /* > * In the Synopsis DesignWare Cores USB3 Databook Rev. 3.30a > * Section 4.1.8 Table 4-7, it states that for a device-initiated > @@ -3329,8 +3330,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 > @@ -3370,6 +3369,7 @@ static void dwc3_gadget_reset_interrupt(struct dwc3 *dwc) > * transfers." > */ > dwc3_stop_active_transfers(dwc); > + dwc->connected = true; > > reg = dwc3_readl(dwc->regs, DWC3_DCTL); > reg &= ~DWC3_DCTL_TSTCTRL_MASK; >