Re: possible deadlock in open_rio

[Andrey Konovalov]

On Wed, Aug 7, 2019 at 4:24 PM Andrey Konovalov <andreyknvl@xxxxxxxxxx> wrote:
>
> On Wed, Aug 7, 2019 at 4:01 PM Alan Stern <stern@xxxxxxxxxxxxxxxxxxx> wrote:
> >
> > On Wed, 7 Aug 2019, Andrey Konovalov wrote:
> >
> > > On Tue, Aug 6, 2019 at 9:13 PM Alan Stern <stern@xxxxxxxxxxxxxxxxxxx> wrote:
> > > >
> > > > On Thu, 1 Aug 2019, syzbot wrote:
> > > >
> > > > > Hello,
> > > > >
> > > > > syzbot found the following crash on:
> > > > >
> > > > > HEAD commit:    7f7867ff 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=136b6aec600000
> > > > > kernel config:  https://syzkaller.appspot.com/x/.config?x=792eb47789f57810
> > > > > dashboard link: https://syzkaller.appspot.com/bug?extid=7bbcbe9c9ff0cd49592a
> > > > > compiler:       gcc (GCC) 9.0.0 20181231 (experimental)
> > > > >
> > > > > Unfortunately, I don't have any reproducer for this crash yet.
> > > > >
> > > > > IMPORTANT: if you fix the bug, please add the following tag to the commit:
> > > > > Reported-by: syzbot+7bbcbe9c9ff0cd49592a@xxxxxxxxxxxxxxxxxxxxxxxxx
> > > > >
> > > > > ======================================================
> > > > > WARNING: possible circular locking dependency detected
> > > > > 5.3.0-rc2+ #23 Not tainted
> > > > > ------------------------------------------------------
> > > >
> > > > Andrey:
> > > >
> > > > This should be completely reproducible, since it's a simple ABBA
> > > > locking violation.  Maybe just introducing a time delay (to avoid races
> > > > and give the open() call time to run) between the gadget creation and
> > > > gadget removal would be enough to do it.
> > >
> > > I've tried some simple approaches to reproducing this, but failed.
> > > Should this require two rio500 devices to trigger?
> >
> > No, one device should be enough.  Just plug it in and then try to open
> > the character device file.
>
> OK, I've reproduced it, so I can test a patch manually. The reason
> syzbot couldn't do that, is because it doesn't open character devices.
> Right now the USB fuzzing instance only opens /dev/input*,
> /dev/hidraw* and /dev/usb/hiddev* (only the devices that are created
> by USB HID devices as I've been working on adding USB HID targeted
> fuzzing support lately).
>
> I guess we should open /dev/chr/* as well. The problem is that there
> 300+ devices there even without connecting USB devices and opening
> them blindly probably won't work. Is there a way to know which
> character devices are created by USB devices? Maybe they are exposed
> over /sys/bus/usb or via some other way?

Ah, OK, I see that it's also exposed as /dev/rio500 for this
particular driver. This doesn't really help, as these names will
differ for different drivers, and this will require custom syzkaller
descriptions for each driver. I'm planning to add them for some
widely-used (i.e. enabled on Android) drivers at some point, but it's
too much work to do it for all the drivers enabled on e.g. Ubuntu.

>
> >
> > Alan Stern
> >
> > > > Is there any way you can test this?
> > >
> > > Not yet.
> > >
> > > >
> > > > Alan Stern
> >