Skip to content

Commit

Permalink
USB: dummy-hcd: fix infinite-loop resubmission bug
Browse files Browse the repository at this point in the history
The dummy-hcd HCD/UDC emulator tries not to do too much work during
each timer interrupt.  But it doesn't try very hard; currently all
it does is limit the total amount of bulk data transferred.  Other
transfer types aren't limited, and URBs that transfer no data (because
of an error, perhaps) don't count toward the limit, even though on a
real USB bus they would consume at least a minimum overhead.

This means it's possible to get the driver stuck in an infinite loop,
for example, if the host class driver resubmits an URB every time it
completes (which is common for interrupt URBs).  Each time the URB is
resubmitted it gets added to the end of the pending-URBs list, and
dummy-hcd doesn't stop until that list is empty.  Andrey Konovalov was
able to trigger this failure mode using the syzkaller fuzzer.

This patch fixes the infinite-loop problem by restricting the URBs
handled during each timer interrupt to those that were already on the
pending list when the interrupt routine started.  Newly added URBs
won't be processed until the next timer interrupt.  The problem of
properly accounting for non-bulk bandwidth (as well as packet and
transaction overhead) is not addressed here.

Signed-off-by: Alan Stern <stern@rowland.harvard.edu>
Reported-by: Andrey Konovalov <andreyknvl@google.com>
Tested-by: Andrey Konovalov <andreyknvl@google.com>
CC: <stable@vger.kernel.org>
Signed-off-by: Felipe Balbi <felipe.balbi@linux.intel.com>
  • Loading branch information
AlanStern authored and Felipe Balbi committed Sep 28, 2017
1 parent fe659bc commit 0173a68
Showing 1 changed file with 9 additions and 0 deletions.
9 changes: 9 additions & 0 deletions drivers/usb/gadget/udc/dummy_hcd.c
Original file line number Diff line number Diff line change
Expand Up @@ -237,6 +237,8 @@ struct dummy_hcd {

struct usb_device *udev;
struct list_head urbp_list;
struct urbp *next_frame_urbp;

u32 stream_en_ep;
u8 num_stream[30 / 2];

Expand Down Expand Up @@ -1250,6 +1252,8 @@ static int dummy_urb_enqueue(

list_add_tail(&urbp->urbp_list, &dum_hcd->urbp_list);
urb->hcpriv = urbp;
if (!dum_hcd->next_frame_urbp)
dum_hcd->next_frame_urbp = urbp;
if (usb_pipetype(urb->pipe) == PIPE_CONTROL)
urb->error_count = 1; /* mark as a new urb */

Expand Down Expand Up @@ -1766,6 +1770,7 @@ static void dummy_timer(unsigned long _dum_hcd)
spin_unlock_irqrestore(&dum->lock, flags);
return;
}
dum_hcd->next_frame_urbp = NULL;

for (i = 0; i < DUMMY_ENDPOINTS; i++) {
if (!ep_info[i].name)
Expand All @@ -1782,6 +1787,10 @@ static void dummy_timer(unsigned long _dum_hcd)
int type;
int status = -EINPROGRESS;

/* stop when we reach URBs queued after the timer interrupt */
if (urbp == dum_hcd->next_frame_urbp)
break;

urb = urbp->urb;
if (urb->unlinked)
goto return_urb;
Expand Down

0 comments on commit 0173a68

Please sign in to comment.