Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751516AbdISRRT (ORCPT ); Tue, 19 Sep 2017 13:17:19 -0400 Received: from iolanthe.rowland.org ([192.131.102.54]:48974 "HELO iolanthe.rowland.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with SMTP id S1751237AbdISRRS (ORCPT ); Tue, 19 Sep 2017 13:17:18 -0400 Date: Tue, 19 Sep 2017 13:17:16 -0400 (EDT) From: Alan Stern X-X-Sender: stern@iolanthe.rowland.org To: Andrey Konovalov cc: Dmitry Torokhov , Henrik Rydberg , "linux-input@vger.kernel.org" , Felipe Balbi , Greg Kroah-Hartman , Johan Hovold , Peter Chen , Yuyang Du , USB list , LKML , Dmitry Vyukov , Kostya Serebryany , syzkaller Subject: Re: usb/gadget: stalls in dummy_timer In-Reply-To: Message-ID: MIME-Version: 1.0 Content-Type: TEXT/PLAIN; charset=US-ASCII Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 3322 Lines: 94 On Tue, 19 Sep 2017, Andrey Konovalov wrote: > On Fri, Sep 15, 2017 at 8:57 PM, Alan Stern wrote: > > On Thu, 14 Sep 2017, Andrey Konovalov wrote: > > > >> On Thu, Sep 14, 2017 at 7:49 PM, Alan Stern wrote: > >> > On Thu, 14 Sep 2017, Andrey Konovalov wrote: > >> > > >> >> Looked at this a little more. > >> >> > >> >> dummy_timer() stucks in an infinite loop. It calls > >> >> usb_hcd_giveback_urb(), which in turn calls usbtouch_irq(), which > >> >> calls usb_submit_urb(), which calls dummy_urb_enqueue() and puts urb > >> >> back into dummy urb queue. dummy_timer() then does goto restart, finds > >> >> the urb and calls usb_hcd_giveback_urb() again. And this process goes > >> >> on again and again. It seems that something should either process the > >> >> urb and set urb->status or it should just expire. > >> > > >> > There is some throttling code, but it applies only to bulk transfers. > >> > Probably because the bandwidth limits for other types are slightly > >> > different. However, I don't think we need to worry about this level of > >> > detail, since the driver makes a number of other approximations anyway. > >> > > >> > Try the patch below; it should fix the problem. > >> > >> Hi Alan, > >> > >> Just tried your patch, my reproducer still hangs the kernel until all > >> memory is exhausted. > >> > >> Thanks! > > > > Hmmm. Your reproducer doesn't run on my system. The mmap system call > > fails, perhaps because this laptop has a 32-bit kernel. So I can't > > tell what's going on. > > > > Can you collect a usbmon trace that shows what happens while the > > reproducer runs? If it turns out to be extremely large, just post an > > initial portion of it. > > I've attached the usbmon trace. It's actually quite short, probably > due to the fact that the kernel enters infinite loop. > > I've also attached a reproducer that should compile on a 32 bit > system, however I haven't tested whether it reproduces the issue. Got it, thanks. Can you test the patch below in place of (or in addition to) the earlier patch? Alan Stern Index: usb-4.x/drivers/usb/gadget/udc/dummy_hcd.c =================================================================== --- usb-4.x.orig/drivers/usb/gadget/udc/dummy_hcd.c +++ usb-4.x/drivers/usb/gadget/udc/dummy_hcd.c @@ -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]; @@ -1252,6 +1254,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 */ @@ -1768,6 +1772,7 @@ static void dummy_timer(unsigned long _d 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) @@ -1784,6 +1789,10 @@ restart: 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;