KASAN: use-after-free Read in iowarrior_disconnect

KASAN: use-after-free Read in iowarrior_disconnect

[syzbot]

Hello,

syzbot found the following crash on:

HEAD commit:    d0847550 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=139be302600000
kernel config:  https://syzkaller.appspot.com/x/.config?x=dbc9c80cc095da19
dashboard link: https://syzkaller.appspot.com/bug?extid=cfe6d93e0abab9a0de05
compiler:       gcc (GCC) 9.0.0 20181231 (experimental)
syz repro:      https://syzkaller.appspot.com/x/repro.syz?x=12fe6b02600000
C reproducer:   https://syzkaller.appspot.com/x/repro.c?x=1548189c600000

IMPORTANT: if you fix the bug, please add the following tag to the commit:
Reported-by: syzbot+cfe6d93e0abab9a0de05@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 ffff8881cc866f58 by task kworker/1:5/1755

CPU: 1 PID: 1755 Comm: kworker/1:5 Not tainted 5.3.0-rc4+ #26
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+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
  iowarrior_disconnect+0xf0/0x2c0 drivers/usb/misc/iowarrior.c:878
  usb_unbind_interface+0x1bd/0x8a0 drivers/usb/core/driver.c:423
  __device_release_driver drivers/base/dd.c:1134 [inline]
  device_release_driver_internal+0x42f/0x500 drivers/base/dd.c:1165
  bus_remove_device+0x2dc/0x4a0 drivers/base/bus.c:556
  device_del+0x420/0xb10 drivers/base/core.c:2339
  usb_disable_device+0x211/0x690 drivers/usb/core/message.c:1237
  usb_disconnect+0x284/0x8d0 drivers/usb/core/hub.c:2199
  hub_port_connect drivers/usb/core/hub.c:4949 [inline]
  hub_port_connect_change drivers/usb/core/hub.c:5213 [inline]
  port_event drivers/usb/core/hub.c:5359 [inline]
  hub_event+0x1454/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

Allocated by task 1755:
  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]
  iowarrior_probe+0x7a/0x10b2 drivers/usb/misc/iowarrior.c:753
  usb_probe_interface+0x305/0x7a0 drivers/usb/core/driver.c:361
  really_probe+0x281/0x6d0 drivers/base/dd.c:548
  driver_probe_device+0x101/0x1b0 drivers/base/dd.c:721
  __device_attach_driver+0x1c2/0x220 drivers/base/dd.c:828
  bus_for_each_drv+0x162/0x1e0 drivers/base/bus.c:454
  __device_attach+0x217/0x360 drivers/base/dd.c:894
  bus_probe_device+0x1e4/0x290 drivers/base/bus.c:514
  device_add+0xae6/0x16f0 drivers/base/core.c:2165
  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/0x6d0 drivers/base/dd.c:548
  driver_probe_device+0x101/0x1b0 drivers/base/dd.c:721
  __device_attach_driver+0x1c2/0x220 drivers/base/dd.c:828
  bus_for_each_drv+0x162/0x1e0 drivers/base/bus.c:454
  __device_attach+0x217/0x360 drivers/base/dd.c:894
  bus_probe_device+0x1e4/0x290 drivers/base/bus.c:514
  device_add+0xae6/0x16f0 drivers/base/core.c:2165
  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 1857:
  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:1474 [inline]
  slab_free mm/slub.c:3016 [inline]
  kfree+0xe4/0x2f0 mm/slub.c:3957
  iowarrior_delete drivers/usb/misc/iowarrior.c:246 [inline]
  iowarrior_release+0x187/0x280 drivers/usb/misc/iowarrior.c:670
  __fput+0x2d7/0x840 fs/file_table.c:280
  task_work_run+0x13f/0x1c0 kernel/task_work.c:113
  exit_task_work include/linux/task_work.h:22 [inline]
  do_exit+0x8ef/0x2c00 kernel/exit.c:879
  do_group_exit+0x125/0x340 kernel/exit.c:983
  __do_sys_exit_group kernel/exit.c:994 [inline]
  __se_sys_exit_group kernel/exit.c:992 [inline]
  __x64_sys_exit_group+0x3a/0x50 kernel/exit.c:992
  do_syscall_64+0xb7/0x580 arch/x86/entry/common.c:296
  entry_SYSCALL_64_after_hwframe+0x49/0xbe

The buggy address belongs to the object at ffff8881cc866f00
  which belongs to the cache kmalloc-512 of size 512
The buggy address is located 88 bytes inside of
  512-byte region [ffff8881cc866f00, ffff8881cc867100)
The buggy address belongs to the page:
page:ffffea0007321980 refcount:1 mapcount:0 mapping:ffff8881da002500  
index:0x0 compound_mapcount: 0
flags: 0x200000000010200(slab|head)
raw: 0200000000010200 ffffea000733fc80 0000000200000002 ffff8881da002500
raw: 0000000000000000 00000000000c000c 00000001ffffffff 0000000000000000
page dumped because: kasan: bad access detected

Memory state around the buggy address:
  ffff8881cc866e00: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
  ffff8881cc866e80: fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc
> ffff8881cc866f00: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
                                                     ^
  ffff8881cc866f80: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
  ffff8881cc867000: 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

Re: KASAN: use-after-free Read in iowarrior_disconnect

[Oliver Neukum]

[-- Attachment #1: Type: text/plain, Size: 894 bytes --]

Am Montag, den 19.08.2019, 07:48 -0700 schrieb syzbot:
> Hello,
> 
> syzbot found the following crash on:
> 
> HEAD commit:    d0847550 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=139be302600000
> kernel config:  https://syzkaller.appspot.com/x/.config?x=dbc9c80cc095da19
> dashboard link: https://syzkaller.appspot.com/bug?extid=cfe6d93e0abab9a0de05
> compiler:       gcc (GCC) 9.0.0 20181231 (experimental)
> syz repro:      https://syzkaller.appspot.com/x/repro.syz?x=12fe6b02600000
> C reproducer:   https://syzkaller.appspot.com/x/repro.c?x=1548189c600000
> 
> IMPORTANT: if you fix the bug, please add the following tag to the commit:
> Reported-by: syzbot+cfe6d93e0abab9a0de05@syzkaller.appspotmail.com
> 

#syz test: https://github.com/google/kasan.git d0847550


[-- Attachment #2: 0001-Revert-usb-iowarrior-fix-deadlock-on-disconnect.patch --]
[-- Type: text/x-patch, Size: 1340 bytes --]

From 43c4270a424052dcb168a0fea5a9ad89778eadc7 Mon Sep 17 00:00:00 2001
From: Oliver Neukum <oneukum@suse.com>
Date: Mon, 19 Aug 2019 17:22:53 +0200
Subject: [PATCH] Revert "usb: iowarrior: fix deadlock on disconnect"

This reverts commit aa40cfb4d2f134322a782b18a687d04300f50f60.
---
 drivers/usb/misc/iowarrior.c | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/drivers/usb/misc/iowarrior.c b/drivers/usb/misc/iowarrior.c
index f5bed9f29e56..ba05dd80a020 100644
--- a/drivers/usb/misc/iowarrior.c
+++ b/drivers/usb/misc/iowarrior.c
@@ -866,20 +866,19 @@ static void iowarrior_disconnect(struct usb_interface *interface)
 	dev = usb_get_intfdata(interface);
 	mutex_lock(&iowarrior_open_disc_lock);
 	usb_set_intfdata(interface, NULL);
-	/* prevent device read, write and ioctl */
-	dev->present = 0;
 
 	minor = dev->minor;
-	mutex_unlock(&iowarrior_open_disc_lock);
-	/* give back our minor - this will call close() locks need to be dropped at this point*/
 
+	/* give back our minor */
 	usb_deregister_dev(interface, &iowarrior_class);
 
 	mutex_lock(&dev->mutex);
 
 	/* prevent device read, write and ioctl */
+	dev->present = 0;
 
 	mutex_unlock(&dev->mutex);
+	mutex_unlock(&iowarrior_open_disc_lock);
 
 	if (dev->opened) {
 		/* There is a process that holds a filedescriptor to the device ,
-- 
2.16.4

Re: KASAN: use-after-free Read in iowarrior_disconnect

[syzbot]

Hello,

syzbot has tested the proposed patch but the reproducer still triggered  
crash:
possible deadlock in usb_deregister_dev

usb 4-1: USB disconnect, device number 2
======================================================
WARNING: possible circular locking dependency detected
5.3.0-rc4+ #1 Not tainted
------------------------------------------------------
kworker/1:1/21 is trying to acquire lock:
00000000bfac431a (minor_rwsem){++++}, at: usb_deregister_dev  
drivers/usb/core/file.c:238 [inline]
00000000bfac431a (minor_rwsem){++++}, at: usb_deregister_dev+0x61/0x270  
drivers/usb/core/file.c:230

but task is already holding lock:
000000007638fa06 (iowarrior_open_disc_lock){+.+.}, at:  
iowarrior_disconnect+0x45/0x2c0 drivers/usb/misc/iowarrior.c:867

which lock already depends on the new lock.


the existing dependency chain (in reverse order) is:

-> #2 (iowarrior_open_disc_lock){+.+.}:
        __mutex_lock_common kernel/locking/mutex.c:930 [inline]
        __mutex_lock+0x158/0x1360 kernel/locking/mutex.c:1077
        iowarrior_open+0x8a/0x2a0 drivers/usb/misc/iowarrior.c:600
        usb_open+0x1df/0x270 drivers/usb/core/file.c:48
        chrdev_open+0x219/0x5c0 fs/char_dev.c:414
        do_dentry_open+0x494/0x1120 fs/open.c:797
        do_last fs/namei.c:3416 [inline]
        path_openat+0x1430/0x3f50 fs/namei.c:3533
        do_filp_open+0x1a1/0x280 fs/namei.c:3563
        do_sys_open+0x3c0/0x580 fs/open.c:1089
        do_syscall_64+0xb7/0x580 arch/x86/entry/common.c:296
        entry_SYSCALL_64_after_hwframe+0x49/0xbe

-> #1 (iowarrior_mutex){+.+.}:
        __mutex_lock_common kernel/locking/mutex.c:930 [inline]
        __mutex_lock+0x158/0x1360 kernel/locking/mutex.c:1077
        iowarrior_open+0x23/0x2a0 drivers/usb/misc/iowarrior.c:589
        usb_open+0x1df/0x270 drivers/usb/core/file.c:48
        chrdev_open+0x219/0x5c0 fs/char_dev.c:414
        do_dentry_open+0x494/0x1120 fs/open.c:797
        do_last fs/namei.c:3416 [inline]
        path_openat+0x1430/0x3f50 fs/namei.c:3533
        do_filp_open+0x1a1/0x280 fs/namei.c:3563
        do_sys_open+0x3c0/0x580 fs/open.c:1089
        do_syscall_64+0xb7/0x580 arch/x86/entry/common.c:296
        entry_SYSCALL_64_after_hwframe+0x49/0xbe

-> #0 (minor_rwsem){++++}:
        check_prev_add kernel/locking/lockdep.c:2405 [inline]
        check_prevs_add kernel/locking/lockdep.c:2507 [inline]
        validate_chain kernel/locking/lockdep.c:2897 [inline]
        __lock_acquire+0x1f7c/0x3b50 kernel/locking/lockdep.c:3880
        lock_acquire+0x127/0x320 kernel/locking/lockdep.c:4412
        down_write+0x92/0x150 kernel/locking/rwsem.c:1500
        usb_deregister_dev drivers/usb/core/file.c:238 [inline]
        usb_deregister_dev+0x61/0x270 drivers/usb/core/file.c:230
        iowarrior_disconnect+0xa8/0x2c0 drivers/usb/misc/iowarrior.c:873
        usb_unbind_interface+0x1bd/0x8a0 drivers/usb/core/driver.c:423
        __device_release_driver drivers/base/dd.c:1134 [inline]
        device_release_driver_internal+0x42f/0x500 drivers/base/dd.c:1165
        bus_remove_device+0x2dc/0x4a0 drivers/base/bus.c:556
        device_del+0x420/0xb10 drivers/base/core.c:2339
        usb_disable_device+0x211/0x690 drivers/usb/core/message.c:1237
        usb_disconnect+0x284/0x8d0 drivers/usb/core/hub.c:2199
        hub_port_connect drivers/usb/core/hub.c:4949 [inline]
        hub_port_connect_change drivers/usb/core/hub.c:5213 [inline]
        port_event drivers/usb/core/hub.c:5359 [inline]
        hub_event+0x1454/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

other info that might help us debug this:

Chain exists of:
   minor_rwsem --> iowarrior_mutex --> iowarrior_open_disc_lock

  Possible unsafe locking scenario:

        CPU0                    CPU1
        ----                    ----
   lock(iowarrior_open_disc_lock);
                                lock(iowarrior_mutex);
                                lock(iowarrior_open_disc_lock);
   lock(minor_rwsem);

  *** DEADLOCK ***

6 locks held by kworker/1:1/21:
  #0: 00000000ffafc5b3 ((wq_completion)usb_hub_wq){+.+.}, at:  
__write_once_size include/linux/compiler.h:226 [inline]
  #0: 00000000ffafc5b3 ((wq_completion)usb_hub_wq){+.+.}, at:  
arch_atomic64_set arch/x86/include/asm/atomic64_64.h:34 [inline]
  #0: 00000000ffafc5b3 ((wq_completion)usb_hub_wq){+.+.}, at: atomic64_set  
include/asm-generic/atomic-instrumented.h:855 [inline]
  #0: 00000000ffafc5b3 ((wq_completion)usb_hub_wq){+.+.}, at:  
atomic_long_set include/asm-generic/atomic-long.h:40 [inline]
  #0: 00000000ffafc5b3 ((wq_completion)usb_hub_wq){+.+.}, at: set_work_data  
kernel/workqueue.c:620 [inline]
  #0: 00000000ffafc5b3 ((wq_completion)usb_hub_wq){+.+.}, at:  
set_work_pool_and_clear_pending kernel/workqueue.c:647 [inline]
  #0: 00000000ffafc5b3 ((wq_completion)usb_hub_wq){+.+.}, at:  
process_one_work+0x827/0x1530 kernel/workqueue.c:2240
  #1: 000000005bc0df0d ((work_completion)(&hub->events)){+.+.}, at:  
process_one_work+0x85b/0x1530 kernel/workqueue.c:2244
  #2: 00000000f73a9504 (&dev->mutex){....}, at: device_lock  
include/linux/device.h:1223 [inline]
  #2: 00000000f73a9504 (&dev->mutex){....}, at: hub_event+0x17c/0x3640  
drivers/usb/core/hub.c:5387
  #3: 000000006fe9ca35 (&dev->mutex){....}, at: device_lock  
include/linux/device.h:1223 [inline]
  #3: 000000006fe9ca35 (&dev->mutex){....}, at: usb_disconnect+0x91/0x8d0  
drivers/usb/core/hub.c:2190
  #4: 0000000044c331cb (&dev->mutex){....}, at:  
device_release_driver_internal+0x23/0x500 drivers/base/dd.c:1162
  #5: 000000007638fa06 (iowarrior_open_disc_lock){+.+.}, at:  
iowarrior_disconnect+0x45/0x2c0 drivers/usb/misc/iowarrior.c:867

stack backtrace:
CPU: 1 PID: 21 Comm: kworker/1:1 Not tainted 5.3.0-rc4+ #1
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+0xca/0x13e lib/dump_stack.c:113
  check_noncircular+0x345/0x3e0 kernel/locking/lockdep.c:1741
  check_prev_add kernel/locking/lockdep.c:2405 [inline]
  check_prevs_add kernel/locking/lockdep.c:2507 [inline]
  validate_chain kernel/locking/lockdep.c:2897 [inline]
  __lock_acquire+0x1f7c/0x3b50 kernel/locking/lockdep.c:3880
  lock_acquire+0x127/0x320 kernel/locking/lockdep.c:4412
  down_write+0x92/0x150 kernel/locking/rwsem.c:1500
  usb_deregister_dev drivers/usb/core/file.c:238 [inline]
  usb_deregister_dev+0x61/0x270 drivers/usb/core/file.c:230
  iowarrior_disconnect+0xa8/0x2c0 drivers/usb/misc/iowarrior.c:873
  usb_unbind_interface+0x1bd/0x8a0 drivers/usb/core/driver.c:423
  __device_release_driver drivers/base/dd.c:1134 [inline]
  device_release_driver_internal+0x42f/0x500 drivers/base/dd.c:1165
  bus_remove_device+0x2dc/0x4a0 drivers/base/bus.c:556
  device_del+0x420/0xb10 drivers/base/core.c:2339
  usb_disable_device+0x211/0x690 drivers/usb/core/message.c:1237
  usb_disconnect+0x284/0x8d0 drivers/usb/core/hub.c:2199
  hub_port_connect drivers/usb/core/hub.c:4949 [inline]
  hub_port_connect_change drivers/usb/core/hub.c:5213 [inline]
  port_event drivers/usb/core/hub.c:5359 [inline]
  hub_event+0x1454/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
iowarrior 4-1:0.236: I/O-Warror #0 now disconnected
usb 4-1: new low-speed USB device number 3 using dummy_hcd
usb 4-1: device descriptor read/all, error -71
usb 4-1: new low-speed USB device number 4 using dummy_hcd
usb 4-1: config 0 has an invalid interface number: 236 but max is 2
usb 4-1: config 0 has an invalid descriptor of length 99, skipping  
remainder of the config
usb 4-1: config 0 has 1 interface, different from the descriptor's value: 3
usb 4-1: config 0 has no interface number 0
usb 4-1: config 0 interface 236 altsetting 0 endpoint 0x81 is Bulk;  
changing to Interrupt
usb 4-1: New USB device found, idVendor=07c0, idProduct=1501,  
bcdDevice=74.a0
usb 4-1: New USB device strings: Mfr=0, Product=0, SerialNumber=0
usb 4-1: config 0 descriptor??


Tested on:

commit:         d0847550 usb-fuzzer: main usb gadget fuzzer driver
git tree:       https://github.com/google/kasan.git
console output: https://syzkaller.appspot.com/x/log.txt?x=154c4522600000
kernel config:  https://syzkaller.appspot.com/x/.config?x=dbc9c80cc095da19
compiler:       gcc (GCC) 9.0.0 20181231 (experimental)
patch:          https://syzkaller.appspot.com/x/patch.diff?x=11898be2600000

Re: KASAN: use-after-free Read in iowarrior_disconnect

[Alan Stern]

On Mon, 19 Aug 2019, Oliver Neukum wrote:

> Am Montag, den 19.08.2019, 07:48 -0700 schrieb syzbot:
> > Hello,
> > 
> > syzbot found the following crash on:
> > 
> > HEAD commit:    d0847550 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=139be302600000
> > kernel config:  https://syzkaller.appspot.com/x/.config?x=dbc9c80cc095da19
> > dashboard link: https://syzkaller.appspot.com/bug?extid=cfe6d93e0abab9a0de05
> > compiler:       gcc (GCC) 9.0.0 20181231 (experimental)
> > syz repro:      https://syzkaller.appspot.com/x/repro.syz?x=12fe6b02600000
> > C reproducer:   https://syzkaller.appspot.com/x/repro.c?x=1548189c600000
> > 
> > IMPORTANT: if you fix the bug, please add the following tag to the commit:
> > Reported-by: syzbot+cfe6d93e0abab9a0de05@syzkaller.appspotmail.com
> > 
> 
> #syz test: https://github.com/google/kasan.git d0847550

There's no need for us to work at cross purposes on this.  We can go 
with your approach.

However, the code is more complicated than your patch accounts for.  
The wait can finish in several different ways:

(1)	The control URB succeeds and the interrupt URB gets an 
	acknowledgment.

(2)	The control URB completes with an error.

(3)	The wait times out.

(4)	A disconnect occurs.

Your patch doesn't handle cases (1) and (3).  (And it doesn't get rid 
of the dev->waitq field, which is no longer used.)

In fact, (1) is a little ambiguous.  When the interrupt URB gets a 
command acknowledgment, there's no way (as far as I can tell) to know 
which command was acknowledged -- particularly if a prior command URB 
had to be cancelled because it timed out.

And as it turns out, the driver neglects to kill the command URB in
case (3).  Furthermore, the driver doesn't have mutual exclusion for 
writes.  So there's nothing to prevent the command URB from being 
submitted while it is still active (syzbot's new crash).

I have to wonder if anybody's actually using this driver.  It seems to
be pretty broken.  Maybe we should just mark it as such and forget
about fixing it.

Alan Stern

Re: KASAN: use-after-free Read in iowarrior_disconnect

[Alan Stern]

Oops, I replied to the wrong email message -- sorry.  This was intended
to be about the problem with the yurex driver, not the iowarrior
driver.

Alan Stern


On Tue, 20 Aug 2019, Alan Stern wrote:

> On Mon, 19 Aug 2019, Oliver Neukum wrote:
> 
> > Am Montag, den 19.08.2019, 07:48 -0700 schrieb syzbot:
> > > Hello,
> > > 
> > > syzbot found the following crash on:
> > > 
> > > HEAD commit:    d0847550 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=139be302600000
> > > kernel config:  https://syzkaller.appspot.com/x/.config?x=dbc9c80cc095da19
> > > dashboard link: https://syzkaller.appspot.com/bug?extid=cfe6d93e0abab9a0de05
> > > compiler:       gcc (GCC) 9.0.0 20181231 (experimental)
> > > syz repro:      https://syzkaller.appspot.com/x/repro.syz?x=12fe6b02600000
> > > C reproducer:   https://syzkaller.appspot.com/x/repro.c?x=1548189c600000
> > > 
> > > IMPORTANT: if you fix the bug, please add the following tag to the commit:
> > > Reported-by: syzbot+cfe6d93e0abab9a0de05@syzkaller.appspotmail.com
> > > 
> > 
> > #syz test: https://github.com/google/kasan.git d0847550
> 
> There's no need for us to work at cross purposes on this.  We can go 
> with your approach.
> 
> However, the code is more complicated than your patch accounts for.  
> The wait can finish in several different ways:
> 
> (1)	The control URB succeeds and the interrupt URB gets an 
> 	acknowledgment.
> 
> (2)	The control URB completes with an error.
> 
> (3)	The wait times out.
> 
> (4)	A disconnect occurs.
> 
> Your patch doesn't handle cases (1) and (3).  (And it doesn't get rid 
> of the dev->waitq field, which is no longer used.)
> 
> In fact, (1) is a little ambiguous.  When the interrupt URB gets a 
> command acknowledgment, there's no way (as far as I can tell) to know 
> which command was acknowledged -- particularly if a prior command URB 
> had to be cancelled because it timed out.
> 
> And as it turns out, the driver neglects to kill the command URB in
> case (3).  Furthermore, the driver doesn't have mutual exclusion for 
> writes.  So there's nothing to prevent the command URB from being 
> submitted while it is still active (syzbot's new crash).
> 
> I have to wonder if anybody's actually using this driver.  It seems to
> be pretty broken.  Maybe we should just mark it as such and forget
> about fixing it.
> 
> Alan Stern

Re: KASAN: use-after-free Read in iowarrior_disconnect

[Oliver Neukum]

Am Dienstag, den 20.08.2019, 10:18 -0400 schrieb Alan Stern:
> On Mon, 19 Aug 2019, Oliver Neukum wrote:
> 
> > Am Montag, den 19.08.2019, 07:48 -0700 schrieb syzbot:
> > > Hello,
> > > 
> > > syzbot found the following crash on:
> > > 
> > > HEAD commit:    d0847550 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=139be302600000
> > > kernel config:  https://syzkaller.appspot.com/x/.config?x=dbc9c80cc095da19
> > > dashboard link: https://syzkaller.appspot.com/bug?extid=cfe6d93e0abab9a0de05
> > > compiler:       gcc (GCC) 9.0.0 20181231 (experimental)
> > > syz repro:      https://syzkaller.appspot.com/x/repro.syz?x=12fe6b02600000
> > > C reproducer:   https://syzkaller.appspot.com/x/repro.c?x=1548189c600000
> > > 
> > > IMPORTANT: if you fix the bug, please add the following tag to the commit:
> > > Reported-by: syzbot+cfe6d93e0abab9a0de05@syzkaller.appspotmail.com
> > > 
> > 
> > #syz test: https://github.com/google/kasan.git d0847550
> 
> There's no need for us to work at cross purposes on this.  We can go 
> with your approach.
> 
> However, the code is more complicated than your patch accounts for.  
> The wait can finish in several different ways:
> 
> (1)	The control URB succeeds and the interrupt URB gets an 
> 	acknowledgment.
> 
> (2)	The control URB completes with an error.
> 
> (3)	The wait times out.
> 
> (4)	A disconnect occurs.

I absolutely agree. There is something quite wrong in this driver.
Unfortunately this is likely exploitable by a malicious gadget,
so just ignoring this is a bad option. I will need to go through the
logic. Or do you want to have a shot at it?

The patch was really only for testing. I wanted to know whether
I was hitting this very issue. This driver will need more surgery.

	Regards
		Oliver

Re: KASAN: use-after-free Read in iowarrior_disconnect

[Oliver Neukum]

Am Dienstag, den 20.08.2019, 16:38 +0200 schrieb Oliver Neukum:
> Am Dienstag, den 20.08.2019, 10:18 -0400 schrieb Alan Stern:
> > On Mon, 19 Aug 2019, Oliver Neukum wrote:
> > 
> > > Am Montag, den 19.08.2019, 07:48 -0700 schrieb syzbot:
> > > > Hello,
> > > > 
> > > > syzbot found the following crash on:
> > > > 
> > > > HEAD commit:    d0847550 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=139be302600000
> > > > kernel config:  https://syzkaller.appspot.com/x/.config?x=dbc9c80cc095da19
> > > > dashboard link: https://syzkaller.appspot.com/bug?extid=cfe6d93e0abab9a0de05
> > > > compiler:       gcc (GCC) 9.0.0 20181231 (experimental)
> > > > syz repro:      https://syzkaller.appspot.com/x/repro.syz?x=12fe6b02600000
> > > > C reproducer:   https://syzkaller.appspot.com/x/repro.c?x=1548189c600000
> > > > 
> > > > IMPORTANT: if you fix the bug, please add the following tag to the commit:
> > > > Reported-by: syzbot+cfe6d93e0abab9a0de05@syzkaller.appspotmail.com
> > > > 
> > > 
> > > #syz test: https://github.com/google/kasan.git d0847550
> > 
> > There's no need for us to work at cross purposes on this.  We can go 
> > with your approach.
> > 
> > However, the code is more complicated than your patch accounts for.  
> > The wait can finish in several different ways:
> > 
> > (1)	The control URB succeeds and the interrupt URB gets an 
> > 	acknowledgment.
> > 
> > (2)	The control URB completes with an error.
> > 
> > (3)	The wait times out.
> > 
> > (4)	A disconnect occurs.
> 
> I absolutely agree. There is something quite wrong in this driver.
> Unfortunately this is likely exploitable by a malicious gadget,
> so just ignoring this is a bad option. I will need to go through the
> logic. Or do you want to have a shot at it?
> 
> The patch was really only for testing. I wanted to know whether
> I was hitting this very issue. This driver will need more surgery.

PS: Referring to yurex

	Regards
		Oliver

Re: KASAN: use-after-free Read in iowarrior_disconnect

[Alan Stern]

On Tue, 20 Aug 2019, Oliver Neukum wrote:

> Am Dienstag, den 20.08.2019, 10:18 -0400 schrieb Alan Stern:
> > On Mon, 19 Aug 2019, Oliver Neukum wrote:
> > 
> > > Am Montag, den 19.08.2019, 07:48 -0700 schrieb syzbot:
> > > > Hello,
> > > > 
> > > > syzbot found the following crash on:
> > > > 
> > > > HEAD commit:    d0847550 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=139be302600000
> > > > kernel config:  https://syzkaller.appspot.com/x/.config?x=dbc9c80cc095da19
> > > > dashboard link: https://syzkaller.appspot.com/bug?extid=cfe6d93e0abab9a0de05
> > > > compiler:       gcc (GCC) 9.0.0 20181231 (experimental)
> > > > syz repro:      https://syzkaller.appspot.com/x/repro.syz?x=12fe6b02600000
> > > > C reproducer:   https://syzkaller.appspot.com/x/repro.c?x=1548189c600000
> > > > 
> > > > IMPORTANT: if you fix the bug, please add the following tag to the commit:
> > > > Reported-by: syzbot+cfe6d93e0abab9a0de05@syzkaller.appspotmail.com
> > > > 
> > > 
> > > #syz test: https://github.com/google/kasan.git d0847550
> > 
> > There's no need for us to work at cross purposes on this.  We can go 
> > with your approach.
> > 
> > However, the code is more complicated than your patch accounts for.  
> > The wait can finish in several different ways:
> > 
> > (1)	The control URB succeeds and the interrupt URB gets an 
> > 	acknowledgment.
> > 
> > (2)	The control URB completes with an error.
> > 
> > (3)	The wait times out.
> > 
> > (4)	A disconnect occurs.
> 
> I absolutely agree. There is something quite wrong in this driver.
> Unfortunately this is likely exploitable by a malicious gadget,
> so just ignoring this is a bad option. I will need to go through the
> logic. Or do you want to have a shot at it?
> 
> The patch was really only for testing. I wanted to know whether
> I was hitting this very issue. This driver will need more surgery.

If you would like to work on it, that's fine with me.

Alan Stern

Re: KASAN: use-after-free Read in iowarrior_disconnect

[Andrey Konovalov]

On Mon, Aug 19, 2019 at 4:48 PM syzbot
<syzbot+cfe6d93e0abab9a0de05@syzkaller.appspotmail.com> wrote:
>
> Hello,
>
> syzbot found the following crash on:
>
> HEAD commit:    d0847550 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=139be302600000
> kernel config:  https://syzkaller.appspot.com/x/.config?x=dbc9c80cc095da19
> dashboard link: https://syzkaller.appspot.com/bug?extid=cfe6d93e0abab9a0de05
> compiler:       gcc (GCC) 9.0.0 20181231 (experimental)
> syz repro:      https://syzkaller.appspot.com/x/repro.syz?x=12fe6b02600000
> C reproducer:   https://syzkaller.appspot.com/x/repro.c?x=1548189c600000
>
> IMPORTANT: if you fix the bug, please add the following tag to the commit:
> Reported-by: syzbot+cfe6d93e0abab9a0de05@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 ffff8881cc866f58 by task kworker/1:5/1755
>
> CPU: 1 PID: 1755 Comm: kworker/1:5 Not tainted 5.3.0-rc4+ #26
> 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+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
>   iowarrior_disconnect+0xf0/0x2c0 drivers/usb/misc/iowarrior.c:878
>   usb_unbind_interface+0x1bd/0x8a0 drivers/usb/core/driver.c:423
>   __device_release_driver drivers/base/dd.c:1134 [inline]
>   device_release_driver_internal+0x42f/0x500 drivers/base/dd.c:1165
>   bus_remove_device+0x2dc/0x4a0 drivers/base/bus.c:556
>   device_del+0x420/0xb10 drivers/base/core.c:2339
>   usb_disable_device+0x211/0x690 drivers/usb/core/message.c:1237
>   usb_disconnect+0x284/0x8d0 drivers/usb/core/hub.c:2199
>   hub_port_connect drivers/usb/core/hub.c:4949 [inline]
>   hub_port_connect_change drivers/usb/core/hub.c:5213 [inline]
>   port_event drivers/usb/core/hub.c:5359 [inline]
>   hub_event+0x1454/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
>
> Allocated by task 1755:
>   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]
>   iowarrior_probe+0x7a/0x10b2 drivers/usb/misc/iowarrior.c:753
>   usb_probe_interface+0x305/0x7a0 drivers/usb/core/driver.c:361
>   really_probe+0x281/0x6d0 drivers/base/dd.c:548
>   driver_probe_device+0x101/0x1b0 drivers/base/dd.c:721
>   __device_attach_driver+0x1c2/0x220 drivers/base/dd.c:828
>   bus_for_each_drv+0x162/0x1e0 drivers/base/bus.c:454
>   __device_attach+0x217/0x360 drivers/base/dd.c:894
>   bus_probe_device+0x1e4/0x290 drivers/base/bus.c:514
>   device_add+0xae6/0x16f0 drivers/base/core.c:2165
>   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/0x6d0 drivers/base/dd.c:548
>   driver_probe_device+0x101/0x1b0 drivers/base/dd.c:721
>   __device_attach_driver+0x1c2/0x220 drivers/base/dd.c:828
>   bus_for_each_drv+0x162/0x1e0 drivers/base/bus.c:454
>   __device_attach+0x217/0x360 drivers/base/dd.c:894
>   bus_probe_device+0x1e4/0x290 drivers/base/bus.c:514
>   device_add+0xae6/0x16f0 drivers/base/core.c:2165
>   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 1857:
>   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:1474 [inline]
>   slab_free mm/slub.c:3016 [inline]
>   kfree+0xe4/0x2f0 mm/slub.c:3957
>   iowarrior_delete drivers/usb/misc/iowarrior.c:246 [inline]
>   iowarrior_release+0x187/0x280 drivers/usb/misc/iowarrior.c:670
>   __fput+0x2d7/0x840 fs/file_table.c:280
>   task_work_run+0x13f/0x1c0 kernel/task_work.c:113
>   exit_task_work include/linux/task_work.h:22 [inline]
>   do_exit+0x8ef/0x2c00 kernel/exit.c:879
>   do_group_exit+0x125/0x340 kernel/exit.c:983
>   __do_sys_exit_group kernel/exit.c:994 [inline]
>   __se_sys_exit_group kernel/exit.c:992 [inline]
>   __x64_sys_exit_group+0x3a/0x50 kernel/exit.c:992
>   do_syscall_64+0xb7/0x580 arch/x86/entry/common.c:296
>   entry_SYSCALL_64_after_hwframe+0x49/0xbe
>
> The buggy address belongs to the object at ffff8881cc866f00
>   which belongs to the cache kmalloc-512 of size 512
> The buggy address is located 88 bytes inside of
>   512-byte region [ffff8881cc866f00, ffff8881cc867100)
> The buggy address belongs to the page:
> page:ffffea0007321980 refcount:1 mapcount:0 mapping:ffff8881da002500
> index:0x0 compound_mapcount: 0
> flags: 0x200000000010200(slab|head)
> raw: 0200000000010200 ffffea000733fc80 0000000200000002 ffff8881da002500
> raw: 0000000000000000 00000000000c000c 00000001ffffffff 0000000000000000
> page dumped because: kasan: bad access detected
>
> Memory state around the buggy address:
>   ffff8881cc866e00: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
>   ffff8881cc866e80: fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc
> > ffff8881cc866f00: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
>                                                      ^
>   ffff8881cc866f80: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
>   ffff8881cc867000: 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

#syz fix: USB: iowarrior: fix use-after-free on disconnect