[lkml]   [2019]   [Aug]   [20]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
Messages in this thread
SubjectRe: KASAN: use-after-free Read in iowarrior_disconnect
On Mon, 19 Aug 2019, Oliver Neukum wrote:

> Am Montag, den 19.08.2019, 07:48 -0700 schrieb syzbot:
> > Hello,
> >
> > syzbot found the following crash on:
> >
> > HEAD commit: d0847550 usb-fuzzer: main usb gadget fuzzer driver
> > git tree: usb-fuzzer
> > console output:
> > kernel config:
> > dashboard link:
> > compiler: gcc (GCC) 9.0.0 20181231 (experimental)
> > syz repro:
> > C reproducer:
> >
> > IMPORTANT: if you fix the bug, please add the following tag to the commit:
> > Reported-by:
> >
> #syz test: d0847550

There's no need for us to work at cross purposes on this. We can go
with your approach.

However, the code is more complicated than your patch accounts for.
The wait can finish in several different ways:

(1) The control URB succeeds and the interrupt URB gets an

(2) The control URB completes with an error.

(3) The wait times out.

(4) A disconnect occurs.

Your patch doesn't handle cases (1) and (3). (And it doesn't get rid
of the dev->waitq field, which is no longer used.)

In fact, (1) is a little ambiguous. When the interrupt URB gets a
command acknowledgment, there's no way (as far as I can tell) to know
which command was acknowledged -- particularly if a prior command URB
had to be cancelled because it timed out.

And as it turns out, the driver neglects to kill the command URB in
case (3). Furthermore, the driver doesn't have mutual exclusion for
writes. So there's nothing to prevent the command URB from being
submitted while it is still active (syzbot's new crash).

I have to wonder if anybody's actually using this driver. It seems to
be pretty broken. Maybe we should just mark it as such and forget
about fixing it.

Alan Stern

 \ /
  Last update: 2019-08-20 16:18    [W:0.121 / U:2.628 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site