Skip to content

Commit c91815b

Browse files
Felipe Balbigregkh
Felipe Balbi
authored andcommitted
usb: dwc3: gadget: never call ->complete() from ->ep_queue()
This is a requirement which has always existed but, somehow, wasn't reflected in the documentation and problems weren't found until now when Tuba Yavuz found a possible deadlock happening between dwc3 and f_hid. She described the situation as follows: spin_lock_irqsave(&hidg->write_spinlock, flags); // first acquire /* we our function has been disabled by host */ if (!hidg->req) { free_ep_req(hidg->in_ep, hidg->req); goto try_again; } [...] status = usb_ep_queue(hidg->in_ep, hidg->req, GFP_ATOMIC); => [...] => usb_gadget_giveback_request => f_hidg_req_complete => spin_lock_irqsave(&hidg->write_spinlock, flags); // second acquire Note that this happens because dwc3 would call ->complete() on a failed usb_ep_queue() due to failed Start Transfer command. This is, anyway, a theoretical situation because dwc3 currently uses "No Response Update Transfer" command for Bulk and Interrupt endpoints. It's still good to make this case impossible to happen even if the "No Reponse Update Transfer" command is changed. Reported-by: Tuba Yavuz <tuba@ece.ufl.edu> Signed-off-by: Felipe Balbi <felipe.balbi@linux.intel.com> Cc: stable <stable@vger.kernel.org> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
1 parent eaa358c commit c91815b

File tree

1 file changed

+25
-18
lines changed

1 file changed

+25
-18
lines changed

Diff for: drivers/usb/dwc3/gadget.c

+25-18
Original file line numberDiff line numberDiff line change
@@ -166,18 +166,8 @@ static void dwc3_ep_inc_deq(struct dwc3_ep *dep)
166166
dwc3_ep_inc_trb(&dep->trb_dequeue);
167167
}
168168

169-
/**
170-
* dwc3_gadget_giveback - call struct usb_request's ->complete callback
171-
* @dep: The endpoint to whom the request belongs to
172-
* @req: The request we're giving back
173-
* @status: completion code for the request
174-
*
175-
* Must be called with controller's lock held and interrupts disabled. This
176-
* function will unmap @req and call its ->complete() callback to notify upper
177-
* layers that it has completed.
178-
*/
179-
void dwc3_gadget_giveback(struct dwc3_ep *dep, struct dwc3_request *req,
180-
int status)
169+
void dwc3_gadget_del_and_unmap_request(struct dwc3_ep *dep,
170+
struct dwc3_request *req, int status)
181171
{
182172
struct dwc3 *dwc = dep->dwc;
183173

@@ -190,18 +180,35 @@ void dwc3_gadget_giveback(struct dwc3_ep *dep, struct dwc3_request *req,
190180

191181
if (req->trb)
192182
usb_gadget_unmap_request_by_dev(dwc->sysdev,
193-
&req->request, req->direction);
183+
&req->request, req->direction);
194184

195185
req->trb = NULL;
196-
197186
trace_dwc3_gadget_giveback(req);
198187

188+
if (dep->number > 1)
189+
pm_runtime_put(dwc->dev);
190+
}
191+
192+
/**
193+
* dwc3_gadget_giveback - call struct usb_request's ->complete callback
194+
* @dep: The endpoint to whom the request belongs to
195+
* @req: The request we're giving back
196+
* @status: completion code for the request
197+
*
198+
* Must be called with controller's lock held and interrupts disabled. This
199+
* function will unmap @req and call its ->complete() callback to notify upper
200+
* layers that it has completed.
201+
*/
202+
void dwc3_gadget_giveback(struct dwc3_ep *dep, struct dwc3_request *req,
203+
int status)
204+
{
205+
struct dwc3 *dwc = dep->dwc;
206+
207+
dwc3_gadget_del_and_unmap_request(dep, req, status);
208+
199209
spin_unlock(&dwc->lock);
200210
usb_gadget_giveback_request(&dep->endpoint, &req->request);
201211
spin_lock(&dwc->lock);
202-
203-
if (dep->number > 1)
204-
pm_runtime_put(dwc->dev);
205212
}
206213

207214
/**
@@ -1227,7 +1234,7 @@ static int __dwc3_gadget_kick_transfer(struct dwc3_ep *dep)
12271234
if (req->trb)
12281235
memset(req->trb, 0, sizeof(struct dwc3_trb));
12291236
dep->queued_requests--;
1230-
dwc3_gadget_giveback(dep, req, ret);
1237+
dwc3_gadget_del_and_unmap_request(dep, req, ret);
12311238
return ret;
12321239
}
12331240

0 commit comments

Comments
 (0)