On Thu, 8 Jun 2017, Andrey Konovalov wrote: > On Wed, Jun 7, 2017 at 11:20 PM, Alan Stern <stern@xxxxxxxxxxxxxxxxxxx> wrote: > > On Wed, 7 Jun 2017, Andrey Konovalov wrote: > > > >> On Wed, Jun 7, 2017 at 4:43 PM, Alan Stern <stern@xxxxxxxxxxxxxxxxxxx> wrote: > >> > On Wed, 7 Jun 2017, Andrey Konovalov wrote: > >> > > >> >> Hi, > >> >> > >> >> I've got the following error report while fuzzing the kernel with syzkaller. > >> >> > >> >> On commit b29794ec95c6856b316c2295904208bf11ffddd9 (4.12-rc4+). > >> >> > >> >> This looks quite similar to > >> >> https://groups.google.com/forum/#!topic/syzkaller/HDawLBeeORI > >> > > >> > It does look very similar, but that problem was supposed to have been > >> > fixed by commit 7b0173811260 ("usb: gadget: udc: core: fix return code > >> > of usb_gadget_probe_driver()"). > >> > > >> >> I'm able to reproduce this, so I can collect some debug traces if needed. > >> > > >> > Can you provide an strace or the equivalent? > >> > >> Here's the syzkaller program (which is actually two programs executed > >> consequently): > >> https://gist.github.com/xairy/fe0a7531e00df5e8bc23e2e56e413510 > >> > >> Here's the strace log: > >> https://gist.github.com/xairy/5fadc3b5d8b2b80c97e566538de08bc4 > > > > Do you know which of the two programs got the GPF? I can't tell from > > the strace log. > > > >> Unfortunately there's a lot of unrelated garbage, but I can't extract > >> a simple C reproducer. > > > > That's okay, it's easy enough to see what's going on. One program > > opens /dev/gadget/dummy_udc, writes an invalid setup string, then > > writes a valid setup string, and then exits. The other program just > > opens the file and then exits. > > > >> I can also apply patches with debug printk's, run the reproducer and > >> send you the result if that will help. > > I've extract another crash log, which is a little simpler: > https://gist.github.com/xairy/b8c814cbd731e4632e8e8fa0f51a29e8 > > > > > Maybe you can patch usb_gadget_probe_driver() in > > drivers/usb/gadget/udc/core.c. Find out whether the "if > > (!driver->match_existing_only)" test is executed and whether it > > succeeds, and find out whether the code following "found:" is executed. > > I would expect that the test is not executed and the jump to "found:" > > is taken, so udc_bind_to_driver() is called and returns 0. Thus, > > udc->driver should be set to driver. > > Here's the funcgraph for usb_gadget_probe_driver: > https://gist.github.com/xairy/3221e2cb9c59514880d24c955de30b80 > > The (!driver->match_existing_only) test is not executed. > The code following "found:" is executed. > > > > > Also, in usb_gadget_unregister_driver(), in the list_for_each_entry() > > loop, we should have udc->driver == driver and therefore ret should get > > set to 0. Consequently, the list_del() near the end should not be > > executed and so the GPF should not occur. > > Here's the funcgraph for usb_gadget_unregister_driver: > https://gist.github.com/xairy/887c52a12af8c9f9fe8ba3e4fa0ef1f0 > > What you described happens during the first call of > usb_gadget_unregister_driver(), however there's another one after > that, which is probably triggered by the second program. > > > > > In particular, do these subroutines get called more than once? > > usb_gadget_unregister_driver() is called twice, the GPF happens during > the second call. Good, that's definitive. And I feel stupid for missing this bug. The patch is below. Alan Stern Index: usb-4.x/drivers/usb/gadget/legacy/inode.c =================================================================== --- usb-4.x.orig/drivers/usb/gadget/legacy/inode.c +++ usb-4.x/drivers/usb/gadget/legacy/inode.c @@ -1183,8 +1183,10 @@ dev_release (struct inode *inode, struct /* closing ep0 === shutdown all */ - if (dev->gadget_registered) + if (dev->gadget_registered) { usb_gadget_unregister_driver (&gadgetfs_driver); + dev->gadget_registered = false; + } /* at this point "good" hardware has disconnected the * device from USB; the host won't see it any more. -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
