Linux-USB Archive on lore.kernel.org
 help / color / Atom feed
From: Greg KH <greg@kroah.com>
To: Alan Stern <stern@rowland.harvard.edu>
Cc: syzbot <syzbot+30cf45ebfe0b0c4847a1@syzkaller.appspotmail.com>,
	andreyknvl@google.com, bhelgaas@google.com, kirr@nexedi.com,
	Kernel development list <linux-kernel@vger.kernel.org>,
	USB list <linux-usb@vger.kernel.org>,
	linux@roeck-us.net, lkundrak@v3.sk, logang@deltatee.com,
	syzkaller-bugs@googlegroups.com
Subject: Re: KASAN: use-after-free Read in ld_usb_release
Date: Mon, 12 Aug 2019 17:31:00 +0200
Message-ID: <20190812153100.GA28429@kroah.com> (raw)
In-Reply-To: <Pine.LNX.4.44L0.1908091229540.1630-100000@iolanthe.rowland.org>

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

  parent reply index

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-08-09 13:18 syzbot
2019-08-09 16:51 ` Alan Stern
2019-08-09 17:33   ` Andrey Konovalov
2019-08-09 17:53     ` syzbot
2019-08-12 12:07   ` Andrey Konovalov
2019-08-12 14:21     ` Alan Stern
2019-08-12 14:31       ` Greg KH
2019-08-12 15:31   ` Greg KH [this message]
2019-08-12 20:11   ` [PATCH] USB: core: Fix races in character device registration and deregistraion Alan Stern
2019-08-12 20:52     ` Greg KH

Reply instructions:

You may reply publically to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20190812153100.GA28429@kroah.com \
    --to=greg@kroah.com \
    --cc=andreyknvl@google.com \
    --cc=bhelgaas@google.com \
    --cc=kirr@nexedi.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-usb@vger.kernel.org \
    --cc=linux@roeck-us.net \
    --cc=lkundrak@v3.sk \
    --cc=logang@deltatee.com \
    --cc=stern@rowland.harvard.edu \
    --cc=syzbot+30cf45ebfe0b0c4847a1@syzkaller.appspotmail.com \
    --cc=syzkaller-bugs@googlegroups.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

Linux-USB Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-usb/0 linux-usb/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-usb linux-usb/ https://lore.kernel.org/linux-usb \
		linux-usb@vger.kernel.org linux-usb@archiver.kernel.org
	public-inbox-index linux-usb


Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-usb


AGPL code for this site: git clone https://public-inbox.org/ public-inbox