| From fc834e607ae3d18e1a20bca3f9a2d7f52ea7a2be Mon Sep 17 00:00:00 2001 |
| From: Alan Stern <stern@rowland.harvard.edu> |
| Date: Thu, 18 Apr 2019 13:12:07 -0400 |
| Subject: USB: dummy-hcd: Fix failure to give back unlinked URBs |
| |
| From: Alan Stern <stern@rowland.harvard.edu> |
| |
| commit fc834e607ae3d18e1a20bca3f9a2d7f52ea7a2be upstream. |
| |
| The syzkaller USB fuzzer identified a failure mode in which dummy-hcd |
| would never give back an unlinked URB. This causes usb_kill_urb() to |
| hang, leading to WARNINGs and unkillable threads. |
| |
| In dummy-hcd, all URBs are given back by the dummy_timer() routine as |
| it scans through the list of pending URBS. Failure to give back URBs |
| can be caused by failure to start or early exit from the scanning |
| loop. The code currently has two such pathways: One is triggered when |
| an unsupported bus transfer speed is encountered, and the other by |
| exhausting the simulated bandwidth for USB transfers during a frame. |
| |
| This patch removes those two paths, thereby allowing all unlinked URBs |
| to be given back in a timely manner. It adds a check for the bus |
| speed when the gadget first starts running, so that dummy_timer() will |
| never thereafter encounter an unsupported speed. And it prevents the |
| loop from exiting as soon as the total bandwidth has been used up (the |
| scanning loop continues, giving back unlinked URBs as they are found, |
| but not transferring any more data). |
| |
| Thanks to Andrey Konovalov for manually running the syzkaller fuzzer |
| to help track down the source of the bug. |
| |
| Signed-off-by: Alan Stern <stern@rowland.harvard.edu> |
| Reported-and-tested-by: syzbot+d919b0f29d7b5a4994b9@syzkaller.appspotmail.com |
| CC: <stable@vger.kernel.org> |
| Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> |
| |
| --- |
| drivers/usb/gadget/udc/dummy_hcd.c | 19 +++++++++++++++---- |
| 1 file changed, 15 insertions(+), 4 deletions(-) |
| |
| --- a/drivers/usb/gadget/udc/dummy_hcd.c |
| +++ b/drivers/usb/gadget/udc/dummy_hcd.c |
| @@ -979,8 +979,18 @@ static int dummy_udc_start(struct usb_ga |
| struct dummy_hcd *dum_hcd = gadget_to_dummy_hcd(g); |
| struct dummy *dum = dum_hcd->dum; |
| |
| - if (driver->max_speed == USB_SPEED_UNKNOWN) |
| + switch (g->speed) { |
| + /* All the speeds we support */ |
| + case USB_SPEED_LOW: |
| + case USB_SPEED_FULL: |
| + case USB_SPEED_HIGH: |
| + case USB_SPEED_SUPER: |
| + break; |
| + default: |
| + dev_err(dummy_dev(dum_hcd), "Unsupported driver max speed %d\n", |
| + driver->max_speed); |
| return -EINVAL; |
| + } |
| |
| /* |
| * SLAVE side init ... the layer above hardware, which |
| @@ -1784,9 +1794,10 @@ static void dummy_timer(struct timer_lis |
| /* Bus speed is 500000 bytes/ms, so use a little less */ |
| total = 490000; |
| break; |
| - default: |
| + default: /* Can't happen */ |
| dev_err(dummy_dev(dum_hcd), "bogus device speed\n"); |
| - return; |
| + total = 0; |
| + break; |
| } |
| |
| /* FIXME if HZ != 1000 this will probably misbehave ... */ |
| @@ -1828,7 +1839,7 @@ restart: |
| |
| /* Used up this frame's bandwidth? */ |
| if (total <= 0) |
| - break; |
| + continue; |
| |
| /* find the gadget's ep for this request (if configured) */ |
| address = usb_pipeendpoint (urb->pipe); |