Hello, syzbot found the following crash on: HEAD commit: e96407b4 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=15a16f26600000 kernel config: https://syzkaller.appspot.com/x/.config?x=cfa2c18fb6a8068e dashboard link: https://syzkaller.appspot.com/bug?extid=30cf45ebfe0b0c4847a1 compiler: gcc (GCC) 9.0.0 20181231 (experimental) syz repro: https://syzkaller.appspot.com/x/repro.syz?x=1416df26600000 C reproducer: https://syzkaller.appspot.com/x/repro.c?x=11ce511c600000 IMPORTANT: if you fix the bug, please add the following tag to the commit: Reported-by: syzbot+30cf45ebfe0b0c4847a1@syzkaller.appspotmail.com ================================================================== BUG: KASAN: use-after-free in __mutex_lock_common kernel/locking/mutex.c:912 [inline] BUG: KASAN: use-after-free in __mutex_lock+0xf23/0x1360 kernel/locking/mutex.c:1077 Read of size 8 at addr ffff8881d21fc2d8 by task syz-executor834/1878 CPU: 0 PID: 1878 Comm: syz-executor834 Not tainted 5.3.0-rc2+ #25 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+0xca/0x13e lib/dump_stack.c:113 print_address_description+0x6a/0x32c mm/kasan/report.c:351 __kasan_report.cold+0x1a/0x33 mm/kasan/report.c:482 kasan_report+0xe/0x12 mm/kasan/common.c:612 __mutex_lock_common kernel/locking/mutex.c:912 [inline] __mutex_lock+0xf23/0x1360 kernel/locking/mutex.c:1077 ld_usb_release+0xb1/0x400 drivers/usb/misc/ldusb.c:386 __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:163 prepare_exit_to_usermode arch/x86/entry/common.c:194 [inline] syscall_return_slowpath arch/x86/entry/common.c:274 [inline] do_syscall_64+0x45f/0x580 arch/x86/entry/common.c:299 entry_SYSCALL_64_after_hwframe+0x49/0xbe RIP: 0033:0x406b31 Code: 75 14 b8 03 00 00 00 0f 05 48 3d 01 f0 ff ff 0f 83 04 19 00 00 c3 48 83 ec 08 e8 6a fc ff ff 48 89 04 24 b8 03 00 00 00 0f 05 <48> 8b 3c 24 48 89 c2 e8 b3 fc ff ff 48 89 d0 48 83 c4 08 48 3d 01 RSP: 002b:00007ffcf13bd080 EFLAGS: 00000293 ORIG_RAX: 0000000000000003 RAX: 0000000000000000 RBX: 0000000000000005 RCX: 0000000000406b31 RDX: fffffffffffffff7 RSI: 0000000000000080 RDI: 0000000000000004 RBP: 0000000000000159 R08: 0000000000000020 R09: 0000000000000020 R10: 00007ffcf13bd0b0 R11: 0000000000000293 R12: 000000000001d884 R13: 0000000000000004 R14: 00000000006e39ec R15: 0000000000000064 Allocated by task 1775: save_stack+0x1b/0x80 mm/kasan/common.c:69 set_track mm/kasan/common.c:77 [inline] __kasan_kmalloc mm/kasan/common.c:487 [inline] __kasan_kmalloc.constprop.0+0xbf/0xd0 mm/kasan/common.c:460 kmalloc include/linux/slab.h:552 [inline] kzalloc include/linux/slab.h:748 [inline] ld_usb_probe+0x6e/0xa65 drivers/usb/misc/ldusb.c:661 usb_probe_interface+0x305/0x7a0 drivers/usb/core/driver.c:361 really_probe+0x281/0x650 drivers/base/dd.c:548 driver_probe_device+0x101/0x1b0 drivers/base/dd.c:709 __device_attach_driver+0x1c2/0x220 drivers/base/dd.c:816 bus_for_each_drv+0x15c/0x1e0 drivers/base/bus.c:454 __device_attach+0x217/0x360 drivers/base/dd.c:882 bus_probe_device+0x1e4/0x290 drivers/base/bus.c:514 device_add+0xae6/0x16f0 drivers/base/core.c:2114 usb_set_configuration+0xdf6/0x1670 drivers/usb/core/message.c:2023 generic_probe+0x9d/0xd5 drivers/usb/core/generic.c:210 usb_probe_device+0x99/0x100 drivers/usb/core/driver.c:266 really_probe+0x281/0x650 drivers/base/dd.c:548 driver_probe_device+0x101/0x1b0 drivers/base/dd.c:709 __device_attach_driver+0x1c2/0x220 drivers/base/dd.c:816 bus_for_each_drv+0x15c/0x1e0 drivers/base/bus.c:454 __device_attach+0x217/0x360 drivers/base/dd.c:882 bus_probe_device+0x1e4/0x290 drivers/base/bus.c:514 device_add+0xae6/0x16f0 drivers/base/core.c:2114 usb_new_device.cold+0x6a4/0xe79 drivers/usb/core/hub.c:2536 hub_port_connect drivers/usb/core/hub.c:5098 [inline] hub_port_connect_change drivers/usb/core/hub.c:5213 [inline] port_event drivers/usb/core/hub.c:5359 [inline] hub_event+0x1b5c/0x3640 drivers/usb/core/hub.c:5441 process_one_work+0x92b/0x1530 kernel/workqueue.c:2269 worker_thread+0x96/0xe20 kernel/workqueue.c:2415 kthread+0x318/0x420 kernel/kthread.c:255 ret_from_fork+0x24/0x30 arch/x86/entry/entry_64.S:352 Freed by task 1775: save_stack+0x1b/0x80 mm/kasan/common.c:69 set_track mm/kasan/common.c:77 [inline] __kasan_slab_free+0x130/0x180 mm/kasan/common.c:449 slab_free_hook mm/slub.c:1423 [inline] slab_free_freelist_hook mm/slub.c:1470 [inline] slab_free mm/slub.c:3012 [inline] kfree+0xe4/0x2f0 mm/slub.c:3953 ld_usb_probe+0x728/0xa65 drivers/usb/misc/ldusb.c:744 usb_probe_interface+0x305/0x7a0 drivers/usb/core/driver.c:361 really_probe+0x281/0x650 drivers/base/dd.c:548 driver_probe_device+0x101/0x1b0 drivers/base/dd.c:709 __device_attach_driver+0x1c2/0x220 drivers/base/dd.c:816 bus_for_each_drv+0x15c/0x1e0 drivers/base/bus.c:454 __device_attach+0x217/0x360 drivers/base/dd.c:882 bus_probe_device+0x1e4/0x290 drivers/base/bus.c:514 device_add+0xae6/0x16f0 drivers/base/core.c:2114 usb_set_configuration+0xdf6/0x1670 drivers/usb/core/message.c:2023 generic_probe+0x9d/0xd5 drivers/usb/core/generic.c:210 usb_probe_device+0x99/0x100 drivers/usb/core/driver.c:266 really_probe+0x281/0x650 drivers/base/dd.c:548 driver_probe_device+0x101/0x1b0 drivers/base/dd.c:709 __device_attach_driver+0x1c2/0x220 drivers/base/dd.c:816 bus_for_each_drv+0x15c/0x1e0 drivers/base/bus.c:454 __device_attach+0x217/0x360 drivers/base/dd.c:882 bus_probe_device+0x1e4/0x290 drivers/base/bus.c:514 device_add+0xae6/0x16f0 drivers/base/core.c:2114 usb_new_device.cold+0x6a4/0xe79 drivers/usb/core/hub.c:2536 hub_port_connect drivers/usb/core/hub.c:5098 [inline] hub_port_connect_change drivers/usb/core/hub.c:5213 [inline] port_event drivers/usb/core/hub.c:5359 [inline] hub_event+0x1b5c/0x3640 drivers/usb/core/hub.c:5441 process_one_work+0x92b/0x1530 kernel/workqueue.c:2269 worker_thread+0x96/0xe20 kernel/workqueue.c:2415 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 ffff8881d21fc280 which belongs to the cache kmalloc-512 of size 512 The buggy address is located 88 bytes inside of 512-byte region [ffff8881d21fc280, ffff8881d21fc480) The buggy address belongs to the page: page:ffffea0007487f00 refcount:1 mapcount:0 mapping:ffff8881da002500 index:0x0 compound_mapcount: 0 flags: 0x200000000010200(slab|head) raw: 0200000000010200 ffffea000739fc80 0000000900000009 ffff8881da002500 raw: 0000000000000000 00000000000c000c 00000001ffffffff 0000000000000000 page dumped because: kasan: bad access detected Memory state around the buggy address: ffff8881d21fc180: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb ffff8881d21fc200: fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc > ffff8881d21fc280: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb ^ ffff8881d21fc300: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb ffff8881d21fc380: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb ================================================================== --- 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. syzbot can test patches for this bug, for details see: https://goo.gl/tpsmEJ#testing-patches
Greg:
See below...
On Fri, 9 Aug 2019, syzbot wrote:
> Hello,
>
> syzbot found the following crash on:
>
> HEAD commit: e96407b4 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=15a16f26600000
> kernel config: https://syzkaller.appspot.com/x/.config?x=cfa2c18fb6a8068e
> dashboard link: https://syzkaller.appspot.com/bug?extid=30cf45ebfe0b0c4847a1
> compiler: gcc (GCC) 9.0.0 20181231 (experimental)
> syz repro: https://syzkaller.appspot.com/x/repro.syz?x=1416df26600000
> C reproducer: https://syzkaller.appspot.com/x/repro.c?x=11ce511c600000
>
> IMPORTANT: if you fix the bug, please add the following tag to the commit:
> Reported-by: syzbot+30cf45ebfe0b0c4847a1@syzkaller.appspotmail.com
>
> ==================================================================
> BUG: KASAN: use-after-free in __mutex_lock_common
> kernel/locking/mutex.c:912 [inline]
> BUG: KASAN: use-after-free in __mutex_lock+0xf23/0x1360
> kernel/locking/mutex.c:1077
> Read of size 8 at addr ffff8881d21fc2d8 by task syz-executor834/1878
>
> CPU: 0 PID: 1878 Comm: syz-executor834 Not tainted 5.3.0-rc2+ #25
> 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+0xca/0x13e lib/dump_stack.c:113
> print_address_description+0x6a/0x32c mm/kasan/report.c:351
> __kasan_report.cold+0x1a/0x33 mm/kasan/report.c:482
> kasan_report+0xe/0x12 mm/kasan/common.c:612
> __mutex_lock_common kernel/locking/mutex.c:912 [inline]
> __mutex_lock+0xf23/0x1360 kernel/locking/mutex.c:1077
> ld_usb_release+0xb1/0x400 drivers/usb/misc/ldusb.c:386
> __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:163
> prepare_exit_to_usermode arch/x86/entry/common.c:194 [inline]
> syscall_return_slowpath arch/x86/entry/common.c:274 [inline]
> do_syscall_64+0x45f/0x580 arch/x86/entry/common.c:299
> entry_SYSCALL_64_after_hwframe+0x49/0xbe
> RIP: 0033:0x406b31
> Code: 75 14 b8 03 00 00 00 0f 05 48 3d 01 f0 ff ff 0f 83 04 19 00 00 c3 48
> 83 ec 08 e8 6a fc ff ff 48 89 04 24 b8 03 00 00 00 0f 05 <48> 8b 3c 24 48
> 89 c2 e8 b3 fc ff ff 48 89 d0 48 83 c4 08 48 3d 01
> RSP: 002b:00007ffcf13bd080 EFLAGS: 00000293 ORIG_RAX: 0000000000000003
> RAX: 0000000000000000 RBX: 0000000000000005 RCX: 0000000000406b31
> RDX: fffffffffffffff7 RSI: 0000000000000080 RDI: 0000000000000004
> RBP: 0000000000000159 R08: 0000000000000020 R09: 0000000000000020
> R10: 00007ffcf13bd0b0 R11: 0000000000000293 R12: 000000000001d884
> R13: 0000000000000004 R14: 00000000006e39ec R15: 0000000000000064
>
> Allocated by task 1775:
> save_stack+0x1b/0x80 mm/kasan/common.c:69
> set_track mm/kasan/common.c:77 [inline]
> __kasan_kmalloc mm/kasan/common.c:487 [inline]
> __kasan_kmalloc.constprop.0+0xbf/0xd0 mm/kasan/common.c:460
> kmalloc include/linux/slab.h:552 [inline]
> kzalloc include/linux/slab.h:748 [inline]
> ld_usb_probe+0x6e/0xa65 drivers/usb/misc/ldusb.c:661
> usb_probe_interface+0x305/0x7a0 drivers/usb/core/driver.c:361
> really_probe+0x281/0x650 drivers/base/dd.c:548
> driver_probe_device+0x101/0x1b0 drivers/base/dd.c:709
> __device_attach_driver+0x1c2/0x220 drivers/base/dd.c:816
> bus_for_each_drv+0x15c/0x1e0 drivers/base/bus.c:454
> __device_attach+0x217/0x360 drivers/base/dd.c:882
> bus_probe_device+0x1e4/0x290 drivers/base/bus.c:514
> device_add+0xae6/0x16f0 drivers/base/core.c:2114
> usb_set_configuration+0xdf6/0x1670 drivers/usb/core/message.c:2023
> generic_probe+0x9d/0xd5 drivers/usb/core/generic.c:210
> usb_probe_device+0x99/0x100 drivers/usb/core/driver.c:266
> really_probe+0x281/0x650 drivers/base/dd.c:548
> driver_probe_device+0x101/0x1b0 drivers/base/dd.c:709
> __device_attach_driver+0x1c2/0x220 drivers/base/dd.c:816
> bus_for_each_drv+0x15c/0x1e0 drivers/base/bus.c:454
> __device_attach+0x217/0x360 drivers/base/dd.c:882
> bus_probe_device+0x1e4/0x290 drivers/base/bus.c:514
> device_add+0xae6/0x16f0 drivers/base/core.c:2114
> usb_new_device.cold+0x6a4/0xe79 drivers/usb/core/hub.c:2536
> hub_port_connect drivers/usb/core/hub.c:5098 [inline]
> hub_port_connect_change drivers/usb/core/hub.c:5213 [inline]
> port_event drivers/usb/core/hub.c:5359 [inline]
> hub_event+0x1b5c/0x3640 drivers/usb/core/hub.c:5441
> process_one_work+0x92b/0x1530 kernel/workqueue.c:2269
> worker_thread+0x96/0xe20 kernel/workqueue.c:2415
> kthread+0x318/0x420 kernel/kthread.c:255
> ret_from_fork+0x24/0x30 arch/x86/entry/entry_64.S:352
>
> Freed by task 1775:
> save_stack+0x1b/0x80 mm/kasan/common.c:69
> set_track mm/kasan/common.c:77 [inline]
> __kasan_slab_free+0x130/0x180 mm/kasan/common.c:449
> slab_free_hook mm/slub.c:1423 [inline]
> slab_free_freelist_hook mm/slub.c:1470 [inline]
> slab_free mm/slub.c:3012 [inline]
> kfree+0xe4/0x2f0 mm/slub.c:3953
> ld_usb_probe+0x728/0xa65 drivers/usb/misc/ldusb.c:744
> usb_probe_interface+0x305/0x7a0 drivers/usb/core/driver.c:361
> really_probe+0x281/0x650 drivers/base/dd.c:548
> driver_probe_device+0x101/0x1b0 drivers/base/dd.c:709
> __device_attach_driver+0x1c2/0x220 drivers/base/dd.c:816
> bus_for_each_drv+0x15c/0x1e0 drivers/base/bus.c:454
> __device_attach+0x217/0x360 drivers/base/dd.c:882
> bus_probe_device+0x1e4/0x290 drivers/base/bus.c:514
> device_add+0xae6/0x16f0 drivers/base/core.c:2114
> usb_set_configuration+0xdf6/0x1670 drivers/usb/core/message.c:2023
> generic_probe+0x9d/0xd5 drivers/usb/core/generic.c:210
> usb_probe_device+0x99/0x100 drivers/usb/core/driver.c:266
> really_probe+0x281/0x650 drivers/base/dd.c:548
> driver_probe_device+0x101/0x1b0 drivers/base/dd.c:709
> __device_attach_driver+0x1c2/0x220 drivers/base/dd.c:816
> bus_for_each_drv+0x15c/0x1e0 drivers/base/bus.c:454
> __device_attach+0x217/0x360 drivers/base/dd.c:882
> bus_probe_device+0x1e4/0x290 drivers/base/bus.c:514
> device_add+0xae6/0x16f0 drivers/base/core.c:2114
> usb_new_device.cold+0x6a4/0xe79 drivers/usb/core/hub.c:2536
> hub_port_connect drivers/usb/core/hub.c:5098 [inline]
> hub_port_connect_change drivers/usb/core/hub.c:5213 [inline]
> port_event drivers/usb/core/hub.c:5359 [inline]
> hub_event+0x1b5c/0x3640 drivers/usb/core/hub.c:5441
> process_one_work+0x92b/0x1530 kernel/workqueue.c:2269
> worker_thread+0x96/0xe20 kernel/workqueue.c:2415
> 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 ffff8881d21fc280
> which belongs to the cache kmalloc-512 of size 512
> The buggy address is located 88 bytes inside of
> 512-byte region [ffff8881d21fc280, ffff8881d21fc480)
> The buggy address belongs to the page:
> page:ffffea0007487f00 refcount:1 mapcount:0 mapping:ffff8881da002500
> index:0x0 compound_mapcount: 0
> flags: 0x200000000010200(slab|head)
> raw: 0200000000010200 ffffea000739fc80 0000000900000009 ffff8881da002500
> raw: 0000000000000000 00000000000c000c 00000001ffffffff 0000000000000000
> page dumped because: kasan: bad access detected
>
> Memory state around the buggy address:
> ffff8881d21fc180: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
> ffff8881d21fc200: fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc
> > ffff8881d21fc280: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
> ^
> ffff8881d21fc300: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
> ffff8881d21fc380: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
> ==================================================================
I have tracked this bug down. The root causes lie in
usb_register_dev() and usb_deregister_dev().
The first problem is that usb_deregister_dev() sets
usb_minors[intf->minor] to NULL before calling device_destroy() on the
class device. This leaves a window during which another thread can
allocate the same minor number but will encounter a duplicate name
error when it tries to register its own class device.
This error shows up in the kernel log from the syzbot test (not shown
in the bug report above, though):
[ 120.583776][ T1775] sysfs: cannot create duplicate filename '/class/usbmisc/ldusb0'
This can be fixed easily enough by reordering the statements in
usb_deregister_dev().
The second problem is in usb_register_dev(). When that routine runs,
it first allocates a minor number, then drops minor_rwsem, and then
creates the class device. If the device creation fails, the minor
number is deallocated and the whole routine returns an error. But
during the time while minor_rwsem was dropped, there is a window in
which the minor number is allocated and so another thread could
successfully open the device file!
These two scenarios are exactly what happened during the syzbot test.
Minor number 0 was deallocated and then allocated in another thread.
The second allocation failed because the old class device was still in
existence. As a result of this failure, ldusb's private data structure
was released. Nevertheless, a third thread managed to open the device
file during the brief time that minor number 0 was re-allocated. When
that thread closed the file, it tried to access the private data
structure that had already been released.
I believe the patch below will fix both problems. But you should take
a look at it first to see if it seems right; syzbot can't really test
the patch because it involves two separate races both coming out wrong!
Alan Stern
drivers/usb/core/file.c | 10 +++++-----
1 file changed, 5 insertions(+), 5 deletions(-)
Index: usb-devel/drivers/usb/core/file.c
===================================================================
--- usb-devel.orig/drivers/usb/core/file.c
+++ usb-devel/drivers/usb/core/file.c
@@ -193,9 +193,10 @@ int usb_register_dev(struct usb_interfac
intf->minor = minor;
break;
}
- up_write(&minor_rwsem);
- if (intf->minor < 0)
+ if (intf->minor < 0) {
+ up_write(&minor_rwsem);
return -EXFULL;
+ }
/* create a usb class device for this usb interface */
snprintf(name, sizeof(name), class_driver->name, minor - minor_base);
@@ -203,12 +204,11 @@ int usb_register_dev(struct usb_interfac
MKDEV(USB_MAJOR, minor), class_driver,
"%s", kbasename(name));
if (IS_ERR(intf->usb_dev)) {
- down_write(&minor_rwsem);
usb_minors[minor] = NULL;
intf->minor = -1;
- up_write(&minor_rwsem);
retval = PTR_ERR(intf->usb_dev);
}
+ up_write(&minor_rwsem);
return retval;
}
EXPORT_SYMBOL_GPL(usb_register_dev);
@@ -234,12 +234,12 @@ void usb_deregister_dev(struct usb_inter
return;
dev_dbg(&intf->dev, "removing %d minor\n", intf->minor);
+ device_destroy(usb_class->class, MKDEV(USB_MAJOR, intf->minor));
down_write(&minor_rwsem);
usb_minors[intf->minor] = NULL;
up_write(&minor_rwsem);
- device_destroy(usb_class->class, MKDEV(USB_MAJOR, intf->minor));
intf->usb_dev = NULL;
intf->minor = -1;
destroy_usb_class();
[-- Attachment #1: Type: text/plain, Size: 12189 bytes --] On Fri, Aug 9, 2019 at 6:51 PM Alan Stern <stern@rowland.harvard.edu> wrote: > > Greg: > > See below... > > On Fri, 9 Aug 2019, syzbot wrote: > > > Hello, > > > > syzbot found the following crash on: > > > > HEAD commit: e96407b4 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=15a16f26600000 > > kernel config: https://syzkaller.appspot.com/x/.config?x=cfa2c18fb6a8068e > > dashboard link: https://syzkaller.appspot.com/bug?extid=30cf45ebfe0b0c4847a1 > > compiler: gcc (GCC) 9.0.0 20181231 (experimental) > > syz repro: https://syzkaller.appspot.com/x/repro.syz?x=1416df26600000 > > C reproducer: https://syzkaller.appspot.com/x/repro.c?x=11ce511c600000 > > > > IMPORTANT: if you fix the bug, please add the following tag to the commit: > > Reported-by: syzbot+30cf45ebfe0b0c4847a1@syzkaller.appspotmail.com > > > > ================================================================== > > BUG: KASAN: use-after-free in __mutex_lock_common > > kernel/locking/mutex.c:912 [inline] > > BUG: KASAN: use-after-free in __mutex_lock+0xf23/0x1360 > > kernel/locking/mutex.c:1077 > > Read of size 8 at addr ffff8881d21fc2d8 by task syz-executor834/1878 > > > > CPU: 0 PID: 1878 Comm: syz-executor834 Not tainted 5.3.0-rc2+ #25 > > 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+0xca/0x13e lib/dump_stack.c:113 > > print_address_description+0x6a/0x32c mm/kasan/report.c:351 > > __kasan_report.cold+0x1a/0x33 mm/kasan/report.c:482 > > kasan_report+0xe/0x12 mm/kasan/common.c:612 > > __mutex_lock_common kernel/locking/mutex.c:912 [inline] > > __mutex_lock+0xf23/0x1360 kernel/locking/mutex.c:1077 > > ld_usb_release+0xb1/0x400 drivers/usb/misc/ldusb.c:386 > > __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:163 > > prepare_exit_to_usermode arch/x86/entry/common.c:194 [inline] > > syscall_return_slowpath arch/x86/entry/common.c:274 [inline] > > do_syscall_64+0x45f/0x580 arch/x86/entry/common.c:299 > > entry_SYSCALL_64_after_hwframe+0x49/0xbe > > RIP: 0033:0x406b31 > > Code: 75 14 b8 03 00 00 00 0f 05 48 3d 01 f0 ff ff 0f 83 04 19 00 00 c3 48 > > 83 ec 08 e8 6a fc ff ff 48 89 04 24 b8 03 00 00 00 0f 05 <48> 8b 3c 24 48 > > 89 c2 e8 b3 fc ff ff 48 89 d0 48 83 c4 08 48 3d 01 > > RSP: 002b:00007ffcf13bd080 EFLAGS: 00000293 ORIG_RAX: 0000000000000003 > > RAX: 0000000000000000 RBX: 0000000000000005 RCX: 0000000000406b31 > > RDX: fffffffffffffff7 RSI: 0000000000000080 RDI: 0000000000000004 > > RBP: 0000000000000159 R08: 0000000000000020 R09: 0000000000000020 > > R10: 00007ffcf13bd0b0 R11: 0000000000000293 R12: 000000000001d884 > > R13: 0000000000000004 R14: 00000000006e39ec R15: 0000000000000064 > > > > Allocated by task 1775: > > save_stack+0x1b/0x80 mm/kasan/common.c:69 > > set_track mm/kasan/common.c:77 [inline] > > __kasan_kmalloc mm/kasan/common.c:487 [inline] > > __kasan_kmalloc.constprop.0+0xbf/0xd0 mm/kasan/common.c:460 > > kmalloc include/linux/slab.h:552 [inline] > > kzalloc include/linux/slab.h:748 [inline] > > ld_usb_probe+0x6e/0xa65 drivers/usb/misc/ldusb.c:661 > > usb_probe_interface+0x305/0x7a0 drivers/usb/core/driver.c:361 > > really_probe+0x281/0x650 drivers/base/dd.c:548 > > driver_probe_device+0x101/0x1b0 drivers/base/dd.c:709 > > __device_attach_driver+0x1c2/0x220 drivers/base/dd.c:816 > > bus_for_each_drv+0x15c/0x1e0 drivers/base/bus.c:454 > > __device_attach+0x217/0x360 drivers/base/dd.c:882 > > bus_probe_device+0x1e4/0x290 drivers/base/bus.c:514 > > device_add+0xae6/0x16f0 drivers/base/core.c:2114 > > usb_set_configuration+0xdf6/0x1670 drivers/usb/core/message.c:2023 > > generic_probe+0x9d/0xd5 drivers/usb/core/generic.c:210 > > usb_probe_device+0x99/0x100 drivers/usb/core/driver.c:266 > > really_probe+0x281/0x650 drivers/base/dd.c:548 > > driver_probe_device+0x101/0x1b0 drivers/base/dd.c:709 > > __device_attach_driver+0x1c2/0x220 drivers/base/dd.c:816 > > bus_for_each_drv+0x15c/0x1e0 drivers/base/bus.c:454 > > __device_attach+0x217/0x360 drivers/base/dd.c:882 > > bus_probe_device+0x1e4/0x290 drivers/base/bus.c:514 > > device_add+0xae6/0x16f0 drivers/base/core.c:2114 > > usb_new_device.cold+0x6a4/0xe79 drivers/usb/core/hub.c:2536 > > hub_port_connect drivers/usb/core/hub.c:5098 [inline] > > hub_port_connect_change drivers/usb/core/hub.c:5213 [inline] > > port_event drivers/usb/core/hub.c:5359 [inline] > > hub_event+0x1b5c/0x3640 drivers/usb/core/hub.c:5441 > > process_one_work+0x92b/0x1530 kernel/workqueue.c:2269 > > worker_thread+0x96/0xe20 kernel/workqueue.c:2415 > > kthread+0x318/0x420 kernel/kthread.c:255 > > ret_from_fork+0x24/0x30 arch/x86/entry/entry_64.S:352 > > > > Freed by task 1775: > > save_stack+0x1b/0x80 mm/kasan/common.c:69 > > set_track mm/kasan/common.c:77 [inline] > > __kasan_slab_free+0x130/0x180 mm/kasan/common.c:449 > > slab_free_hook mm/slub.c:1423 [inline] > > slab_free_freelist_hook mm/slub.c:1470 [inline] > > slab_free mm/slub.c:3012 [inline] > > kfree+0xe4/0x2f0 mm/slub.c:3953 > > ld_usb_probe+0x728/0xa65 drivers/usb/misc/ldusb.c:744 > > usb_probe_interface+0x305/0x7a0 drivers/usb/core/driver.c:361 > > really_probe+0x281/0x650 drivers/base/dd.c:548 > > driver_probe_device+0x101/0x1b0 drivers/base/dd.c:709 > > __device_attach_driver+0x1c2/0x220 drivers/base/dd.c:816 > > bus_for_each_drv+0x15c/0x1e0 drivers/base/bus.c:454 > > __device_attach+0x217/0x360 drivers/base/dd.c:882 > > bus_probe_device+0x1e4/0x290 drivers/base/bus.c:514 > > device_add+0xae6/0x16f0 drivers/base/core.c:2114 > > usb_set_configuration+0xdf6/0x1670 drivers/usb/core/message.c:2023 > > generic_probe+0x9d/0xd5 drivers/usb/core/generic.c:210 > > usb_probe_device+0x99/0x100 drivers/usb/core/driver.c:266 > > really_probe+0x281/0x650 drivers/base/dd.c:548 > > driver_probe_device+0x101/0x1b0 drivers/base/dd.c:709 > > __device_attach_driver+0x1c2/0x220 drivers/base/dd.c:816 > > bus_for_each_drv+0x15c/0x1e0 drivers/base/bus.c:454 > > __device_attach+0x217/0x360 drivers/base/dd.c:882 > > bus_probe_device+0x1e4/0x290 drivers/base/bus.c:514 > > device_add+0xae6/0x16f0 drivers/base/core.c:2114 > > usb_new_device.cold+0x6a4/0xe79 drivers/usb/core/hub.c:2536 > > hub_port_connect drivers/usb/core/hub.c:5098 [inline] > > hub_port_connect_change drivers/usb/core/hub.c:5213 [inline] > > port_event drivers/usb/core/hub.c:5359 [inline] > > hub_event+0x1b5c/0x3640 drivers/usb/core/hub.c:5441 > > process_one_work+0x92b/0x1530 kernel/workqueue.c:2269 > > worker_thread+0x96/0xe20 kernel/workqueue.c:2415 > > 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 ffff8881d21fc280 > > which belongs to the cache kmalloc-512 of size 512 > > The buggy address is located 88 bytes inside of > > 512-byte region [ffff8881d21fc280, ffff8881d21fc480) > > The buggy address belongs to the page: > > page:ffffea0007487f00 refcount:1 mapcount:0 mapping:ffff8881da002500 > > index:0x0 compound_mapcount: 0 > > flags: 0x200000000010200(slab|head) > > raw: 0200000000010200 ffffea000739fc80 0000000900000009 ffff8881da002500 > > raw: 0000000000000000 00000000000c000c 00000001ffffffff 0000000000000000 > > page dumped because: kasan: bad access detected > > > > Memory state around the buggy address: > > ffff8881d21fc180: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb > > ffff8881d21fc200: fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc > > > ffff8881d21fc280: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb > > ^ > > ffff8881d21fc300: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb > > ffff8881d21fc380: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb > > ================================================================== > > I have tracked this bug down. The root causes lie in > usb_register_dev() and usb_deregister_dev(). > > The first problem is that usb_deregister_dev() sets > usb_minors[intf->minor] to NULL before calling device_destroy() on the > class device. This leaves a window during which another thread can > allocate the same minor number but will encounter a duplicate name > error when it tries to register its own class device. > > This error shows up in the kernel log from the syzbot test (not shown > in the bug report above, though): > > [ 120.583776][ T1775] sysfs: cannot create duplicate filename '/class/usbmisc/ldusb0' > > This can be fixed easily enough by reordering the statements in > usb_deregister_dev(). > > The second problem is in usb_register_dev(). When that routine runs, > it first allocates a minor number, then drops minor_rwsem, and then > creates the class device. If the device creation fails, the minor > number is deallocated and the whole routine returns an error. But > during the time while minor_rwsem was dropped, there is a window in > which the minor number is allocated and so another thread could > successfully open the device file! > > These two scenarios are exactly what happened during the syzbot test. > Minor number 0 was deallocated and then allocated in another thread. > The second allocation failed because the old class device was still in > existence. As a result of this failure, ldusb's private data structure > was released. Nevertheless, a third thread managed to open the device > file during the brief time that minor number 0 was re-allocated. When > that thread closed the file, it tried to access the private data > structure that had already been released. > > I believe the patch below will fix both problems. But you should take > a look at it first to see if it seems right; syzbot can't really test > the patch because it involves two separate races both coming out wrong! This report has a reproducer, so we can try it: #syz test: https://github.com/google/kasan.git e96407b4 > > Alan Stern > > > drivers/usb/core/file.c | 10 +++++----- > 1 file changed, 5 insertions(+), 5 deletions(-) > > Index: usb-devel/drivers/usb/core/file.c > =================================================================== > --- usb-devel.orig/drivers/usb/core/file.c > +++ usb-devel/drivers/usb/core/file.c > @@ -193,9 +193,10 @@ int usb_register_dev(struct usb_interfac > intf->minor = minor; > break; > } > - up_write(&minor_rwsem); > - if (intf->minor < 0) > + if (intf->minor < 0) { > + up_write(&minor_rwsem); > return -EXFULL; > + } > > /* create a usb class device for this usb interface */ > snprintf(name, sizeof(name), class_driver->name, minor - minor_base); > @@ -203,12 +204,11 @@ int usb_register_dev(struct usb_interfac > MKDEV(USB_MAJOR, minor), class_driver, > "%s", kbasename(name)); > if (IS_ERR(intf->usb_dev)) { > - down_write(&minor_rwsem); > usb_minors[minor] = NULL; > intf->minor = -1; > - up_write(&minor_rwsem); > retval = PTR_ERR(intf->usb_dev); > } > + up_write(&minor_rwsem); > return retval; > } > EXPORT_SYMBOL_GPL(usb_register_dev); > @@ -234,12 +234,12 @@ void usb_deregister_dev(struct usb_inter > return; > > dev_dbg(&intf->dev, "removing %d minor\n", intf->minor); > + device_destroy(usb_class->class, MKDEV(USB_MAJOR, intf->minor)); > > down_write(&minor_rwsem); > usb_minors[intf->minor] = NULL; > up_write(&minor_rwsem); > > - device_destroy(usb_class->class, MKDEV(USB_MAJOR, intf->minor)); > intf->usb_dev = NULL; > intf->minor = -1; > destroy_usb_class(); > [-- Attachment #2: usb_core_file.patch --] [-- Type: text/x-patch, Size: 1382 bytes --] Index: usb-devel/drivers/usb/core/file.c =================================================================== --- usb-devel.orig/drivers/usb/core/file.c +++ usb-devel/drivers/usb/core/file.c @@ -193,9 +193,10 @@ int usb_register_dev(struct usb_interfac intf->minor = minor; break; } - up_write(&minor_rwsem); - if (intf->minor < 0) + if (intf->minor < 0) { + up_write(&minor_rwsem); return -EXFULL; + } /* create a usb class device for this usb interface */ snprintf(name, sizeof(name), class_driver->name, minor - minor_base); @@ -203,12 +204,11 @@ int usb_register_dev(struct usb_interfac MKDEV(USB_MAJOR, minor), class_driver, "%s", kbasename(name)); if (IS_ERR(intf->usb_dev)) { - down_write(&minor_rwsem); usb_minors[minor] = NULL; intf->minor = -1; - up_write(&minor_rwsem); retval = PTR_ERR(intf->usb_dev); } + up_write(&minor_rwsem); return retval; } EXPORT_SYMBOL_GPL(usb_register_dev); @@ -234,12 +234,12 @@ void usb_deregister_dev(struct usb_inter return; dev_dbg(&intf->dev, "removing %d minor\n", intf->minor); + device_destroy(usb_class->class, MKDEV(USB_MAJOR, intf->minor)); down_write(&minor_rwsem); usb_minors[intf->minor] = NULL; up_write(&minor_rwsem); - device_destroy(usb_class->class, MKDEV(USB_MAJOR, intf->minor)); intf->usb_dev = NULL; intf->minor = -1; destroy_usb_class();
Hello, syzbot has tested the proposed patch and the reproducer did not trigger crash: Reported-and-tested-by: syzbot+30cf45ebfe0b0c4847a1@syzkaller.appspotmail.com Tested on: commit: e96407b4 usb-fuzzer: main usb gadget fuzzer driver git tree: https://github.com/google/kasan.git kernel config: https://syzkaller.appspot.com/x/.config?x=cfa2c18fb6a8068e compiler: gcc (GCC) 9.0.0 20181231 (experimental) patch: https://syzkaller.appspot.com/x/patch.diff?x=15b4c802600000 Note: testing is done by a robot and is best-effort only.
On Fri, Aug 9, 2019 at 6:51 PM Alan Stern <stern@rowland.harvard.edu> wrote: > > Greg: > > See below... > > On Fri, 9 Aug 2019, syzbot wrote: > > > Hello, > > > > syzbot found the following crash on: > > > > HEAD commit: e96407b4 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=15a16f26600000 > > kernel config: https://syzkaller.appspot.com/x/.config?x=cfa2c18fb6a8068e > > dashboard link: https://syzkaller.appspot.com/bug?extid=30cf45ebfe0b0c4847a1 > > compiler: gcc (GCC) 9.0.0 20181231 (experimental) > > syz repro: https://syzkaller.appspot.com/x/repro.syz?x=1416df26600000 > > C reproducer: https://syzkaller.appspot.com/x/repro.c?x=11ce511c600000 > > > > IMPORTANT: if you fix the bug, please add the following tag to the commit: > > Reported-by: syzbot+30cf45ebfe0b0c4847a1@syzkaller.appspotmail.com > > > > ================================================================== > > BUG: KASAN: use-after-free in __mutex_lock_common > > kernel/locking/mutex.c:912 [inline] > > BUG: KASAN: use-after-free in __mutex_lock+0xf23/0x1360 > > kernel/locking/mutex.c:1077 > > Read of size 8 at addr ffff8881d21fc2d8 by task syz-executor834/1878 > > > > CPU: 0 PID: 1878 Comm: syz-executor834 Not tainted 5.3.0-rc2+ #25 > > 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+0xca/0x13e lib/dump_stack.c:113 > > print_address_description+0x6a/0x32c mm/kasan/report.c:351 > > __kasan_report.cold+0x1a/0x33 mm/kasan/report.c:482 > > kasan_report+0xe/0x12 mm/kasan/common.c:612 > > __mutex_lock_common kernel/locking/mutex.c:912 [inline] > > __mutex_lock+0xf23/0x1360 kernel/locking/mutex.c:1077 > > ld_usb_release+0xb1/0x400 drivers/usb/misc/ldusb.c:386 > > __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:163 > > prepare_exit_to_usermode arch/x86/entry/common.c:194 [inline] > > syscall_return_slowpath arch/x86/entry/common.c:274 [inline] > > do_syscall_64+0x45f/0x580 arch/x86/entry/common.c:299 > > entry_SYSCALL_64_after_hwframe+0x49/0xbe > > RIP: 0033:0x406b31 > > Code: 75 14 b8 03 00 00 00 0f 05 48 3d 01 f0 ff ff 0f 83 04 19 00 00 c3 48 > > 83 ec 08 e8 6a fc ff ff 48 89 04 24 b8 03 00 00 00 0f 05 <48> 8b 3c 24 48 > > 89 c2 e8 b3 fc ff ff 48 89 d0 48 83 c4 08 48 3d 01 > > RSP: 002b:00007ffcf13bd080 EFLAGS: 00000293 ORIG_RAX: 0000000000000003 > > RAX: 0000000000000000 RBX: 0000000000000005 RCX: 0000000000406b31 > > RDX: fffffffffffffff7 RSI: 0000000000000080 RDI: 0000000000000004 > > RBP: 0000000000000159 R08: 0000000000000020 R09: 0000000000000020 > > R10: 00007ffcf13bd0b0 R11: 0000000000000293 R12: 000000000001d884 > > R13: 0000000000000004 R14: 00000000006e39ec R15: 0000000000000064 > > > > Allocated by task 1775: > > save_stack+0x1b/0x80 mm/kasan/common.c:69 > > set_track mm/kasan/common.c:77 [inline] > > __kasan_kmalloc mm/kasan/common.c:487 [inline] > > __kasan_kmalloc.constprop.0+0xbf/0xd0 mm/kasan/common.c:460 > > kmalloc include/linux/slab.h:552 [inline] > > kzalloc include/linux/slab.h:748 [inline] > > ld_usb_probe+0x6e/0xa65 drivers/usb/misc/ldusb.c:661 > > usb_probe_interface+0x305/0x7a0 drivers/usb/core/driver.c:361 > > really_probe+0x281/0x650 drivers/base/dd.c:548 > > driver_probe_device+0x101/0x1b0 drivers/base/dd.c:709 > > __device_attach_driver+0x1c2/0x220 drivers/base/dd.c:816 > > bus_for_each_drv+0x15c/0x1e0 drivers/base/bus.c:454 > > __device_attach+0x217/0x360 drivers/base/dd.c:882 > > bus_probe_device+0x1e4/0x290 drivers/base/bus.c:514 > > device_add+0xae6/0x16f0 drivers/base/core.c:2114 > > usb_set_configuration+0xdf6/0x1670 drivers/usb/core/message.c:2023 > > generic_probe+0x9d/0xd5 drivers/usb/core/generic.c:210 > > usb_probe_device+0x99/0x100 drivers/usb/core/driver.c:266 > > really_probe+0x281/0x650 drivers/base/dd.c:548 > > driver_probe_device+0x101/0x1b0 drivers/base/dd.c:709 > > __device_attach_driver+0x1c2/0x220 drivers/base/dd.c:816 > > bus_for_each_drv+0x15c/0x1e0 drivers/base/bus.c:454 > > __device_attach+0x217/0x360 drivers/base/dd.c:882 > > bus_probe_device+0x1e4/0x290 drivers/base/bus.c:514 > > device_add+0xae6/0x16f0 drivers/base/core.c:2114 > > usb_new_device.cold+0x6a4/0xe79 drivers/usb/core/hub.c:2536 > > hub_port_connect drivers/usb/core/hub.c:5098 [inline] > > hub_port_connect_change drivers/usb/core/hub.c:5213 [inline] > > port_event drivers/usb/core/hub.c:5359 [inline] > > hub_event+0x1b5c/0x3640 drivers/usb/core/hub.c:5441 > > process_one_work+0x92b/0x1530 kernel/workqueue.c:2269 > > worker_thread+0x96/0xe20 kernel/workqueue.c:2415 > > kthread+0x318/0x420 kernel/kthread.c:255 > > ret_from_fork+0x24/0x30 arch/x86/entry/entry_64.S:352 > > > > Freed by task 1775: > > save_stack+0x1b/0x80 mm/kasan/common.c:69 > > set_track mm/kasan/common.c:77 [inline] > > __kasan_slab_free+0x130/0x180 mm/kasan/common.c:449 > > slab_free_hook mm/slub.c:1423 [inline] > > slab_free_freelist_hook mm/slub.c:1470 [inline] > > slab_free mm/slub.c:3012 [inline] > > kfree+0xe4/0x2f0 mm/slub.c:3953 > > ld_usb_probe+0x728/0xa65 drivers/usb/misc/ldusb.c:744 > > usb_probe_interface+0x305/0x7a0 drivers/usb/core/driver.c:361 > > really_probe+0x281/0x650 drivers/base/dd.c:548 > > driver_probe_device+0x101/0x1b0 drivers/base/dd.c:709 > > __device_attach_driver+0x1c2/0x220 drivers/base/dd.c:816 > > bus_for_each_drv+0x15c/0x1e0 drivers/base/bus.c:454 > > __device_attach+0x217/0x360 drivers/base/dd.c:882 > > bus_probe_device+0x1e4/0x290 drivers/base/bus.c:514 > > device_add+0xae6/0x16f0 drivers/base/core.c:2114 > > usb_set_configuration+0xdf6/0x1670 drivers/usb/core/message.c:2023 > > generic_probe+0x9d/0xd5 drivers/usb/core/generic.c:210 > > usb_probe_device+0x99/0x100 drivers/usb/core/driver.c:266 > > really_probe+0x281/0x650 drivers/base/dd.c:548 > > driver_probe_device+0x101/0x1b0 drivers/base/dd.c:709 > > __device_attach_driver+0x1c2/0x220 drivers/base/dd.c:816 > > bus_for_each_drv+0x15c/0x1e0 drivers/base/bus.c:454 > > __device_attach+0x217/0x360 drivers/base/dd.c:882 > > bus_probe_device+0x1e4/0x290 drivers/base/bus.c:514 > > device_add+0xae6/0x16f0 drivers/base/core.c:2114 > > usb_new_device.cold+0x6a4/0xe79 drivers/usb/core/hub.c:2536 > > hub_port_connect drivers/usb/core/hub.c:5098 [inline] > > hub_port_connect_change drivers/usb/core/hub.c:5213 [inline] > > port_event drivers/usb/core/hub.c:5359 [inline] > > hub_event+0x1b5c/0x3640 drivers/usb/core/hub.c:5441 > > process_one_work+0x92b/0x1530 kernel/workqueue.c:2269 > > worker_thread+0x96/0xe20 kernel/workqueue.c:2415 > > 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 ffff8881d21fc280 > > which belongs to the cache kmalloc-512 of size 512 > > The buggy address is located 88 bytes inside of > > 512-byte region [ffff8881d21fc280, ffff8881d21fc480) > > The buggy address belongs to the page: > > page:ffffea0007487f00 refcount:1 mapcount:0 mapping:ffff8881da002500 > > index:0x0 compound_mapcount: 0 > > flags: 0x200000000010200(slab|head) > > raw: 0200000000010200 ffffea000739fc80 0000000900000009 ffff8881da002500 > > raw: 0000000000000000 00000000000c000c 00000001ffffffff 0000000000000000 > > page dumped because: kasan: bad access detected > > > > Memory state around the buggy address: > > ffff8881d21fc180: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb > > ffff8881d21fc200: fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc > > > ffff8881d21fc280: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb > > ^ > > ffff8881d21fc300: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb > > ffff8881d21fc380: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb > > ================================================================== > > I have tracked this bug down. The root causes lie in > usb_register_dev() and usb_deregister_dev(). > > The first problem is that usb_deregister_dev() sets > usb_minors[intf->minor] to NULL before calling device_destroy() on the > class device. This leaves a window during which another thread can > allocate the same minor number but will encounter a duplicate name > error when it tries to register its own class device. > > This error shows up in the kernel log from the syzbot test (not shown > in the bug report above, though): > > [ 120.583776][ T1775] sysfs: cannot create duplicate filename '/class/usbmisc/ldusb0' > > This can be fixed easily enough by reordering the statements in > usb_deregister_dev(). > > The second problem is in usb_register_dev(). When that routine runs, > it first allocates a minor number, then drops minor_rwsem, and then > creates the class device. If the device creation fails, the minor > number is deallocated and the whole routine returns an error. But > during the time while minor_rwsem was dropped, there is a window in > which the minor number is allocated and so another thread could > successfully open the device file! > > These two scenarios are exactly what happened during the syzbot test. > Minor number 0 was deallocated and then allocated in another thread. > The second allocation failed because the old class device was still in > existence. As a result of this failure, ldusb's private data structure > was released. Nevertheless, a third thread managed to open the device > file during the brief time that minor number 0 was re-allocated. When > that thread closed the file, it tried to access the private data > structure that had already been released. > > I believe the patch below will fix both problems. But you should take > a look at it first to see if it seems right; syzbot can't really test > the patch because it involves two separate races both coming out wrong! > > Alan Stern Alan, could you submit this patch (if you haven't already)? Looks like it fixes this bug (and might fix some others). > > > drivers/usb/core/file.c | 10 +++++----- > 1 file changed, 5 insertions(+), 5 deletions(-) > > Index: usb-devel/drivers/usb/core/file.c > =================================================================== > --- usb-devel.orig/drivers/usb/core/file.c > +++ usb-devel/drivers/usb/core/file.c > @@ -193,9 +193,10 @@ int usb_register_dev(struct usb_interfac > intf->minor = minor; > break; > } > - up_write(&minor_rwsem); > - if (intf->minor < 0) > + if (intf->minor < 0) { > + up_write(&minor_rwsem); > return -EXFULL; > + } > > /* create a usb class device for this usb interface */ > snprintf(name, sizeof(name), class_driver->name, minor - minor_base); > @@ -203,12 +204,11 @@ int usb_register_dev(struct usb_interfac > MKDEV(USB_MAJOR, minor), class_driver, > "%s", kbasename(name)); > if (IS_ERR(intf->usb_dev)) { > - down_write(&minor_rwsem); > usb_minors[minor] = NULL; > intf->minor = -1; > - up_write(&minor_rwsem); > retval = PTR_ERR(intf->usb_dev); > } > + up_write(&minor_rwsem); > return retval; > } > EXPORT_SYMBOL_GPL(usb_register_dev); > @@ -234,12 +234,12 @@ void usb_deregister_dev(struct usb_inter > return; > > dev_dbg(&intf->dev, "removing %d minor\n", intf->minor); > + device_destroy(usb_class->class, MKDEV(USB_MAJOR, intf->minor)); > > down_write(&minor_rwsem); > usb_minors[intf->minor] = NULL; > up_write(&minor_rwsem); > > - device_destroy(usb_class->class, MKDEV(USB_MAJOR, intf->minor)); > intf->usb_dev = NULL; > intf->minor = -1; > destroy_usb_class(); >
On Mon, 12 Aug 2019, Andrey Konovalov wrote: > Alan, could you submit this patch (if you haven't already)? Looks like > it fixes this bug (and might fix some others). I will. I was waiting to see if Greg KH had any comments. Alan Stern > > drivers/usb/core/file.c | 10 +++++----- > > 1 file changed, 5 insertions(+), 5 deletions(-) > > > > Index: usb-devel/drivers/usb/core/file.c > > =================================================================== > > --- usb-devel.orig/drivers/usb/core/file.c > > +++ usb-devel/drivers/usb/core/file.c > > @@ -193,9 +193,10 @@ int usb_register_dev(struct usb_interfac > > intf->minor = minor; > > break; > > } > > - up_write(&minor_rwsem); > > - if (intf->minor < 0) > > + if (intf->minor < 0) { > > + up_write(&minor_rwsem); > > return -EXFULL; > > + } > > > > /* create a usb class device for this usb interface */ > > snprintf(name, sizeof(name), class_driver->name, minor - minor_base); > > @@ -203,12 +204,11 @@ int usb_register_dev(struct usb_interfac > > MKDEV(USB_MAJOR, minor), class_driver, > > "%s", kbasename(name)); > > if (IS_ERR(intf->usb_dev)) { > > - down_write(&minor_rwsem); > > usb_minors[minor] = NULL; > > intf->minor = -1; > > - up_write(&minor_rwsem); > > retval = PTR_ERR(intf->usb_dev); > > } > > + up_write(&minor_rwsem); > > return retval; > > } > > EXPORT_SYMBOL_GPL(usb_register_dev); > > @@ -234,12 +234,12 @@ void usb_deregister_dev(struct usb_inter > > return; > > > > dev_dbg(&intf->dev, "removing %d minor\n", intf->minor); > > + device_destroy(usb_class->class, MKDEV(USB_MAJOR, intf->minor)); > > > > down_write(&minor_rwsem); > > usb_minors[intf->minor] = NULL; > > up_write(&minor_rwsem); > > > > - device_destroy(usb_class->class, MKDEV(USB_MAJOR, intf->minor)); > > intf->usb_dev = NULL; > > intf->minor = -1; > > destroy_usb_class();
On Mon, Aug 12, 2019 at 10:21:14AM -0400, Alan Stern wrote:
> On Mon, 12 Aug 2019, Andrey Konovalov wrote:
>
> > Alan, could you submit this patch (if you haven't already)? Looks like
> > it fixes this bug (and might fix some others).
>
> I will. I was waiting to see if Greg KH had any comments.
Give me a few hours, it's in my queue to review...
On Fri, Aug 09, 2019 at 12:51:00PM -0400, Alan Stern wrote:
> Greg:
>
> See below...
>
> On Fri, 9 Aug 2019, syzbot wrote:
>
> > Hello,
> >
> > syzbot found the following crash on:
> >
> > HEAD commit: e96407b4 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=15a16f26600000
> > kernel config: https://syzkaller.appspot.com/x/.config?x=cfa2c18fb6a8068e
> > dashboard link: https://syzkaller.appspot.com/bug?extid=30cf45ebfe0b0c4847a1
> > compiler: gcc (GCC) 9.0.0 20181231 (experimental)
> > syz repro: https://syzkaller.appspot.com/x/repro.syz?x=1416df26600000
> > C reproducer: https://syzkaller.appspot.com/x/repro.c?x=11ce511c600000
> >
> > IMPORTANT: if you fix the bug, please add the following tag to the commit:
> > Reported-by: syzbot+30cf45ebfe0b0c4847a1@syzkaller.appspotmail.com
> >
> > ==================================================================
> > BUG: KASAN: use-after-free in __mutex_lock_common
> > kernel/locking/mutex.c:912 [inline]
> > BUG: KASAN: use-after-free in __mutex_lock+0xf23/0x1360
> > kernel/locking/mutex.c:1077
> > Read of size 8 at addr ffff8881d21fc2d8 by task syz-executor834/1878
> >
> > CPU: 0 PID: 1878 Comm: syz-executor834 Not tainted 5.3.0-rc2+ #25
> > 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+0xca/0x13e lib/dump_stack.c:113
> > print_address_description+0x6a/0x32c mm/kasan/report.c:351
> > __kasan_report.cold+0x1a/0x33 mm/kasan/report.c:482
> > kasan_report+0xe/0x12 mm/kasan/common.c:612
> > __mutex_lock_common kernel/locking/mutex.c:912 [inline]
> > __mutex_lock+0xf23/0x1360 kernel/locking/mutex.c:1077
> > ld_usb_release+0xb1/0x400 drivers/usb/misc/ldusb.c:386
> > __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:163
> > prepare_exit_to_usermode arch/x86/entry/common.c:194 [inline]
> > syscall_return_slowpath arch/x86/entry/common.c:274 [inline]
> > do_syscall_64+0x45f/0x580 arch/x86/entry/common.c:299
> > entry_SYSCALL_64_after_hwframe+0x49/0xbe
> > RIP: 0033:0x406b31
> > Code: 75 14 b8 03 00 00 00 0f 05 48 3d 01 f0 ff ff 0f 83 04 19 00 00 c3 48
> > 83 ec 08 e8 6a fc ff ff 48 89 04 24 b8 03 00 00 00 0f 05 <48> 8b 3c 24 48
> > 89 c2 e8 b3 fc ff ff 48 89 d0 48 83 c4 08 48 3d 01
> > RSP: 002b:00007ffcf13bd080 EFLAGS: 00000293 ORIG_RAX: 0000000000000003
> > RAX: 0000000000000000 RBX: 0000000000000005 RCX: 0000000000406b31
> > RDX: fffffffffffffff7 RSI: 0000000000000080 RDI: 0000000000000004
> > RBP: 0000000000000159 R08: 0000000000000020 R09: 0000000000000020
> > R10: 00007ffcf13bd0b0 R11: 0000000000000293 R12: 000000000001d884
> > R13: 0000000000000004 R14: 00000000006e39ec R15: 0000000000000064
> >
> > Allocated by task 1775:
> > save_stack+0x1b/0x80 mm/kasan/common.c:69
> > set_track mm/kasan/common.c:77 [inline]
> > __kasan_kmalloc mm/kasan/common.c:487 [inline]
> > __kasan_kmalloc.constprop.0+0xbf/0xd0 mm/kasan/common.c:460
> > kmalloc include/linux/slab.h:552 [inline]
> > kzalloc include/linux/slab.h:748 [inline]
> > ld_usb_probe+0x6e/0xa65 drivers/usb/misc/ldusb.c:661
> > usb_probe_interface+0x305/0x7a0 drivers/usb/core/driver.c:361
> > really_probe+0x281/0x650 drivers/base/dd.c:548
> > driver_probe_device+0x101/0x1b0 drivers/base/dd.c:709
> > __device_attach_driver+0x1c2/0x220 drivers/base/dd.c:816
> > bus_for_each_drv+0x15c/0x1e0 drivers/base/bus.c:454
> > __device_attach+0x217/0x360 drivers/base/dd.c:882
> > bus_probe_device+0x1e4/0x290 drivers/base/bus.c:514
> > device_add+0xae6/0x16f0 drivers/base/core.c:2114
> > usb_set_configuration+0xdf6/0x1670 drivers/usb/core/message.c:2023
> > generic_probe+0x9d/0xd5 drivers/usb/core/generic.c:210
> > usb_probe_device+0x99/0x100 drivers/usb/core/driver.c:266
> > really_probe+0x281/0x650 drivers/base/dd.c:548
> > driver_probe_device+0x101/0x1b0 drivers/base/dd.c:709
> > __device_attach_driver+0x1c2/0x220 drivers/base/dd.c:816
> > bus_for_each_drv+0x15c/0x1e0 drivers/base/bus.c:454
> > __device_attach+0x217/0x360 drivers/base/dd.c:882
> > bus_probe_device+0x1e4/0x290 drivers/base/bus.c:514
> > device_add+0xae6/0x16f0 drivers/base/core.c:2114
> > usb_new_device.cold+0x6a4/0xe79 drivers/usb/core/hub.c:2536
> > hub_port_connect drivers/usb/core/hub.c:5098 [inline]
> > hub_port_connect_change drivers/usb/core/hub.c:5213 [inline]
> > port_event drivers/usb/core/hub.c:5359 [inline]
> > hub_event+0x1b5c/0x3640 drivers/usb/core/hub.c:5441
> > process_one_work+0x92b/0x1530 kernel/workqueue.c:2269
> > worker_thread+0x96/0xe20 kernel/workqueue.c:2415
> > kthread+0x318/0x420 kernel/kthread.c:255
> > ret_from_fork+0x24/0x30 arch/x86/entry/entry_64.S:352
> >
> > Freed by task 1775:
> > save_stack+0x1b/0x80 mm/kasan/common.c:69
> > set_track mm/kasan/common.c:77 [inline]
> > __kasan_slab_free+0x130/0x180 mm/kasan/common.c:449
> > slab_free_hook mm/slub.c:1423 [inline]
> > slab_free_freelist_hook mm/slub.c:1470 [inline]
> > slab_free mm/slub.c:3012 [inline]
> > kfree+0xe4/0x2f0 mm/slub.c:3953
> > ld_usb_probe+0x728/0xa65 drivers/usb/misc/ldusb.c:744
> > usb_probe_interface+0x305/0x7a0 drivers/usb/core/driver.c:361
> > really_probe+0x281/0x650 drivers/base/dd.c:548
> > driver_probe_device+0x101/0x1b0 drivers/base/dd.c:709
> > __device_attach_driver+0x1c2/0x220 drivers/base/dd.c:816
> > bus_for_each_drv+0x15c/0x1e0 drivers/base/bus.c:454
> > __device_attach+0x217/0x360 drivers/base/dd.c:882
> > bus_probe_device+0x1e4/0x290 drivers/base/bus.c:514
> > device_add+0xae6/0x16f0 drivers/base/core.c:2114
> > usb_set_configuration+0xdf6/0x1670 drivers/usb/core/message.c:2023
> > generic_probe+0x9d/0xd5 drivers/usb/core/generic.c:210
> > usb_probe_device+0x99/0x100 drivers/usb/core/driver.c:266
> > really_probe+0x281/0x650 drivers/base/dd.c:548
> > driver_probe_device+0x101/0x1b0 drivers/base/dd.c:709
> > __device_attach_driver+0x1c2/0x220 drivers/base/dd.c:816
> > bus_for_each_drv+0x15c/0x1e0 drivers/base/bus.c:454
> > __device_attach+0x217/0x360 drivers/base/dd.c:882
> > bus_probe_device+0x1e4/0x290 drivers/base/bus.c:514
> > device_add+0xae6/0x16f0 drivers/base/core.c:2114
> > usb_new_device.cold+0x6a4/0xe79 drivers/usb/core/hub.c:2536
> > hub_port_connect drivers/usb/core/hub.c:5098 [inline]
> > hub_port_connect_change drivers/usb/core/hub.c:5213 [inline]
> > port_event drivers/usb/core/hub.c:5359 [inline]
> > hub_event+0x1b5c/0x3640 drivers/usb/core/hub.c:5441
> > process_one_work+0x92b/0x1530 kernel/workqueue.c:2269
> > worker_thread+0x96/0xe20 kernel/workqueue.c:2415
> > 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 ffff8881d21fc280
> > which belongs to the cache kmalloc-512 of size 512
> > The buggy address is located 88 bytes inside of
> > 512-byte region [ffff8881d21fc280, ffff8881d21fc480)
> > The buggy address belongs to the page:
> > page:ffffea0007487f00 refcount:1 mapcount:0 mapping:ffff8881da002500
> > index:0x0 compound_mapcount: 0
> > flags: 0x200000000010200(slab|head)
> > raw: 0200000000010200 ffffea000739fc80 0000000900000009 ffff8881da002500
> > raw: 0000000000000000 00000000000c000c 00000001ffffffff 0000000000000000
> > page dumped because: kasan: bad access detected
> >
> > Memory state around the buggy address:
> > ffff8881d21fc180: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
> > ffff8881d21fc200: fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc
> > > ffff8881d21fc280: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
> > ^
> > ffff8881d21fc300: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
> > ffff8881d21fc380: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
> > ==================================================================
>
> I have tracked this bug down. The root causes lie in
> usb_register_dev() and usb_deregister_dev().
>
> The first problem is that usb_deregister_dev() sets
> usb_minors[intf->minor] to NULL before calling device_destroy() on the
> class device. This leaves a window during which another thread can
> allocate the same minor number but will encounter a duplicate name
> error when it tries to register its own class device.
>
> This error shows up in the kernel log from the syzbot test (not shown
> in the bug report above, though):
>
> [ 120.583776][ T1775] sysfs: cannot create duplicate filename '/class/usbmisc/ldusb0'
>
> This can be fixed easily enough by reordering the statements in
> usb_deregister_dev().
>
> The second problem is in usb_register_dev(). When that routine runs,
> it first allocates a minor number, then drops minor_rwsem, and then
> creates the class device. If the device creation fails, the minor
> number is deallocated and the whole routine returns an error. But
> during the time while minor_rwsem was dropped, there is a window in
> which the minor number is allocated and so another thread could
> successfully open the device file!
>
> These two scenarios are exactly what happened during the syzbot test.
> Minor number 0 was deallocated and then allocated in another thread.
> The second allocation failed because the old class device was still in
> existence. As a result of this failure, ldusb's private data structure
> was released. Nevertheless, a third thread managed to open the device
> file during the brief time that minor number 0 was re-allocated. When
> that thread closed the file, it tried to access the private data
> structure that had already been released.
>
> I believe the patch below will fix both problems. But you should take
> a look at it first to see if it seems right; syzbot can't really test
> the patch because it involves two separate races both coming out wrong!
Ugh, nasty races, good find. Your fixes look fine, feel free to turn
this into a patch I can submit and I will do so.
thanks for finding this!
greg k-h
The syzbot fuzzer has found two (!) races in the USB character device registration and deregistration routines. This patch fixes the races. The first race results from the fact that usb_deregister_dev() sets usb_minors[intf->minor] to NULL before calling device_destroy() on the class device. This leaves a window during which another thread can allocate the same minor number but will encounter a duplicate name error when it tries to register its own class device. A typical error message in the system log would look like: sysfs: cannot create duplicate filename '/class/usbmisc/ldusb0' The patch fixes this race by destroying the class device first. The second race is in usb_register_dev(). When that routine runs, it first allocates a minor number, then drops minor_rwsem, and then creates the class device. If the device creation fails, the minor number is deallocated and the whole routine returns an error. But during the time while minor_rwsem was dropped, there is a window in which the minor number is allocated and so another thread can successfully open the device file. Typically this results in use-after-free errors or invalid accesses when the other thread closes its open file reference, because the kernel then tries to release resources that were already deallocated when usb_register_dev() failed. The patch fixes this race by keeping minor_rwsem locked throughout the entire routine. Reported-and-tested-by: syzbot+30cf45ebfe0b0c4847a1@syzkaller.appspotmail.com Signed-off-by: Alan Stern <stern@rowland.harvard.edu> CC: <stable@vger.kernel.org> --- [as1907] drivers/usb/core/file.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) Index: usb-devel/drivers/usb/core/file.c =================================================================== --- usb-devel.orig/drivers/usb/core/file.c +++ usb-devel/drivers/usb/core/file.c @@ -193,9 +193,10 @@ int usb_register_dev(struct usb_interfac intf->minor = minor; break; } - up_write(&minor_rwsem); - if (intf->minor < 0) + if (intf->minor < 0) { + up_write(&minor_rwsem); return -EXFULL; + } /* create a usb class device for this usb interface */ snprintf(name, sizeof(name), class_driver->name, minor - minor_base); @@ -203,12 +204,11 @@ int usb_register_dev(struct usb_interfac MKDEV(USB_MAJOR, minor), class_driver, "%s", kbasename(name)); if (IS_ERR(intf->usb_dev)) { - down_write(&minor_rwsem); usb_minors[minor] = NULL; intf->minor = -1; - up_write(&minor_rwsem); retval = PTR_ERR(intf->usb_dev); } + up_write(&minor_rwsem); return retval; } EXPORT_SYMBOL_GPL(usb_register_dev); @@ -234,12 +234,12 @@ void usb_deregister_dev(struct usb_inter return; dev_dbg(&intf->dev, "removing %d minor\n", intf->minor); + device_destroy(usb_class->class, MKDEV(USB_MAJOR, intf->minor)); down_write(&minor_rwsem); usb_minors[intf->minor] = NULL; up_write(&minor_rwsem); - device_destroy(usb_class->class, MKDEV(USB_MAJOR, intf->minor)); intf->usb_dev = NULL; intf->minor = -1; destroy_usb_class();
On Mon, Aug 12, 2019 at 04:11:07PM -0400, Alan Stern wrote:
> The syzbot fuzzer has found two (!) races in the USB character device
> registration and deregistration routines. This patch fixes the races.
>
> The first race results from the fact that usb_deregister_dev() sets
> usb_minors[intf->minor] to NULL before calling device_destroy() on the
> class device. This leaves a window during which another thread can
> allocate the same minor number but will encounter a duplicate name
> error when it tries to register its own class device. A typical error
> message in the system log would look like:
>
> sysfs: cannot create duplicate filename '/class/usbmisc/ldusb0'
>
> The patch fixes this race by destroying the class device first.
>
> The second race is in usb_register_dev(). When that routine runs, it
> first allocates a minor number, then drops minor_rwsem, and then
> creates the class device. If the device creation fails, the minor
> number is deallocated and the whole routine returns an error. But
> during the time while minor_rwsem was dropped, there is a window in
> which the minor number is allocated and so another thread can
> successfully open the device file. Typically this results in
> use-after-free errors or invalid accesses when the other thread closes
> its open file reference, because the kernel then tries to release
> resources that were already deallocated when usb_register_dev()
> failed. The patch fixes this race by keeping minor_rwsem locked
> throughout the entire routine.
>
> Reported-and-tested-by: syzbot+30cf45ebfe0b0c4847a1@syzkaller.appspotmail.com
> Signed-off-by: Alan Stern <stern@rowland.harvard.edu>
> CC: <stable@vger.kernel.org>
>
> ---
>
> [as1907]
Thanks for this, now queued up.
greg k-h