From: Alan Stern <stern@rowland.harvard.edu>
To: Andrey Konovalov <andreyknvl@google.com>
Cc: "Felipe Balbi" <balbi@kernel.org>,
"Greg Kroah-Hartman" <gregkh@linuxfoundation.org>,
"Peter Chen" <peter.chen@nxp.com>,
"Krzysztof Opasiak" <k.opasiak@samsung.com>,
"Colin Ian King" <colin.king@canonical.com>,
"Felix Hädicke" <felixhaedicke@web.de>,
"Roger Quadros" <rogerq@ti.com>,
"USB list" <linux-usb@vger.kernel.org>,
LKML <linux-kernel@vger.kernel.org>,
"Dmitry Vyukov" <dvyukov@google.com>,
"Kostya Serebryany" <kcc@google.com>,
syzkaller <syzkaller@googlegroups.com>
Subject: Re: usb/gadget: another GPF in usb_gadget_unregister_driver
Date: Thu, 8 Jun 2017 11:55:14 -0400 (EDT) [thread overview]
Message-ID: <Pine.LNX.4.44L0.1706081153080.1949-100000@iolanthe.rowland.org> (raw)
In-Reply-To: <CAAeHK+xdfFYXpKeAEWM+s7u1m3FBFc9iGtU+WFaOhyYi09xKPg@mail.gmail.com>
On Thu, 8 Jun 2017, Andrey Konovalov wrote:
> On Wed, Jun 7, 2017 at 11:20 PM, Alan Stern <stern@rowland.harvard.edu> wrote:
> > On Wed, 7 Jun 2017, Andrey Konovalov wrote:
> >
> >> On Wed, Jun 7, 2017 at 4:43 PM, Alan Stern <stern@rowland.harvard.edu> 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.
next prev parent reply other threads:[~2017-06-08 15:55 UTC|newest]
Thread overview: 7+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-06-07 13:39 usb/gadget: another GPF in usb_gadget_unregister_driver Andrey Konovalov
2017-06-07 14:43 ` Alan Stern
2017-06-07 15:15 ` Andrey Konovalov
2017-06-07 21:20 ` Alan Stern
2017-06-08 11:40 ` Andrey Konovalov
2017-06-08 15:55 ` Alan Stern [this message]
2017-06-08 16:34 ` 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.1706081153080.1949-100000@iolanthe.rowland.org \
--to=stern@rowland.harvard.edu \
--cc=andreyknvl@google.com \
--cc=balbi@kernel.org \
--cc=colin.king@canonical.com \
--cc=dvyukov@google.com \
--cc=felixhaedicke@web.de \
--cc=gregkh@linuxfoundation.org \
--cc=k.opasiak@samsung.com \
--cc=kcc@google.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-usb@vger.kernel.org \
--cc=peter.chen@nxp.com \
--cc=rogerq@ti.com \
--cc=syzkaller@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: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).