WARNING in usbhid_raw_request/usb_submit_urb

[syzbot]

Hello,

syzbot found the following crash on:

HEAD commit: 6a3599ce 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=12386cb4600000
kernel config: https://syzkaller.appspot.com/x/.config?x=700ca426ab83faae
dashboard link: https://syzkaller.appspot.com/bug?extid=a7a6b9c609b9457c62c6
compiler: gcc (GCC) 9.0.0 20181231 (experimental)

Unfortunately, I don't have any reproducer for this crash yet.

IMPORTANT: if you fix the bug, please add the following tag to the commit:
Reported-by: syzbot+a7a6b9...@syzkaller.appspotmail.com

------------[ cut here ]------------
usb 2-1: BOGUS urb xfer, pipe 2 != type 2
WARNING: CPU: 0 PID: 3730 at drivers/usb/core/urb.c:477
usb_submit_urb+0x1188/0x13b0 drivers/usb/core/urb.c:477
Kernel panic - not syncing: panic_on_warn set ...
CPU: 0 PID: 3730 Comm: syz-executor.1 Not tainted 5.2.0-rc6+ #15
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
panic+0x292/0x6c9 kernel/panic.c:219
__warn.cold+0x20/0x4b kernel/panic.c:576
report_bug+0x262/0x2a0 lib/bug.c:186
fixup_bug arch/x86/kernel/traps.c:179 [inline]
fixup_bug arch/x86/kernel/traps.c:174 [inline]
do_error_trap+0x12b/0x1e0 arch/x86/kernel/traps.c:272
do_invalid_op+0x32/0x40 arch/x86/kernel/traps.c:291
invalid_op+0x14/0x20 arch/x86/entry/entry_64.S:986
RIP: 0010:usb_submit_urb+0x1188/0x13b0 drivers/usb/core/urb.c:477
Code: 4d 85 ed 74 2c e8 f8 d3 f4 fd 4c 89 f7 e8 a0 51 1c ff 41 89 d8 44 89
e1 4c 89 ea 48 89 c6 48 c7 c7 00 0e f7 85 e8 83 98 ca fd <0f> 0b e9 20 f4
ff ff e8 cc d3 f4 fd 4c 89 f2 48 b8 00 00 00 00 00
RSP: 0018:ffff8881d4f479d0 EFLAGS: 00010282
RAX: 0000000000000000 RBX: 0000000000000002 RCX: 0000000000000000
RDX: 0000000000005dfa RSI: ffffffff8127ef3d RDI: ffffed103a9e8f2c
RBP: 0000000000000000 R08: ffff8881af663000 R09: 0000000000000000
R10: 0000000000000000 R11: 0000000000000000 R12: 0000000000000002
R13: ffff8881d462ed38 R14: ffff8881d18f9a20 R15: ffff8881d80e1c00
usb_start_wait_urb+0x108/0x2b0 drivers/usb/core/message.c:57
usb_internal_control_msg drivers/usb/core/message.c:101 [inline]
usb_control_msg+0x31c/0x4a0 drivers/usb/core/message.c:152
usbhid_set_raw_report drivers/hid/usbhid/hid-core.c:917 [inline]
usbhid_raw_request+0x21f/0x640 drivers/hid/usbhid/hid-core.c:1265
hid_hw_raw_request include/linux/hid.h:1079 [inline]
hidraw_send_report+0x296/0x500 drivers/hid/hidraw.c:151
hidraw_ioctl+0x5b4/0xaf0 drivers/hid/hidraw.c:421
vfs_ioctl fs/ioctl.c:46 [inline]
file_ioctl fs/ioctl.c:509 [inline]
do_vfs_ioctl+0xcda/0x12e0 fs/ioctl.c:696
ksys_ioctl+0x9b/0xc0 fs/ioctl.c:713
__do_sys_ioctl fs/ioctl.c:720 [inline]
__se_sys_ioctl fs/ioctl.c:718 [inline]
__x64_sys_ioctl+0x6f/0xb0 fs/ioctl.c:718
do_syscall_64+0xb7/0x560 arch/x86/entry/common.c:301
entry_SYSCALL_64_after_hwframe+0x49/0xbe
RIP: 0033:0x459829
Code: fd b7 fb ff c3 66 2e 0f 1f 84 00 00 00 00 00 66 90 48 89 f8 48 89 f7
48 89 d6 48 89 ca 4d 89 c2 4d 89 c8 4c 8b 4c 24 08 0f 05 <48> 3d 01 f0 ff
ff 0f 83 cb b7 fb ff c3 66 2e 0f 1f 84 00 00 00 00
RSP: 002b:00007fe142c43c78 EFLAGS: 00000246 ORIG_RAX: 0000000000000010
RAX: ffffffffffffffda RBX: 0000000000000003 RCX: 0000000000459829
RDX: 0000000020000000 RSI: 00000000c0404806 RDI: 0000000000000004
RBP: 000000000075bfc8 R08: 0000000000000000 R09: 0000000000000000
R10: 0000000000000000 R11: 0000000000000246 R12: 00007fe142c446d4
R13: 00000000004c22ab R14: 00000000004d5630 R15: 00000000ffffffff
Kernel Offset: disabled
Rebooting in 86400 seconds..


---
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 syzk...@googlegroups.com.

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

[Alan Stern]

This is very strange. It looks like the kernel is complaining that 2
!= 2.

A more likely explanation is a race in the usbhid driver. If
usbhid_set_raw_report() gets called _after_ usbhid has been unbound
from the device and while the endpoint is being destroyed, we could get
something like this.

Perhaps one of Oliver's patches will also fix this.

Alan Stern

[Andrey Konovalov]

Since there's no reproducer this is quite likely some kind of race. We
can close this bug once Oliver's patches are applied, and if it gets
triggered again syzbot will rereport it.

>
> Alan Stern
>

[Oliver Neukum]

AFAICT my patch right now introduces another race. This will require
a fresh look.

Regards
Oliver

[syzbot]

syzbot has found a reproducer for the following crash on:

HEAD commit: 7f7867ff 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=10619cec600000
kernel config: https://syzkaller.appspot.com/x/.config?x=792eb47789f57810

dashboard link: https://syzkaller.appspot.com/bug?extid=a7a6b9c609b9457c62c6
compiler: gcc (GCC) 9.0.0 20181231 (experimental)

syz repro: https://syzkaller.appspot.com/x/repro.syz?x=10606c42600000

IMPORTANT: if you fix the bug, please add the following tag to the commit:
Reported-by: syzbot+a7a6b9...@syzkaller.appspotmail.com

------------[ cut here ]------------
usb 2-1: BOGUS urb xfer, pipe 2 != type 2

WARNING: CPU: 1 PID: 7429 at drivers/usb/core/urb.c:477

usb_submit_urb+0x1188/0x13b0 drivers/usb/core/urb.c:477
Kernel panic - not syncing: panic_on_warn set ...

CPU: 1 PID: 7429 Comm: syz-executor.1 Not tainted 5.3.0-rc2+ #23

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

panic+0x2a3/0x6da kernel/panic.c:219
__warn.cold+0x20/0x4a kernel/panic.c:576

report_bug+0x262/0x2a0 lib/bug.c:186
fixup_bug arch/x86/kernel/traps.c:179 [inline]
fixup_bug arch/x86/kernel/traps.c:174 [inline]
do_error_trap+0x12b/0x1e0 arch/x86/kernel/traps.c:272
do_invalid_op+0x32/0x40 arch/x86/kernel/traps.c:291

invalid_op+0x23/0x30 arch/x86/entry/entry_64.S:1026

RIP: 0010:usb_submit_urb+0x1188/0x13b0 drivers/usb/core/urb.c:477

Code: 4d 85 ed 74 2c e8 38 e8 ed fd 4c 89 f7 e8 70 dc 1a ff 41 89 d8 44 89
e1 4c 89 ea 48 89 c6 48 c7 c7 60 cc f8 85 e8 4d b9 c3 fd <0f> 0b e9 20 f4
ff ff e8 0c e8 ed fd 4c 89 f2 48 b8 00 00 00 00 00
RSP: 0018:ffff8881cef0f9d0 EFLAGS: 00010282

RAX: 0000000000000000 RBX: 0000000000000002 RCX: 0000000000000000

RDX: 0000000000000000 RSI: ffffffff812830fd RDI: ffffed1039de1f2c
RBP: 0000000000000000 R08: ffff8881c853e000 R09: fffffbfff115e1a2
R10: fffffbfff115e1a1 R11: ffffffff88af0d0f R12: 0000000000000002
R13: ffff8881d976b0a8 R14: ffff8881d0e02b20 R15: ffff8881d1720600

usb_start_wait_urb+0x108/0x2b0 drivers/usb/core/message.c:57
usb_internal_control_msg drivers/usb/core/message.c:101 [inline]
usb_control_msg+0x31c/0x4a0 drivers/usb/core/message.c:152
usbhid_set_raw_report drivers/hid/usbhid/hid-core.c:917 [inline]
usbhid_raw_request+0x21f/0x640 drivers/hid/usbhid/hid-core.c:1265
hid_hw_raw_request include/linux/hid.h:1079 [inline]
hidraw_send_report+0x296/0x500 drivers/hid/hidraw.c:151

hidraw_ioctl+0x5b4/0xae0 drivers/hid/hidraw.c:421

vfs_ioctl fs/ioctl.c:46 [inline]
file_ioctl fs/ioctl.c:509 [inline]

do_vfs_ioctl+0xd2d/0x1330 fs/ioctl.c:696

ksys_ioctl+0x9b/0xc0 fs/ioctl.c:713
__do_sys_ioctl fs/ioctl.c:720 [inline]
__se_sys_ioctl fs/ioctl.c:718 [inline]
__x64_sys_ioctl+0x6f/0xb0 fs/ioctl.c:718

do_syscall_64+0xb7/0x580 arch/x86/entry/common.c:296

entry_SYSCALL_64_after_hwframe+0x49/0xbe
RIP: 0033:0x459829
Code: fd b7 fb ff c3 66 2e 0f 1f 84 00 00 00 00 00 66 90 48 89 f8 48 89 f7
48 89 d6 48 89 ca 4d 89 c2 4d 89 c8 4c 8b 4c 24 08 0f 05 <48> 3d 01 f0 ff
ff 0f 83 cb b7 fb ff c3 66 2e 0f 1f 84 00 00 00 00

RSP: 002b:00007f6a91f44c78 EFLAGS: 00000246 ORIG_RAX: 0000000000000010

RAX: ffffffffffffffda RBX: 0000000000000003 RCX: 0000000000459829

RDX: 0000000020000240 RSI: 00000000c0404806 RDI: 0000000000000004
RBP: 000000000075bf20 R08: 0000000000000000 R09: 0000000000000000
R10: 0000000000000000 R11: 0000000000000246 R12: 00007f6a91f456d4
R13: 00000000004c22c3 R14: 00000000004d5688 R15: 00000000ffffffff

[Oliver Neukum]

Hi,

this makes relatively little sense. This goes to endpoint 0.
This must be a control endpoint, or the system should have
triggered the warning during enumeration. Do you trigger the
panic_on_warn on the fly in your kernel?

Also, could you include "lsusb -v" in the USB fuzzer bugs?

Regards
Oliver

[Alan Stern]

It's probably caused by a race with disconnect. By the time
usb_submit_urb() performs the endpoint type check, the endpoint data
has been wiped out or overwritten.

At least, that's the only reason I can imagine for the code to
complain that 2 != 2. (Note that the same warning gets printed,
misleading though it may be, if the endpoint structure is deallocated
while usb_submit_urb is running.)

> Do you trigger the
> panic_on_warn on the fly in your kernel?
>
> Also, could you include "lsusb -v" in the USB fuzzer bugs?

Or the contents of /sys/kernel/debug/usb/devices, which might be easier
to read. However I'm not sure it would help in this case; the device
is attached only for a very brief time. After all, the bug occurs
because probe races with disconnect.

Alan Stern

[Andrey Konovalov]

Yeah, unfortunately this is a race, so the reproducers connects some
USB devices in a loop, so even if we collect lsusb of some other debug
info, it's not clear at which point in time to do that.

One way to debug this is to write a kernel patch that prints some
debugging info and then ask syzbot to test it. However, note that
adding printk's might affect the timings and the race might become
harder to reproduce.

>
> Alan Stern
>

[Andrey Konovalov]

On Tue, Jul 30, 2019 at 6:58 PM syzbot
<syzbot+a7a6b9...@syzkaller.appspotmail.com> wrote:
>
> syzbot has found a reproducer for the following crash on:
>
> HEAD commit: 7f7867ff 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=10619cec600000
> kernel config: https://syzkaller.appspot.com/x/.config?x=792eb47789f57810
> dashboard link: https://syzkaller.appspot.com/bug?extid=a7a6b9c609b9457c62c6
> compiler: gcc (GCC) 9.0.0 20181231 (experimental)
> syz repro: https://syzkaller.appspot.com/x/repro.syz?x=10606c42600000
>
> IMPORTANT: if you fix the bug, please add the following tag to the commit:
> Reported-by: syzbot+a7a6b9...@syzkaller.appspotmail.com

Let's try Hillf's patch here as well:

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

[syzbot]

Hello,

syzbot has tested the proposed patch but the reproducer still triggered
crash:
KASAN: invalid-free in hcd_buffer_free

usb 5-1: USB disconnect, device number 2
==================================================================
BUG: KASAN: double-free or invalid-free in hcd_buffer_free+0x199/0x260
drivers/usb/core/buffer.c:165

CPU: 0 PID: 1745 Comm: kworker/0:2 Not tainted 5.3.0-rc2+ #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

print_address_description+0x6a/0x32c mm/kasan/report.c:351
kasan_report_invalid_free+0x61/0xa0 mm/kasan/report.c:444
__kasan_slab_free+0x162/0x180 mm/kasan/common.c:428
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
hcd_buffer_free+0x199/0x260 drivers/usb/core/buffer.c:165
usb_free_coherent+0x67/0x80 drivers/usb/core/usb.c:932
hid_free_buffers.isra.0+0x94/0x290 drivers/hid/usbhid/hid-core.c:964
usbhid_stop+0x308/0x450 drivers/hid/usbhid/hid-core.c:1224
logi_dj_remove+0x107/0x210 drivers/hid/hid-logitech-dj.c:1797
hid_device_remove+0xed/0x240 drivers/hid/hid-core.c:2242
__device_release_driver drivers/base/dd.c:1118 [inline]
device_release_driver_internal+0x206/0x4c0 drivers/base/dd.c:1151
bus_remove_device+0x2dc/0x4a0 drivers/base/bus.c:556
device_del+0x420/0xb10 drivers/base/core.c:2288
hid_remove_device drivers/hid/hid-core.c:2413 [inline]
hid_destroy_device+0xe1/0x150 drivers/hid/hid-core.c:2432
usbhid_disconnect+0xad/0xd0 drivers/hid/usbhid/hid-core.c:1414
usb_unbind_interface+0x1bd/0x8a0 drivers/usb/core/driver.c:423
__device_release_driver drivers/base/dd.c:1120 [inline]
device_release_driver_internal+0x404/0x4c0 drivers/base/dd.c:1151
bus_remove_device+0x2dc/0x4a0 drivers/base/bus.c:556
device_del+0x420/0xb10 drivers/base/core.c:2288
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
process_scheduled_works kernel/workqueue.c:2331 [inline]
worker_thread+0x7ab/0xe20 kernel/workqueue.c:2417
kthread+0x318/0x420 kernel/kthread.c:255
ret_from_fork+0x24/0x30 arch/x86/entry/entry_64.S:352

Allocated by task 1745:
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:557 [inline]
hcd_buffer_alloc+0x1ca/0x290 drivers/usb/core/buffer.c:135
usb_alloc_coherent+0x5d/0x80 drivers/usb/core/usb.c:910
hid_alloc_buffers drivers/hid/usbhid/hid-core.c:846 [inline]
usbhid_start+0x60b/0x22f0 drivers/hid/usbhid/hid-core.c:1075
hid_hw_start+0x5d/0x130 drivers/hid/hid-core.c:1976
logi_dj_probe+0x808/0xcd7 drivers/hid/hid-logitech-dj.c:1703
hid_device_probe+0x2be/0x3f0 drivers/hid/hid-core.c:2209
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
hid_add_device+0x33c/0x990 drivers/hid/hid-core.c:2365
usbhid_probe+0xa81/0xfa0 drivers/hid/usbhid/hid-core.c:1386
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 1745:
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
hcd_buffer_free+0x199/0x260 drivers/usb/core/buffer.c:165
usb_free_coherent+0x67/0x80 drivers/usb/core/usb.c:932
hid_free_buffers.isra.0+0x94/0x290 drivers/hid/usbhid/hid-core.c:964
usbhid_stop+0x308/0x450 drivers/hid/usbhid/hid-core.c:1224
usbhid_disconnect+0xa5/0xd0 drivers/hid/usbhid/hid-core.c:1413
usb_unbind_interface+0x1bd/0x8a0 drivers/usb/core/driver.c:423
__device_release_driver drivers/base/dd.c:1120 [inline]
device_release_driver_internal+0x404/0x4c0 drivers/base/dd.c:1151
bus_remove_device+0x2dc/0x4a0 drivers/base/bus.c:556
device_del+0x420/0xb10 drivers/base/core.c:2288
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
process_scheduled_works kernel/workqueue.c:2331 [inline]
worker_thread+0x7ab/0xe20 kernel/workqueue.c:2417
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 ffff8881d5875500
which belongs to the cache kmalloc-4k of size 4096
The buggy address is located 0 bytes inside of
4096-byte region [ffff8881d5875500, ffff8881d5876500)
The buggy address belongs to the page:
page:ffffea0007561c00 refcount:1 mapcount:0 mapping:ffff8881da00c280
index:0x0 compound_mapcount: 0
flags: 0x200000000010200(slab|head)
raw: 0200000000010200 dead000000000100 dead000000000122 ffff8881da00c280
raw: 0000000000000000 0000000000070007 00000001ffffffff 0000000000000000
page dumped because: kasan: bad access detected

Memory state around the buggy address:
ffff8881d5875400: fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc
ffff8881d5875480: fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc
> ffff8881d5875500: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
^
ffff8881d5875580: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
ffff8881d5875600: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
==================================================================


Tested on:

commit: 7f7867ff 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=13833b9a600000
kernel config: https://syzkaller.appspot.com/x/.config?x=792eb47789f57810

compiler: gcc (GCC) 9.0.0 20181231 (experimental)

patch: https://syzkaller.appspot.com/x/patch.diff?x=167d2a0e600000

[Andrey Konovalov]

On Mon, Aug 12, 2019 at 3:03 PM syzbot
<syzbot+a7a6b9...@syzkaller.appspotmail.com> wrote:
>
> Hello,
>
> syzbot has tested the proposed patch but the reproducer still triggered
> crash:
> KASAN: invalid-free in hcd_buffer_free
>
> usb 5-1: USB disconnect, device number 2
> ==================================================================
> BUG: KASAN: double-free or invalid-free in hcd_buffer_free+0x199/0x260
> drivers/usb/core/buffer.c:165

Hm, looks like a different bug...

[Hillf Danton]

Hi Andrey

On Mon, 12 Aug 2019 06:03:01 -0700

> Hello,
>
> syzbot has tested the proposed patch but the reproducer still triggered
> crash:
> KASAN: invalid-free in hcd_buffer_free
>
> usb 5-1: USB disconnect, device number 2
> ==================================================================
> BUG: KASAN: double-free or invalid-free in hcd_buffer_free+0x199/0x260
> drivers/usb/core/buffer.c:165

JFYI:
1, the hid-core.patch in the attachment at
https://lore.kernel.org/lkml/CAAeHK+z-uCr-bWu9uVDynU2S=wCrtxRbuA-Cut=h5zYu...@mail.gmail.com/

===quote begin===

> IMPORTANT: if you fix the bug, please add the following tag to the commit:
> Reported-by: syzbot+a7a6b9...@syzkaller.appspotmail.com

Let's try Hillf's patch here as well:

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

>
> ------------[ cut here ]------------
> usb 2-1: BOGUS urb xfer, pipe 2 != type 2
> WARNING: CPU: 1 PID: 7429 at drivers/usb/core/urb.c:477
> usb_submit_urb+0x1188/0x13b0 drivers/usb/core/urb.c:477

===quote end===

is identical to the patch for
#syz test: https://github.com/google/kasan.git 6a3599ce
Reported-by: syzbot+3cbe5c...@syzkaller.appspotmail.com

--- a/drivers/hid/usbhid/hid-core.c
+++ b/drivers/hid/usbhid/hid-core.c
@@ -1410,6 +1410,7 @@ static void usbhid_disconnect(struct usb
spin_lock_irq(&usbhid->lock); /* Sync with error and led handlers */
set_bit(HID_DISCONNECTED, &usbhid->iofl);
spin_unlock_irq(&usbhid->lock);
+ hid_hw_stop(hid);
hid_destroy_device(hid);
kfree(usbhid);
}


2, based on the report itself in the quote section above,

> usb 2-1: BOGUS urb xfer, pipe 2 != type 2

I suggest that we get it invalidated for now. If unfortunately it comes
up again we know at least what once happened.

3, this is a new report I did not see before, and worth a new report
thread with a new subject line. See it soon after 7 hours of sleep and
prepare patch with Test-by.

Thanks
Hillf

>
> CPU: 0 PID: 1745 Comm: kworker/0:2 Not tainted 5.3.0-rc2+ #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

> commit: 7f7867ff 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=13833b9a600000
> kernel config: https://syzkaller.appspot.com/x/.config?x=792eb47789f57810

> compiler: gcc (GCC) 9.0.0 20181231 (experimental)

> patch: https://syzkaller.appspot.com/x/patch.diff?x=167d2a0e600000

[Andrey Konovalov]

On Mon, Aug 12, 2019 at 4:47 PM Hillf Danton <hda...@sina.com> wrote:
>
>
> Hi Andrey
>
> On Mon, 12 Aug 2019 06:03:01 -0700
> > Hello,
> >
> > syzbot has tested the proposed patch but the reproducer still triggered
> > crash:
> > KASAN: invalid-free in hcd_buffer_free
> >
> > usb 5-1: USB disconnect, device number 2
> > ==================================================================
> > BUG: KASAN: double-free or invalid-free in hcd_buffer_free+0x199/0x260
> > drivers/usb/core/buffer.c:165
>
> JFYI:
> 1, the hid-core.patch in the attachment at
> https://lore.kernel.org/lkml/CAAeHK+z-uCr-bWu9uVDynU2S=wCrtxRbuA-Cut=h5zYu...@mail.gmail.com/
>
> ===quote begin===
> > IMPORTANT: if you fix the bug, please add the following tag to the commit:
> > Reported-by: syzbot+a7a6b9...@syzkaller.appspotmail.com
>
> Let's try Hillf's patch here as well:
>
> #syz test: https://github.com/google/kasan.git 7f7867ff

Remove # when quoting syzbot commands, as this will trigger syzbot
testing again =)

>
> >
> > ------------[ cut here ]------------
> > usb 2-1: BOGUS urb xfer, pipe 2 != type 2
> > WARNING: CPU: 1 PID: 7429 at drivers/usb/core/urb.c:477
> > usb_submit_urb+0x1188/0x13b0 drivers/usb/core/urb.c:477
> ===quote end===
>
> is identical to the patch for
> #syz test: https://github.com/google/kasan.git 6a3599ce
> Reported-by: syzbot+3cbe5c...@syzkaller.appspotmail.com
>
> --- a/drivers/hid/usbhid/hid-core.c
> +++ b/drivers/hid/usbhid/hid-core.c
> @@ -1410,6 +1410,7 @@ static void usbhid_disconnect(struct usb
> spin_lock_irq(&usbhid->lock); /* Sync with error and led handlers */
> set_bit(HID_DISCONNECTED, &usbhid->iofl);
> spin_unlock_irq(&usbhid->lock);
> + hid_hw_stop(hid);
> hid_destroy_device(hid);
> kfree(usbhid);
> }
>
>
> 2, based on the report itself in the quote section above,
>
> > usb 2-1: BOGUS urb xfer, pipe 2 != type 2
>
> I suggest that we get it invalidated for now. If unfortunately it comes
> up again we know at least what once happened.
>
> 3, this is a new report I did not see before, and worth a new report
> thread with a new subject line. See it soon after 7 hours of sleep and
> prepare patch with Test-by.

Let's dup it into the PM report:

#syz dup: general protection fault in __pm_runtime_resume

Once the fix is in the USB fuzzer tree, the double-free will be
reported in a separate thread (once it gets hit again).

[syzbot]

Hello,

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

Reported-and-tested-by:
syzbot+3cbe5c...@syzkaller.appspotmail.com

Tested on:

commit: 7f7867ff usb-fuzzer: main usb gadget fuzzer driver
git tree: https://github.com/google/kasan.git

kernel config: https://syzkaller.appspot.com/x/.config?x=792eb47789f57810
compiler: gcc (GCC) 9.0.0 20181231 (experimental)

patch: https://syzkaller.appspot.com/x/patch.diff?x=177252d2600000

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

[Hillf Danton]

[respin with the mess in Cc list cleaned up]

On Mon, 12 Aug 2019 06:03:01 -0700

> Hello,
>

> syzbot has tested the proposed patch but the reproducer still triggered crash:
> KASAN: invalid-free in hcd_buffer_free
>
> usb 5-1: USB disconnect, device number 2
> ==================================================================
> BUG: KASAN: double-free or invalid-free in hcd_buffer_free+0x199/0x260
> drivers/usb/core/buffer.c:165
>

> CPU: 0 PID: 1745 Comm: kworker/0:2 Not tainted 5.3.0-rc2+ #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

> commit: 7f7867ff 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=13833b9a600000
> kernel config: https://syzkaller.appspot.com/x/.config?x=792eb47789f57810

> compiler: gcc (GCC) 9.0.0 20181231 (experimental)

> patch: https://syzkaller.appspot.com/x/patch.diff?x=167d2a0e600000

Followup of commit e3e14de50dff ("HID: fix start/stop cycle in usbhid driver")

--- a/drivers/hid/usbhid/hid-core.c
+++ b/drivers/hid/usbhid/hid-core.c
@@ -1214,6 +1214,8 @@ static void usbhid_stop(struct hid_devic

hid->claimed = 0;

+ if (!usbhid->urbin) /* freeing buffers only once */
+ return;
usb_free_urb(usbhid->urbin);
usb_free_urb(usbhid->urbctrl);
usb_free_urb(usbhid->urbout);
--

[Dmitry Vyukov]

I have not read all the code, but the last report was a double-free,
which suggests that usbhid->urbin is actually not reset to NULL, so
this check won't help. Potentially we need both this check and reset
the fields to NULL after freeing.

[Oliver Neukum]

Am Dienstag, den 13.08.2019, 12:26 +0800 schrieb Hillf Danton:
> [respin with the mess in Cc list cleaned up]

> Followup of commit e3e14de50dff ("HID: fix start/stop cycle in usbhid driver")
>
> --- a/drivers/hid/usbhid/hid-core.c
> +++ b/drivers/hid/usbhid/hid-core.c
> @@ -1214,6 +1214,8 @@ static void usbhid_stop(struct hid_devic
>
> hid->claimed = 0;
>
> + if (!usbhid->urbin) /* freeing buffers only once */
> + return;
> usb_free_urb(usbhid->urbin);
> usb_free_urb(usbhid->urbctrl);
> usb_free_urb(usbhid->urbout);

This looks rather suspicious. Why is stop() called multiple times?
Do we have a refcounting issue? If not, what controls locking?

Regards
Oliver

[Hillf Danton]

Hi Oliver and Dmitry

On Tue, 13 Aug 2019 16:15:02 +0800 Oliver Neukum wrote:
>
> Am Dienstag, den 13.08.2019, 12:26 +0800 schrieb Hillf Danton:
> > [respin with the mess in Cc list cleaned up]
>
> > Followup of commit e3e14de50dff ("HID: fix start/stop cycle in usbhid driver")
> >
> > --- a/drivers/hid/usbhid/hid-core.c
> > +++ b/drivers/hid/usbhid/hid-core.c
> > @@ -1214,6 +1214,8 @@ static void usbhid_stop(struct hid_devic
> >
> > hid->claimed = 0;
> >
> > + if (!usbhid->urbin) /* freeing buffers only once */
> > + return;
> > usb_free_urb(usbhid->urbin);
> > usb_free_urb(usbhid->urbctrl);
> > usb_free_urb(usbhid->urbout);
>
> This looks rather suspicious.

You are right, Oliver.
It is a simple fence added for blocking double free.

> Why is stop() called multiple times?

Good question really.

And the answer can be found in the diff below as disconnecting hid
is what's needed, and no more, before destroying it IMO.
Currently it is the only way for ll_driver to info hidraw what's
happening by calling hidraw_disconnect().
Otherwise no disconnect no exist reset. There are a few sites where
exist is checked.

And s/hid_hw_stop/hid_disconnect/ can do the work.

Thanks
Hillf

--- a/drivers/hid/usbhid/hid-core.c
+++ b/drivers/hid/usbhid/hid-core.c

@@ -1410,6 +1410,7 @@ static void usbhid_disconnect(struct usb
spin_lock_irq(&usbhid->lock); /* Sync with error and led handlers */
set_bit(HID_DISCONNECTED, &usbhid->iofl);
spin_unlock_irq(&usbhid->lock);
+ hid_hw_stop(hid);
hid_destroy_device(hid);
kfree(usbhid);
}

--

[Alan Stern]

On Mon, 12 Aug 2019, syzbot wrote:

> Hello,
>
> syzbot has tested the proposed patch but the reproducer still triggered
> crash:
> KASAN: invalid-free in hcd_buffer_free

This bug report shows that Hillf's fix isn't exactly right.

> usb 5-1: USB disconnect, device number 2
> ==================================================================
> BUG: KASAN: double-free or invalid-free in hcd_buffer_free+0x199/0x260
> drivers/usb/core/buffer.c:165
>
> CPU: 0 PID: 1745 Comm: kworker/0:2 Not tainted 5.3.0-rc2+ #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
> print_address_description+0x6a/0x32c mm/kasan/report.c:351
> kasan_report_invalid_free+0x61/0xa0 mm/kasan/report.c:444
> __kasan_slab_free+0x162/0x180 mm/kasan/common.c:428
> 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
> hcd_buffer_free+0x199/0x260 drivers/usb/core/buffer.c:165
> usb_free_coherent+0x67/0x80 drivers/usb/core/usb.c:932
> hid_free_buffers.isra.0+0x94/0x290 drivers/hid/usbhid/hid-core.c:964
> usbhid_stop+0x308/0x450 drivers/hid/usbhid/hid-core.c:1224
> logi_dj_remove+0x107/0x210 drivers/hid/hid-logitech-dj.c:1797

Here the double-free occurred when logi_dj_remove() called
hd_hw_stop()...

> hid_device_remove+0xed/0x240 drivers/hid/hid-core.c:2242
> __device_release_driver drivers/base/dd.c:1118 [inline]
> device_release_driver_internal+0x206/0x4c0 drivers/base/dd.c:1151
> bus_remove_device+0x2dc/0x4a0 drivers/base/bus.c:556
> device_del+0x420/0xb10 drivers/base/core.c:2288
> hid_remove_device drivers/hid/hid-core.c:2413 [inline]
> hid_destroy_device+0xe1/0x150 drivers/hid/hid-core.c:2432
> usbhid_disconnect+0xad/0xd0 drivers/hid/usbhid/hid-core.c:1414

which occurred inside usbhid_disconnect()'s call to
hid_destroy_device().

But just above the call to hid_destroy_device(), Hillf's patch adds a
direct call to hid_hw_stop(), which is what did the original free.

So it looks like the problem here is that some paths in the original
unpatched code end up calling hid_hw_stop() by way of the hid_device's
driver, and other paths do not.

I haven't had time to track down this difference. Maybe somebody
on the mailing list already knows why it occurs.

Alan Stern

[syzbot]

Hello,

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

WARNING: ODEBUG bug in __free_pages_ok

------------[ cut here ]------------
ODEBUG: free active (active state 0) object type: timer_list hint:
hid_retry_timeout+0x0/0xd0 drivers/hid/usbhid/hid-core.c:716
WARNING: CPU: 1 PID: 22 at lib/debugobjects.c:325
debug_print_object+0x160/0x250 lib/debugobjects.c:325

Kernel panic - not syncing: panic_on_warn set ...

CPU: 1 PID: 22 Comm: kworker/1:1 Not tainted 5.2.0-rc6+ #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

panic+0x292/0x6c9 kernel/panic.c:219
__warn.cold+0x20/0x4b kernel/panic.c:576

report_bug+0x262/0x2a0 lib/bug.c:186
fixup_bug arch/x86/kernel/traps.c:179 [inline]
fixup_bug arch/x86/kernel/traps.c:174 [inline]
do_error_trap+0x12b/0x1e0 arch/x86/kernel/traps.c:272
do_invalid_op+0x32/0x40 arch/x86/kernel/traps.c:291

invalid_op+0x14/0x20 arch/x86/entry/entry_64.S:986
RIP: 0010:debug_print_object+0x160/0x250 lib/debugobjects.c:325
Code: dd a0 16 ba 85 48 89 fa 48 c1 ea 03 80 3c 02 00 0f 85 bf 00 00 00 48
8b 14 dd a0 16 ba 85 48 c7 c7 80 0c ba 85 e8 db c7 33 ff <0f> 0b 83 05 83
6e 86 05 01 48 83 c4 20 5b 5d 41 5c 41 5d c3 48 89
RSP: 0018:ffff8881d9f8f710 EFLAGS: 00010086
RAX: 0000000000000000 RBX: 0000000000000003 RCX: 0000000000000000
RDX: 0000000000000000 RSI: ffffffff8127ef3d RDI: ffffed103b3f1ed4
RBP: 0000000000000001 R08: ffff8881d9f80000 R09: ffffed103b663ed7
R10: ffffed103b663ed6 R11: ffff8881db31f6b7 R12: ffffffff86b04760
R13: ffffffff812db3c0 R14: ffffffff88f07d68 R15: ffff8881d56da8c8
__debug_check_no_obj_freed lib/debugobjects.c:785 [inline]
debug_check_no_obj_freed+0x2a3/0x42e lib/debugobjects.c:817
free_pages_prepare mm/page_alloc.c:1140 [inline]
__free_pages_ok+0x215/0x1bb0 mm/page_alloc.c:1366
usbhid_disconnect+0x98/0xd0 drivers/hid/usbhid/hid-core.c:1414
usb_unbind_interface+0x1bd/0x8a0 drivers/usb/core/driver.c:423
__device_release_driver drivers/base/dd.c:1081 [inline]
device_release_driver_internal+0x404/0x4c0 drivers/base/dd.c:1112
bus_remove_device+0x2dc/0x4a0 drivers/base/bus.c:556
device_del+0x460/0xb80 drivers/base/core.c:2274
usb_disable_device+0x211/0x690 drivers/usb/core/message.c:1237
usb_disconnect+0x284/0x830 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+0x13bd/0x3550 drivers/usb/core/hub.c:5441
process_one_work+0x905/0x1570 kernel/workqueue.c:2269

process_scheduled_works kernel/workqueue.c:2331 [inline]
worker_thread+0x7ab/0xe20 kernel/workqueue.c:2417

kthread+0x30b/0x410 kernel/kthread.c:255
ret_from_fork+0x24/0x30 arch/x86/entry/entry_64.S:352

======================================================


Tested on:

commit: 6a3599ce 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=13a615ba600000
kernel config: https://syzkaller.appspot.com/x/.config?x=700ca426ab83faae

compiler: gcc (GCC) 9.0.0 20181231 (experimental)

patch: https://syzkaller.appspot.com/x/patch.diff?x=12a74d3c600000

[Alan Stern]

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

drivers/hid/hid-core.c | 3 +++
drivers/hid/hid-lg.c | 12 +++++++-----
drivers/hid/hid-lg4ff.c | 1 -
3 files changed, 10 insertions(+), 6 deletions(-)

Index: usb-devel/drivers/hid/hid-core.c
===================================================================
--- usb-devel.orig/drivers/hid/hid-core.c
+++ usb-devel/drivers/hid/hid-core.c
@@ -1973,6 +1973,8 @@ int hid_hw_start(struct hid_device *hdev
{
int error;

+ dev_info(&hdev->dev, "In hid_hw_start for %p\n", hdev);
+ dump_stack();
error = hdev->ll_driver->start(hdev);
if (error)
return error;
@@ -1998,6 +2000,7 @@ EXPORT_SYMBOL_GPL(hid_hw_start);
*/
void hid_hw_stop(struct hid_device *hdev)
{
+ dev_info(&hdev->dev, "In hid_hw_stop for %p\n", hdev);
hid_disconnect(hdev);
hdev->ll_driver->stop(hdev);
}
Index: usb-devel/drivers/hid/hid-lg.c
===================================================================
--- usb-devel.orig/drivers/hid/hid-lg.c
+++ usb-devel/drivers/hid/hid-lg.c
@@ -806,7 +806,7 @@ static int lg_probe(struct hid_device *h
ret = hid_hw_start(hdev, connect_mask);
if (ret) {
hid_err(hdev, "hw start failed\n");
- goto err_free;
+ goto err_stop;
}

/* Setup wireless link with Logitech Wii wheel */
@@ -818,7 +818,7 @@ static int lg_probe(struct hid_device *h

if (!buf) {
ret = -ENOMEM;
- goto err_free;
+ goto err_stop;
}

ret = hid_hw_raw_request(hdev, buf[0], buf, sizeof(cbuf),
@@ -850,9 +850,12 @@ static int lg_probe(struct hid_device *h
ret = lg4ff_init(hdev);

if (ret)
- goto err_free;
+ goto err_stop;

return 0;
+
+err_stop:
+ hid_hw_stop(hdev);
err_free:
kfree(drv_data);
return ret;
@@ -863,8 +866,7 @@ static void lg_remove(struct hid_device
struct lg_drv_data *drv_data = hid_get_drvdata(hdev);
if (drv_data->quirks & LG_FF4)
lg4ff_deinit(hdev);
- else
- hid_hw_stop(hdev);
+ hid_hw_stop(hdev);
kfree(drv_data);
}

Index: usb-devel/drivers/hid/hid-lg4ff.c
===================================================================
--- usb-devel.orig/drivers/hid/hid-lg4ff.c
+++ usb-devel/drivers/hid/hid-lg4ff.c
@@ -1477,7 +1477,6 @@ int lg4ff_deinit(struct hid_device *hid)
}
}
#endif
- hid_hw_stop(hid);
drv_data->device_props = NULL;

kfree(entry);

[syzbot]

Hello,

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

Reported-and-tested-by:
syzbot+3cbe5c...@syzkaller.appspotmail.com

Tested on:

commit: 6a3599ce usb-fuzzer: main usb gadget fuzzer driver
git tree: https://github.com/google/kasan.git

kernel config: https://syzkaller.appspot.com/x/.config?x=700ca426ab83faae
compiler: gcc (GCC) 9.0.0 20181231 (experimental)

patch: https://syzkaller.appspot.com/x/patch.diff?x=170b66a6600000

[Alan Stern]

On Mon, 12 Aug 2019, syzbot wrote:

That was the result from testing Hillf's patch:

--- a/drivers/hid/usbhid/hid-core.c
+++ b/drivers/hid/usbhid/hid-core.c
@@ -1410,6 +1410,7 @@ static void usbhid_disconnect(struct usb
spin_lock_irq(&usbhid->lock); /* Sync with error and led handlers */
set_bit(HID_DISCONNECTED, &usbhid->iofl);
spin_unlock_irq(&usbhid->lock);
+ hid_hw_stop(hid);
hid_destroy_device(hid);
kfree(usbhid);
}

There is very good reason to believe this patch is not the correct
solution to the problem. For one thing, in some circumstances the
patch ends up calling hid_hw_stop() twice (not shown here, but we have
seen this in other bug reports from syzbot).

For another, I have just tested a different patch and found that it
also prevents this particular crash:

> Hello,
>
> syzbot has tested the proposed patch and the reproducer did not trigger
> crash:
>
> Reported-and-tested-by:
> syzbot+3cbe5c...@syzkaller.appspotmail.com
>
> Tested on:
>

> commit: 6a3599ce usb-fuzzer: main usb gadget fuzzer driver
> git tree: https://github.com/google/kasan.git
> kernel config: https://syzkaller.appspot.com/x/.config?x=700ca426ab83faae

> compiler: gcc (GCC) 9.0.0 20181231 (experimental)

> patch: https://syzkaller.appspot.com/x/patch.diff?x=170b66a6600000

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

My patch:

Index: usb-devel/drivers/hid/hid-lg.c
===================================================================
--- usb-devel.orig/drivers/hid/hid-lg.c
+++ usb-devel/drivers/hid/hid-lg.c

This fixes a fairly obvious bug in the hid-lg driver: It does not
always call hid_hw_stop() in all pathways after calling hid_hw_start().

Presumably the same is true for the other related bugs found by syzbot.
I'm doing some more testing and we will see...

Alan Stern

[syzbot]

Hello,

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

Reported-and-tested-by:

syzbot+a7a6b9...@syzkaller.appspotmail.com

Tested on:

commit: 7f7867ff usb-fuzzer: main usb gadget fuzzer driver
git tree: https://github.com/google/kasan.git
kernel config: https://syzkaller.appspot.com/x/.config?x=792eb47789f57810
compiler: gcc (GCC) 9.0.0 20181231 (experimental)

patch: https://syzkaller.appspot.com/x/patch.diff?x=17d74372600000

[Alan Stern]

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

[syzbot]

Hello,

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

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

Tested on:

commit: 6a3599ce usb-fuzzer: main usb gadget fuzzer driver
git tree: https://github.com/google/kasan.git
kernel config: https://syzkaller.appspot.com/x/.config?x=e9e71c41d2fd1aa9

compiler: gcc (GCC) 9.0.0 20181231 (experimental)

patch: https://syzkaller.appspot.com/x/patch.diff?x=16298b72600000

[Alan Stern]

I want to make sure that the patch itself is responsible for fixing the
problem, and not the extra debugging it added for testing.

Alan Stern

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

[syzbot]

Hello,

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

Reported-and-tested-by:

syzbot+3cbe5c...@syzkaller.appspotmail.com

Tested on:

commit: 7f7867ff usb-fuzzer: main usb gadget fuzzer driver
git tree: https://github.com/google/kasan.git
kernel config: https://syzkaller.appspot.com/x/.config?x=792eb47789f57810

compiler: gcc (GCC) 9.0.0 20181231 (experimental)

patch: https://syzkaller.appspot.com/x/patch.diff?x=148edbe2600000

[Alan Stern]

The syzbot fuzzer found a general protection fault in the HID subsystem:

kasan: CONFIG_KASAN_INLINE enabled
kasan: GPF could be caused by NULL-ptr deref or user memory access
general protection fault: 0000 [#1] SMP KASAN
CPU: 0 PID: 3715 Comm: syz-executor.3 Not tainted 5.2.0-rc6+ #15

Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS
Google 01/01/2011

RIP: 0010:__pm_runtime_resume+0x49/0x180 drivers/base/power/runtime.c:1069
Code: ed 74 d5 fe 45 85 ed 0f 85 9a 00 00 00 e8 6f 73 d5 fe 48 8d bd c1 02
00 00 48 b8 00 00 00 00 00 fc ff df 48 89 fa 48 c1 ea 03 <0f> b6 04 02 48
89 fa 83 e2 07 38 d0 7f 08 84 c0 0f 85 fe 00 00 00
RSP: 0018:ffff8881d99d78e0 EFLAGS: 00010202
RAX: dffffc0000000000 RBX: 0000000000000020 RCX: ffffc90003f3f000
RDX: 0000000416d8686d RSI: ffffffff82676841 RDI: 00000020b6c3436a
RBP: 00000020b6c340a9 R08: ffff8881c6d64800 R09: fffffbfff0e84c25
R10: ffff8881d99d7940 R11: ffffffff87426127 R12: 0000000000000004
R13: 0000000000000000 R14: ffff8881d9b94000 R15: ffffffff897f9048
FS: 00007f047f542700(0000) GS:ffff8881db200000(0000) knlGS:0000000000000000
CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
CR2: 0000001b30f21000 CR3: 00000001ca032000 CR4: 00000000001406f0
DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
Call Trace:
pm_runtime_get_sync include/linux/pm_runtime.h:226 [inline]
usb_autopm_get_interface+0x1b/0x50 drivers/usb/core/driver.c:1707
usbhid_power+0x7c/0xe0 drivers/hid/usbhid/hid-core.c:1234
hid_hw_power include/linux/hid.h:1038 [inline]
hidraw_open+0x20d/0x740 drivers/hid/hidraw.c:282
chrdev_open+0x219/0x5c0 fs/char_dev.c:413
do_dentry_open+0x497/0x1040 fs/open.c:778
do_last fs/namei.c:3416 [inline]
path_openat+0x1430/0x3ff0 fs/namei.c:3533
do_filp_open+0x1a1/0x280 fs/namei.c:3563
do_sys_open+0x3c0/0x580 fs/open.c:1070
do_syscall_64+0xb7/0x560 arch/x86/entry/common.c:301
entry_SYSCALL_64_after_hwframe+0x49/0xbe

It turns out the fault was caused by a bug in the HID Logitech driver,
which violates the requirement that every pathway calling
hid_hw_start() must also call hid_hw_stop(). This patch fixes the bug
by making sure the requirement is met.

Reported-and-tested-by: syzbot+3cbe5c...@syzkaller.appspotmail.com
Signed-off-by: Alan Stern <st...@rowland.harvard.edu>
CC: <sta...@vger.kernel.org>

---

[as1909]


drivers/hid/hid-lg.c | 10 ++++++----
drivers/hid/hid-lg4ff.c | 1 -
2 files changed, 6 insertions(+), 5 deletions(-)

[Andrey Konovalov]

Trying Alan's fix from another thread here:

[syzbot]

Hello,

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

Reported-and-tested-by:

syzbot+a7a6b9...@syzkaller.appspotmail.com

Tested on:

commit: 7f7867ff usb-fuzzer: main usb gadget fuzzer driver
git tree: https://github.com/google/kasan.git
kernel config: https://syzkaller.appspot.com/x/.config?x=792eb47789f57810
compiler: gcc (GCC) 9.0.0 20181231 (experimental)

patch: https://syzkaller.appspot.com/x/patch.diff?x=140148ca600000

[Andrey Konovalov]

Looks like the same bug:

[Andrey Konovalov]

This bug has manifested in a bunch of different ways and produced
multiple confusing syzbot reports. Thank you for tracking this down
and fixing it, Alan!

[Jiri Kosina]

On Tue, 20 Aug 2019, Alan Stern wrote:

> The syzbot fuzzer found a general protection fault in the HID subsystem:

Applied, thanks a lot Alan.

--
Jiri Kosina
SUSE Labs

[Andrey Konovalov]

On Tue, Aug 20, 2019 at 10:00 PM Alan Stern <st...@rowland.harvard.edu> wrote:
>

Hi Alan,

I've ran the fuzzer with your patches applied overnight and noticed
another fallout of similar bugs. I think they are caused by a similar
issue in the sony HID driver. There's no hid_hw_stop() call in the "if
(!(hdev->claimed & HID_CLAIMED_INPUT))" case in sony_probe(). Does it
look like a bug to you?

Thanks!

[Alan Stern]

On Thu, 22 Aug 2019, Andrey Konovalov wrote:

> Hi Alan,
>
> I've ran the fuzzer with your patches applied overnight and noticed
> another fallout of similar bugs. I think they are caused by a similar
> issue in the sony HID driver. There's no hid_hw_stop() call in the "if
> (!(hdev->claimed & HID_CLAIMED_INPUT))" case in sony_probe(). Does it
> look like a bug to you?

It looks like the relevant hid_hw_stop() call is the one at the end of
sony_configure_input(). But I can't tell if doing that way is valid or
not -- in practice the code would end up calling hid_disconnect() while
hid_connect() was still running, which doesn't seem like a good idea.

There's a comment about this near the end of sony_probe(). I suspect
it would be better to call hid_hw_stop() in the conditional code
following that comment rather than in sony_configure_input().

Either way, these are all things Jiri should know about or check up on.

Have you gotten any test results from syzbot exercising these pathways?
You ought to be able to tell which HID driver is involved by looking
through the console output.

Alan Stern

[Andrey Konovalov]

Yes, a typical crash is below, that's why I thought it's the sony
driver. Adding hid_hw_stop() in sony_probe() stops the issue from
happening, but I don't know whether it's the right fix.

usb 1-1: new high-speed USB device number 3 using dummy_hcd
usb 1-1: Using ep0 maxpacket: 8
usb 1-1: config 0 interface 0 altsetting 0 endpoint 0x81 has an
invalid bInterval 0, changing7
usb 1-1: config 0 interface 0 altsetting 0 has 1 endpoint descriptor,
different from the inte9
usb 1-1: New USB device found, idVendor=054c, idProduct=024b, bcdDevice= 0.00
usb 1-1: New USB device strings: Mfr=0, Product=0, SerialNumber=0
usb 1-1: config 0 descriptor??
sony 0003:054C:024B.0002: unknown main item tag 0x0
sony 0003:054C:024B.0002: unknown main item tag 0x2
sony 0003:054C:024B.0002: unknown main item tag 0x0
sony 0003:054C:024B.0002: unknown main item tag 0x0
...
sony 0003:054C:024B.0002: unknown main item tag 0x0
==================================================================
BUG: KASAN: use-after-free in usbhid_power+0xca/0xe0
Read of size 8 at addr ffff88805d590008 by task syz-executor/1808

CPU: 1 PID: 1808 Comm: syz-executor Not tainted 5.3.0-rc5+ #203
Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.12.0-1 04/01/2014

Call Trace:
__dump_stack lib/dump_stack.c:77

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
usbhid_power+0xca/0xe0 drivers/hid/usbhid/hid-core.c:1234
hid_hw_power ./include/linux/hid.h:1038
hidraw_open+0x20d/0x740 drivers/hid/hidraw.c:282
chrdev_open+0x219/0x5c0 fs/char_dev.c:414
do_dentry_open+0x494/0x1120 fs/open.c:797
do_last fs/namei.c:3416
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 arch/x86/entry/entry_64.S:175
RIP: 0033:0x413a0e
Code: 89 54 24 08 e8 a3 f9 ff ff 8b 74 24 0c 48 8b 3c 24 41 89 c0 44
8b 54 24 08 b8 01 01 00 4
RSP: 002b:00007f7bf0a66730 EFLAGS: 00000293 ORIG_RAX: 0000000000000101
RAX: ffffffffffffffda RBX: 6666666666666667 RCX: 0000000000413a0e
RDX: 0000000000000200 RSI: 00007f7bf0a66840 RDI: 00000000ffffff9c
RBP: 00007f7bf0a66840 R08: 0000000000000000 R09: 0000000000000000
R10: 0000000000000000 R11: 0000000000000293 R12: 00000000004a521c
R13: 00000000004ef7d0 R14: 00000000004ae881 R15: 00007f7bf0a676bc

Allocated by task 78:

save_stack+0x1b/0x80 mm/kasan/common.c:69
set_track mm/kasan/common.c:77

__kasan_kmalloc mm/kasan/common.c:487
__kasan_kmalloc.constprop.0+0xbf/0xd0 mm/kasan/common.c:460
slab_post_alloc_hook mm/slab.h:520
slab_alloc_node mm/slub.c:2770
__kmalloc_node_track_caller+0xfc/0x380 mm/slub.c:4365
__kmalloc_reserve.isra.0+0x39/0xe0 net/core/skbuff.c:141
__alloc_skb+0xef/0x5a0 net/core/skbuff.c:209
alloc_skb ./include/linux/skbuff.h:1055
alloc_uevent_skb+0x7b/0x210 lib/kobject_uevent.c:289
uevent_net_broadcast_untagged lib/kobject_uevent.c:325
kobject_uevent_net_broadcast lib/kobject_uevent.c:408
kobject_uevent_env+0x8ee/0x1160 lib/kobject_uevent.c:592
device_del+0x6b2/0xb10 drivers/base/core.c:2349
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
hub_port_connect_change drivers/usb/core/hub.c:5213
port_event drivers/usb/core/hub.c:5359

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

Freed by task 242:

save_stack+0x1b/0x80 mm/kasan/common.c:69
set_track mm/kasan/common.c:77

__kasan_slab_free+0x130/0x180 mm/kasan/common.c:449
slab_free_hook mm/slub.c:1423

slab_free_freelist_hook mm/slub.c:1474
slab_free mm/slub.c:3016
kfree+0xe4/0x2f0 mm/slub.c:3957
skb_free_head+0x8b/0xa0 net/core/skbuff.c:591
skb_release_data+0x41f/0x7c0 net/core/skbuff.c:611
skb_release_all+0x46/0x60 net/core/skbuff.c:665
__kfree_skb net/core/skbuff.c:679
consume_skb net/core/skbuff.c:838
consume_skb+0xd9/0x320 net/core/skbuff.c:832
skb_free_datagram+0x16/0xf0 net/core/datagram.c:328
netlink_recvmsg+0x65e/0xee0 net/netlink/af_netlink.c:1996
sock_recvmsg_nosec net/socket.c:871
sock_recvmsg net/socket.c:889
sock_recvmsg+0xca/0x110 net/socket.c:885
___sys_recvmsg+0x271/0x5a0 net/socket.c:2480
__sys_recvmsg+0xe9/0x1b0 net/socket.c:2537
do_syscall_64+0xb7/0x580 arch/x86/entry/common.c:296
entry_SYSCALL_64_after_hwframe+0x49/0xbe arch/x86/entry/entry_64.S:175

The buggy address belongs to the object at ffff88805d590000
which belongs to the cache kmalloc-1k of size 1024
The buggy address is located 8 bytes inside of
1024-byte region [ffff88805d590000, ffff88805d590400)

The buggy address belongs to the page:

page:ffffea0001756400 refcount:1 mapcount:0 mapping:ffff88806c402280
index:0x0 compound_mapco0
flags: 0x100000000010200(slab|head)
raw: 0100000000010200 dead000000000100 dead000000000122 ffff88806c402280
raw: 0000000000000000 00000000000e000e 00000001ffffffff 0000000000000000

page dumped because: kasan: bad access detected

Memory state around the buggy address:

ffff88805d58ff00: fe fe fe fe fe fe fe fe fe fe fe fe fe fe fe fe
ffff88805d58ff80: fe fe fe fe fe fe fe fe fe fe fe fe fe fe fe fe
>ffff88805d590000: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
^
ffff88805d590080: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
ffff88805d590100: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
==================================================================

[Alan Stern]

On Thu, 22 Aug 2019, Andrey Konovalov wrote:

> On Thu, Aug 22, 2019 at 7:11 PM Alan Stern <st...@rowland.harvard.edu> wrote:
> >
> > On Thu, 22 Aug 2019, Andrey Konovalov wrote:
> >
> > > Hi Alan,
> > >
> > > I've ran the fuzzer with your patches applied overnight and noticed
> > > another fallout of similar bugs. I think they are caused by a similar
> > > issue in the sony HID driver. There's no hid_hw_stop() call in the "if
> > > (!(hdev->claimed & HID_CLAIMED_INPUT))" case in sony_probe(). Does it
> > > look like a bug to you?
> >
> > It looks like the relevant hid_hw_stop() call is the one at the end of
> > sony_configure_input(). But I can't tell if doing that way is valid or
> > not -- in practice the code would end up calling hid_disconnect() while
> > hid_connect() was still running, which doesn't seem like a good idea.
> >
> > There's a comment about this near the end of sony_probe(). I suspect
> > it would be better to call hid_hw_stop() in the conditional code
> > following that comment rather than in sony_configure_input().
> >
> > Either way, these are all things Jiri should know about or check up on.
> >
> > Have you gotten any test results from syzbot exercising these pathways?
> > You ought to be able to tell which HID driver is involved by looking
> > through the console output.
>
> Yes, a typical crash is below, that's why I thought it's the sony
> driver. Adding hid_hw_stop() in sony_probe() stops the issue from
> happening, but I don't know whether it's the right fix.

Probably you have to add hid_hw_stop() in sony_probe() and remove it
from sony_configure_input(). But like I said above, Jiri should look
into this.

Alan Stern

[Hillf Danton]

On Fri, 23 Aug 2019 02:26:08 +0800 Andrey Konovalov wrote:
> ====================================================================

> ====================================================================

1, a while after first time to see

usbhid_power+0xca/0xe0 drivers/hid/usbhid/hid-core.c:1234
hid_hw_power ./include/linux/hid.h:1038
hidraw_open+0x20d/0x740 drivers/hid/hidraw.c:282

2, check ->exist under minors_lock upon opening.

3, reset ->exist under minors_lock while disconnecting.

4, add one line in ll_driver to nudge hidraw into disconnecting.

--- a/drivers/hid/usbhid/hid-core.c
+++ b/drivers/hid/usbhid/hid-core.c
@@ -1410,6 +1410,7 @@ static void usbhid_disconnect(struct usb
spin_lock_irq(&usbhid->lock); /* Sync with error and led handlers */
set_bit(HID_DISCONNECTED, &usbhid->iofl);
spin_unlock_irq(&usbhid->lock);

+ hid_disconnect(hid);
hid_destroy_device(hid);
kfree(usbhid);
}

[Jiri Kosina]

On Thu, 22 Aug 2019, Alan Stern wrote:

> > > > I've ran the fuzzer with your patches applied overnight and noticed
> > > > another fallout of similar bugs. I think they are caused by a similar
> > > > issue in the sony HID driver. There's no hid_hw_stop() call in the "if
> > > > (!(hdev->claimed & HID_CLAIMED_INPUT))" case in sony_probe(). Does it
> > > > look like a bug to you?
> > >
> > > It looks like the relevant hid_hw_stop() call is the one at the end of
> > > sony_configure_input(). But I can't tell if doing that way is valid or
> > > not -- in practice the code would end up calling hid_disconnect() while
> > > hid_connect() was still running, which doesn't seem like a good idea.
> > >
> > > There's a comment about this near the end of sony_probe(). I suspect
> > > it would be better to call hid_hw_stop() in the conditional code
> > > following that comment rather than in sony_configure_input().
> > >
> > > Either way, these are all things Jiri should know about or check up on.
> > >
> > > Have you gotten any test results from syzbot exercising these pathways?
> > > You ought to be able to tell which HID driver is involved by looking
> > > through the console output.
> >
> > Yes, a typical crash is below, that's why I thought it's the sony
> > driver. Adding hid_hw_stop() in sony_probe() stops the issue from
> > happening, but I don't know whether it's the right fix.
>
> Probably you have to add hid_hw_stop() in sony_probe() and remove it
> from sony_configure_input(). But like I said above, Jiri should look
> into this.

It almost certainly is, thanks.

Adding Roderick to CC ... Roderick, will you be able to test and submit
a patch fixing that?

[Roderick.C...@sony.com]

Sure we will have a look and do some testing. Hopefully we can share a patch some time next week.

Thanks,
Roderick

[Andrey Konovalov]

Hi Roderick,

I was wondering if you had a chance to look into this?

Once the Logitech fix is upstream, this similar Sony bug will start
producing a large number of similar syzbot reports since it causes a
major memory corruption and we'll need to triage them all again. It
would be nice to get the Sony fix merged together with the Logitech
one. Or at least to have it available so I can apply it manually until
it is merged.

Thanks!

[Roderick.C...@sony.com]

> From: Andrey Konovalov [andre...@google.com]
> Sent: Tuesday, September 03, 2019 3:46 AM
> To: Colenbrander, Roderick
> Cc: Jiri Kosina; Alan Stern; Gustavo A. R. Silva; Hillf Danton; syzkaller-bugs; linux...@vger.kernel.org; USB list
> Subject: Re: [PATCH] HID: USB: Fix general protection fault caused by Logitech driver

Hi Andrey,

We are still looking at the issue. I had one of my guys try the fix (and trigger an error in sony_input_configured), but he claimed he crashed his kernel. Due to the long weekend in the US, we didn't get back to looking at it yet. We are looking at it some more today or tomorrow.

Thanks,
Roderick