On Sun, 12 Apr 2020, syzbot wrote: > syzbot has found a reproducer for the following crash on: > > HEAD commit: 0fa84af8 Merge tag 'usb-serial-5.7-rc1' of https://git.ker.. > git tree: https://github.com/google/kasan.git usb-fuzzer > console output: https://syzkaller.appspot.com/x/log.txt?x=14453a8be00000 > kernel config: https://syzkaller.appspot.com/x/.config?x=6b9c154b0c23aecf > dashboard link: https://syzkaller.appspot.com/bug?extid=7bf5a7b0f0a1f9446f4c > compiler: gcc (GCC) 9.0.0 20181231 (experimental) > syz repro: https://syzkaller.appspot.com/x/repro.syz?x=140c644fe00000 > C reproducer: https://syzkaller.appspot.com/x/repro.c?x=163fb28be00000 > > IMPORTANT: if you fix the bug, please add the following tag to the commit: > Reported-by: syzbot+7bf5a7b0f0a1f9446f4c@xxxxxxxxxxxxxxxxxxxxxxxxx > > ================================================================== > BUG: KASAN: use-after-free in atomic_read include/asm-generic/atomic-instrumented.h:26 [inline] > BUG: KASAN: use-after-free in usb_kill_urb drivers/usb/core/urb.c:696 [inline] > BUG: KASAN: use-after-free in usb_kill_urb+0x24b/0x2c0 drivers/usb/core/urb.c:688 > Read of size 4 at addr ffff8881c6d6e210 by task systemd-udevd/1137 Details removed. Given how hard this is to reproduce, it definitely seems like some sort of race. But it's very hard to tell what is racing with what. Let's start off easy and get a little extra information at the point where the bug occurs. As far as I can tell, usbhid_close() is always supposed to be called before usbhid_stop(). Alan Stern #syz test: https://github.com/google/kasan.git 0fa84af8 Index: usb-devel/drivers/hid/usbhid/hid-core.c =================================================================== --- usb-devel.orig/drivers/hid/usbhid/hid-core.c +++ usb-devel/drivers/hid/usbhid/hid-core.c @@ -747,6 +747,13 @@ static void usbhid_close(struct hid_devi return; hid_cancel_delayed_stuff(usbhid); + if (usbhid->alan_alloc == 0) + dev_WARN(&usbhid->intf->dev, "Close after dealloc (open %d)\n", + usbhid->alan_open); + if (usbhid->alan_open != 1) + dev_WARN(&usbhid->intf->dev, "Close while open = %d\n", + usbhid->alan_open); + --usbhid->alan_open; usb_kill_urb(usbhid->urbin); usbhid->intf->needs_remote_wakeup = 0; } @@ -1120,6 +1127,7 @@ static int usbhid_start(struct hid_devic continue; if (!(usbhid->urbin = usb_alloc_urb(0, GFP_KERNEL))) goto fail; + ++usbhid->alan_alloc; pipe = usb_rcvintpipe(dev, endpoint->bEndpointAddress); usb_fill_int_urb(usbhid->urbin, dev, pipe, usbhid->inbuf, insize, hid_irq_in, hid, interval); @@ -1177,6 +1185,7 @@ static int usbhid_start(struct hid_devic usbhid_set_leds(hid); device_set_wakeup_enable(&dev->dev, 1); } + ++usbhid->alan_open; return 0; fail: @@ -1197,6 +1206,9 @@ static void usbhid_stop(struct hid_devic if (WARN_ON(!usbhid)) return; + if (usbhid->alan_open > 0) + dev_WARN(&usbhid->intf->dev, "Stop while open (alloc = %d)\n", + usbhid->alan_alloc); if (hid->quirks & HID_QUIRK_ALWAYS_POLL) { clear_bit(HID_IN_POLLING, &usbhid->iofl); usbhid->intf->needs_remote_wakeup = 0; @@ -1206,6 +1218,7 @@ static void usbhid_stop(struct hid_devic spin_lock_irq(&usbhid->lock); /* Sync with error and led handlers */ set_bit(HID_DISCONNECTED, &usbhid->iofl); spin_unlock_irq(&usbhid->lock); + --usbhid->alan_alloc; usb_kill_urb(usbhid->urbin); usb_kill_urb(usbhid->urbout); usb_kill_urb(usbhid->urbctrl); Index: usb-devel/drivers/hid/usbhid/usbhid.h =================================================================== --- usb-devel.orig/drivers/hid/usbhid/usbhid.h +++ usb-devel/drivers/hid/usbhid/usbhid.h @@ -87,6 +87,9 @@ struct usbhid_device { unsigned int retry_delay; /* Delay length in ms */ struct work_struct reset_work; /* Task context for resets */ wait_queue_head_t wait; /* For sleeping */ + + int alan_alloc; + int alan_open; }; #define hid_to_usb_dev(hid_dev) \
