Re: [PATCH] usb: dwc2: extend treatment for incomplete transfer

[Boris ARZUR <boris@konbu.org>]

Hi,

First post in this list, please be lenient.

Replying to self to give some context: I'm on a Asus c201 (rk3288)
and I see some crashes with cdc_ether.

Here is how to repro:
- create heavy usb network load: I tether my phone and
  netcat some file from it;
- create heavy CPU load (pushd linux; make -j 6)
- observe kernel messages:
dwc2 ff580000.usb: dwc2_hc_chhltd_intr_dma: Channel 7 - ChHltd set, but reason is unknown
dwc2 ff580000.usb: hcint 0x00000002, intsts 0x04200021
dwc2 ff580000.usb: ep_type 0x00000002 bulk /* ba: ADDED LOG */

The kernel will write to 0 at line 2494 below in file drivers/usb/dwc2/hcd.c
2474 static void dwc2_free_dma_aligned_buffer(struct urb *urb)
2475 {
/* ... */
2482 	/* Restore urb->transfer_buffer from the end of the allocated area */
2483 	memcpy(&stored_xfer_buffer,
2484 	       PTR_ALIGN(urb->transfer_buffer + urb->transfer_buffer_length,
2485 			 dma_get_cache_alignment()),
2486 	       sizeof(urb->transfer_buffer));
/* ... */
2494 		memcpy(stored_xfer_buffer, urb->transfer_buffer, length);
/* ... */
2500 }

The fix I propose has been working fine on my machine, but I confess
I am less than familiar with this area...

My guess is that the kernel misses some deadlines due to contention and we
see channel halts. I tried treating these as we do the other (with other end
point types) and it solved the crashes. I verified on next-20191030 that the
data is correctly transfered over the network (no corruption).

Thank you & regards,
Boris.

>Channel halt can happen with BULK endpoints when the
>cpu is under high load. Treating it as an error leads
>to a null-pointer dereference in dwc2_free_dma_aligned_buffer().
>
>Signed-off-by: Boris Arzur <boris@konbu.org>
>---
> drivers/usb/dwc2/hcd_intr.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
>diff --git a/drivers/usb/dwc2/hcd_intr.c b/drivers/usb/dwc2/hcd_intr.c
>index a052d39b4375..697fed530aeb 100644
>--- a/drivers/usb/dwc2/hcd_intr.c
>+++ b/drivers/usb/dwc2/hcd_intr.c
>@@ -1944,7 +1944,8 @@ static void dwc2_hc_chhltd_intr_dma(struct dwc2_hsotg
>*hsotg,
>                         */
>                        dwc2_hc_ack_intr(hsotg, chan, chnum, qtd);
>                } else {
>-                       if (chan->ep_type == USB_ENDPOINT_XFER_INT ||
>+                       if (chan->ep_type == USB_ENDPOINT_XFER_BULK ||
>+                           chan->ep_type == USB_ENDPOINT_XFER_INT ||
>                            chan->ep_type == USB_ENDPOINT_XFER_ISOC) {
>                                /*
>                                 * A periodic transfer halted with no other
>--
>2.23.0