linux-kernel - Re: Frequent dwc3 crashes on suspend or reboot since 5.0-rc1

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Date:   Tue, 5 Feb 2019 10:58:00 -0500 (EST)
From:   Alan Stern <stern@...land.harvard.edu>
To:     John Stultz <john.stultz@...aro.org>
cc:     Felipe Balbi <balbi@...nel.org>,
        Zeng Tao <prime.zeng@...ilicon.com>,
        Jack Pham <jackp@...eaurora.org>,
        Thinh Nguyen <thinh.nguyen@...opsys.com>,
        Chen Yu <chenyu56@...wei.com>,
        lkml <linux-kernel@...r.kernel.org>,
        Linux USB List <linux-usb@...r.kernel.org>,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>
Subject: Re: Frequent dwc3 crashes on suspend or reboot since 5.0-rc1

On Mon, 4 Feb 2019, John Stultz wrote:

> On Sat, Feb 2, 2019 at 9:00 AM Alan Stern <stern@...land.harvard.edu> wrote:
> >
> > On Fri, 1 Feb 2019, John Stultz wrote:
> >
> > > Hey all,
> > >   Since the 5.0 merge window opened, I've been tripping on frequent
> > > dwc3 crashes on reboot and suspend, which I've added an example to the
> > > bottom of this mail.
> > >
> > > I've dug in a little bit and sort of have a sense of whats going on.
> > >
> > > In ffs_epfile_io():
> > > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/usb/gadget/function/f_fs.c#n1065
> > >
> > > The completion done is setup on the stack:
> > >   DECLARE_COMPLETION_ONSTACK(done);
> > >
> > > Then later we setup a request and queue it:
> > >   req->context  = &done;
> > >   ...
> > >   ret = usb_ep_queue(ep->ep, req, GFP_ATOMIC);
> > >
> > > Then wait for it:
> > >   if (unlikely(wait_for_completion_interruptible(&done))) {
> > >     /*
> > >     * To avoid race condition with ffs_epfile_io_complete,
> > >     * dequeue the request first then check
> > >     * status. usb_ep_dequeue API should guarantee no race
> > >     * condition with req->complete callback.
> > >     */
> > >     usb_ep_dequeue(ep->ep, req);
> >
> > This code contains a bug: It assumes that usb_ep_dequeue() waits until
> > the request has been completed.  You should insert
> >
> >         wait_for_completion(&done);
> >
> > right here.
> >
> > >     interrupted = ep->status < 0;
> > >   }
> >
> 
> Thanks! This does seem to resolve the issue I'm seeing.
> 
> Are you planning to send in such a patch, or would it help if I sent it out?

Either way, whatever you prefer.

If you decide to write the patch yourself, consider editing the
preceding comment (which is confusing if not outright wrong).  Also,
there is an earlier call of usb_ep_dequeue() in __ffs_ep0_queue_wait()
which might need to be fixed as well -- I'm not sure.

Alan Stern