KASAN: use-after-free Read in usbhid_close (3)

KASAN: use-after-free Read in usbhid_close (3)

[syzbot]

Hello,

syzbot found 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=14f1695de00000
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)

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+7bf5a7b0f0a1f9446f4c@syzkaller.appspotmail.com

==================================================================
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 ffff8881d8e72310 by task systemd-udevd/5778

CPU: 1 PID: 5778 Comm: systemd-udevd Not tainted 5.6.0-rc7-syzkaller #0
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 01/01/2011
Call Trace:
 __dump_stack lib/dump_stack.c:77 [inline]
 dump_stack+0xef/0x16e lib/dump_stack.c:118
 print_address_description.constprop.0.cold+0xd3/0x314 mm/kasan/report.c:374
 __kasan_report.cold+0x37/0x77 mm/kasan/report.c:506
 kasan_report+0xe/0x20 mm/kasan/common.c:641
 check_memory_region_inline mm/kasan/generic.c:185 [inline]
 check_memory_region+0x152/0x1c0 mm/kasan/generic.c:192
 atomic_read include/asm-generic/atomic-instrumented.h:26 [inline]
 usb_kill_urb drivers/usb/core/urb.c:696 [inline]
 usb_kill_urb+0x24b/0x2c0 drivers/usb/core/urb.c:688
 usbhid_close+0x15c/0x210 drivers/hid/usbhid/hid-core.c:750
 hid_hw_close+0xa8/0xd0 drivers/hid/hid-core.c:2100
 wacom_close+0x66/0x80 drivers/hid/wacom_sys.c:192
 input_close_device+0x110/0x1a0 drivers/input/input.c:682
 evdev_close_device drivers/input/evdev.c:428 [inline]
 evdev_release+0x188/0x1c0 drivers/input/evdev.c:466
 __fput+0x2d7/0x840 fs/file_table.c:280
 task_work_run+0x13f/0x1c0 kernel/task_work.c:113
 tracehook_notify_resume include/linux/tracehook.h:188 [inline]
 exit_to_usermode_loop+0x1d2/0x200 arch/x86/entry/common.c:164
 prepare_exit_to_usermode arch/x86/entry/common.c:195 [inline]
 syscall_return_slowpath arch/x86/entry/common.c:278 [inline]
 do_syscall_64+0x4e0/0x5a0 arch/x86/entry/common.c:304
 entry_SYSCALL_64_after_hwframe+0x49/0xbe
RIP: 0033:0x7fc61b05f270
Code: 73 01 c3 48 8b 0d 38 7d 20 00 f7 d8 64 89 01 48 83 c8 ff c3 66 0f 1f 44 00 00 83 3d 59 c1 20 00 00 75 10 b8 03 00 00 00 0f 05 <48> 3d 01 f0 ff ff 73 31 c3 48 83 ec 08 e8 ee fb ff ff 48 89 04 24
RSP: 002b:00007ffc7605c618 EFLAGS: 00000246 ORIG_RAX: 0000000000000003
RAX: 0000000000000000 RBX: 0000000000000007 RCX: 00007fc61b05f270
RDX: 0000000000000000 RSI: 0000000000000000 RDI: 0000000000000007
RBP: 00007fc61bf18710 R08: 0000556bb325a33c R09: 0000000000000078
R10: 0000000000000000 R11: 0000000000000246 R12: 0000000000000016
R13: 0000000000000000 R14: 00007ffc7605c6c4 R15: 0000000000000000

Allocated by task 5:
 save_stack+0x1b/0x80 mm/kasan/common.c:72
 set_track mm/kasan/common.c:80 [inline]
 __kasan_kmalloc mm/kasan/common.c:515 [inline]
 __kasan_kmalloc.constprop.0+0xbf/0xd0 mm/kasan/common.c:488
 kmalloc include/linux/slab.h:560 [inline]
 usb_alloc_urb+0x65/0xb0 drivers/usb/core/urb.c:74
 usbhid_start+0xb26/0x22f0 drivers/hid/usbhid/hid-core.c:1121
 hid_hw_start+0x5d/0x130 drivers/hid/hid-core.c:2030
 wacom_parse_and_register+0x28f3/0x5260 drivers/hid/wacom_sys.c:2365
 wacom_probe+0x94d/0xba0 drivers/hid/wacom_sys.c:2748
 hid_device_probe+0x2be/0x3f0 drivers/hid/hid-core.c:2263
 really_probe+0x290/0xac0 drivers/base/dd.c:551
 driver_probe_device+0x223/0x350 drivers/base/dd.c:724
 __device_attach_driver+0x1d1/0x290 drivers/base/dd.c:831
 bus_for_each_drv+0x162/0x1e0 drivers/base/bus.c:431
 __device_attach+0x217/0x390 drivers/base/dd.c:897
 bus_probe_device+0x1e4/0x290 drivers/base/bus.c:491
 device_add+0x1459/0x1bf0 drivers/base/core.c:2500
 hid_add_device drivers/hid/hid-core.c:2419 [inline]
 hid_add_device+0x33c/0x9a0 drivers/hid/hid-core.c:2368
 usbhid_probe+0xa81/0xfa0 drivers/hid/usbhid/hid-core.c:1386
 usb_probe_interface+0x310/0x800 drivers/usb/core/driver.c:374
 really_probe+0x290/0xac0 drivers/base/dd.c:551
 driver_probe_device+0x223/0x350 drivers/base/dd.c:724
 __device_attach_driver+0x1d1/0x290 drivers/base/dd.c:831
 bus_for_each_drv+0x162/0x1e0 drivers/base/bus.c:431
 __device_attach+0x217/0x390 drivers/base/dd.c:897
 bus_probe_device+0x1e4/0x290 drivers/base/bus.c:491
 device_add+0x1459/0x1bf0 drivers/base/core.c:2500
 usb_set_configuration+0xece/0x1840 drivers/usb/core/message.c:2025
 usb_generic_driver_probe+0x9d/0xe0 drivers/usb/core/generic.c:241
 usb_probe_device+0xd9/0x230 drivers/usb/core/driver.c:272
 really_probe+0x290/0xac0 drivers/base/dd.c:551
 driver_probe_device+0x223/0x350 drivers/base/dd.c:724
 __device_attach_driver+0x1d1/0x290 drivers/base/dd.c:831
 bus_for_each_drv+0x162/0x1e0 drivers/base/bus.c:431
 __device_attach+0x217/0x390 drivers/base/dd.c:897
 bus_probe_device+0x1e4/0x290 drivers/base/bus.c:491
 device_add+0x1459/0x1bf0 drivers/base/core.c:2500
 usb_new_device.cold+0x540/0xcd0 drivers/usb/core/hub.c:2548
 hub_port_connect drivers/usb/core/hub.c:5195 [inline]
 hub_port_connect_change drivers/usb/core/hub.c:5335 [inline]
 port_event drivers/usb/core/hub.c:5481 [inline]
 hub_event+0x21cb/0x4300 drivers/usb/core/hub.c:5563
 process_one_work+0x94b/0x1620 kernel/workqueue.c:2266
 worker_thread+0x96/0xe20 kernel/workqueue.c:2412
 kthread+0x318/0x420 kernel/kthread.c:255
 ret_from_fork+0x24/0x30 arch/x86/entry/entry_64.S:352

Freed by task 5:
 save_stack+0x1b/0x80 mm/kasan/common.c:72
 set_track mm/kasan/common.c:80 [inline]
 kasan_set_free_info mm/kasan/common.c:337 [inline]
 __kasan_slab_free+0x117/0x160 mm/kasan/common.c:476
 slab_free_hook mm/slub.c:1444 [inline]
 slab_free_freelist_hook mm/slub.c:1477 [inline]
 slab_free mm/slub.c:3034 [inline]
 kfree+0xd5/0x300 mm/slub.c:3995
 urb_destroy drivers/usb/core/urb.c:26 [inline]
 kref_put include/linux/kref.h:65 [inline]
 usb_free_urb.part.0+0xaf/0x110 drivers/usb/core/urb.c:96
 usb_free_urb+0x1b/0x30 drivers/usb/core/urb.c:95
 usbhid_stop+0x1d4/0x460 drivers/hid/usbhid/hid-core.c:1217
 wacom_remove+0x88/0x3b0 drivers/hid/wacom_sys.c:2772
 hid_device_remove+0xed/0x240 drivers/hid/hid-core.c:2296
 __device_release_driver drivers/base/dd.c:1135 [inline]
 device_release_driver_internal+0x231/0x500 drivers/base/dd.c:1168
 bus_remove_device+0x2eb/0x5a0 drivers/base/bus.c:533
 device_del+0x481/0xd30 drivers/base/core.c:2677
 hid_remove_device drivers/hid/hid-core.c:2467 [inline]
 hid_destroy_device+0xe1/0x150 drivers/hid/hid-core.c:2486
 usbhid_disconnect+0x9f/0xe0 drivers/hid/usbhid/hid-core.c:1413
 usb_unbind_interface+0x1bd/0x8a0 drivers/usb/core/driver.c:436
 __device_release_driver drivers/base/dd.c:1137 [inline]
 device_release_driver_internal+0x42f/0x500 drivers/base/dd.c:1168
 bus_remove_device+0x2eb/0x5a0 drivers/base/bus.c:533
 device_del+0x481/0xd30 drivers/base/core.c:2677
 usb_disable_device+0x23d/0x790 drivers/usb/core/message.c:1238
 usb_disconnect+0x293/0x900 drivers/usb/core/hub.c:2211
 hub_port_connect drivers/usb/core/hub.c:5046 [inline]
 hub_port_connect_change drivers/usb/core/hub.c:5335 [inline]
 port_event drivers/usb/core/hub.c:5481 [inline]
 hub_event+0x1a1d/0x4300 drivers/usb/core/hub.c:5563
 process_one_work+0x94b/0x1620 kernel/workqueue.c:2266
 worker_thread+0x96/0xe20 kernel/workqueue.c:2412
 kthread+0x318/0x420 kernel/kthread.c:255
 ret_from_fork+0x24/0x30 arch/x86/entry/entry_64.S:352

The buggy address belongs to the object at ffff8881d8e72300
 which belongs to the cache kmalloc-192 of size 192
The buggy address is located 16 bytes inside of
 192-byte region [ffff8881d8e72300, ffff8881d8e723c0)
The buggy address belongs to the page:
page:ffffea0007639c80 refcount:1 mapcount:0 mapping:ffff8881da002a00 index:0x0
flags: 0x200000000000200(slab)
raw: 0200000000000200 ffffea0007628f80 0000000400000004 ffff8881da002a00
raw: 0000000000000000 0000000000100010 00000001ffffffff 0000000000000000
page dumped because: kasan: bad access detected

Memory state around the buggy address:
 ffff8881d8e72200: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
 ffff8881d8e72280: fb fb fb fb fb fb fb fb fc fc fc fc fc fc fc fc
>ffff8881d8e72300: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
                         ^
 ffff8881d8e72380: fb fb fb fb fb fb fb fb fc fc fc fc fc fc fc fc
 ffff8881d8e72400: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
==================================================================


---
This bug is generated by a bot. It may contain errors.
See https://goo.gl/tpsmEJ for more information about syzbot.
syzbot engineers can be reached at syzkaller@googlegroups.com.

syzbot will keep track of this bug report. See:
https://goo.gl/tpsmEJ#status for how to communicate with syzbot.

Re: KASAN: use-after-free Read in usbhid_close (3)

[syzbot]

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@syzkaller.appspotmail.com

==================================================================
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

CPU: 0 PID: 1137 Comm: systemd-udevd Not tainted 5.6.0-rc7-syzkaller #0
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 01/01/2011
Call Trace:
 __dump_stack lib/dump_stack.c:77 [inline]
 dump_stack+0xef/0x16e lib/dump_stack.c:118
 print_address_description.constprop.0.cold+0xd3/0x314 mm/kasan/report.c:374
 __kasan_report.cold+0x37/0x77 mm/kasan/report.c:506
 kasan_report+0xe/0x20 mm/kasan/common.c:641
 check_memory_region_inline mm/kasan/generic.c:185 [inline]
 check_memory_region+0x152/0x1c0 mm/kasan/generic.c:192
 atomic_read include/asm-generic/atomic-instrumented.h:26 [inline]
 usb_kill_urb drivers/usb/core/urb.c:696 [inline]
 usb_kill_urb+0x24b/0x2c0 drivers/usb/core/urb.c:688
 usbhid_close+0x15c/0x210 drivers/hid/usbhid/hid-core.c:750
 hid_hw_close+0xa8/0xd0 drivers/hid/hid-core.c:2100
 wacom_close+0x66/0x80 drivers/hid/wacom_sys.c:192
 input_close_device+0x110/0x1a0 drivers/input/input.c:682
 evdev_close_device drivers/input/evdev.c:428 [inline]
 evdev_release+0x188/0x1c0 drivers/input/evdev.c:466
 __fput+0x2d7/0x840 fs/file_table.c:280
 task_work_run+0x13f/0x1c0 kernel/task_work.c:113
 tracehook_notify_resume include/linux/tracehook.h:188 [inline]
 exit_to_usermode_loop+0x1d2/0x200 arch/x86/entry/common.c:164
 prepare_exit_to_usermode arch/x86/entry/common.c:195 [inline]
 syscall_return_slowpath arch/x86/entry/common.c:278 [inline]
 do_syscall_64+0x4e0/0x5a0 arch/x86/entry/common.c:304
 entry_SYSCALL_64_after_hwframe+0x49/0xbe
RIP: 0033:0x7fed9d0bf270
Code: 73 01 c3 48 8b 0d 38 7d 20 00 f7 d8 64 89 01 48 83 c8 ff c3 66 0f 1f 44 00 00 83 3d 59 c1 20 00 00 75 10 b8 03 00 00 00 0f 05 <48> 3d 01 f0 ff ff 73 31 c3 48 83 ec 08 e8 ee fb ff ff 48 89 04 24
RSP: 002b:00007fff038ab128 EFLAGS: 00000246 ORIG_RAX: 0000000000000003
RAX: 0000000000000000 RBX: 0000000000000007 RCX: 00007fed9d0bf270
RDX: 0000000000000000 RSI: 0000000000000000 RDI: 0000000000000007
RBP: 00007fed9df78710 R08: 0000557eb19b2f3b R09: 0000000000000078
R10: 0000000000000000 R11: 0000000000000246 R12: 0000000000000016
R13: 0000000000000000 R14: 00007fff038ab1d4 R15: 0000000000000000

Allocated by task 418:
 save_stack+0x1b/0x80 mm/kasan/common.c:72
 set_track mm/kasan/common.c:80 [inline]
 __kasan_kmalloc mm/kasan/common.c:515 [inline]
 __kasan_kmalloc.constprop.0+0xbf/0xd0 mm/kasan/common.c:488
 kmalloc include/linux/slab.h:560 [inline]
 usb_alloc_urb+0x65/0xb0 drivers/usb/core/urb.c:74
 usbhid_start+0xb26/0x22f0 drivers/hid/usbhid/hid-core.c:1121
 hid_hw_start+0x5d/0x130 drivers/hid/hid-core.c:2030
 wacom_parse_and_register+0x28f3/0x5260 drivers/hid/wacom_sys.c:2365
 wacom_probe+0x94d/0xba0 drivers/hid/wacom_sys.c:2748
 hid_device_probe+0x2be/0x3f0 drivers/hid/hid-core.c:2263
 really_probe+0x290/0xac0 drivers/base/dd.c:551
 driver_probe_device+0x223/0x350 drivers/base/dd.c:724
 __device_attach_driver+0x1d1/0x290 drivers/base/dd.c:831
 bus_for_each_drv+0x162/0x1e0 drivers/base/bus.c:431
 __device_attach+0x217/0x390 drivers/base/dd.c:897
 bus_probe_device+0x1e4/0x290 drivers/base/bus.c:491
 device_add+0x1459/0x1bf0 drivers/base/core.c:2500
 hid_add_device drivers/hid/hid-core.c:2419 [inline]
 hid_add_device+0x33c/0x9a0 drivers/hid/hid-core.c:2368
 usbhid_probe+0xa81/0xfa0 drivers/hid/usbhid/hid-core.c:1386
 usb_probe_interface+0x310/0x800 drivers/usb/core/driver.c:374
 really_probe+0x290/0xac0 drivers/base/dd.c:551
 driver_probe_device+0x223/0x350 drivers/base/dd.c:724
 __device_attach_driver+0x1d1/0x290 drivers/base/dd.c:831
 bus_for_each_drv+0x162/0x1e0 drivers/base/bus.c:431
 __device_attach+0x217/0x390 drivers/base/dd.c:897
 bus_probe_device+0x1e4/0x290 drivers/base/bus.c:491
 device_add+0x1459/0x1bf0 drivers/base/core.c:2500
 usb_set_configuration+0xece/0x1840 drivers/usb/core/message.c:2025
 usb_generic_driver_probe+0x9d/0xe0 drivers/usb/core/generic.c:241
 usb_probe_device+0xd9/0x230 drivers/usb/core/driver.c:272
 really_probe+0x290/0xac0 drivers/base/dd.c:551
 driver_probe_device+0x223/0x350 drivers/base/dd.c:724
 __device_attach_driver+0x1d1/0x290 drivers/base/dd.c:831
 bus_for_each_drv+0x162/0x1e0 drivers/base/bus.c:431
 __device_attach+0x217/0x390 drivers/base/dd.c:897
 bus_probe_device+0x1e4/0x290 drivers/base/bus.c:491
 device_add+0x1459/0x1bf0 drivers/base/core.c:2500
 usb_new_device.cold+0x540/0xcd0 drivers/usb/core/hub.c:2548
 hub_port_connect drivers/usb/core/hub.c:5195 [inline]
 hub_port_connect_change drivers/usb/core/hub.c:5335 [inline]
 port_event drivers/usb/core/hub.c:5481 [inline]
 hub_event+0x21cb/0x4300 drivers/usb/core/hub.c:5563
 process_one_work+0x94b/0x1620 kernel/workqueue.c:2266
 worker_thread+0x96/0xe20 kernel/workqueue.c:2412
 kthread+0x318/0x420 kernel/kthread.c:255
 ret_from_fork+0x24/0x30 arch/x86/entry/entry_64.S:352

Freed by task 95:
 save_stack+0x1b/0x80 mm/kasan/common.c:72
 set_track mm/kasan/common.c:80 [inline]
 kasan_set_free_info mm/kasan/common.c:337 [inline]
 __kasan_slab_free+0x117/0x160 mm/kasan/common.c:476
 slab_free_hook mm/slub.c:1444 [inline]
 slab_free_freelist_hook mm/slub.c:1477 [inline]
 slab_free mm/slub.c:3034 [inline]
 kfree+0xd5/0x300 mm/slub.c:3995
 urb_destroy drivers/usb/core/urb.c:26 [inline]
 kref_put include/linux/kref.h:65 [inline]
 usb_free_urb.part.0+0xaf/0x110 drivers/usb/core/urb.c:96
 usb_free_urb+0x1b/0x30 drivers/usb/core/urb.c:95
 usbhid_stop+0x1d4/0x460 drivers/hid/usbhid/hid-core.c:1217
 wacom_remove+0x88/0x3b0 drivers/hid/wacom_sys.c:2772
 hid_device_remove+0xed/0x240 drivers/hid/hid-core.c:2296
 __device_release_driver drivers/base/dd.c:1135 [inline]
 device_release_driver_internal+0x231/0x500 drivers/base/dd.c:1168
 bus_remove_device+0x2eb/0x5a0 drivers/base/bus.c:533
 device_del+0x481/0xd30 drivers/base/core.c:2677
 hid_remove_device drivers/hid/hid-core.c:2467 [inline]
 hid_destroy_device+0xe1/0x150 drivers/hid/hid-core.c:2486
 usbhid_disconnect+0x9f/0xe0 drivers/hid/usbhid/hid-core.c:1413
 usb_unbind_interface+0x1bd/0x8a0 drivers/usb/core/driver.c:436
 __device_release_driver drivers/base/dd.c:1137 [inline]
 device_release_driver_internal+0x42f/0x500 drivers/base/dd.c:1168
 bus_remove_device+0x2eb/0x5a0 drivers/base/bus.c:533
 device_del+0x481/0xd30 drivers/base/core.c:2677
 usb_disable_device+0x23d/0x790 drivers/usb/core/message.c:1238
 usb_disconnect+0x293/0x900 drivers/usb/core/hub.c:2211
 hub_port_connect drivers/usb/core/hub.c:5046 [inline]
 hub_port_connect_change drivers/usb/core/hub.c:5335 [inline]
 port_event drivers/usb/core/hub.c:5481 [inline]
 hub_event+0x1a1d/0x4300 drivers/usb/core/hub.c:5563
 process_one_work+0x94b/0x1620 kernel/workqueue.c:2266
 worker_thread+0x96/0xe20 kernel/workqueue.c:2412
 kthread+0x318/0x420 kernel/kthread.c:255
 ret_from_fork+0x24/0x30 arch/x86/entry/entry_64.S:352

The buggy address belongs to the object at ffff8881c6d6e200
 which belongs to the cache kmalloc-192 of size 192
The buggy address is located 16 bytes inside of
 192-byte region [ffff8881c6d6e200, ffff8881c6d6e2c0)
The buggy address belongs to the page:
page:ffffea00071b5b80 refcount:1 mapcount:0 mapping:ffff8881da002a00 index:0x0
flags: 0x200000000000200(slab)
raw: 0200000000000200 ffffea00071c2e40 0000000500000005 ffff8881da002a00
raw: 0000000000000000 0000000000100010 00000001ffffffff 0000000000000000
page dumped because: kasan: bad access detected

Memory state around the buggy address:
 ffff8881c6d6e100: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
 ffff8881c6d6e180: fb fb fb fb fb fb fb fb fc fc fc fc fc fc fc fc
>ffff8881c6d6e200: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
                         ^
 ffff8881c6d6e280: fb fb fb fb fb fb fb fb fc fc fc fc fc fc fc fc
 ffff8881c6d6e300: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
==================================================================

Re: KASAN: use-after-free Read in usbhid_close (3)

[Alan Stern]

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@syzkaller.appspotmail.com
> 
> ==================================================================
> 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) \

Re: KASAN: use-after-free Read in usbhid_close (3)

[syzbot]

Hello,

syzbot has tested the proposed patch but the reproducer still triggered crash:
WARNING in usbhid_stop

usb 2-1: USB disconnect, device number 5
------------[ cut here ]------------
usbhid 2-1:0.0: Stop while open (alloc = 1)
WARNING: CPU: 0 PID: 12 at drivers/hid/usbhid/hid-core.c:1210 usbhid_stop+0x156/0x610 drivers/hid/usbhid/hid-core.c:1210
Kernel panic - not syncing: panic_on_warn set ...
CPU: 0 PID: 12 Comm: kworker/0:1 Not tainted 5.6.0-rc7-syzkaller #0
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 01/01/2011
Workqueue: usb_hub_wq hub_event
Call Trace:
 __dump_stack lib/dump_stack.c:77 [inline]
 dump_stack+0xef/0x16e lib/dump_stack.c:118
 panic+0x2aa/0x6e1 kernel/panic.c:221
 __warn.cold+0x2f/0x30 kernel/panic.c:582
 report_bug+0x27b/0x2f0 lib/bug.c:195
 fixup_bug arch/x86/kernel/traps.c:174 [inline]
 fixup_bug arch/x86/kernel/traps.c:169 [inline]
 do_error_trap+0x12b/0x1e0 arch/x86/kernel/traps.c:267
 do_invalid_op+0x32/0x40 arch/x86/kernel/traps.c:286
 invalid_op+0x23/0x30 arch/x86/entry/entry_64.S:1027
RIP: 0010:usbhid_stop+0x156/0x610 drivers/hid/usbhid/hid-core.c:1210
Code: 48 89 7c 24 08 e8 ea c4 bd fc 48 8b 7c 24 08 e8 10 95 f7 fd 48 8b 14 24 44 89 f9 48 c7 c7 80 45 84 86 48 89 c6 e8 92 55 92 fc <0f> 0b e8 c3 c4 bd fc 48 8d bd ac 1e 00 00 48 b8 00 00 00 00 00 fc
RSP: 0018:ffff8881da227640 EFLAGS: 00010282
RAX: 0000000000000000 RBX: ffff8881d8b48000 RCX: 0000000000000000
RDX: 0000000000000000 RSI: ffffffff812974dd RDI: ffffed103b444eba
RBP: ffff8881c7710000 R08: ffff8881da211880 R09: ffffed103b64439f
R10: ffffed103b64439e R11: ffff8881db221cf3 R12: ffff8881c54b0000
R13: ffff8881d8b4a9a8 R14: ffff8881c7711fd8 R15: 0000000000000001
 wacom_remove+0x88/0x3b0 drivers/hid/wacom_sys.c:2772
 hid_device_remove+0xed/0x240 drivers/hid/hid-core.c:2296
 __device_release_driver drivers/base/dd.c:1135 [inline]
 device_release_driver_internal+0x231/0x500 drivers/base/dd.c:1168
 bus_remove_device+0x2eb/0x5a0 drivers/base/bus.c:533
 device_del+0x481/0xd30 drivers/base/core.c:2677
 hid_remove_device drivers/hid/hid-core.c:2467 [inline]
 hid_destroy_device+0xe1/0x150 drivers/hid/hid-core.c:2486
 usbhid_disconnect+0x9f/0xe0 drivers/hid/usbhid/hid-core.c:1426
 usb_unbind_interface+0x1bd/0x8a0 drivers/usb/core/driver.c:436
 __device_release_driver drivers/base/dd.c:1137 [inline]
 device_release_driver_internal+0x42f/0x500 drivers/base/dd.c:1168
 bus_remove_device+0x2eb/0x5a0 drivers/base/bus.c:533
 device_del+0x481/0xd30 drivers/base/core.c:2677
 usb_disable_device+0x23d/0x790 drivers/usb/core/message.c:1238
 usb_disconnect+0x293/0x900 drivers/usb/core/hub.c:2211
 hub_port_connect drivers/usb/core/hub.c:5046 [inline]
 hub_port_connect_change drivers/usb/core/hub.c:5335 [inline]
 port_event drivers/usb/core/hub.c:5481 [inline]
 hub_event+0x1a1d/0x4300 drivers/usb/core/hub.c:5563
 process_one_work+0x94b/0x1620 kernel/workqueue.c:2266
 worker_thread+0x96/0xe20 kernel/workqueue.c:2412
 kthread+0x318/0x420 kernel/kthread.c:255
 ret_from_fork+0x24/0x30 arch/x86/entry/entry_64.S:352
Kernel Offset: disabled
Rebooting in 86400 seconds..


Tested on:

commit:         0fa84af8 Merge tag 'usb-serial-5.7-rc1' of https://git.ker..
git tree:       https://github.com/google/kasan.git
console output: https://syzkaller.appspot.com/x/log.txt?x=12dbb8f7e00000
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)
patch:          https://syzkaller.appspot.com/x/patch.diff?x=105bb8f7e00000

Re: KASAN: use-after-free Read in usbhid_close (3)

[Alan Stern]

On Fri, 17 Apr 2020, syzbot wrote:

> Hello,
> 
> syzbot has tested the proposed patch but the reproducer still triggered crash:
> WARNING in usbhid_stop
> 
> usb 2-1: USB disconnect, device number 5
> ------------[ cut here ]------------
> usbhid 2-1:0.0: Stop while open (alloc = 1)

Okay, good.  usbhid_close() should have been called before this 
happened.  Let's try tracing the pathway; maybe this will show where it 
goes off the rails.

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,7 @@ static void usbhid_close(struct hid_devi
 		return;
 
 	hid_cancel_delayed_stuff(usbhid);
+	--hid->alan_open;
 	usb_kill_urb(usbhid->urbin);
 	usbhid->intf->needs_remote_wakeup = 0;
 }
@@ -1177,6 +1178,7 @@ static int usbhid_start(struct hid_devic
 		usbhid_set_leds(hid);
 		device_set_wakeup_enable(&dev->dev, 1);
 	}
+	++hid->alan_open;
 	return 0;
 
 fail:
@@ -1197,6 +1199,10 @@ static void usbhid_stop(struct hid_devic
 	if (WARN_ON(!usbhid))
 		return;
 
+	if (hid->alan_open > 0)
+		dev_WARN(&usbhid->intf->dev, "Stop while open = %d: %d %d %d\n",
+				hid->alan_open,
+				hid->alan1, hid->alan2, hid->alan3);
 	if (hid->quirks & HID_QUIRK_ALWAYS_POLL) {
 		clear_bit(HID_IN_POLLING, &usbhid->iofl);
 		usbhid->intf->needs_remote_wakeup = 0;
Index: usb-devel/drivers/hid/hid-input.c
===================================================================
--- usb-devel.orig/drivers/hid/hid-input.c
+++ usb-devel/drivers/hid/hid-input.c
@@ -1960,6 +1960,7 @@ void hidinput_disconnect(struct hid_devi
 {
 	struct hid_input *hidinput, *next;
 
+	++hid->alan1;
 	hidinput_cleanup_battery(hid);
 
 	list_for_each_entry_safe(hidinput, next, &hid->inputs, list) {
Index: usb-devel/drivers/input/evdev.c
===================================================================
--- usb-devel.orig/drivers/input/evdev.c
+++ usb-devel/drivers/input/evdev.c
@@ -23,6 +23,7 @@
 #include <linux/major.h>
 #include <linux/device.h>
 #include <linux/cdev.h>
+#include <linux/hid.h>
 #include "input-compat.h"
 
 struct evdev {
@@ -1329,6 +1330,11 @@ static void evdev_mark_dead(struct evdev
 static void evdev_cleanup(struct evdev *evdev)
 {
 	struct input_handle *handle = &evdev->handle;
+	struct hid_device *hid;
+
+	hid = (struct hid_device *) input_get_drvdata(evdev->handle.dev);
+	if (hid)
+		++hid->alan3;
 
 	evdev_mark_dead(evdev);
 	evdev_hangup(evdev);
Index: usb-devel/drivers/input/input.c
===================================================================
--- usb-devel.orig/drivers/input/input.c
+++ usb-devel/drivers/input/input.c
@@ -23,6 +23,7 @@
 #include <linux/device.h>
 #include <linux/mutex.h>
 #include <linux/rcupdate.h>
+#include <linux/hid.h>
 #include "input-compat.h"
 #include "input-poller.h"
 
@@ -2081,7 +2082,11 @@ static void input_cleanse_bitmasks(struc
 static void __input_unregister_device(struct input_dev *dev)
 {
 	struct input_handle *handle, *next;
+	struct hid_device *hid;
 
+	hid = (struct hid_device *) input_get_drvdata(dev);
+	if (hid)
+		++hid->alan2;
 	input_disconnect_device(dev);
 
 	mutex_lock(&input_mutex);
Index: usb-devel/include/linux/hid.h
===================================================================
--- usb-devel.orig/include/linux/hid.h
+++ usb-devel/include/linux/hid.h
@@ -618,6 +618,9 @@ struct hid_device {							/* device repo
 	struct list_head debug_list;
 	spinlock_t  debug_list_lock;
 	wait_queue_head_t debug_wait;
+
+	int alan_open;
+	int alan1, alan2, alan3;
 };
 
 #define to_hid_device(pdev) \

Re: KASAN: use-after-free Read in usbhid_close (3)

[syzbot]

Hello,

syzbot has tested the proposed patch but the reproducer still triggered crash:
KASAN: use-after-free Read in __input_unregister_device

wacom 0003:056A:0061.0002: Unknown device_type for 'HID 056a:0061'. Assuming pen.
input: Wacom PenStation2 Pen as /devices/platform/dummy_hcd.1/usb2/2-1/2-1:0.0/0003:056A:0061.0002/input/input8
wacom 0003:056A:0061.0002: hidraw1: USB HID v0.00 Device [HID 056a:0061] on usb-dummy_hcd.1-1/input0
usb 6-1: USB disconnect, device number 2
==================================================================
BUG: KASAN: use-after-free in __input_unregister_device+0x4a6/0x4c0 drivers/input/input.c:2089
Read of size 4 at addr ffff8881cc1380e8 by task kworker/0:1/12

CPU: 0 PID: 12 Comm: kworker/0:1 Not tainted 5.6.0-rc7-syzkaller #0
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 01/01/2011
Workqueue: usb_hub_wq hub_event
Call Trace:
 __dump_stack lib/dump_stack.c:77 [inline]
 dump_stack+0xef/0x16e lib/dump_stack.c:118
 print_address_description.constprop.0.cold+0xd3/0x314 mm/kasan/report.c:374
 __kasan_report.cold+0x37/0x77 mm/kasan/report.c:506
 kasan_report+0xe/0x20 mm/kasan/common.c:641
 __input_unregister_device+0x4a6/0x4c0 drivers/input/input.c:2089
 release_nodes+0x499/0x910 drivers/base/devres.c:507
 devres_release_group+0x160/0x210 drivers/base/devres.c:678
 wacom_release_resources drivers/hid/wacom_sys.c:2238 [inline]
 wacom_remove+0x244/0x3b0 drivers/hid/wacom_sys.c:2786
 hid_device_remove+0xed/0x240 drivers/hid/hid-core.c:2296
 __device_release_driver drivers/base/dd.c:1135 [inline]
 device_release_driver_internal+0x231/0x500 drivers/base/dd.c:1168
 bus_remove_device+0x2eb/0x5a0 drivers/base/bus.c:533
 device_del+0x481/0xd30 drivers/base/core.c:2677
 hid_remove_device drivers/hid/hid-core.c:2467 [inline]
 hid_destroy_device+0xe1/0x150 drivers/hid/hid-core.c:2486
 usbhid_disconnect+0x9f/0xe0 drivers/hid/usbhid/hid-core.c:1419
 usb_unbind_interface+0x1bd/0x8a0 drivers/usb/core/driver.c:436
 __device_release_driver drivers/base/dd.c:1137 [inline]
 device_release_driver_internal+0x42f/0x500 drivers/base/dd.c:1168
 bus_remove_device+0x2eb/0x5a0 drivers/base/bus.c:533
 device_del+0x481/0xd30 drivers/base/core.c:2677
 usb_disable_device+0x23d/0x790 drivers/usb/core/message.c:1238
 usb_disconnect+0x293/0x900 drivers/usb/core/hub.c:2211
 hub_port_connect drivers/usb/core/hub.c:5046 [inline]
 hub_port_connect_change drivers/usb/core/hub.c:5335 [inline]
 port_event drivers/usb/core/hub.c:5481 [inline]
 hub_event+0x1a1d/0x4300 drivers/usb/core/hub.c:5563
 process_one_work+0x94b/0x1620 kernel/workqueue.c:2266
 worker_thread+0x96/0xe20 kernel/workqueue.c:2412
 kthread+0x318/0x420 kernel/kthread.c:255
 ret_from_fork+0x24/0x30 arch/x86/entry/entry_64.S:352

Allocated by task 3265:
 save_stack+0x1b/0x80 mm/kasan/common.c:72
 set_track mm/kasan/common.c:80 [inline]
 __kasan_kmalloc mm/kasan/common.c:515 [inline]
 __kasan_kmalloc.constprop.0+0xbf/0xd0 mm/kasan/common.c:488
 slab_post_alloc_hook mm/slab.h:584 [inline]
 slab_alloc_node mm/slub.c:2786 [inline]
 slab_alloc mm/slub.c:2794 [inline]
 kmem_cache_alloc+0xd8/0x300 mm/slub.c:2799
 getname_flags fs/namei.c:138 [inline]
 getname_flags+0xd2/0x5b0 fs/namei.c:128
 user_path_at_empty+0x2a/0x50 fs/namei.c:2746
 user_path_at include/linux/namei.h:58 [inline]
 do_faccessat+0x248/0x7a0 fs/open.c:398
 do_syscall_64+0xb6/0x5a0 arch/x86/entry/common.c:294
 entry_SYSCALL_64_after_hwframe+0x49/0xbe

Freed by task 3265:
 save_stack+0x1b/0x80 mm/kasan/common.c:72
 set_track mm/kasan/common.c:80 [inline]
 kasan_set_free_info mm/kasan/common.c:337 [inline]
 __kasan_slab_free+0x117/0x160 mm/kasan/common.c:476
 slab_free_hook mm/slub.c:1444 [inline]
 slab_free_freelist_hook mm/slub.c:1477 [inline]
 slab_free mm/slub.c:3034 [inline]
 kmem_cache_free+0x9b/0x360 mm/slub.c:3050
 putname+0xe1/0x120 fs/namei.c:259
 filename_lookup+0x282/0x3e0 fs/namei.c:2475
 user_path_at include/linux/namei.h:58 [inline]
 do_faccessat+0x248/0x7a0 fs/open.c:398
 do_syscall_64+0xb6/0x5a0 arch/x86/entry/common.c:294
 entry_SYSCALL_64_after_hwframe+0x49/0xbe

The buggy address belongs to the object at ffff8881cc138000
 which belongs to the cache names_cache of size 4096
The buggy address is located 232 bytes inside of
 4096-byte region [ffff8881cc138000, ffff8881cc139000)
The buggy address belongs to the page:
page:ffffea0007304e00 refcount:1 mapcount:0 mapping:ffff8881da11c000 index:0x0 compound_mapcount: 0
flags: 0x200000000010200(slab|head)
raw: 0200000000010200 dead000000000100 dead000000000122 ffff8881da11c000
raw: 0000000000000000 0000000000070007 00000001ffffffff 0000000000000000
page dumped because: kasan: bad access detected

Memory state around the buggy address:
 ffff8881cc137f80: fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc
 ffff8881cc138000: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
>ffff8881cc138080: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
                                                          ^
 ffff8881cc138100: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
 ffff8881cc138180: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
==================================================================


Tested on:

commit:         0fa84af8 Merge tag 'usb-serial-5.7-rc1' of https://git.ker..
git tree:       https://github.com/google/kasan.git
console output: https://syzkaller.appspot.com/x/log.txt?x=169d38d7e00000
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)
patch:          https://syzkaller.appspot.com/x/patch.diff?x=17a95273e00000

Re: KASAN: use-after-free Read in usbhid_close (3)

[Alan Stern]

On Fri, 17 Apr 2020, syzbot wrote:

> Hello,
> 
> syzbot has tested the proposed patch but the reproducer still triggered crash:
> KASAN: use-after-free Read in __input_unregister_device
> 
> wacom 0003:056A:0061.0002: Unknown device_type for 'HID 056a:0061'. Assuming pen.
> input: Wacom PenStation2 Pen as /devices/platform/dummy_hcd.1/usb2/2-1/2-1:0.0/0003:056A:0061.0002/input/input8
> wacom 0003:056A:0061.0002: hidraw1: USB HID v0.00 Device [HID 056a:0061] on usb-dummy_hcd.1-1/input0
> usb 6-1: USB disconnect, device number 2
> ==================================================================
> BUG: KASAN: use-after-free in __input_unregister_device+0x4a6/0x4c0 drivers/input/input.c:2089
> Read of size 4 at addr ffff8881cc1380e8 by task kworker/0:1/12

Oops.  Wrong kind of error and in the wrong place.  So I need to be a
little more careful about the debugging code.  Let's try again.

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,7 @@ static void usbhid_close(struct hid_devi
 		return;
 
 	hid_cancel_delayed_stuff(usbhid);
+	--hid->alan_open;
 	usb_kill_urb(usbhid->urbin);
 	usbhid->intf->needs_remote_wakeup = 0;
 }
@@ -1177,6 +1178,7 @@ static int usbhid_start(struct hid_devic
 		usbhid_set_leds(hid);
 		device_set_wakeup_enable(&dev->dev, 1);
 	}
+	++hid->alan_open;
 	return 0;
 
 fail:
@@ -1197,6 +1199,11 @@ static void usbhid_stop(struct hid_devic
 	if (WARN_ON(!usbhid))
 		return;
 
+	dev_info(&usbhid->intf->dev, "Stop: %d %d %d\n",
+			hid->alan1, hid->alan2, hid->alan3);
+	if (hid->alan_open > 0)
+		dev_WARN(&usbhid->intf->dev, "Stop while open = %d\n",
+				hid->alan_open);
 	if (hid->quirks & HID_QUIRK_ALWAYS_POLL) {
 		clear_bit(HID_IN_POLLING, &usbhid->iofl);
 		usbhid->intf->needs_remote_wakeup = 0;
Index: usb-devel/drivers/hid/hid-input.c
===================================================================
--- usb-devel.orig/drivers/hid/hid-input.c
+++ usb-devel/drivers/hid/hid-input.c
@@ -1960,6 +1960,7 @@ void hidinput_disconnect(struct hid_devi
 {
 	struct hid_input *hidinput, *next;
 
+	++hid->alan1;
 	hidinput_cleanup_battery(hid);
 
 	list_for_each_entry_safe(hidinput, next, &hid->inputs, list) {
Index: usb-devel/drivers/input/evdev.c
===================================================================
--- usb-devel.orig/drivers/input/evdev.c
+++ usb-devel/drivers/input/evdev.c
@@ -23,6 +23,7 @@
 #include <linux/major.h>
 #include <linux/device.h>
 #include <linux/cdev.h>
+#include <linux/hid.h>
 #include "input-compat.h"
 
 struct evdev {
@@ -1329,6 +1330,11 @@ static void evdev_mark_dead(struct evdev
 static void evdev_cleanup(struct evdev *evdev)
 {
 	struct input_handle *handle = &evdev->handle;
+	struct hid_device *hid;
+
+	hid = evdev->handle.dev->hid;
+	if (hid)
+		++hid->alan3;
 
 	evdev_mark_dead(evdev);
 	evdev_hangup(evdev);
Index: usb-devel/drivers/input/input.c
===================================================================
--- usb-devel.orig/drivers/input/input.c
+++ usb-devel/drivers/input/input.c
@@ -23,6 +23,7 @@
 #include <linux/device.h>
 #include <linux/mutex.h>
 #include <linux/rcupdate.h>
+#include <linux/hid.h>
 #include "input-compat.h"
 #include "input-poller.h"
 
@@ -2081,7 +2082,11 @@ static void input_cleanse_bitmasks(struc
 static void __input_unregister_device(struct input_dev *dev)
 {
 	struct input_handle *handle, *next;
+	struct hid_device *hid;
 
+	hid = dev->hid;
+	if (hid)
+		++hid->alan2;
 	input_disconnect_device(dev);
 
 	mutex_lock(&input_mutex);
Index: usb-devel/include/linux/hid.h
===================================================================
--- usb-devel.orig/include/linux/hid.h
+++ usb-devel/include/linux/hid.h
@@ -618,6 +618,9 @@ struct hid_device {							/* device repo
 	struct list_head debug_list;
 	spinlock_t  debug_list_lock;
 	wait_queue_head_t debug_wait;
+
+	int alan_open;
+	int alan1, alan2, alan3;
 };
 
 #define to_hid_device(pdev) \
Index: usb-devel/include/linux/input.h
===================================================================
--- usb-devel.orig/include/linux/input.h
+++ usb-devel/include/linux/input.h
@@ -22,6 +22,7 @@
 #include <linux/mod_devicetable.h>
 
 struct input_dev_poller;
+struct hid_device;
 
 /**
  * struct input_value - input value representation
@@ -129,6 +130,7 @@ enum input_clock_type {
  *  by a driver
  */
 struct input_dev {
+	struct hid_device *hid;
 	const char *name;
 	const char *phys;
 	const char *uniq;
Index: usb-devel/drivers/hid/wacom_sys.c
===================================================================
--- usb-devel.orig/drivers/hid/wacom_sys.c
+++ usb-devel/drivers/hid/wacom_sys.c
@@ -2017,6 +2017,7 @@ static struct input_dev *wacom_allocate_
 	if (!input_dev)
 		return NULL;
 
+	input_dev->hid = hdev;
 	input_dev->name = wacom_wac->features.name;
 	input_dev->phys = hdev->phys;
 	input_dev->dev.parent = &hdev->dev;

Re: KASAN: use-after-free Read in usbhid_close (3)

[syzbot]

Hello,

syzbot has tested the proposed patch but the reproducer still triggered crash:
WARNING in usbhid_stop

usb 6-1: USB disconnect, device number 2
usbhid 6-1:0.0: Stop: 0 0 0
------------[ cut here ]------------
usbhid 6-1:0.0: Stop while open = 1
WARNING: CPU: 1 PID: 17 at drivers/hid/usbhid/hid-core.c:1205 usbhid_stop.cold+0x1c7/0x5a6 drivers/hid/usbhid/hid-core.c:1205
Kernel panic - not syncing: panic_on_warn set ...
CPU: 1 PID: 17 Comm: kworker/1:0 Not tainted 5.6.0-rc7-syzkaller #0
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 01/01/2011
Workqueue: usb_hub_wq hub_event
Call Trace:
 __dump_stack lib/dump_stack.c:77 [inline]
 dump_stack+0xef/0x16e lib/dump_stack.c:118
 panic+0x2aa/0x6e1 kernel/panic.c:221
 __warn.cold+0x2f/0x30 kernel/panic.c:582
 report_bug+0x27b/0x2f0 lib/bug.c:195
 fixup_bug arch/x86/kernel/traps.c:174 [inline]
 fixup_bug arch/x86/kernel/traps.c:169 [inline]
 do_error_trap+0x12b/0x1e0 arch/x86/kernel/traps.c:267
 do_invalid_op+0x32/0x40 arch/x86/kernel/traps.c:286
 invalid_op+0x23/0x30 arch/x86/entry/entry_64.S:1027
RIP: 0010:usbhid_stop.cold+0x1c7/0x5a6 drivers/hid/usbhid/hid-core.c:1205
Code: 48 89 7c 24 08 e8 9f 5e bd fc 48 8b 7c 24 08 e8 c5 2e f7 fd 48 8b 14 24 44 89 f9 48 c7 c7 20 4b 84 86 48 89 c6 e8 47 ef 91 fc <0f> 0b e8 78 5e bd fc 48 8d bb ac 1e 00 00 b8 ff ff 37 00 48 89 fa
RSP: 0018:ffff8881da267640 EFLAGS: 00010282
RAX: 0000000000000000 RBX: ffff8881cd788000 RCX: 0000000000000000
RDX: 0000000000000000 RSI: ffffffff812974dd RDI: ffffed103b44ceba
RBP: ffff8881cd22c000 R08: ffff8881da24b100 R09: ffffed103b66439f
R10: ffffed103b66439e R11: ffff8881db321cf3 R12: ffff8881cd789fd8
R13: ffff8881cd22c008 R14: ffff8881d6c41000 R15: 0000000000000001
 wacom_remove+0x88/0x3b0 drivers/hid/wacom_sys.c:2773
 hid_device_remove+0xed/0x240 drivers/hid/hid-core.c:2296
 __device_release_driver drivers/base/dd.c:1135 [inline]
 device_release_driver_internal+0x231/0x500 drivers/base/dd.c:1168
 bus_remove_device+0x2eb/0x5a0 drivers/base/bus.c:533
 device_del+0x481/0xd30 drivers/base/core.c:2677
 hid_remove_device drivers/hid/hid-core.c:2467 [inline]
 hid_destroy_device+0xe1/0x150 drivers/hid/hid-core.c:2486
 usbhid_disconnect+0x9f/0xe0 drivers/hid/usbhid/hid-core.c:1420
 usb_unbind_interface+0x1bd/0x8a0 drivers/usb/core/driver.c:436
 __device_release_driver drivers/base/dd.c:1137 [inline]
 device_release_driver_internal+0x42f/0x500 drivers/base/dd.c:1168
 bus_remove_device+0x2eb/0x5a0 drivers/base/bus.c:533
 device_del+0x481/0xd30 drivers/base/core.c:2677
 usb_disable_device+0x23d/0x790 drivers/usb/core/message.c:1238
 usb_disconnect+0x293/0x900 drivers/usb/core/hub.c:2211
 hub_port_connect drivers/usb/core/hub.c:5046 [inline]
 hub_port_connect_change drivers/usb/core/hub.c:5335 [inline]
 port_event drivers/usb/core/hub.c:5481 [inline]
 hub_event+0x1a1d/0x4300 drivers/usb/core/hub.c:5563
 process_one_work+0x94b/0x1620 kernel/workqueue.c:2266
 worker_thread+0x96/0xe20 kernel/workqueue.c:2412
 kthread+0x318/0x420 kernel/kthread.c:255
 ret_from_fork+0x24/0x30 arch/x86/entry/entry_64.S:352
Kernel Offset: disabled
Rebooting in 86400 seconds..


Tested on:

commit:         0fa84af8 Merge tag 'usb-serial-5.7-rc1' of https://git.ker..
git tree:       https://github.com/google/kasan.git
console output: https://syzkaller.appspot.com/x/log.txt?x=165078f7e00000
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)
patch:          https://syzkaller.appspot.com/x/patch.diff?x=15cef627e00000

Re: KASAN: use-after-free Read in usbhid_close (3)

[Alan Stern]

On Sat, 18 Apr 2020, syzbot wrote:

> Hello,
> 
> syzbot has tested the proposed patch but the reproducer still triggered crash:
> WARNING in usbhid_stop
> 
> usb 6-1: USB disconnect, device number 2
> usbhid 6-1:0.0: Stop: 0 0 0
> ------------[ cut here ]------------
> usbhid 6-1:0.0: Stop while open = 1

Okay, now we're getting somewhere.  That "Stop: 0 0 0" is unexpected.  
Let's probe the starting part of the close pathway more closely.

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,7 @@ static void usbhid_close(struct hid_devi
 		return;
 
 	hid_cancel_delayed_stuff(usbhid);
+	--hid->alan_open;
 	usb_kill_urb(usbhid->urbin);
 	usbhid->intf->needs_remote_wakeup = 0;
 }
@@ -1177,6 +1178,7 @@ static int usbhid_start(struct hid_devic
 		usbhid_set_leds(hid);
 		device_set_wakeup_enable(&dev->dev, 1);
 	}
+	++hid->alan_open;
 	return 0;
 
 fail:
@@ -1197,6 +1199,11 @@ static void usbhid_stop(struct hid_devic
 	if (WARN_ON(!usbhid))
 		return;
 
+	dev_info(&usbhid->intf->dev, "Stop: %d %d %d %d\n",
+			hid->alan1, hid->alan2, hid->alan3, hid->alan4);
+	if (hid->alan_open > 0)
+		dev_WARN(&usbhid->intf->dev, "Stop while open = %d\n",
+				hid->alan_open);
 	if (hid->quirks & HID_QUIRK_ALWAYS_POLL) {
 		clear_bit(HID_IN_POLLING, &usbhid->iofl);
 		usbhid->intf->needs_remote_wakeup = 0;
Index: usb-devel/drivers/hid/hid-input.c
===================================================================
--- usb-devel.orig/drivers/hid/hid-input.c
+++ usb-devel/drivers/hid/hid-input.c
@@ -1960,6 +1960,7 @@ void hidinput_disconnect(struct hid_devi
 {
 	struct hid_input *hidinput, *next;
 
+	++hid->alan3;
 	hidinput_cleanup_battery(hid);
 
 	list_for_each_entry_safe(hidinput, next, &hid->inputs, list) {
Index: usb-devel/drivers/input/input.c
===================================================================
--- usb-devel.orig/drivers/input/input.c
+++ usb-devel/drivers/input/input.c
@@ -23,6 +23,7 @@
 #include <linux/device.h>
 #include <linux/mutex.h>
 #include <linux/rcupdate.h>
+#include <linux/hid.h>
 #include "input-compat.h"
 #include "input-poller.h"
 
@@ -2258,6 +2259,11 @@ EXPORT_SYMBOL(input_register_device);
  */
 void input_unregister_device(struct input_dev *dev)
 {
+	struct hid_device *hid;
+
+	hid = dev->hid;
+	if (hid)
+		++hid->alan4;
 	if (dev->devres_managed) {
 		WARN_ON(devres_destroy(dev->dev.parent,
 					devm_input_device_unregister,
Index: usb-devel/include/linux/hid.h
===================================================================
--- usb-devel.orig/include/linux/hid.h
+++ usb-devel/include/linux/hid.h
@@ -618,6 +618,9 @@ struct hid_device {							/* device repo
 	struct list_head debug_list;
 	spinlock_t  debug_list_lock;
 	wait_queue_head_t debug_wait;
+
+	int alan_open;
+	int alan1, alan2, alan3, alan4;
 };
 
 #define to_hid_device(pdev) \
Index: usb-devel/include/linux/input.h
===================================================================
--- usb-devel.orig/include/linux/input.h
+++ usb-devel/include/linux/input.h
@@ -22,6 +22,7 @@
 #include <linux/mod_devicetable.h>
 
 struct input_dev_poller;
+struct hid_device;
 
 /**
  * struct input_value - input value representation
@@ -129,6 +130,7 @@ enum input_clock_type {
  *  by a driver
  */
 struct input_dev {
+	struct hid_device *hid;
 	const char *name;
 	const char *phys;
 	const char *uniq;
Index: usb-devel/drivers/hid/wacom_sys.c
===================================================================
--- usb-devel.orig/drivers/hid/wacom_sys.c
+++ usb-devel/drivers/hid/wacom_sys.c
@@ -2017,6 +2017,7 @@ static struct input_dev *wacom_allocate_
 	if (!input_dev)
 		return NULL;
 
+	input_dev->hid = hdev;
 	input_dev->name = wacom_wac->features.name;
 	input_dev->phys = hdev->phys;
 	input_dev->dev.parent = &hdev->dev;
Index: usb-devel/drivers/hid/hid-core.c
===================================================================
--- usb-devel.orig/drivers/hid/hid-core.c
+++ usb-devel/drivers/hid/hid-core.c
@@ -2001,6 +2001,7 @@ EXPORT_SYMBOL_GPL(hid_connect);
 
 void hid_disconnect(struct hid_device *hdev)
 {
+	++hdev->alan2;
 	device_remove_file(&hdev->dev, &dev_attr_country);
 	if (hdev->claimed & HID_CLAIMED_INPUT)
 		hidinput_disconnect(hdev);
@@ -2050,6 +2051,7 @@ EXPORT_SYMBOL_GPL(hid_hw_start);
  */
 void hid_hw_stop(struct hid_device *hdev)
 {
+	++hdev->alan1;
 	hid_disconnect(hdev);
 	hdev->ll_driver->stop(hdev);
 }

Re: KASAN: use-after-free Read in usbhid_close (3)

[syzbot]

Hello,

syzbot has tested the proposed patch but the reproducer still triggered crash:
WARNING in usbhid_stop

usbhid 6-1:0.0: Stop: 1 1 0 0
------------[ cut here ]------------
usbhid 6-1:0.0: Stop while open = 1
WARNING: CPU: 0 PID: 12 at drivers/hid/usbhid/hid-core.c:1205 usbhid_stop.cold+0x1fb/0x5e6 drivers/hid/usbhid/hid-core.c:1205
Kernel panic - not syncing: panic_on_warn set ...
CPU: 0 PID: 12 Comm: kworker/0:1 Not tainted 5.6.0-rc7-syzkaller #0
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 01/01/2011
Workqueue: usb_hub_wq hub_event
Call Trace:
 __dump_stack lib/dump_stack.c:77 [inline]
 dump_stack+0xef/0x16e lib/dump_stack.c:118
 panic+0x2aa/0x6e1 kernel/panic.c:221
 __warn.cold+0x2f/0x30 kernel/panic.c:582
 report_bug+0x27b/0x2f0 lib/bug.c:195
 fixup_bug arch/x86/kernel/traps.c:174 [inline]
 fixup_bug arch/x86/kernel/traps.c:169 [inline]
 do_error_trap+0x12b/0x1e0 arch/x86/kernel/traps.c:267
 do_invalid_op+0x32/0x40 arch/x86/kernel/traps.c:286
 invalid_op+0x23/0x30 arch/x86/entry/entry_64.S:1027
RIP: 0010:usbhid_stop.cold+0x1fb/0x5e6 drivers/hid/usbhid/hid-core.c:1205
Code: 48 89 7c 24 08 e8 0b 5f bd fc 48 8b 7c 24 08 e8 31 2f f7 fd 48 8b 14 24 44 89 f1 48 c7 c7 20 4b 84 86 48 89 c6 e8 b3 ef 91 fc <0f> 0b e8 e4 5e bd fc 48 8d bb ac 1e 00 00 b8 ff ff 37 00 48 89 fa
RSP: 0018:ffff8881da227640 EFLAGS: 00010282
RAX: 0000000000000000 RBX: ffff8881cf7a8000 RCX: 0000000000000000
RDX: 0000000000000000 RSI: ffffffff812974dd RDI: ffffed103b444eba
RBP: ffff8881d1de4000 R08: ffff8881da211880 R09: ffffed103b64439f
R10: ffffed103b64439e R11: ffff8881db221cf3 R12: ffff8881d1de4008
R13: ffff8881c5ff4000 R14: 0000000000000001 R15: ffff8881cf7a9fd8
 wacom_remove+0x88/0x3b0 drivers/hid/wacom_sys.c:2773
 hid_device_remove+0xed/0x1d0 drivers/hid/hid-core.c:2298
 __device_release_driver drivers/base/dd.c:1135 [inline]
 device_release_driver_internal+0x231/0x500 drivers/base/dd.c:1168
 bus_remove_device+0x2eb/0x5a0 drivers/base/bus.c:533
 device_del+0x481/0xd30 drivers/base/core.c:2677
 hid_remove_device drivers/hid/hid-core.c:2469 [inline]
 hid_destroy_device+0xe1/0x150 drivers/hid/hid-core.c:2488
 usbhid_disconnect+0x9f/0xe0 drivers/hid/usbhid/hid-core.c:1420
 usb_unbind_interface+0x1bd/0x8a0 drivers/usb/core/driver.c:436
 __device_release_driver drivers/base/dd.c:1137 [inline]
 device_release_driver_internal+0x42f/0x500 drivers/base/dd.c:1168
 bus_remove_device+0x2eb/0x5a0 drivers/base/bus.c:533
 device_del+0x481/0xd30 drivers/base/core.c:2677
 usb_disable_device+0x23d/0x790 drivers/usb/core/message.c:1238
 usb_disconnect+0x293/0x900 drivers/usb/core/hub.c:2211
 hub_port_connect drivers/usb/core/hub.c:5046 [inline]
 hub_port_connect_change drivers/usb/core/hub.c:5335 [inline]
 port_event drivers/usb/core/hub.c:5481 [inline]
 hub_event+0x1a1d/0x4300 drivers/usb/core/hub.c:5563
 process_one_work+0x94b/0x1620 kernel/workqueue.c:2266
 worker_thread+0x96/0xe20 kernel/workqueue.c:2412
 kthread+0x318/0x420 kernel/kthread.c:255
 ret_from_fork+0x24/0x30 arch/x86/entry/entry_64.S:352
Kernel Offset: disabled
Rebooting in 86400 seconds..


Tested on:

commit:         0fa84af8 Merge tag 'usb-serial-5.7-rc1' of https://git.ker..
git tree:       https://github.com/google/kasan.git
console output: https://syzkaller.appspot.com/x/log.txt?x=17f91227e00000
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)
patch:          https://syzkaller.appspot.com/x/patch.diff?x=1216d073e00000

Re: KASAN: use-after-free Read in usbhid_close (3)

[Alan Stern]

On Sat, 18 Apr 2020, syzbot wrote:

> Hello,
> 
> syzbot has tested the proposed patch but the reproducer still triggered crash:
> WARNING in usbhid_stop
> 
> usbhid 6-1:0.0: Stop: 1 1 0 0
> ------------[ cut here ]------------
> usbhid 6-1:0.0: Stop while open = 1

This indicates there is some confusion about whether the wacom device
is an input device.  Apparently hidinput_connect() either doesn't get
called or fails, but nevertheless an evdev input handler gets
associated with the device.

Maybe the confusion is only in my head.  This test may help clear it up 
a little.

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,7 @@ static void usbhid_close(struct hid_devi
 		return;
 
 	hid_cancel_delayed_stuff(usbhid);
+	--hid->alan_open;
 	usb_kill_urb(usbhid->urbin);
 	usbhid->intf->needs_remote_wakeup = 0;
 }
@@ -1177,6 +1178,7 @@ static int usbhid_start(struct hid_devic
 		usbhid_set_leds(hid);
 		device_set_wakeup_enable(&dev->dev, 1);
 	}
+	++hid->alan_open;
 	return 0;
 
 fail:
@@ -1197,6 +1199,11 @@ static void usbhid_stop(struct hid_devic
 	if (WARN_ON(!usbhid))
 		return;
 
+	dev_info(&usbhid->intf->dev, "Stop: %d %d %d %d\n",
+			hid->alan1, hid->alan2, hid->alan3, hid->alan4);
+	if (hid->alan_open > 0)
+		dev_WARN(&usbhid->intf->dev, "Stop while open = %d\n",
+				hid->alan_open);
 	if (hid->quirks & HID_QUIRK_ALWAYS_POLL) {
 		clear_bit(HID_IN_POLLING, &usbhid->iofl);
 		usbhid->intf->needs_remote_wakeup = 0;
Index: usb-devel/drivers/hid/hid-input.c
===================================================================
--- usb-devel.orig/drivers/hid/hid-input.c
+++ usb-devel/drivers/hid/hid-input.c
@@ -1861,6 +1861,7 @@ int hidinput_connect(struct hid_device *
 	unsigned int application;
 	int i, k;
 
+	dev_info(&hid->dev, "In hidinput_connect\n");
 	INIT_LIST_HEAD(&hid->inputs);
 	INIT_WORK(&hid->led_work, hidinput_led_worker);
 
@@ -1960,6 +1961,7 @@ void hidinput_disconnect(struct hid_devi
 {
 	struct hid_input *hidinput, *next;
 
+	++hid->alan3;
 	hidinput_cleanup_battery(hid);
 
 	list_for_each_entry_safe(hidinput, next, &hid->inputs, list) {
Index: usb-devel/drivers/input/evdev.c
===================================================================
--- usb-devel.orig/drivers/input/evdev.c
+++ usb-devel/drivers/input/evdev.c
@@ -23,6 +23,7 @@
 #include <linux/major.h>
 #include <linux/device.h>
 #include <linux/cdev.h>
+#include <linux/hid.h>
 #include "input-compat.h"
 
 struct evdev {
@@ -1329,6 +1330,11 @@ static void evdev_mark_dead(struct evdev
 static void evdev_cleanup(struct evdev *evdev)
 {
 	struct input_handle *handle = &evdev->handle;
+//	struct hid_device *hid;
+
+//	hid = evdev->handle.dev->hid;
+//	if (hid)
+//		++hid->alan3;
 
 	evdev_mark_dead(evdev);
 	evdev_hangup(evdev);
Index: usb-devel/drivers/input/input.c
===================================================================
--- usb-devel.orig/drivers/input/input.c
+++ usb-devel/drivers/input/input.c
@@ -23,6 +23,7 @@
 #include <linux/device.h>
 #include <linux/mutex.h>
 #include <linux/rcupdate.h>
+#include <linux/hid.h>
 #include "input-compat.h"
 #include "input-poller.h"
 
@@ -1031,6 +1032,10 @@ static int input_attach_handler(struct i
 	if (error && error != -ENODEV)
 		pr_err("failed to attach handler %s to device %s, error: %d\n",
 		       handler->name, kobject_name(&dev->dev.kobj), error);
+	if (!error && dev->hid) {
+		dev_info(&dev->dev, "Attached to handler %s\n", handler->name);
+		dump_stack();
+	}
 
 	return error;
 }
@@ -2157,6 +2162,7 @@ int input_register_device(struct input_d
 	const char *path;
 	int error;
 
+	dev_info(&dev->dev, "In input_register_device\n");
 	if (test_bit(EV_ABS, dev->evbit) && !dev->absinfo) {
 		dev_err(&dev->dev,
 			"Absolute device without dev->absinfo, refusing to register\n");
@@ -2258,6 +2264,11 @@ EXPORT_SYMBOL(input_register_device);
  */
 void input_unregister_device(struct input_dev *dev)
 {
+	struct hid_device *hid;
+
+	hid = dev->hid;
+	if (hid)
+		++hid->alan4;
 	if (dev->devres_managed) {
 		WARN_ON(devres_destroy(dev->dev.parent,
 					devm_input_device_unregister,
Index: usb-devel/include/linux/hid.h
===================================================================
--- usb-devel.orig/include/linux/hid.h
+++ usb-devel/include/linux/hid.h
@@ -618,6 +618,9 @@ struct hid_device {							/* device repo
 	struct list_head debug_list;
 	spinlock_t  debug_list_lock;
 	wait_queue_head_t debug_wait;
+
+	int alan_open;
+	int alan1, alan2, alan3, alan4;
 };
 
 #define to_hid_device(pdev) \
Index: usb-devel/include/linux/input.h
===================================================================
--- usb-devel.orig/include/linux/input.h
+++ usb-devel/include/linux/input.h
@@ -22,6 +22,7 @@
 #include <linux/mod_devicetable.h>
 
 struct input_dev_poller;
+struct hid_device;
 
 /**
  * struct input_value - input value representation
@@ -129,6 +130,7 @@ enum input_clock_type {
  *  by a driver
  */
 struct input_dev {
+	struct hid_device *hid;
 	const char *name;
 	const char *phys;
 	const char *uniq;
Index: usb-devel/drivers/hid/wacom_sys.c
===================================================================
--- usb-devel.orig/drivers/hid/wacom_sys.c
+++ usb-devel/drivers/hid/wacom_sys.c
@@ -2017,6 +2017,7 @@ static struct input_dev *wacom_allocate_
 	if (!input_dev)
 		return NULL;
 
+	input_dev->hid = hdev;
 	input_dev->name = wacom_wac->features.name;
 	input_dev->phys = hdev->phys;
 	input_dev->dev.parent = &hdev->dev;
Index: usb-devel/drivers/hid/hid-core.c
===================================================================
--- usb-devel.orig/drivers/hid/hid-core.c
+++ usb-devel/drivers/hid/hid-core.c
@@ -2001,6 +2001,7 @@ EXPORT_SYMBOL_GPL(hid_connect);
 
 void hid_disconnect(struct hid_device *hdev)
 {
+	++hdev->alan2;
 	device_remove_file(&hdev->dev, &dev_attr_country);
 	if (hdev->claimed & HID_CLAIMED_INPUT)
 		hidinput_disconnect(hdev);
@@ -2050,6 +2051,7 @@ EXPORT_SYMBOL_GPL(hid_hw_start);
  */
 void hid_hw_stop(struct hid_device *hdev)
 {
+	++hdev->alan1;
 	hid_disconnect(hdev);
 	hdev->ll_driver->stop(hdev);
 }

Re: KASAN: use-after-free Read in usbhid_close (3)

[syzbot]

Hello,

syzbot has tested the proposed patch but the reproducer still triggered crash:
WARNING in usbhid_stop

usbhid 5-1:0.0: Stop while open = 1
WARNING: CPU: 0 PID: 95 at drivers/hid/usbhid/hid-core.c:1205 usbhid_stop.cold+0x1fb/0x5e6 drivers/hid/usbhid/hid-core.c:1205
Kernel panic - not syncing: panic_on_warn set ...
CPU: 0 PID: 95 Comm: kworker/0:2 Not tainted 5.6.0-rc7-syzkaller #0
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 01/01/2011
Workqueue: usb_hub_wq hub_event
Call Trace:
 __dump_stack lib/dump_stack.c:77 [inline]
 dump_stack+0xef/0x16e lib/dump_stack.c:118
 panic+0x2aa/0x6e1 kernel/panic.c:221
 __warn.cold+0x2f/0x30 kernel/panic.c:582
 report_bug+0x27b/0x2f0 lib/bug.c:195
 fixup_bug arch/x86/kernel/traps.c:174 [inline]
 fixup_bug arch/x86/kernel/traps.c:169 [inline]
 do_error_trap+0x12b/0x1e0 arch/x86/kernel/traps.c:267
 do_invalid_op+0x32/0x40 arch/x86/kernel/traps.c:286
 invalid_op+0x23/0x30 arch/x86/entry/entry_64.S:1027
RIP: 0010:usbhid_stop.cold+0x1fb/0x5e6 drivers/hid/usbhid/hid-core.c:1205
Code: 48 89 7c 24 08 e8 7b 87 bd fc 48 8b 7c 24 08 e8 a1 57 f7 fd 48 8b 14 24 44 89 f1 48 c7 c7 60 3d 84 86 48 89 c6 e8 23 18 92 fc <0f> 0b e8 54 87 bd fc 48 8d bb ac 1e 00 00 b8 ff ff 37 00 48 89 fa
RSP: 0018:ffff8881d5cbf640 EFLAGS: 00010282
RAX: 0000000000000000 RBX: ffff8881bcd1c000 RCX: 0000000000000000
RDX: 0000000000000000 RSI: ffffffff812974dd RDI: ffffed103ab97eba
RBP: ffff8881ccac4000 R08: ffff8881d712e200 R09: ffffed103b646248
R10: ffffed103b646247 R11: ffff8881db23123f R12: ffff8881ccac4008
R13: ffff8881d8859000 R14: 0000000000000001 R15: ffff8881bcd1dfd8
 wacom_remove+0x88/0x3b0 drivers/hid/wacom_sys.c:2773
 hid_device_remove+0xed/0x1d0 drivers/hid/hid-core.c:2298
 __device_release_driver drivers/base/dd.c:1135 [inline]
 device_release_driver_internal+0x231/0x500 drivers/base/dd.c:1168
 bus_remove_device+0x2eb/0x5a0 drivers/base/bus.c:533
 device_del+0x481/0xd30 drivers/base/core.c:2677
 hid_remove_device drivers/hid/hid-core.c:2469 [inline]
 hid_destroy_device+0xe1/0x150 drivers/hid/hid-core.c:2488
 usbhid_disconnect+0x9f/0xe0 drivers/hid/usbhid/hid-core.c:1420
 usb_unbind_interface+0x1bd/0x8a0 drivers/usb/core/driver.c:436
 __device_release_driver drivers/base/dd.c:1137 [inline]
 device_release_driver_internal+0x42f/0x500 drivers/base/dd.c:1168
 bus_remove_device+0x2eb/0x5a0 drivers/base/bus.c:533
 device_del+0x481/0xd30 drivers/base/core.c:2677
 usb_disable_device+0x23d/0x790 drivers/usb/core/message.c:1238
 usb_disconnect+0x293/0x900 drivers/usb/core/hub.c:2211
 hub_port_connect drivers/usb/core/hub.c:5046 [inline]
 hub_port_connect_change drivers/usb/core/hub.c:5335 [inline]
 port_event drivers/usb/core/hub.c:5481 [inline]
 hub_event+0x1a1d/0x4300 drivers/usb/core/hub.c:5563
 process_one_work+0x94b/0x1620 kernel/workqueue.c:2266
 process_scheduled_works kernel/workqueue.c:2328 [inline]
 worker_thread+0x7ab/0xe20 kernel/workqueue.c:2414
 kthread+0x318/0x420 kernel/kthread.c:255
 ret_from_fork+0x24/0x30 arch/x86/entry/entry_64.S:352
Kernel Offset: disabled
Rebooting in 86400 seconds..


Tested on:

commit:         0fa84af8 Merge tag 'usb-serial-5.7-rc1' of https://git.ker..
git tree:       https://github.com/google/kasan.git
console output: https://syzkaller.appspot.com/x/log.txt?x=16275720100000
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)
patch:          https://syzkaller.appspot.com/x/patch.diff?x=175205d7e00000

Re: KASAN: use-after-free Read in usbhid_close (3)

[Alan Stern]

linux-input people:

syzbot has found a bug related to USB/HID/input, and I have narrowed it
down to the wacom driver.  As far as I can tell, the problem is caused
the fact that drivers/hid/wacom_sys.c calls input_register_device()
in several places, but it never calls input_unregister_device().

I know very little about the input subsystem, but this certainly seems 
like a bug.

When the device is unplugged, the disconnect pathway doesn't call
hid_hw_close().  That routine doesn't get called until the user closes
the device file (which can be long after the device is gone and
hid_hw_stop() has run).  Then usbhid_close() gets a use-after-free
error when it tries to access data structures that were deallocated by
usbhid_stop().  No doubt there are other problems too, but this is
the one that syzbot found.

Can any of you help fix this?  Thanks.

Alan Stern

Re: KASAN: use-after-free Read in usbhid_close (3)

[Dmitry Torokhov]

Hi Alan,

On Sat, Apr 18, 2020 at 10:16:32PM -0400, Alan Stern wrote:
> linux-input people:
> 
> syzbot has found a bug related to USB/HID/input, and I have narrowed it
> down to the wacom driver.  As far as I can tell, the problem is caused
> the fact that drivers/hid/wacom_sys.c calls input_register_device()
> in several places, but it never calls input_unregister_device().
> 
> I know very little about the input subsystem, but this certainly seems 
> like a bug.

Wacom driver uses devm_input_allocate_device(), so unregister should
happen automatically on device removal once we exit wacom_probe().

> 
> When the device is unplugged, the disconnect pathway doesn't call
> hid_hw_close().  That routine doesn't get called until the user closes
> the device file (which can be long after the device is gone and
> hid_hw_stop() has run).  Then usbhid_close() gets a use-after-free
> error when it tries to access data structures that were deallocated by
> usbhid_stop().  No doubt there are other problems too, but this is
> the one that syzbot found.

Unregistering the input device should result in calling wacom_close()
(if device was previously opened), which, as far as I can tell, calls
hid_hw_close().

I wonder if it is valid to call hid_hw_stop() before hid_hw_close()?

It could be that we again get confused by the "easiness" of devm APIs
and completely screwing up unwind order.

Thanks.

-- 
Dmitry

Re: KASAN: use-after-free Read in usbhid_close (3)

[Dmitry Torokhov]

On Sat, Apr 18, 2020 at 09:09:44PM -0700, Dmitry Torokhov wrote:
> Hi Alan,
> 
> On Sat, Apr 18, 2020 at 10:16:32PM -0400, Alan Stern wrote:
> > linux-input people:
> > 
> > syzbot has found a bug related to USB/HID/input, and I have narrowed it
> > down to the wacom driver.  As far as I can tell, the problem is caused
> > the fact that drivers/hid/wacom_sys.c calls input_register_device()
> > in several places, but it never calls input_unregister_device().
> > 
> > I know very little about the input subsystem, but this certainly seems 
> > like a bug.
> 
> Wacom driver uses devm_input_allocate_device(), so unregister should
> happen automatically on device removal once we exit wacom_probe().
> 
> > 
> > When the device is unplugged, the disconnect pathway doesn't call
> > hid_hw_close().  That routine doesn't get called until the user closes
> > the device file (which can be long after the device is gone and
> > hid_hw_stop() has run).  Then usbhid_close() gets a use-after-free
> > error when it tries to access data structures that were deallocated by
> > usbhid_stop().  No doubt there are other problems too, but this is
> > the one that syzbot found.
> 
> Unregistering the input device should result in calling wacom_close()
> (if device was previously opened), which, as far as I can tell, calls
> hid_hw_close().
> 
> I wonder if it is valid to call hid_hw_stop() before hid_hw_close()?
> 
> It could be that we again get confused by the "easiness" of devm APIs
> and completely screwing up unwind order.

Let's also add Ping and Jason to the conversation...

-- 
Dmitry

Re: KASAN: use-after-free Read in usbhid_close (3)

[Alan Stern]

On Sat, 18 Apr 2020, Dmitry Torokhov wrote:

> On Sat, Apr 18, 2020 at 09:09:44PM -0700, Dmitry Torokhov wrote:
> > Hi Alan,
> > 
> > On Sat, Apr 18, 2020 at 10:16:32PM -0400, Alan Stern wrote:
> > > linux-input people:
> > > 
> > > syzbot has found a bug related to USB/HID/input, and I have narrowed it
> > > down to the wacom driver.  As far as I can tell, the problem is caused
> > > the fact that drivers/hid/wacom_sys.c calls input_register_device()
> > > in several places, but it never calls input_unregister_device().
> > > 
> > > I know very little about the input subsystem, but this certainly seems 
> > > like a bug.
> > 
> > Wacom driver uses devm_input_allocate_device(), so unregister should
> > happen automatically on device removal once we exit wacom_probe().
> > 
> > > 
> > > When the device is unplugged, the disconnect pathway doesn't call
> > > hid_hw_close().  That routine doesn't get called until the user closes
> > > the device file (which can be long after the device is gone and
> > > hid_hw_stop() has run).  Then usbhid_close() gets a use-after-free
> > > error when it tries to access data structures that were deallocated by
> > > usbhid_stop().  No doubt there are other problems too, but this is
> > > the one that syzbot found.
> > 
> > Unregistering the input device should result in calling wacom_close()
> > (if device was previously opened), which, as far as I can tell, calls
> > hid_hw_close().
> > 
> > I wonder if it is valid to call hid_hw_stop() before hid_hw_close()?

No, it isn't.  If it were, for example, why would evdev_disconnect() -> 
evdev_cleanup() need to call input_close_device()?  And why would 
usbhid_disconnect() deallocate the usbhid structure which usbhid_stop()
accesses?

> > It could be that we again get confused by the "easiness" of devm APIs
> > and completely screwing up unwind order.

That's probably what happened.

Alan Stern

> Let's also add Ping and Jason to the conversation...

Re: KASAN: use-after-free Read in usbhid_close (3)

[Dmitry Torokhov]

On Sun, Apr 19, 2020 at 10:07:34AM -0400, Alan Stern wrote:
> On Sat, 18 Apr 2020, Dmitry Torokhov wrote:
> 
> > On Sat, Apr 18, 2020 at 09:09:44PM -0700, Dmitry Torokhov wrote:
> > > Hi Alan,
> > > 
> > > On Sat, Apr 18, 2020 at 10:16:32PM -0400, Alan Stern wrote:
> > > > linux-input people:
> > > > 
> > > > syzbot has found a bug related to USB/HID/input, and I have narrowed it
> > > > down to the wacom driver.  As far as I can tell, the problem is caused
> > > > the fact that drivers/hid/wacom_sys.c calls input_register_device()
> > > > in several places, but it never calls input_unregister_device().
> > > > 
> > > > I know very little about the input subsystem, but this certainly seems 
> > > > like a bug.
> > > 
> > > Wacom driver uses devm_input_allocate_device(), so unregister should
> > > happen automatically on device removal once we exit wacom_probe().
> > > 
> > > > 
> > > > When the device is unplugged, the disconnect pathway doesn't call
> > > > hid_hw_close().  That routine doesn't get called until the user closes
> > > > the device file (which can be long after the device is gone and
> > > > hid_hw_stop() has run).  Then usbhid_close() gets a use-after-free
> > > > error when it tries to access data structures that were deallocated by
> > > > usbhid_stop().  No doubt there are other problems too, but this is
> > > > the one that syzbot found.
> > > 
> > > Unregistering the input device should result in calling wacom_close()
> > > (if device was previously opened), which, as far as I can tell, calls
> > > hid_hw_close().
> > > 
> > > I wonder if it is valid to call hid_hw_stop() before hid_hw_close()?
> 
> No, it isn't.  If it were, for example, why would evdev_disconnect() -> 
> evdev_cleanup() need to call input_close_device()?

Because input and HID are not the same. For input, when we attempt to
unregister an input device we will go through all attached input
handlers (like evdev) and if they believe they have the device open they
will attempt to close it. How close is implemented is up to particular
driver.

I am not sure about HID implementation details, but I could envision
transports where you can tell the transport that you no longer want
events to be delivered to you ("close") vs you want to disable hardware
("stop") and support any order of them.

> And why would 
> usbhid_disconnect() deallocate the usbhid structure which usbhid_stop()
> accesses?

This happens only after we return from hid_destroy_device(), so
even in the presence of devm I'd expect that all devm-related stuff
instantiated by hid-wacom would have been completed before we get back
to usbhid_disconnect().

Can we validate that calls to wacom_close() happen?

> 
> > > It could be that we again get confused by the "easiness" of devm APIs
> > > and completely screwing up unwind order.
> 
> That's probably what happened.

Thanks.

-- 
Dmitry

Re: KASAN: use-after-free Read in usbhid_close (3)

[Alan Stern]

On Sun, 19 Apr 2020, Dmitry Torokhov wrote:

> On Sun, Apr 19, 2020 at 10:07:34AM -0400, Alan Stern wrote:
> > On Sat, 18 Apr 2020, Dmitry Torokhov wrote:
> > 
> > > On Sat, Apr 18, 2020 at 09:09:44PM -0700, Dmitry Torokhov wrote:
> > > > Hi Alan,
> > > > 
> > > > On Sat, Apr 18, 2020 at 10:16:32PM -0400, Alan Stern wrote:
> > > > > linux-input people:
> > > > > 
> > > > > syzbot has found a bug related to USB/HID/input, and I have narrowed it
> > > > > down to the wacom driver.  As far as I can tell, the problem is caused
> > > > > the fact that drivers/hid/wacom_sys.c calls input_register_device()
> > > > > in several places, but it never calls input_unregister_device().
> > > > > 
> > > > > I know very little about the input subsystem, but this certainly seems 
> > > > > like a bug.
> > > > 
> > > > Wacom driver uses devm_input_allocate_device(), so unregister should
> > > > happen automatically on device removal once we exit wacom_probe().
> > > > 
> > > > > 
> > > > > When the device is unplugged, the disconnect pathway doesn't call
> > > > > hid_hw_close().  That routine doesn't get called until the user closes
> > > > > the device file (which can be long after the device is gone and
> > > > > hid_hw_stop() has run).  Then usbhid_close() gets a use-after-free
> > > > > error when it tries to access data structures that were deallocated by
> > > > > usbhid_stop().  No doubt there are other problems too, but this is
> > > > > the one that syzbot found.
> > > > 
> > > > Unregistering the input device should result in calling wacom_close()
> > > > (if device was previously opened), which, as far as I can tell, calls
> > > > hid_hw_close().
> > > > 
> > > > I wonder if it is valid to call hid_hw_stop() before hid_hw_close()?
> > 
> > No, it isn't.  If it were, for example, why would evdev_disconnect() -> 
> > evdev_cleanup() need to call input_close_device()?
> 
> Because input and HID are not the same. For input, when we attempt to
> unregister an input device we will go through all attached input
> handlers (like evdev) and if they believe they have the device open they
> will attempt to close it. How close is implemented is up to particular
> driver.
> 
> I am not sure about HID implementation details, but I could envision
> transports where you can tell the transport that you no longer want
> events to be delivered to you ("close") vs you want to disable hardware
> ("stop") and support any order of them.

Jiri, you should know: Are HID drivers supposed to work okay when the
->close callback is issued after (or concurrently with) the ->stop
callback?

The actual bug found by syzbot was a race between those two routines in 
usbhid.

> > And why would 
> > usbhid_disconnect() deallocate the usbhid structure which usbhid_stop()
> > accesses?
> 
> This happens only after we return from hid_destroy_device(), so
> even in the presence of devm I'd expect that all devm-related stuff
> instantiated by hid-wacom would have been completed before we get back
> to usbhid_disconnect().
> 
> Can we validate that calls to wacom_close() happen?

I could find out if you think it's important.  In the syzbot tests, the 
crash occurs before wacom_close() is called.

Alan Stern

Re: KASAN: use-after-free Read in usbhid_close (3)

[Alan Stern]

On Sun, 19 Apr 2020, Alan Stern wrote:

> Jiri, you should know: Are HID drivers supposed to work okay when the
> ->close callback is issued after (or concurrently with) the ->stop
> callback?

No response.  I'll assume that strange callback orderings should be 
supported.  Let's see if the patch below fixes the race in usbhid.

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
@@ -682,16 +682,21 @@ static int usbhid_open(struct hid_device
 	struct usbhid_device *usbhid = hid->driver_data;
 	int res;
 
+	mutex_lock(&usbhid->mutex);
+
 	set_bit(HID_OPENED, &usbhid->iofl);
 
-	if (hid->quirks & HID_QUIRK_ALWAYS_POLL)
-		return 0;
+	if (hid->quirks & HID_QUIRK_ALWAYS_POLL) {
+		res = 0;
+		goto Done;
+	}
 
 	res = usb_autopm_get_interface(usbhid->intf);
 	/* the device must be awake to reliably request remote wakeup */
 	if (res < 0) {
 		clear_bit(HID_OPENED, &usbhid->iofl);
-		return -EIO;
+		res = -EIO;
+		goto Done;
 	}
 
 	usbhid->intf->needs_remote_wakeup = 1;
@@ -725,6 +730,9 @@ static int usbhid_open(struct hid_device
 		msleep(50);
 
 	clear_bit(HID_RESUME_RUNNING, &usbhid->iofl);
+
+ Done:
+	mutex_unlock(&usbhid->mutex);
 	return res;
 }
 
@@ -732,6 +740,8 @@ static void usbhid_close(struct hid_devi
 {
 	struct usbhid_device *usbhid = hid->driver_data;
 
+	mutex_lock(&usbhid->mutex);
+
 	/*
 	 * Make sure we don't restart data acquisition due to
 	 * a resumption we no longer care about by avoiding racing
@@ -743,12 +753,13 @@ static void usbhid_close(struct hid_devi
 		clear_bit(HID_IN_POLLING, &usbhid->iofl);
 	spin_unlock_irq(&usbhid->lock);
 
-	if (hid->quirks & HID_QUIRK_ALWAYS_POLL)
-		return;
+	if (!(hid->quirks & HID_QUIRK_ALWAYS_POLL)) {
+		hid_cancel_delayed_stuff(usbhid);
+		usb_kill_urb(usbhid->urbin);
+		usbhid->intf->needs_remote_wakeup = 0;
+	}
 
-	hid_cancel_delayed_stuff(usbhid);
-	usb_kill_urb(usbhid->urbin);
-	usbhid->intf->needs_remote_wakeup = 0;
+	mutex_unlock(&usbhid->mutex);
 }
 
 /*
@@ -1057,6 +1068,8 @@ static int usbhid_start(struct hid_devic
 	unsigned int n, insize = 0;
 	int ret;
 
+	mutex_lock(&usbhid->mutex);
+
 	clear_bit(HID_DISCONNECTED, &usbhid->iofl);
 
 	usbhid->bufsize = HID_MIN_BUFFER_SIZE;
@@ -1177,6 +1190,8 @@ static int usbhid_start(struct hid_devic
 		usbhid_set_leds(hid);
 		device_set_wakeup_enable(&dev->dev, 1);
 	}
+
+	mutex_unlock(&usbhid->mutex);
 	return 0;
 
 fail:
@@ -1187,6 +1202,7 @@ fail:
 	usbhid->urbout = NULL;
 	usbhid->urbctrl = NULL;
 	hid_free_buffers(dev, hid);
+	mutex_unlock(&usbhid->mutex);
 	return ret;
 }
 
@@ -1202,6 +1218,8 @@ static void usbhid_stop(struct hid_devic
 		usbhid->intf->needs_remote_wakeup = 0;
 	}
 
+	mutex_lock(&usbhid->mutex);
+
 	clear_bit(HID_STARTED, &usbhid->iofl);
 	spin_lock_irq(&usbhid->lock);	/* Sync with error and led handlers */
 	set_bit(HID_DISCONNECTED, &usbhid->iofl);
@@ -1222,6 +1240,8 @@ static void usbhid_stop(struct hid_devic
 	usbhid->urbout = NULL;
 
 	hid_free_buffers(hid_to_usb_dev(hid), hid);
+
+	mutex_unlock(&usbhid->mutex);
 }
 
 static int usbhid_power(struct hid_device *hid, int lvl)
@@ -1382,6 +1402,7 @@ static int usbhid_probe(struct usb_inter
 	INIT_WORK(&usbhid->reset_work, hid_reset);
 	timer_setup(&usbhid->io_retry, hid_retry_timeout, 0);
 	spin_lock_init(&usbhid->lock);
+	mutex_init(&usbhid->mutex);
 
 	ret = hid_add_device(hid);
 	if (ret) {
Index: usb-devel/drivers/hid/usbhid/usbhid.h
===================================================================
--- usb-devel.orig/drivers/hid/usbhid/usbhid.h
+++ usb-devel/drivers/hid/usbhid/usbhid.h
@@ -80,6 +80,7 @@ struct usbhid_device {
 	dma_addr_t outbuf_dma;                                          /* Output buffer dma */
 	unsigned long last_out;							/* record of last output for timeouts */
 
+	struct mutex mutex;						/* start/stop/open/close */
 	spinlock_t lock;						/* fifo spinlock */
 	unsigned long iofl;                                             /* I/O flags (CTRL_RUNNING, OUT_RUNNING) */
 	struct timer_list io_retry;                                     /* Retry timer */

Re: KASAN: use-after-free Read in usbhid_close (3)

[syzbot]

Hello,

syzbot has tested the proposed patch and the reproducer did not trigger crash:

Reported-and-tested-by: syzbot+7bf5a7b0f0a1f9446f4c@syzkaller.appspotmail.com

Tested on:

commit:         0fa84af8 Merge tag 'usb-serial-5.7-rc1' of https://git.ker..
git tree:       https://github.com/google/kasan.git
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)
patch:          https://syzkaller.appspot.com/x/patch.diff?x=14f40acfe00000

Note: testing is done by a robot and is best-effort only.

Re: KASAN: use-after-free Read in usbhid_close (3)

[Jiri Kosina]

On Wed, 22 Apr 2020, Alan Stern wrote:

> > Jiri, you should know: Are HID drivers supposed to work okay when the
> > ->close callback is issued after (or concurrently with) the ->stop
> > callback?
> 
> No response.  

Sorry, I've been a bit swamped recently. Thanks a lot for taking care of 
this.

> I'll assume that strange callback orderings should be supported.  Let's 
> see if the patch below fixes the race in usbhid.

Unfortunately I don't believe the supportability of this is fully defined. 
I have tried to quickly go over the few major drivers and didn't find 
anything relying various orderings, but I might have easily missed some 
case.

So unless we have a programatic way to check it, the patch you created for 
mutual exclusion is a good bandaid I believe.

Thanks again Alan, I'll push it to Linus for 5.7.

-- 
Jiri Kosina
SUSE Labs