diff mbox series

usb: dwc3: gadget: fix race when disabling ep with cancelled xfers

Message ID 20191031090713.1452818-1-felipe.balbi@linux.intel.com (mailing list archive)
State Mainlined
Commit d8eca64eec7103ab1fbabc0a187dbf6acfb2af93
Headers show
Series usb: dwc3: gadget: fix race when disabling ep with cancelled xfers | expand

Commit Message

Felipe Balbi Oct. 31, 2019, 9:07 a.m. UTC
When disabling an endpoint which has cancelled requests, we should
make sure to giveback requests that are currently pending in the
cancelled list, otherwise we may fall into a situation where command
completion interrupt fires after endpoint has been disabled, therefore
causing a splat.

Fixes: fec9095bdef4 "usb: dwc3: gadget: remove wait_end_transfer"
Reported-by: Roger Quadros <rogerq@ti.com>
Signed-off-by: Felipe Balbi <felipe.balbi@linux.intel.com>
---

Greg,

please apply this as a patch, if it's not too much to ask. Otherwise I
can add this to my pull request for the next merge window.

cheers

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

Comments

Pavel Hofman Oct. 31, 2019, 9:15 a.m. UTC | #1
Hi,

Dne 31. 10. 19 v 10:07 Felipe Balbi napsal(a):
> When disabling an endpoint which has cancelled requests, we should
> make sure to giveback requests that are currently pending in the
> cancelled list, otherwise we may fall into a situation where command
> completion interrupt fires after endpoint has been disabled, therefore
> causing a splat.
> 

Please is this problem (and its solution) also applicable to dwc2 (i.e. 
e.g. RPi4)?

Thanks a lot,

Pavel.
Felipe Balbi Oct. 31, 2019, 9:23 a.m. UTC | #2
Hi,

Pavel Hofman <pavel.hofman@ivitera.com> writes:

> Hi,
>
> Dne 31. 10. 19 v 10:07 Felipe Balbi napsal(a):
>> When disabling an endpoint which has cancelled requests, we should
>> make sure to giveback requests that are currently pending in the
>> cancelled list, otherwise we may fall into a situation where command
>> completion interrupt fires after endpoint has been disabled, therefore
>> causing a splat.
>> 
>
> Please is this problem (and its solution) also applicable to dwc2 (i.e. 
> e.g. RPi4)?

different driver, so no.
Pavel Hofman Oct. 31, 2019, 10:11 a.m. UTC | #3
Dne 31. 10. 19 v 10:23 Felipe Balbi napsal(a):
> 
>>>
>>
>> Please is this problem (and its solution) also applicable to dwc2 (i.e.
>> e.g. RPi4)?
> 
> different driver, so no.

Yes the driver is different, but I was wondering if such situation can 
occur in dwc2 too and (if so) whether it is already taken care of. 
Disconnecting the USB device is a standard operation.

Thanks a lot. Regards,

Pavel.
Roger Quadros Oct. 31, 2019, 10:47 a.m. UTC | #4
Hi,

On 31/10/2019 12:11, Pavel Hofman wrote:
> Dne 31. 10. 19 v 10:23 Felipe Balbi napsal(a):
>>
>>>>
>>>
>>> Please is this problem (and its solution) also applicable to dwc2 (i.e.
>>> e.g. RPi4)?
>>
>> different driver, so no.
> 
> Yes the driver is different, but I was wondering if such situation can occur in dwc2 too and (if so) whether it is already taken care of. Disconnecting the USB device is a standard operation.

I discovered this problem on dwc3 while using g_audio gadget. see here:
https://www.spinics.net/lists/linux-usb/msg187244.html

You might want to do a similar test with dwc2?

> 
> Thanks a lot. Regards,
> 
> Pavel.
Felipe Balbi Oct. 31, 2019, 10:57 a.m. UTC | #5
Hi,

Pavel Hofman <pavel.hofman@ivitera.com> writes:
> Dne 31. 10. 19 v 10:23 Felipe Balbi napsal(a):
>> 
>>>>
>>>
>>> Please is this problem (and its solution) also applicable to dwc2 (i.e.
>>> e.g. RPi4)?
>> 
>> different driver, so no.
>
> Yes the driver is different, but I was wondering if such situation can 
> occur in dwc2 too and (if so) whether it is already taken care of. 
> Disconnecting the USB device is a standard operation.

technically, it can. But I don't remember of any such bug report for
dwc2. Have you faced something like this?
diff mbox series

Patch

diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
index 86dc1db788a9..a9aba716bf80 100644
--- a/drivers/usb/dwc3/gadget.c
+++ b/drivers/usb/dwc3/gadget.c
@@ -707,6 +707,12 @@  static void dwc3_remove_requests(struct dwc3 *dwc, struct dwc3_ep *dep)
 
 		dwc3_gadget_giveback(dep, req, -ESHUTDOWN);
 	}
+
+	while (!list_empty(&dep->cancelled_list)) {
+		req = next_request(&dep->cancelled_list);
+
+		dwc3_gadget_giveback(dep, req, -ESHUTDOWN);
+	}
 }
 
 /**