From: Alan Stern <stern@rowland.harvard.edu> To: Greg KH <greg@kroah.com> Cc: andreyknvl@google.com, gustavo@embeddedor.com, USB list <linux-usb@vger.kernel.org>, syzkaller-bugs@googlegroups.com Subject: USB: dummy-hcd: Fix failure to give back unlinked URBs Date: Thu, 18 Apr 2019 13:12:07 -0400 (EDT) [thread overview] Message-ID: <Pine.LNX.4.44L0.1904181307350.1303-100000@iolanthe.rowland.org> (raw) 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> --- [as1889] drivers/usb/gadget/udc/dummy_hcd.c | 19 +++++++++++++++---- 1 file changed, 15 insertions(+), 4 deletions(-) Index: usb-devel/drivers/usb/gadget/udc/dummy_hcd.c =================================================================== --- usb-devel.orig/drivers/usb/gadget/udc/dummy_hcd.c +++ usb-devel/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);
WARNING: multiple messages have this Message-ID (diff)
From: Alan Stern <stern@rowland.harvard.edu> To: Greg KH <greg@kroah.com> Cc: andreyknvl@google.com, <gustavo@embeddedor.com>, USB list <linux-usb@vger.kernel.org>, <syzkaller-bugs@googlegroups.com> Subject: [PATCH] USB: dummy-hcd: Fix failure to give back unlinked URBs Date: Thu, 18 Apr 2019 13:12:07 -0400 (EDT) [thread overview] Message-ID: <Pine.LNX.4.44L0.1904181307350.1303-100000@iolanthe.rowland.org> (raw) Message-ID: <20190418171207.P7livWZZ1WKKjCln99lxT0JM8z_xuJUJgOeWyopowDg@z> (raw) In-Reply-To: <00000000000054e5e70586bf46cc@google.com> 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> --- [as1889] drivers/usb/gadget/udc/dummy_hcd.c | 19 +++++++++++++++---- 1 file changed, 15 insertions(+), 4 deletions(-) Index: usb-devel/drivers/usb/gadget/udc/dummy_hcd.c =================================================================== --- usb-devel.orig/drivers/usb/gadget/udc/dummy_hcd.c +++ usb-devel/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);
next reply other threads:[~2019-04-18 17:12 UTC|newest] Thread overview: 26+ messages / expand[flat|nested] mbox.gz Atom feed top 2019-04-18 17:12 Alan Stern [this message] 2019-04-18 17:12 ` [PATCH] USB: dummy-hcd: Fix failure to give back unlinked URBs Alan Stern [not found] <CAAeHK+wDEOpkuh0+OmPra3Yu8ri-8As82CyZ-1KyYC62AJkj1Q@mail.gmail.com> 2019-04-16 15:44 ` INFO: task hung in usb_kill_urb Alan Stern 2019-04-16 15:44 ` Alan Stern 2019-04-16 16:19 ` syzbot 2019-04-16 16:19 ` syzbot 2019-04-16 18:25 ` Alan Stern 2019-04-16 18:25 ` Alan Stern 2019-04-16 19:03 ` syzbot 2019-04-16 19:03 ` syzbot 2019-04-16 21:14 ` Alan Stern 2019-04-16 21:14 ` Alan Stern 2019-04-16 21:53 ` syzbot 2019-04-16 21:53 ` syzbot 2019-04-17 19:09 ` Alan Stern 2019-04-17 19:09 ` Alan Stern 2019-04-17 19:56 ` syzbot 2019-04-17 19:56 ` syzbot 2019-04-18 12:21 ` Andrey Konovalov 2019-04-18 12:21 ` Andrey Konovalov 2019-04-17 11:16 ` Andrey Konovalov 2019-04-17 11:16 ` Andrey Konovalov 2019-04-19 18:36 ` UDC hardware for fuzzing [was: Re: INFO: task hung in usb_kill_urb] Alan Stern 2019-04-19 18:36 ` INFO: task hung in usb_kill_urb Alan Stern 2019-04-23 12:44 ` UDC hardware for fuzzing [was: Re: INFO: task hung in usb_kill_urb] Andrey Konovalov 2019-04-23 12:44 ` INFO: task hung in usb_kill_urb Andrey Konovalov
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=Pine.LNX.4.44L0.1904181307350.1303-100000@iolanthe.rowland.org \ --to=stern@rowland.harvard.edu \ --cc=andreyknvl@google.com \ --cc=greg@kroah.com \ --cc=gustavo@embeddedor.com \ --cc=linux-usb@vger.kernel.org \ --cc=syzkaller-bugs@googlegroups.com \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.