Message-ID: <20190812153100.GA28429@kroah.com>
Date:   Mon, 12 Aug 2019 17:31:00 +0200
From:   Greg KH <greg@...ah.com>
To:     Alan Stern <stern@...land.harvard.edu>
Cc:     syzbot <syzbot+30cf45ebfe0b0c4847a1@...kaller.appspotmail.com>,
        andreyknvl@...gle.com, bhelgaas@...gle.com, kirr@...edi.com,
        Kernel development list <linux-kernel@...r.kernel.org>,
        USB list <linux-usb@...r.kernel.org>, linux@...ck-us.net,
        lkundrak@...sk, logang@...tatee.com,
        syzkaller-bugs@...glegroups.com
Subject: Re: KASAN: use-after-free Read in ld_usb_release

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@...kaller.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

Powered by blists - more mailing lists