linux-kernel - Re: [PATCH V2] usb: gadget: f_fs: don't free buffer prematurely

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] [thread-next>] [day] [month] [year] [list]

Date:   Wed, 20 Mar 2019 16:42:48 -0700
From:   John Stultz <john.stultz@...aro.org>
To:     "Yang, Fei" <fei.yang@...el.com>
Cc:     Felipe Balbi <balbi@...nel.org>,
        Greg KH <gregkh@...uxfoundation.org>,
        "andrzej.p@...labora.com" <andrzej.p@...labora.com>,
        Vincent Pelletier <plr.vincent@...il.com>,
        Linux USB List <linux-usb@...r.kernel.org>,
        lkml <linux-kernel@...r.kernel.org>, Josh Gao <jmgao@...gle.com>,
        Alistair Strachan <astrachan@...gle.com>,
        "Shen, JingX" <jingx.shen@...el.com>
Subject: Re: [PATCH V2] usb: gadget: f_fs: don't free buffer prematurely

On Wed, Mar 20, 2019 at 4:28 PM Yang, Fei <fei.yang@...el.com> wrote:
>
> > Hey Fei,
> >  So while this patch does resolve the issues I was seeing with mainline kernels and recent changes to adbd,
> > Josh pointed out that it wouldn't resolve the issues I was seeing with older kernels which is slightly different (but still related to aio usage).
> >
> > On the older kernels I'm hitting scheduling while atomic on reboot, which seems to be due to ffs_aio_cancel() taking a spinlock then calling usb_ep_dequeue() which might sleep.
> >
> > It seems a fix for this was tried earlier with d52e4d0c0c428 ("usb:
> > gadget: ffs: Fix BUG when userland exits with submitted AIO
> > transfers") which was then reverted by a9c859033f6e.
> >
> > Elsewhere it seems the ffs driver takes effort to drop any locks before calling usb_ep_dequeue(), so this seems like
> > that should be addressed, but it also seems like recent change to the dwc3 driver has been made to avoid sleeping
> > in that path (see fec9095bdef4 ("usb: dwc3: gadget: remove wait_end_transfer")), which may be why I'm not seeing
> > the problem with mainline (and your patch here, of coarse). But that also doesn't clarify if its still a potential issue
> > w/ non-dwc3 platforms.
> >
> > So for older kernels, do you have a suggestion of which approach is advised?  Does usb_ep_dequeue need to avoid
> > sleeping or do we need to rework the ffs_aio_cancel logic?
>
> Are you seeing this issue with Android? When running adb reboot?
> I have tried 4.19 and 4.9 kernel with Android P-dessert on one of the Intel platforms, but no luck on reproducing the issue.

You probably need to be running AOSP/master to trigger this. The
changes which uncovered this landed just last week.

> I will get back to you if I could reproduce the issue. I'm afraid I won’t be able to do much by just looking at the code.

So as discussed in further mails, the main issue seems to be the dwc3
code was sleeping in its ep_dequeue logic, which isn't safe as
ffs_aio_cancel calls it while holding a spinlock. Upstream the dw3c
driver has been fixed, but -stable kernels still have the issue.

thanks
-john