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: https://github.com/google/kasan.git usb-fuzzer > > console output: https://syzkaller.appspot.com/x/log.txt?x=139be302600000 > > kernel config: https://syzkaller.appspot.com/x/.config?x=dbc9c80cc095da19 > > dashboard link: https://syzkaller.appspot.com/bug?extid=cfe6d93e0abab9a0de05 > > compiler: gcc (GCC) 9.0.0 20181231 (experimental) > > syz repro: https://syzkaller.appspot.com/x/repro.syz?x=12fe6b02600000 > > C reproducer: https://syzkaller.appspot.com/x/repro.c?x=1548189c600000 > > > > IMPORTANT: if you fix the bug, please add the following tag to the commit: > > Reported-by: syzbot+cfe6d93e0abab9a0de05@xxxxxxxxxxxxxxxxxxxxxxxxx > > > > #syz test: https://github.com/google/kasan.git 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 acknowledgment. (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
