2020-01-14 13:06:12

by syzbot

[permalink] [raw]
Subject: WARNING in usbhid_raw_request/usb_submit_urb (3)

Hello,

syzbot found the following crash on:

HEAD commit: 5a67532c usb: gadget: add raw-gadget interface
git tree: https://github.com/google/kasan.git usb-fuzzer
console output: https://syzkaller.appspot.com/x/log.txt?x=16e4bb25e00000
kernel config: https://syzkaller.appspot.com/x/.config?x=9ba75825443d54bd
dashboard link: https://syzkaller.appspot.com/bug?extid=db339689b2101f6f6071
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: [email protected]

------------[ cut here ]------------
usb 3-1: BOGUS urb xfer, pipe 2 != type 2
WARNING: CPU: 0 PID: 23154 at drivers/usb/core/urb.c:478
usb_submit_urb+0x1188/0x1460 drivers/usb/core/urb.c:478
Kernel panic - not syncing: panic_on_warn set ...
CPU: 0 PID: 23154 Comm: syz-executor.2 Not tainted 5.5.0-rc6-syzkaller #0
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+0xef/0x16e lib/dump_stack.c:118
panic+0x2aa/0x6e1 kernel/panic.c:221
__warn.cold+0x2f/0x30 kernel/panic.c:582
report_bug+0x27b/0x2f0 lib/bug.c:195
fixup_bug arch/x86/kernel/traps.c:174 [inline]
fixup_bug arch/x86/kernel/traps.c:169 [inline]
do_error_trap+0x12b/0x1e0 arch/x86/kernel/traps.c:267
do_invalid_op+0x32/0x40 arch/x86/kernel/traps.c:286
invalid_op+0x23/0x30 arch/x86/entry/entry_64.S:1027
RIP: 0010:usb_submit_urb+0x1188/0x1460 drivers/usb/core/urb.c:478
Code: 4d 85 ed 74 46 e8 a8 1c e1 fd 4c 89 f7 e8 90 87 17 ff 41 89 d8 44 89
e1 4c 89 ea 48 89 c6 48 c7 c7 60 27 3b 86 e8 30 03 b6 fd <0f> 0b e9 20 f4
ff ff e8 7c 1c e1 fd 0f 1f 44 00 00 e8 72 1c e1 fd
RSP: 0018:ffff8881d4227b18 EFLAGS: 00010282
RAX: 0000000000000000 RBX: 0000000000000002 RCX: 0000000000000000
RDX: 0000000000004c45 RSI: ffffffff8129598d RDI: ffffed103a844f55
RBP: 0000000000000000 R08: ffff8881cdc30000 R09: fffffbfff1269cb7
R10: fffffbfff1269cb6 R11: ffffffff8934e5b7 R12: 0000000000000002
R13: ffff8881caf26ab0 R14: ffff8881d40040a0 R15: ffff8881c6936d00
usb_start_wait_urb+0x108/0x4c0 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_write+0x34/0x50 drivers/hid/hidraw.c:164
__vfs_write+0x76/0x100 fs/read_write.c:494
vfs_write+0x262/0x5c0 fs/read_write.c:558
ksys_write+0x127/0x250 fs/read_write.c:611
do_syscall_64+0xb6/0x5a0 arch/x86/entry/common.c:294
entry_SYSCALL_64_after_hwframe+0x49/0xbe
RIP: 0033:0x45af49
Code: ad b6 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 7b b6 fb ff c3 66 2e 0f 1f 84 00 00 00 00
RSP: 002b:00007f0d8bc69c78 EFLAGS: 00000246 ORIG_RAX: 0000000000000001
RAX: ffffffffffffffda RBX: 0000000000000003 RCX: 000000000045af49
RDX: 000000000000000b RSI: 00000000200000c0 RDI: 0000000000000006
RBP: 000000000075bfc8 R08: 0000000000000000 R09: 0000000000000000
R10: 0000000000000000 R11: 0000000000000246 R12: 00007f0d8bc6a6d4
R13: 00000000004ccfde R14: 00000000004e8570 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 [email protected].

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


2020-04-01 20:55:26

by syzbot

[permalink] [raw]
Subject: Re: WARNING in usbhid_raw_request/usb_submit_urb (3)

syzbot has found a reproducer for the following crash on:

HEAD commit: 0fa84af8 Merge tag 'usb-serial-5.7-rc1' of https://git.ker..
git tree: https://github.com/google/kasan.git usb-fuzzer
console output: https://syzkaller.appspot.com/x/log.txt?x=12aa8567e00000
kernel config: https://syzkaller.appspot.com/x/.config?x=6b9c154b0c23aecf
dashboard link: https://syzkaller.appspot.com/bug?extid=db339689b2101f6f6071
compiler: gcc (GCC) 9.0.0 20181231 (experimental)
syz repro: https://syzkaller.appspot.com/x/repro.syz?x=1342740be00000

IMPORTANT: if you fix the bug, please add the following tag to the commit:
Reported-by: [email protected]

------------[ cut here ]------------
usb 2-1: BOGUS urb xfer, pipe 2 != type 2
WARNING: CPU: 0 PID: 9241 at drivers/usb/core/urb.c:478 usb_submit_urb+0x1188/0x1460 drivers/usb/core/urb.c:478
Kernel panic - not syncing: panic_on_warn set ...
CPU: 0 PID: 9241 Comm: syz-executor.1 Not tainted 5.6.0-rc7-syzkaller #0
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+0xef/0x16e lib/dump_stack.c:118
panic+0x2aa/0x6e1 kernel/panic.c:221
__warn.cold+0x2f/0x30 kernel/panic.c:582
report_bug+0x27b/0x2f0 lib/bug.c:195
fixup_bug arch/x86/kernel/traps.c:174 [inline]
fixup_bug arch/x86/kernel/traps.c:169 [inline]
do_error_trap+0x12b/0x1e0 arch/x86/kernel/traps.c:267
do_invalid_op+0x32/0x40 arch/x86/kernel/traps.c:286
invalid_op+0x23/0x30 arch/x86/entry/entry_64.S:1027
RIP: 0010:usb_submit_urb+0x1188/0x1460 drivers/usb/core/urb.c:478
Code: 4d 85 ed 74 46 e8 68 87 dd fd 4c 89 f7 e8 90 57 17 ff 41 89 d8 44 89 e1 4c 89 ea 48 89 c6 48 c7 c7 80 dd 3b 86 e8 10 18 b2 fd <0f> 0b e9 20 f4 ff ff e8 3c 87 dd fd 0f 1f 44 00 00 e8 32 87 dd fd
RSP: 0018:ffff8881c7a47b38 EFLAGS: 00010286
RAX: 0000000000000000 RBX: 0000000000000002 RCX: 0000000000000000
RDX: 0000000000000000 RSI: ffffffff812974dd RDI: ffffed1038f48f59
RBP: 0000000000000000 R08: ffff8881cd811880 R09: ffffed103b646248
R10: ffffed103b646247 R11: ffff8881db23123f R12: 0000000000000002
R13: ffff8881c5c764b0 R14: ffff8881c8daf0a0 R15: ffff8881d0117500
usb_start_wait_urb+0x108/0x4c0 drivers/usb/core/message.c:58
usb_internal_control_msg drivers/usb/core/message.c:102 [inline]
usb_control_msg+0x31c/0x4a0 drivers/usb/core/message.c:153
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+0x620/0xaf0 drivers/hid/hidraw.c:422
vfs_ioctl fs/ioctl.c:47 [inline]
ksys_ioctl+0x11a/0x180 fs/ioctl.c:763
__do_sys_ioctl fs/ioctl.c:772 [inline]
__se_sys_ioctl fs/ioctl.c:770 [inline]
__x64_sys_ioctl+0x6f/0xb0 fs/ioctl.c:770
do_syscall_64+0xb6/0x5a0 arch/x86/entry/common.c:294
entry_SYSCALL_64_after_hwframe+0x49/0xbe
RIP: 0033:0x45c849
Code: ad b6 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 7b b6 fb ff c3 66 2e 0f 1f 84 00 00 00 00
RSP: 002b:00007fa05ffffc78 EFLAGS: 00000246 ORIG_RAX: 0000000000000010
RAX: ffffffffffffffda RBX: 00007fa0600006d4 RCX: 000000000045c849
RDX: 00000000200000c0 RSI: 0000000080404806 RDI: 0000000000000006
RBP: 000000000076bfa0 R08: 0000000000000000 R09: 0000000000000000
R10: 0000000000000000 R11: 0000000000000246 R12: 00000000ffffffff
R13: 0000000000000335 R14: 00000000004c59df R15: 000000000076bfac
Kernel Offset: disabled
Rebooting in 86400 seconds..

2020-04-02 15:36:38

by Alan Stern

[permalink] [raw]
Subject: Re: WARNING in usbhid_raw_request/usb_submit_urb (3)

On Wed, 1 Apr 2020, syzbot wrote:

> syzbot has found a reproducer for the following crash on:
>
> HEAD commit: 0fa84af8 Merge tag 'usb-serial-5.7-rc1' of https://git.ker..
> git tree: https://github.com/google/kasan.git usb-fuzzer
> console output: https://syzkaller.appspot.com/x/log.txt?x=12aa8567e00000
> kernel config: https://syzkaller.appspot.com/x/.config?x=6b9c154b0c23aecf
> dashboard link: https://syzkaller.appspot.com/bug?extid=db339689b2101f6f6071
> compiler: gcc (GCC) 9.0.0 20181231 (experimental)
> syz repro: https://syzkaller.appspot.com/x/repro.syz?x=1342740be00000
>
> IMPORTANT: if you fix the bug, please add the following tag to the commit:
> Reported-by: [email protected]
>
> ------------[ cut here ]------------
> usb 2-1: BOGUS urb xfer, pipe 2 != type 2
> WARNING: CPU: 0 PID: 9241 at drivers/usb/core/urb.c:478 usb_submit_urb+0x1188/0x1460 drivers/usb/core/urb.c:478

At last! Let's get some more information about this...

Alan Stern

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

Index: usb-devel/drivers/usb/core/urb.c
===================================================================
--- usb-devel.orig/drivers/usb/core/urb.c
+++ usb-devel/drivers/usb/core/urb.c
@@ -475,8 +475,9 @@ int usb_submit_urb(struct urb *urb, gfp_

/* Check that the pipe's type matches the endpoint's type */
if (usb_urb_ep_type_check(urb))
- dev_WARN(&dev->dev, "BOGUS urb xfer, pipe %x != type %x\n",
- usb_pipetype(urb->pipe), pipetypes[xfertype]);
+ dev_WARN(&dev->dev, "BOGUS urb xfer, pipe %x != type %x, ep addr 0x%02x, pipe 0x%x, xfertype %d\n",
+ usb_pipetype(urb->pipe), pipetypes[xfertype],
+ ep->desc.bEndpointAddress, urb->pipe, xfertype);

/* Check against a simple/standard policy */
allowed = (URB_NO_TRANSFER_DMA_MAP | URB_NO_INTERRUPT | URB_DIR_MASK |

2020-04-02 15:58:55

by Andrey Konovalov

[permalink] [raw]
Subject: Re: WARNING in usbhid_raw_request/usb_submit_urb (3)

On Thu, Apr 2, 2020 at 5:53 PM syzbot
<[email protected]> wrote:
>
> Hello,
>
> syzbot has tested the proposed patch and the reproducer did not trigger crash:
>
> Reported-and-tested-by: [email protected]
>
> Tested on:
>
> commit: 0fa84af8 Merge tag 'usb-serial-5.7-rc1' of https://git.ker..
> git tree: https://github.com/google/kasan.git
> kernel config: https://syzkaller.appspot.com/x/.config?x=6b9c154b0c23aecf
> dashboard link: https://syzkaller.appspot.com/bug?extid=db339689b2101f6f6071
> compiler: gcc (GCC) 9.0.0 20181231 (experimental)
> patch: https://syzkaller.appspot.com/x/patch.diff?x=143c2c63e00000
>
> Note: testing is done by a robot and is best-effort only.

Let's try this once again, maybe the reproducer is extremely unreliable.

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


Attachments:
usb.patch (789.00 B)

2020-04-02 16:51:33

by syzbot

[permalink] [raw]
Subject: Re: WARNING in usbhid_raw_request/usb_submit_urb (3)

Hello,

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

Reported-and-tested-by: [email protected]

Tested on:

commit: 0fa84af8 Merge tag 'usb-serial-5.7-rc1' of https://git.ker..
git tree: https://github.com/google/kasan.git
kernel config: https://syzkaller.appspot.com/x/.config?x=6b9c154b0c23aecf
dashboard link: https://syzkaller.appspot.com/bug?extid=db339689b2101f6f6071
compiler: gcc (GCC) 9.0.0 20181231 (experimental)
patch: https://syzkaller.appspot.com/x/patch.diff?x=143c2c63e00000

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

2020-04-02 17:23:08

by syzbot

[permalink] [raw]
Subject: Re: WARNING in usbhid_raw_request/usb_submit_urb (3)

Hello,

syzbot has tested the proposed patch but the reproducer still triggered crash:
WARNING in usbhid_raw_request/usb_submit_urb

------------[ cut here ]------------
usb 3-1: BOGUS urb xfer, pipe 2 != type 2, ep addr 0x00, pipe 0x80001a00, xfertype 0
WARNING: CPU: 1 PID: 4932 at drivers/usb/core/urb.c:478 usb_submit_urb+0x11eb/0x14d0 drivers/usb/core/urb.c:478
Kernel panic - not syncing: panic_on_warn set ...
CPU: 1 PID: 4932 Comm: syz-executor.2 Not tainted 5.6.0-rc7-syzkaller #0
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+0xef/0x16e lib/dump_stack.c:118
panic+0x2aa/0x6e1 kernel/panic.c:221
__warn.cold+0x2f/0x30 kernel/panic.c:582
report_bug+0x27b/0x2f0 lib/bug.c:195
fixup_bug arch/x86/kernel/traps.c:174 [inline]
fixup_bug arch/x86/kernel/traps.c:169 [inline]
do_error_trap+0x12b/0x1e0 arch/x86/kernel/traps.c:267
do_invalid_op+0x32/0x40 arch/x86/kernel/traps.c:286
invalid_op+0x23/0x30 arch/x86/entry/entry_64.S:1027
RIP: 0010:usb_submit_urb+0x11eb/0x14d0 drivers/usb/core/urb.c:478
Code: fd 4c 89 ff e8 36 57 17 ff 45 89 e9 44 89 e1 48 c7 c7 80 dd 3b 86 55 48 89 c6 53 44 8b 44 24 2c 48 8b 54 24 30 e8 ad 17 b2 fd <0f> 0b 58 5a e9 bb f3 ff ff e8 d7 86 dd fd 0f 1f 44 00 00 e8 cd 86
RSP: 0018:ffff8881c7ff7b30 EFLAGS: 00010282
RAX: 0000000000000000 RBX: 0000000080001a00 RCX: 0000000000000000
RDX: 0000000000000000 RSI: ffffffff812974dd RDI: ffffed1038ffef58
RBP: 0000000000000000 R08: ffff8881cd729880 R09: ffffed103b666248
R10: ffffed103b666247 R11: ffff8881db33123f R12: 0000000000000002
R13: 0000000000000000 R14: ffff8881d98d7000 R15: ffff8881d02f30a0
usb_start_wait_urb+0x108/0x4c0 drivers/usb/core/message.c:58
usb_internal_control_msg drivers/usb/core/message.c:102 [inline]
usb_control_msg+0x31c/0x4a0 drivers/usb/core/message.c:153
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+0x620/0xaf0 drivers/hid/hidraw.c:422
vfs_ioctl fs/ioctl.c:47 [inline]
ksys_ioctl+0x11a/0x180 fs/ioctl.c:763
__do_sys_ioctl fs/ioctl.c:772 [inline]
__se_sys_ioctl fs/ioctl.c:770 [inline]
__x64_sys_ioctl+0x6f/0xb0 fs/ioctl.c:770
do_syscall_64+0xb6/0x5a0 arch/x86/entry/common.c:294
entry_SYSCALL_64_after_hwframe+0x49/0xbe
RIP: 0033:0x45c849
Code: ad b6 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 7b b6 fb ff c3 66 2e 0f 1f 84 00 00 00 00
RSP: 002b:00007fa1cf489c78 EFLAGS: 00000246 ORIG_RAX: 0000000000000010
RAX: ffffffffffffffda RBX: 00007fa1cf48a6d4 RCX: 000000000045c849
RDX: 00000000200000c0 RSI: 0000000080404806 RDI: 0000000000000006
RBP: 000000000076bfa0 R08: 0000000000000000 R09: 0000000000000000
R10: 0000000000000000 R11: 0000000000000246 R12: 00000000ffffffff
R13: 0000000000000335 R14: 00000000004c59df R15: 000000000076bfac
Kernel Offset: disabled
Rebooting in 86400 seconds..


Tested on:

commit: 0fa84af8 Merge tag 'usb-serial-5.7-rc1' of https://git.ker..
git tree: https://github.com/google/kasan.git
console output: https://syzkaller.appspot.com/x/log.txt?x=1652148fe00000
kernel config: https://syzkaller.appspot.com/x/.config?x=6b9c154b0c23aecf
dashboard link: https://syzkaller.appspot.com/bug?extid=db339689b2101f6f6071
compiler: gcc (GCC) 9.0.0 20181231 (experimental)
patch: https://syzkaller.appspot.com/x/patch.diff?x=1174b8fbe00000

2020-04-02 19:00:49

by Alan Stern

[permalink] [raw]
Subject: Re: WARNING in usbhid_raw_request/usb_submit_urb (3)

On Thu, 2 Apr 2020, syzbot wrote:

> Hello,
>
> syzbot has tested the proposed patch but the reproducer still triggered crash:
> WARNING in usbhid_raw_request/usb_submit_urb
>
> ------------[ cut here ]------------
> usb 3-1: BOGUS urb xfer, pipe 2 != type 2, ep addr 0x00, pipe 0x80001a00, xfertype 0

Not much help, I'm afraid. The pipe value decodes to:

Direction: OUT
Device address: 26 (which agrees with the console output)
Endpoint: 0
Pipe type: Control (which is appropriate for ep 0
and agrees with xfertype)

Unfortunately, the values printed here need not be the same as the
values checked by usb_urb_ep_type_check(), if either the URB or the
descriptor is modified concurrently by another thread.

Let's try a different approach. Since this modifies the code before
the test, it may not trigger the warning. But it's worth a try.

Alan Stern

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

Index: usb-devel/drivers/usb/core/urb.c
===================================================================
--- usb-devel.orig/drivers/usb/core/urb.c
+++ usb-devel/drivers/usb/core/urb.c
@@ -199,15 +199,27 @@ static const int pipetypes[4] = {
* given urb. It returns 0 if the urb contains a valid endpoint, otherwise
* a negative error code.
*/
+unsigned int alan_pipe, alan_epaddr, alan_epattr;
int usb_urb_ep_type_check(const struct urb *urb)
{
const struct usb_host_endpoint *ep;
+ unsigned int pipe, epattr;

- ep = usb_pipe_endpoint(urb->dev, urb->pipe);
- if (!ep)
+ pipe = READ_ONCE(urb->pipe);
+ ep = usb_pipe_endpoint(urb->dev, pipe);
+ if (!ep) {
+ alan_pipe = pipe;
+ alan_epattr = 0;
+ alan_epaddr = 0xff;
return -EINVAL;
- if (usb_pipetype(urb->pipe) != pipetypes[usb_endpoint_type(&ep->desc)])
+ }
+ epattr = READ_ONCE(ep->desc.bmAttributes);
+ if (usb_pipetype(pipe) != pipetypes[epattr & USB_ENDPOINT_XFERTYPE_MASK]) {
+ alan_pipe = pipe;
+ alan_epattr = epattr;
+ alan_epaddr = ep->desc.bEndpointAddress;
return -EINVAL;
+ }
return 0;
}
EXPORT_SYMBOL_GPL(usb_urb_ep_type_check);
@@ -475,8 +487,9 @@ int usb_submit_urb(struct urb *urb, gfp_

/* Check that the pipe's type matches the endpoint's type */
if (usb_urb_ep_type_check(urb))
- dev_WARN(&dev->dev, "BOGUS urb xfer, pipe %x != type %x\n",
- usb_pipetype(urb->pipe), pipetypes[xfertype]);
+ dev_WARN(&dev->dev, "BOGUS urb xfer, pipe %x != type %x, pipe 0x%x, ep attr 0x%02x, ep addr 0x%02x\n",
+ usb_pipetype(urb->pipe), pipetypes[xfertype],
+ alan_pipe, alan_epattr, alan_epaddr);

/* Check against a simple/standard policy */
allowed = (URB_NO_TRANSFER_DMA_MAP | URB_NO_INTERRUPT | URB_DIR_MASK |


2020-04-02 21:33:26

by syzbot

[permalink] [raw]
Subject: Re: WARNING in usbhid_raw_request/usb_submit_urb (3)

Hello,

syzbot has tested the proposed patch but the reproducer still triggered crash:
WARNING in usbhid_raw_request/usb_submit_urb

------------[ cut here ]------------
usb 1-1: BOGUS urb xfer, pipe 2 != type 2, pipe 0x80005600, ep attr 0x00, ep addr 0xff
WARNING: CPU: 1 PID: 8507 at drivers/usb/core/urb.c:490 usb_submit_urb+0xa41/0x1380 drivers/usb/core/urb.c:490
Kernel panic - not syncing: panic_on_warn set ...
CPU: 1 PID: 8507 Comm: syz-executor.0 Not tainted 5.6.0-rc7-syzkaller #0
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+0xef/0x16e lib/dump_stack.c:118
panic+0x2aa/0x6e1 kernel/panic.c:221
__warn.cold+0x2f/0x30 kernel/panic.c:582
report_bug+0x27b/0x2f0 lib/bug.c:195
fixup_bug arch/x86/kernel/traps.c:174 [inline]
fixup_bug arch/x86/kernel/traps.c:169 [inline]
do_error_trap+0x12b/0x1e0 arch/x86/kernel/traps.c:267
do_invalid_op+0x32/0x40 arch/x86/kernel/traps.c:286
invalid_op+0x23/0x30 arch/x86/entry/entry_64.S:1027
RIP: 0010:usb_submit_urb+0xa41/0x1380 drivers/usb/core/urb.c:490
Code: f7 e8 53 5e 17 ff 41 89 e9 41 89 d8 44 89 e1 41 55 48 89 c6 8b 44 24 30 48 c7 c7 80 dd 3b 86 50 48 8b 54 24 40 e8 c7 1e b2 fd <0f> 0b 58 5a e8 f6 8d dd fd 0f b6 6c 24 08 bb 86 03 00 00 48 c7 c6
RSP: 0018:ffff8881cfef7b28 EFLAGS: 00010282
RAX: 0000000000000000 RBX: 0000000000000002 RCX: 0000000000000000
RDX: 0000000000000000 RSI: ffffffff812974dd RDI: ffffed1039fdef57
RBP: 0000000080005600 R08: ffff8881c528e200 R09: ffffed103b666248
R10: ffffed103b666247 R11: ffff8881db33123f R12: 0000000000000002
R13: 00000000000000ff R14: ffff8881d999d0a0 R15: ffff8881da357800
usb_start_wait_urb+0x108/0x4c0 drivers/usb/core/message.c:58
usb_internal_control_msg drivers/usb/core/message.c:102 [inline]
usb_control_msg+0x31c/0x4a0 drivers/usb/core/message.c:153
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+0x620/0xaf0 drivers/hid/hidraw.c:422
vfs_ioctl fs/ioctl.c:47 [inline]
ksys_ioctl+0x11a/0x180 fs/ioctl.c:763
__do_sys_ioctl fs/ioctl.c:772 [inline]
__se_sys_ioctl fs/ioctl.c:770 [inline]
__x64_sys_ioctl+0x6f/0xb0 fs/ioctl.c:770
do_syscall_64+0xb6/0x5a0 arch/x86/entry/common.c:294
entry_SYSCALL_64_after_hwframe+0x49/0xbe
RIP: 0033:0x45c849
Code: ad b6 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 7b b6 fb ff c3 66 2e 0f 1f 84 00 00 00 00
RSP: 002b:00007fe66e55bc78 EFLAGS: 00000246 ORIG_RAX: 0000000000000010
RAX: ffffffffffffffda RBX: 00007fe66e55c6d4 RCX: 000000000045c849
RDX: 00000000200000c0 RSI: 0000000080404806 RDI: 0000000000000006
RBP: 000000000076bfa0 R08: 0000000000000000 R09: 0000000000000000
R10: 0000000000000000 R11: 0000000000000246 R12: 00000000ffffffff
R13: 0000000000000335 R14: 00000000004c59df R15: 000000000076bfac
Kernel Offset: disabled
Rebooting in 86400 seconds..


Tested on:

commit: 0fa84af8 Merge tag 'usb-serial-5.7-rc1' of https://git.ker..
git tree: https://github.com/google/kasan.git
console output: https://syzkaller.appspot.com/x/log.txt?x=15ddbf2fe00000
kernel config: https://syzkaller.appspot.com/x/.config?x=6b9c154b0c23aecf
dashboard link: https://syzkaller.appspot.com/bug?extid=db339689b2101f6f6071
compiler: gcc (GCC) 9.0.0 20181231 (experimental)
patch: https://syzkaller.appspot.com/x/patch.diff?x=1559e85de00000

2020-04-23 01:20:10

by Alan Stern

[permalink] [raw]
Subject: Re: WARNING in usbhid_raw_request/usb_submit_urb (3)

On Thu, 2 Apr 2020, syzbot wrote:

> Hello,
>
> syzbot has tested the proposed patch but the reproducer still triggered crash:
> WARNING in usbhid_raw_request/usb_submit_urb
>
> ------------[ cut here ]------------
> usb 1-1: BOGUS urb xfer, pipe 2 != type 2, pipe 0x80005600, ep attr 0x00, ep addr 0xff

This means that the ep_out[0] pointer was NULL. Let's try to find out
why.

Alan Stern

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

Index: usb-devel/drivers/usb/core/hub.c
===================================================================
--- usb-devel.orig/drivers/usb/core/hub.c
+++ usb-devel/drivers/usb/core/hub.c
@@ -4437,9 +4437,11 @@ int hub_port_debounce(struct usb_hub *hu

void usb_ep0_reinit(struct usb_device *udev)
{
+ udev->alan1 = 2;
usb_disable_endpoint(udev, 0 + USB_DIR_IN, true);
usb_disable_endpoint(udev, 0 + USB_DIR_OUT, true);
usb_enable_endpoint(udev, &udev->ep0, true);
+ udev->alan1 = 3;
}
EXPORT_SYMBOL_GPL(usb_ep0_reinit);

Index: usb-devel/drivers/usb/core/message.c
===================================================================
--- usb-devel.orig/drivers/usb/core/message.c
+++ usb-devel/drivers/usb/core/message.c
@@ -1198,6 +1198,10 @@ void usb_disable_interface(struct usb_de
int i;

for (i = 0; i < alt->desc.bNumEndpoints; ++i) {
+ if ((alt->endpoint[i].desc.bEndpointAddress & 0x0f) == 0)
+ dev_info(&dev->dev, "Ep 0 is %d in interface %d-%d\n",
+ i, alt->desc.bInterfaceNumber,
+ alt->desc.bAlternateSetting);
usb_disable_endpoint(dev,
alt->endpoint[i].desc.bEndpointAddress,
reset_hardware);
@@ -1275,6 +1279,8 @@ void usb_disable_device(struct usb_devic
mutex_unlock(hcd->bandwidth_mutex);
/* Second pass: remove endpoint pointers */
}
+ if (!skip_ep0)
+ dev->alan1 = 1;
for (i = skip_ep0; i < 16; ++i) {
usb_disable_endpoint(dev, i, true);
usb_disable_endpoint(dev, i + USB_DIR_IN, true);
Index: usb-devel/drivers/usb/core/urb.c
===================================================================
--- usb-devel.orig/drivers/usb/core/urb.c
+++ usb-devel/drivers/usb/core/urb.c
@@ -204,8 +204,12 @@ int usb_urb_ep_type_check(const struct u
const struct usb_host_endpoint *ep;

ep = usb_pipe_endpoint(urb->dev, urb->pipe);
- if (!ep)
+ if (!ep) {
+ dev_info(&urb->dev->dev, "Ep %d disabled: %d %d\n",
+ usb_pipeendpoint(urb->pipe),
+ urb->dev->alan1, urb->dev->alan2);
return -EINVAL;
+ }
if (usb_pipetype(urb->pipe) != pipetypes[usb_endpoint_type(&ep->desc)])
return -EINVAL;
return 0;
Index: usb-devel/include/linux/usb.h
===================================================================
--- usb-devel.orig/include/linux/usb.h
+++ usb-devel/include/linux/usb.h
@@ -629,6 +629,7 @@ struct usb3_lpm_parameters {
* usb_set_device_state().
*/
struct usb_device {
+ int alan1, alan2;
int devnum;
char devpath[16];
u32 route;

2020-04-23 01:40:00

by syzbot

[permalink] [raw]
Subject: Re: WARNING in usbhid_raw_request/usb_submit_urb (3)

Hello,

syzbot has tested the proposed patch but the reproducer still triggered crash:
WARNING in usbhid_raw_request/usb_submit_urb

usb 5-1: Ep 0 disabled: 2 0
------------[ cut here ]------------
usb 5-1: BOGUS urb xfer, pipe 2 != type 2
WARNING: CPU: 0 PID: 6371 at drivers/usb/core/urb.c:482 usb_submit_urb+0xa41/0x1390 drivers/usb/core/urb.c:482
Kernel panic - not syncing: panic_on_warn set ...
CPU: 0 PID: 6371 Comm: syz-executor.4 Not tainted 5.6.0-rc7-syzkaller #0
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+0xef/0x16e lib/dump_stack.c:118
panic+0x2aa/0x6e1 kernel/panic.c:221
__warn.cold+0x2f/0x30 kernel/panic.c:582
report_bug+0x27b/0x2f0 lib/bug.c:195
fixup_bug arch/x86/kernel/traps.c:174 [inline]
fixup_bug arch/x86/kernel/traps.c:169 [inline]
do_error_trap+0x12b/0x1e0 arch/x86/kernel/traps.c:267
do_invalid_op+0x32/0x40 arch/x86/kernel/traps.c:286
invalid_op+0x23/0x30 arch/x86/entry/entry_64.S:1027
RIP: 0010:usb_submit_urb+0xa41/0x1390 drivers/usb/core/urb.c:482
Code: 84 49 04 00 00 e8 7f 89 dd fd 4c 89 ef e8 87 59 17 ff 41 89 d8 44 89 e1 4c 89 f2 48 89 c6 48 c7 c7 c0 dd 3b 86 e8 27 1a b2 fd <0f> 0b e8 58 89 dd fd 0f b6 6c 24 08 bb 86 03 00 00 48 c7 c6 20 df
RSP: 0018:ffff8881be9afb30 EFLAGS: 00010282
RAX: 0000000000000000 RBX: 0000000000000002 RCX: 0000000000000000
RDX: 0000000000000000 RSI: ffffffff812974dd RDI: ffffed1037d35f58
RBP: 0000000000000000 R08: ffff8881d41fe200 R09: ffffed103b64439f
R10: ffffed103b64439e R11: ffff8881db221cf3 R12: 0000000000000002
R13: ffff8881cf1390a8 R14: ffff8881d8fb8e88 R15: ffff8881d89b8e00
usb_start_wait_urb+0x108/0x4c0 drivers/usb/core/message.c:58
usb_internal_control_msg drivers/usb/core/message.c:102 [inline]
usb_control_msg+0x31c/0x4a0 drivers/usb/core/message.c:153
usbhid_set_raw_report drivers/hid/usbhid/hid-core.c:917 [inline]
usbhid_raw_request+0x225/0x620 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+0x620/0xaf0 drivers/hid/hidraw.c:422
vfs_ioctl fs/ioctl.c:47 [inline]
ksys_ioctl+0x11a/0x180 fs/ioctl.c:763
__do_sys_ioctl fs/ioctl.c:772 [inline]
__se_sys_ioctl fs/ioctl.c:770 [inline]
__x64_sys_ioctl+0x6f/0xb0 fs/ioctl.c:770
do_syscall_64+0xb6/0x5a0 arch/x86/entry/common.c:294
entry_SYSCALL_64_after_hwframe+0x49/0xbe
RIP: 0033:0x45c849
Code: ad b6 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 7b b6 fb ff c3 66 2e 0f 1f 84 00 00 00 00
RSP: 002b:00007fa679711c78 EFLAGS: 00000246 ORIG_RAX: 0000000000000010
RAX: ffffffffffffffda RBX: 00007fa6797126d4 RCX: 000000000045c849
RDX: 00000000200000c0 RSI: 0000000080404806 RDI: 0000000000000006
RBP: 000000000076bfa0 R08: 0000000000000000 R09: 0000000000000000
R10: 0000000000000000 R11: 0000000000000246 R12: 00000000ffffffff
R13: 0000000000000335 R14: 00000000004c59df R15: 000000000076bfac
Kernel Offset: disabled
Rebooting in 86400 seconds..


Tested on:

commit: 0fa84af8 Merge tag 'usb-serial-5.7-rc1' of https://git.ker..
git tree: https://github.com/google/kasan.git
console output: https://syzkaller.appspot.com/x/log.txt?x=1120236fe00000
kernel config: https://syzkaller.appspot.com/x/.config?x=6b9c154b0c23aecf
dashboard link: https://syzkaller.appspot.com/bug?extid=db339689b2101f6f6071
compiler: gcc (GCC) 9.0.0 20181231 (experimental)
patch: https://syzkaller.appspot.com/x/patch.diff?x=1799b3bfe00000

2020-04-23 16:39:16

by Alan Stern

[permalink] [raw]
Subject: Re: WARNING in usbhid_raw_request/usb_submit_urb (3)

On Wed, 22 Apr 2020, syzbot wrote:

> Hello,
>
> syzbot has tested the proposed patch but the reproducer still triggered crash:
> WARNING in usbhid_raw_request/usb_submit_urb
>
> usb 5-1: Ep 0 disabled: 2 0

This means that URB submission raced with usb_ep0_reinit(). Let's try
to find out how.

Alan Stern

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

Index: usb-devel/drivers/usb/core/hub.c
===================================================================
--- usb-devel.orig/drivers/usb/core/hub.c
+++ usb-devel/drivers/usb/core/hub.c
@@ -4440,6 +4440,7 @@ void usb_ep0_reinit(struct usb_device *u
usb_disable_endpoint(udev, 0 + USB_DIR_IN, true);
usb_disable_endpoint(udev, 0 + USB_DIR_OUT, true);
usb_enable_endpoint(udev, &udev->ep0, true);
+ udev->alan1 = 0;
}
EXPORT_SYMBOL_GPL(usb_ep0_reinit);

@@ -4471,6 +4472,7 @@ static int hub_set_address(struct usb_de
update_devnum(udev, devnum);
/* Device now using proper address. */
usb_set_device_state(udev, USB_STATE_ADDRESS);
+ udev->alan1 = 1;
usb_ep0_reinit(udev);
}
return retval;
@@ -4838,6 +4840,7 @@ hub_port_init(struct usb_hub *hub, struc
else
dev_warn(&udev->dev, "Using ep0 maxpacket: %d\n", i);
udev->ep0.desc.wMaxPacketSize = cpu_to_le16(i);
+ udev->alan1 = 2;
usb_ep0_reinit(udev);
}

@@ -5226,6 +5229,7 @@ static void hub_port_connect(struct usb_
loop_disable:
hub_port_disable(hub, port1, 1);
loop:
+ udev->alan1 = 3;
usb_ep0_reinit(udev);
release_devnum(udev);
hub_free_dev(udev);
@@ -5766,6 +5770,7 @@ static int usb_reset_and_verify_device(s

/* ep0 maxpacket size may change; let the HCD know about it.
* Other endpoints will be handled by re-enumeration. */
+ udev->alan1 = 4;
usb_ep0_reinit(udev);
ret = hub_port_init(parent_hub, udev, port1, i);
if (ret >= 0 || ret == -ENOTCONN || ret == -ENODEV)
Index: usb-devel/drivers/usb/core/urb.c
===================================================================
--- usb-devel.orig/drivers/usb/core/urb.c
+++ usb-devel/drivers/usb/core/urb.c
@@ -204,8 +204,12 @@ int usb_urb_ep_type_check(const struct u
const struct usb_host_endpoint *ep;

ep = usb_pipe_endpoint(urb->dev, urb->pipe);
- if (!ep)
+ if (!ep) {
+ dev_info(&urb->dev->dev, "Ep %d disabled: %d\n",
+ usb_pipeendpoint(urb->pipe),
+ urb->dev->alan1);
return -EINVAL;
+ }
if (usb_pipetype(urb->pipe) != pipetypes[usb_endpoint_type(&ep->desc)])
return -EINVAL;
return 0;
Index: usb-devel/include/linux/usb.h
===================================================================
--- usb-devel.orig/include/linux/usb.h
+++ usb-devel/include/linux/usb.h
@@ -629,6 +629,7 @@ struct usb3_lpm_parameters {
* usb_set_device_state().
*/
struct usb_device {
+ int alan1;
int devnum;
char devpath[16];
u32 route;

2020-04-23 17:22:32

by syzbot

[permalink] [raw]
Subject: Re: WARNING in usbhid_raw_request/usb_submit_urb (3)

Hello,

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

Reported-and-tested-by: [email protected]

Tested on:

commit: 0fa84af8 Merge tag 'usb-serial-5.7-rc1' of https://git.ker..
git tree: https://github.com/google/kasan.git
kernel config: https://syzkaller.appspot.com/x/.config?x=6b9c154b0c23aecf
dashboard link: https://syzkaller.appspot.com/bug?extid=db339689b2101f6f6071
compiler: gcc (GCC) 9.0.0 20181231 (experimental)
patch: https://syzkaller.appspot.com/x/patch.diff?x=1027bca0100000

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

2020-04-23 19:41:21

by Alan Stern

[permalink] [raw]
Subject: Re: WARNING in usbhid_raw_request/usb_submit_urb (3)

On Thu, 23 Apr 2020, syzbot wrote:

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

Bad luck, I guess. Let's try again.

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

Index: usb-devel/drivers/usb/core/hub.c
===================================================================
--- usb-devel.orig/drivers/usb/core/hub.c
+++ usb-devel/drivers/usb/core/hub.c
@@ -4440,6 +4440,7 @@ void usb_ep0_reinit(struct usb_device *u
usb_disable_endpoint(udev, 0 + USB_DIR_IN, true);
usb_disable_endpoint(udev, 0 + USB_DIR_OUT, true);
usb_enable_endpoint(udev, &udev->ep0, true);
+ udev->alan1 = 0;
}
EXPORT_SYMBOL_GPL(usb_ep0_reinit);

@@ -4471,6 +4472,7 @@ static int hub_set_address(struct usb_de
update_devnum(udev, devnum);
/* Device now using proper address. */
usb_set_device_state(udev, USB_STATE_ADDRESS);
+ udev->alan1 = 1;
usb_ep0_reinit(udev);
}
return retval;
@@ -4838,6 +4840,7 @@ hub_port_init(struct usb_hub *hub, struc
else
dev_warn(&udev->dev, "Using ep0 maxpacket: %d\n", i);
udev->ep0.desc.wMaxPacketSize = cpu_to_le16(i);
+ udev->alan1 = 2;
usb_ep0_reinit(udev);
}

@@ -5226,6 +5229,7 @@ static void hub_port_connect(struct usb_
loop_disable:
hub_port_disable(hub, port1, 1);
loop:
+ udev->alan1 = 3;
usb_ep0_reinit(udev);
release_devnum(udev);
hub_free_dev(udev);
@@ -5766,6 +5770,7 @@ static int usb_reset_and_verify_device(s

/* ep0 maxpacket size may change; let the HCD know about it.
* Other endpoints will be handled by re-enumeration. */
+ udev->alan1 = 4;
usb_ep0_reinit(udev);
ret = hub_port_init(parent_hub, udev, port1, i);
if (ret >= 0 || ret == -ENOTCONN || ret == -ENODEV)
Index: usb-devel/drivers/usb/core/urb.c
===================================================================
--- usb-devel.orig/drivers/usb/core/urb.c
+++ usb-devel/drivers/usb/core/urb.c
@@ -204,8 +204,12 @@ int usb_urb_ep_type_check(const struct u
const struct usb_host_endpoint *ep;

ep = usb_pipe_endpoint(urb->dev, urb->pipe);
- if (!ep)
+ if (!ep) {
+ dev_info(&urb->dev->dev, "Ep %d disabled: %d\n",
+ usb_pipeendpoint(urb->pipe),
+ urb->dev->alan1);
return -EINVAL;
+ }
if (usb_pipetype(urb->pipe) != pipetypes[usb_endpoint_type(&ep->desc)])
return -EINVAL;
return 0;
Index: usb-devel/include/linux/usb.h
===================================================================
--- usb-devel.orig/include/linux/usb.h
+++ usb-devel/include/linux/usb.h
@@ -629,6 +629,7 @@ struct usb3_lpm_parameters {
* usb_set_device_state().
*/
struct usb_device {
+ int alan1;
int devnum;
char devpath[16];
u32 route;

2020-04-23 20:41:47

by syzbot

[permalink] [raw]
Subject: Re: WARNING in usbhid_raw_request/usb_submit_urb (3)

Hello,

syzbot has tested the proposed patch but the reproducer still triggered crash:
WARNING in usbhid_raw_request/usb_submit_urb

usb 2-1: Ep 0 disabled: 4
------------[ cut here ]------------
usb 2-1: BOGUS urb xfer, pipe 2 != type 2
WARNING: CPU: 1 PID: 4177 at drivers/usb/core/urb.c:482 usb_submit_urb+0xa38/0x1380 drivers/usb/core/urb.c:482
Kernel panic - not syncing: panic_on_warn set ...
CPU: 1 PID: 4177 Comm: syz-executor.1 Not tainted 5.6.0-rc7-syzkaller #0
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+0xef/0x16e lib/dump_stack.c:118
panic+0x2aa/0x6e1 kernel/panic.c:221
__warn.cold+0x2f/0x30 kernel/panic.c:582
report_bug+0x27b/0x2f0 lib/bug.c:195
fixup_bug arch/x86/kernel/traps.c:174 [inline]
fixup_bug arch/x86/kernel/traps.c:169 [inline]
do_error_trap+0x12b/0x1e0 arch/x86/kernel/traps.c:267
do_invalid_op+0x32/0x40 arch/x86/kernel/traps.c:286
invalid_op+0x23/0x30 arch/x86/entry/entry_64.S:1027
RIP: 0010:usb_submit_urb+0xa38/0x1380 drivers/usb/core/urb.c:482
Code: 84 40 04 00 00 e8 28 8a dd fd 4c 89 ef e8 30 5a 17 ff 41 89 d8 44 89 e1 4c 89 f2 48 89 c6 48 c7 c7 c0 dd 3b 86 e8 d0 1a b2 fd <0f> 0b e8 01 8a dd fd 0f b6 6c 24 08 bb 86 03 00 00 48 c7 c6 20 df
RSP: 0018:ffff8881cfcefb30 EFLAGS: 00010282
RAX: 0000000000000000 RBX: 0000000000000002 RCX: 0000000000000000
RDX: 0000000000000000 RSI: ffffffff812974dd RDI: ffffed1039f9df58
RBP: 0000000000000000 R08: ffff8881d8d01880 R09: ffffed103b66439f
R10: ffffed103b66439e R11: ffff8881db321cf3 R12: 0000000000000002
R13: ffff8881c5cd60a8 R14: ffff8881c6021ed0 R15: ffff8881d9572100
usb_start_wait_urb+0x108/0x4c0 drivers/usb/core/message.c:58
usb_internal_control_msg drivers/usb/core/message.c:102 [inline]
usb_control_msg+0x31c/0x4a0 drivers/usb/core/message.c:153
usbhid_set_raw_report drivers/hid/usbhid/hid-core.c:917 [inline]
usbhid_raw_request+0x225/0x620 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+0x620/0xaf0 drivers/hid/hidraw.c:422
vfs_ioctl fs/ioctl.c:47 [inline]
ksys_ioctl+0x11a/0x180 fs/ioctl.c:763
__do_sys_ioctl fs/ioctl.c:772 [inline]
__se_sys_ioctl fs/ioctl.c:770 [inline]
__x64_sys_ioctl+0x6f/0xb0 fs/ioctl.c:770
do_syscall_64+0xb6/0x5a0 arch/x86/entry/common.c:294
entry_SYSCALL_64_after_hwframe+0x49/0xbe
RIP: 0033:0x45c849
Code: ad b6 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 7b b6 fb ff c3 66 2e 0f 1f 84 00 00 00 00
RSP: 002b:00007fc247305c78 EFLAGS: 00000246 ORIG_RAX: 0000000000000010
RAX: ffffffffffffffda RBX: 00007fc2473066d4 RCX: 000000000045c849
RDX: 00000000200000c0 RSI: 0000000080404806 RDI: 0000000000000006
RBP: 000000000076bfa0 R08: 0000000000000000 R09: 0000000000000000
R10: 0000000000000000 R11: 0000000000000246 R12: 00000000ffffffff
R13: 0000000000000335 R14: 00000000004c59df R15: 000000000076bfac
Kernel Offset: disabled
Rebooting in 86400 seconds..


Tested on:

commit: 0fa84af8 Merge tag 'usb-serial-5.7-rc1' of https://git.ker..
git tree: https://github.com/google/kasan.git
console output: https://syzkaller.appspot.com/x/log.txt?x=143e9b9be00000
kernel config: https://syzkaller.appspot.com/x/.config?x=6b9c154b0c23aecf
dashboard link: https://syzkaller.appspot.com/bug?extid=db339689b2101f6f6071
compiler: gcc (GCC) 9.0.0 20181231 (experimental)
patch: https://syzkaller.appspot.com/x/patch.diff?x=1067e1bbe00000

2020-04-23 21:11:39

by Alan Stern

[permalink] [raw]
Subject: Re: WARNING in usbhid_raw_request/usb_submit_urb (3)

On Thu, 23 Apr 2020, syzbot wrote:

> Hello,
>
> syzbot has tested the proposed patch but the reproducer still triggered crash:
> WARNING in usbhid_raw_request/usb_submit_urb
>
> usb 2-1: Ep 0 disabled: 4

Nasty! This indicates an URB is being submitted while the device is
being reset.

The URB comes from an hidraw ioctl request. I wonder where the reset
comes from? We can try to find out -- but I don't think this will lead
directly to a solution.

In general there is no mutual exclusion between USB I/O and resets.
Unless the reset came through the usbhid driver, I don't see how we can
prevent this from happening again.

I suppose we _could_ change the code so that it would be treated as an
ordinary I/O error, maybe with dev_warn instead of dev_WARN. After
all, failure to find ep 0 can hardly be called a driver bug. Anyway,
let's see what the next test turns up.

Alan Stern

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

Index: usb-devel/drivers/usb/core/hub.c
===================================================================
--- usb-devel.orig/drivers/usb/core/hub.c
+++ usb-devel/drivers/usb/core/hub.c
@@ -4440,6 +4440,7 @@ void usb_ep0_reinit(struct usb_device *u
usb_disable_endpoint(udev, 0 + USB_DIR_IN, true);
usb_disable_endpoint(udev, 0 + USB_DIR_OUT, true);
usb_enable_endpoint(udev, &udev->ep0, true);
+ udev->alan1 = 0;
}
EXPORT_SYMBOL_GPL(usb_ep0_reinit);

@@ -4471,6 +4472,7 @@ static int hub_set_address(struct usb_de
update_devnum(udev, devnum);
/* Device now using proper address. */
usb_set_device_state(udev, USB_STATE_ADDRESS);
+ udev->alan1 = 1;
usb_ep0_reinit(udev);
}
return retval;
@@ -4838,6 +4840,7 @@ hub_port_init(struct usb_hub *hub, struc
else
dev_warn(&udev->dev, "Using ep0 maxpacket: %d\n", i);
udev->ep0.desc.wMaxPacketSize = cpu_to_le16(i);
+ udev->alan1 = 2;
usb_ep0_reinit(udev);
}

@@ -5226,6 +5229,7 @@ static void hub_port_connect(struct usb_
loop_disable:
hub_port_disable(hub, port1, 1);
loop:
+ udev->alan1 = 3;
usb_ep0_reinit(udev);
release_devnum(udev);
hub_free_dev(udev);
@@ -5762,10 +5766,14 @@ static int usb_reset_and_verify_device(s
bos = udev->bos;
udev->bos = NULL;

+ dev_info(&udev->dev, "Device reset\n");
+ dump_stack();
+
for (i = 0; i < SET_CONFIG_TRIES; ++i) {

/* ep0 maxpacket size may change; let the HCD know about it.
* Other endpoints will be handled by re-enumeration. */
+ udev->alan1 = 4;
usb_ep0_reinit(udev);
ret = hub_port_init(parent_hub, udev, port1, i);
if (ret >= 0 || ret == -ENOTCONN || ret == -ENODEV)
Index: usb-devel/drivers/usb/core/urb.c
===================================================================
--- usb-devel.orig/drivers/usb/core/urb.c
+++ usb-devel/drivers/usb/core/urb.c
@@ -204,8 +204,12 @@ int usb_urb_ep_type_check(const struct u
const struct usb_host_endpoint *ep;

ep = usb_pipe_endpoint(urb->dev, urb->pipe);
- if (!ep)
+ if (!ep) {
+ dev_info(&urb->dev->dev, "Ep %d disabled: %d\n",
+ usb_pipeendpoint(urb->pipe),
+ urb->dev->alan1);
return -EINVAL;
+ }
if (usb_pipetype(urb->pipe) != pipetypes[usb_endpoint_type(&ep->desc)])
return -EINVAL;
return 0;
Index: usb-devel/include/linux/usb.h
===================================================================
--- usb-devel.orig/include/linux/usb.h
+++ usb-devel/include/linux/usb.h
@@ -629,6 +629,7 @@ struct usb3_lpm_parameters {
* usb_set_device_state().
*/
struct usb_device {
+ int alan1;
int devnum;
char devpath[16];
u32 route;

2020-04-23 21:55:20

by syzbot

[permalink] [raw]
Subject: Re: WARNING in usbhid_raw_request/usb_submit_urb (3)

Hello,

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

Reported-and-tested-by: [email protected]

Tested on:

commit: 0fa84af8 Merge tag 'usb-serial-5.7-rc1' of https://git.ker..
git tree: https://github.com/google/kasan.git
kernel config: https://syzkaller.appspot.com/x/.config?x=6b9c154b0c23aecf
dashboard link: https://syzkaller.appspot.com/bug?extid=db339689b2101f6f6071
compiler: gcc (GCC) 9.0.0 20181231 (experimental)
patch: https://syzkaller.appspot.com/x/patch.diff?x=126fd687e00000

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

2020-04-24 01:03:46

by Alan Stern

[permalink] [raw]
Subject: Re: WARNING in usbhid_raw_request/usb_submit_urb (3)

On Thu, 23 Apr 2020, syzbot wrote:

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

Okay, try again.

Alan Stern

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

Index: usb-devel/drivers/usb/core/hub.c
===================================================================
--- usb-devel.orig/drivers/usb/core/hub.c
+++ usb-devel/drivers/usb/core/hub.c
@@ -4440,6 +4440,7 @@ void usb_ep0_reinit(struct usb_device *u
usb_disable_endpoint(udev, 0 + USB_DIR_IN, true);
usb_disable_endpoint(udev, 0 + USB_DIR_OUT, true);
usb_enable_endpoint(udev, &udev->ep0, true);
+ udev->alan1 = 0;
}
EXPORT_SYMBOL_GPL(usb_ep0_reinit);

@@ -4471,6 +4472,7 @@ static int hub_set_address(struct usb_de
update_devnum(udev, devnum);
/* Device now using proper address. */
usb_set_device_state(udev, USB_STATE_ADDRESS);
+ udev->alan1 = 1;
usb_ep0_reinit(udev);
}
return retval;
@@ -4838,6 +4840,7 @@ hub_port_init(struct usb_hub *hub, struc
else
dev_warn(&udev->dev, "Using ep0 maxpacket: %d\n", i);
udev->ep0.desc.wMaxPacketSize = cpu_to_le16(i);
+ udev->alan1 = 2;
usb_ep0_reinit(udev);
}

@@ -5226,6 +5229,7 @@ static void hub_port_connect(struct usb_
loop_disable:
hub_port_disable(hub, port1, 1);
loop:
+ udev->alan1 = 3;
usb_ep0_reinit(udev);
release_devnum(udev);
hub_free_dev(udev);
@@ -5762,10 +5766,14 @@ static int usb_reset_and_verify_device(s
bos = udev->bos;
udev->bos = NULL;

+ dev_info(&udev->dev, "Device reset\n");
+ dump_stack();
+
for (i = 0; i < SET_CONFIG_TRIES; ++i) {

/* ep0 maxpacket size may change; let the HCD know about it.
* Other endpoints will be handled by re-enumeration. */
+ udev->alan1 = 4;
usb_ep0_reinit(udev);
ret = hub_port_init(parent_hub, udev, port1, i);
if (ret >= 0 || ret == -ENOTCONN || ret == -ENODEV)
Index: usb-devel/drivers/usb/core/urb.c
===================================================================
--- usb-devel.orig/drivers/usb/core/urb.c
+++ usb-devel/drivers/usb/core/urb.c
@@ -204,8 +204,12 @@ int usb_urb_ep_type_check(const struct u
const struct usb_host_endpoint *ep;

ep = usb_pipe_endpoint(urb->dev, urb->pipe);
- if (!ep)
+ if (!ep) {
+ dev_info(&urb->dev->dev, "Ep %d disabled: %d\n",
+ usb_pipeendpoint(urb->pipe),
+ urb->dev->alan1);
return -EINVAL;
+ }
if (usb_pipetype(urb->pipe) != pipetypes[usb_endpoint_type(&ep->desc)])
return -EINVAL;
return 0;
Index: usb-devel/include/linux/usb.h
===================================================================
--- usb-devel.orig/include/linux/usb.h
+++ usb-devel/include/linux/usb.h
@@ -629,6 +629,7 @@ struct usb3_lpm_parameters {
* usb_set_device_state().
*/
struct usb_device {
+ int alan1;
int devnum;
char devpath[16];
u32 route;


2020-04-24 01:22:05

by syzbot

[permalink] [raw]
Subject: Re: WARNING in usbhid_raw_request/usb_submit_urb (3)

Hello,

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

Reported-and-tested-by: [email protected]

Tested on:

commit: 0fa84af8 Merge tag 'usb-serial-5.7-rc1' of https://git.ker..
git tree: https://github.com/google/kasan.git
kernel config: https://syzkaller.appspot.com/x/.config?x=6b9c154b0c23aecf
dashboard link: https://syzkaller.appspot.com/bug?extid=db339689b2101f6f6071
compiler: gcc (GCC) 9.0.0 20181231 (experimental)
patch: https://syzkaller.appspot.com/x/patch.diff?x=136ca310100000

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

2020-04-24 01:41:58

by Alan Stern

[permalink] [raw]
Subject: Re: WARNING in usbhid_raw_request/usb_submit_urb (3)

On Thu, 23 Apr 2020, syzbot wrote:

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

Forget about trying to catch the race, let's just see what's going on.

Alan Stern

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

Index: usb-devel/drivers/usb/core/hub.c
===================================================================
--- usb-devel.orig/drivers/usb/core/hub.c
+++ usb-devel/drivers/usb/core/hub.c
@@ -4440,6 +4440,7 @@ void usb_ep0_reinit(struct usb_device *u
usb_disable_endpoint(udev, 0 + USB_DIR_IN, true);
usb_disable_endpoint(udev, 0 + USB_DIR_OUT, true);
usb_enable_endpoint(udev, &udev->ep0, true);
+ udev->alan1 = 0;
}
EXPORT_SYMBOL_GPL(usb_ep0_reinit);

@@ -4471,6 +4472,7 @@ static int hub_set_address(struct usb_de
update_devnum(udev, devnum);
/* Device now using proper address. */
usb_set_device_state(udev, USB_STATE_ADDRESS);
+ udev->alan1 = 1;
usb_ep0_reinit(udev);
}
return retval;
@@ -4838,6 +4840,7 @@ hub_port_init(struct usb_hub *hub, struc
else
dev_warn(&udev->dev, "Using ep0 maxpacket: %d\n", i);
udev->ep0.desc.wMaxPacketSize = cpu_to_le16(i);
+ udev->alan1 = 2;
usb_ep0_reinit(udev);
}

@@ -5226,6 +5229,7 @@ static void hub_port_connect(struct usb_
loop_disable:
hub_port_disable(hub, port1, 1);
loop:
+ udev->alan1 = 3;
usb_ep0_reinit(udev);
release_devnum(udev);
hub_free_dev(udev);
@@ -5762,10 +5766,13 @@ static int usb_reset_and_verify_device(s
bos = udev->bos;
udev->bos = NULL;

+ dev_WARN(&udev->dev, "Device reset\n");
+
for (i = 0; i < SET_CONFIG_TRIES; ++i) {

/* ep0 maxpacket size may change; let the HCD know about it.
* Other endpoints will be handled by re-enumeration. */
+ udev->alan1 = 4;
usb_ep0_reinit(udev);
ret = hub_port_init(parent_hub, udev, port1, i);
if (ret >= 0 || ret == -ENOTCONN || ret == -ENODEV)
Index: usb-devel/drivers/usb/core/urb.c
===================================================================
--- usb-devel.orig/drivers/usb/core/urb.c
+++ usb-devel/drivers/usb/core/urb.c
@@ -204,8 +204,12 @@ int usb_urb_ep_type_check(const struct u
const struct usb_host_endpoint *ep;

ep = usb_pipe_endpoint(urb->dev, urb->pipe);
- if (!ep)
+ if (!ep) {
+ dev_info(&urb->dev->dev, "Ep %d disabled: %d\n",
+ usb_pipeendpoint(urb->pipe),
+ urb->dev->alan1);
return -EINVAL;
+ }
if (usb_pipetype(urb->pipe) != pipetypes[usb_endpoint_type(&ep->desc)])
return -EINVAL;
return 0;
Index: usb-devel/include/linux/usb.h
===================================================================
--- usb-devel.orig/include/linux/usb.h
+++ usb-devel/include/linux/usb.h
@@ -629,6 +629,7 @@ struct usb3_lpm_parameters {
* usb_set_device_state().
*/
struct usb_device {
+ int alan1;
int devnum;
char devpath[16];
u32 route;

2020-04-24 02:14:06

by syzbot

[permalink] [raw]
Subject: Re: WARNING in usbhid_raw_request/usb_submit_urb (3)

Hello,

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

------------[ cut here ]------------
usb 1-1: Device reset
WARNING: CPU: 0 PID: 94 at drivers/usb/core/hub.c:5761 usb_reset_and_verify_device+0x37d/0xce0 drivers/usb/core/hub.c:5761
Kernel panic - not syncing: panic_on_warn set ...
CPU: 0 PID: 94 Comm: kworker/0:2 Not tainted 5.6.0-rc7-syzkaller #0
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 01/01/2011
Workqueue: events __usb_queue_reset_device
Call Trace:
__dump_stack lib/dump_stack.c:77 [inline]
dump_stack+0xef/0x16e lib/dump_stack.c:118
panic+0x2aa/0x6e1 kernel/panic.c:221
__warn.cold+0x2f/0x30 kernel/panic.c:582
report_bug+0x27b/0x2f0 lib/bug.c:195
fixup_bug arch/x86/kernel/traps.c:174 [inline]
fixup_bug arch/x86/kernel/traps.c:169 [inline]
do_error_trap+0x12b/0x1e0 arch/x86/kernel/traps.c:267
do_invalid_op+0x32/0x40 arch/x86/kernel/traps.c:286
invalid_op+0x23/0x30 arch/x86/entry/entry_64.S:1027
RIP: 0010:usb_reset_and_verify_device+0x37d/0xce0 drivers/usb/core/hub.c:5761
Code: 0f 84 9e 06 00 00 e8 82 89 df fd 49 8d bc 24 a8 00 00 00 e8 85 59 19 ff 4c 89 ea 48 c7 c7 e0 86 3b 86 48 89 c6 e8 2b 1a b4 fd <0f> 0b 4d 89 e5 4c 89 34 24 44 8b 74 24 3c 48 b8 00 00 00 00 00 fc
RSP: 0018:ffff8881d5d7fb98 EFLAGS: 00010286
RAX: 0000000000000000 RBX: ffff8881d4d70000 RCX: 0000000000000000
RDX: 0000000000000000 RSI: ffffffff812974dd RDI: ffffed103abaff65
RBP: 0000000000000000 R08: ffff8881d6b01880 R09: ffffed103b646248
R10: ffffed103b646247 R11: ffff8881db23123f R12: ffff8881d8f1e000
R13: ffff8881c9381900 R14: ffff8881d4ce5000 R15: ffff8881d8f1e560
usb_reset_device+0x4bb/0x960 drivers/usb/core/hub.c:5946
__usb_queue_reset_device+0x68/0x90 drivers/usb/core/message.c:1754
process_one_work+0x94b/0x1620 kernel/workqueue.c:2266
worker_thread+0x96/0xe20 kernel/workqueue.c:2412
kthread+0x318/0x420 kernel/kthread.c:255
ret_from_fork+0x24/0x30 arch/x86/entry/entry_64.S:352
Kernel Offset: disabled
Rebooting in 86400 seconds..


Tested on:

commit: 0fa84af8 Merge tag 'usb-serial-5.7-rc1' of https://git.ker..
git tree: https://github.com/google/kasan.git
console output: https://syzkaller.appspot.com/x/log.txt?x=10d0c580100000
kernel config: https://syzkaller.appspot.com/x/.config?x=6b9c154b0c23aecf
dashboard link: https://syzkaller.appspot.com/bug?extid=db339689b2101f6f6071
compiler: gcc (GCC) 9.0.0 20181231 (experimental)
patch: https://syzkaller.appspot.com/x/patch.diff?x=14ab96a0100000

2020-04-24 12:22:41

by Alan Stern

[permalink] [raw]
Subject: Re: WARNING in usbhid_raw_request/usb_submit_urb (3)

On Thu, 23 Apr 2020, syzbot wrote:

> Hello,
>
> syzbot has tested the proposed patch but the reproducer still triggered crash:
> WARNING in usb_reset_and_verify_device
>
> ------------[ cut here ]------------
> usb 1-1: Device reset

> usb_reset_device+0x4bb/0x960 drivers/usb/core/hub.c:5946
> __usb_queue_reset_device+0x68/0x90 drivers/usb/core/message.c:1754
> process_one_work+0x94b/0x1620 kernel/workqueue.c:2266

The reset was queued. Let's see who queued it.

Alan Stern

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

Index: usb-devel/drivers/usb/core/hub.c
===================================================================
--- usb-devel.orig/drivers/usb/core/hub.c
+++ usb-devel/drivers/usb/core/hub.c
@@ -4440,6 +4440,7 @@ void usb_ep0_reinit(struct usb_device *u
usb_disable_endpoint(udev, 0 + USB_DIR_IN, true);
usb_disable_endpoint(udev, 0 + USB_DIR_OUT, true);
usb_enable_endpoint(udev, &udev->ep0, true);
+ udev->alan1 = 0;
}
EXPORT_SYMBOL_GPL(usb_ep0_reinit);

@@ -4471,6 +4472,7 @@ static int hub_set_address(struct usb_de
update_devnum(udev, devnum);
/* Device now using proper address. */
usb_set_device_state(udev, USB_STATE_ADDRESS);
+ udev->alan1 = 1;
usb_ep0_reinit(udev);
}
return retval;
@@ -4838,6 +4840,7 @@ hub_port_init(struct usb_hub *hub, struc
else
dev_warn(&udev->dev, "Using ep0 maxpacket: %d\n", i);
udev->ep0.desc.wMaxPacketSize = cpu_to_le16(i);
+ udev->alan1 = 2;
usb_ep0_reinit(udev);
}

@@ -5226,6 +5229,7 @@ static void hub_port_connect(struct usb_
loop_disable:
hub_port_disable(hub, port1, 1);
loop:
+ udev->alan1 = 3;
usb_ep0_reinit(udev);
release_devnum(udev);
hub_free_dev(udev);
@@ -5766,6 +5770,7 @@ static int usb_reset_and_verify_device(s

/* ep0 maxpacket size may change; let the HCD know about it.
* Other endpoints will be handled by re-enumeration. */
+ udev->alan1 = 4;
usb_ep0_reinit(udev);
ret = hub_port_init(parent_hub, udev, port1, i);
if (ret >= 0 || ret == -ENOTCONN || ret == -ENODEV)
@@ -6007,6 +6012,8 @@ EXPORT_SYMBOL_GPL(usb_reset_device);
*/
void usb_queue_reset_device(struct usb_interface *iface)
{
+ dev_WARN(&iface->dev, "Device reset\n");
+
if (schedule_work(&iface->reset_ws))
usb_get_intf(iface);
}
Index: usb-devel/drivers/usb/core/urb.c
===================================================================
--- usb-devel.orig/drivers/usb/core/urb.c
+++ usb-devel/drivers/usb/core/urb.c
@@ -204,8 +204,12 @@ int usb_urb_ep_type_check(const struct u
const struct usb_host_endpoint *ep;

ep = usb_pipe_endpoint(urb->dev, urb->pipe);
- if (!ep)
+ if (!ep) {
+ dev_info(&urb->dev->dev, "Ep %d disabled: %d\n",
+ usb_pipeendpoint(urb->pipe),
+ urb->dev->alan1);
return -EINVAL;
+ }
if (usb_pipetype(urb->pipe) != pipetypes[usb_endpoint_type(&ep->desc)])
return -EINVAL;
return 0;
Index: usb-devel/include/linux/usb.h
===================================================================
--- usb-devel.orig/include/linux/usb.h
+++ usb-devel/include/linux/usb.h
@@ -629,6 +629,7 @@ struct usb3_lpm_parameters {
* usb_set_device_state().
*/
struct usb_device {
+ int alan1;
int devnum;
char devpath[16];
u32 route;

2020-04-24 12:34:33

by syzbot

[permalink] [raw]
Subject: Re: WARNING in usbhid_raw_request/usb_submit_urb (3)

Hello,

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

------------[ cut here ]------------
usbhid 3-1:0.0: Device reset
WARNING: CPU: 0 PID: 12 at drivers/usb/core/hub.c:6007 usb_queue_reset_device+0x63/0x130 drivers/usb/core/hub.c:6007
Kernel panic - not syncing: panic_on_warn set ...
CPU: 0 PID: 12 Comm: kworker/0:1 Not tainted 5.6.0-rc7-syzkaller #0
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 01/01/2011
Workqueue: events hid_reset
Call Trace:
__dump_stack lib/dump_stack.c:77 [inline]
dump_stack+0xef/0x16e lib/dump_stack.c:118
panic+0x2aa/0x6e1 kernel/panic.c:221
__warn.cold+0x2f/0x30 kernel/panic.c:582
report_bug+0x27b/0x2f0 lib/bug.c:195
fixup_bug arch/x86/kernel/traps.c:174 [inline]
fixup_bug arch/x86/kernel/traps.c:169 [inline]
do_error_trap+0x12b/0x1e0 arch/x86/kernel/traps.c:267
do_invalid_op+0x32/0x40 arch/x86/kernel/traps.c:286
invalid_op+0x23/0x30 arch/x86/entry/entry_64.S:1027
RIP: 0010:usb_queue_reset_device+0x63/0x130 drivers/usb/core/hub.c:6007
Code: 00 00 4c 8d 6d 30 4d 85 e4 74 72 e8 47 02 e0 fd 4c 89 ef e8 4f d2 19 ff 4c 89 e2 48 c7 c7 20 79 3b 86 48 89 c6 e8 f5 92 b4 fd <0f> 0b 48 8d 95 f0 04 00 00 48 c7 c0 88 2a e6 87 48 b9 00 00 00 00
RSP: 0018:ffff8881da227ce0 EFLAGS: 00010286
RAX: 0000000000000000 RBX: ffff8881cd5ae920 RCX: 0000000000000000
RDX: 0000000000000000 RSI: ffffffff812974dd RDI: ffffed103b444f8e
RBP: ffff8881ce92d000 R08: ffff8881da211880 R09: ffffed103b646248
R10: ffffed103b646247 R11: ffff8881db23123f R12: ffff8881d4c25d80
R13: ffff8881ce92d030 R14: ffff8881da0ef400 R15: ffff8881db233e00
hid_reset+0x219/0x3e0 drivers/hid/usbhid/hid-core.c:138
process_one_work+0x94b/0x1620 kernel/workqueue.c:2266
worker_thread+0x96/0xe20 kernel/workqueue.c:2412
kthread+0x318/0x420 kernel/kthread.c:255
ret_from_fork+0x24/0x30 arch/x86/entry/entry_64.S:352
Kernel Offset: disabled
Rebooting in 86400 seconds..


Tested on:

commit: 0fa84af8 Merge tag 'usb-serial-5.7-rc1' of https://git.ker..
git tree: https://github.com/google/kasan.git
console output: https://syzkaller.appspot.com/x/log.txt?x=147a58a7e00000
kernel config: https://syzkaller.appspot.com/x/.config?x=6b9c154b0c23aecf
dashboard link: https://syzkaller.appspot.com/bug?extid=db339689b2101f6f6071
compiler: gcc (GCC) 9.0.0 20181231 (experimental)
patch: https://syzkaller.appspot.com/x/patch.diff?x=1131acdfe00000

2020-04-24 15:22:38

by Alan Stern

[permalink] [raw]
Subject: Re: WARNING in usbhid_raw_request/usb_submit_urb (3)

On Fri, 24 Apr 2020, syzbot wrote:

> Hello,
>
> syzbot has tested the proposed patch but the reproducer still triggered crash:
> WARNING in usb_queue_reset_device
>
> ------------[ cut here ]------------
> usbhid 3-1:0.0: Device reset
...
> hid_reset+0x219/0x3e0 drivers/hid/usbhid/hid-core.c:138
> process_one_work+0x94b/0x1620 kernel/workqueue.c:2266
> worker_thread+0x96/0xe20 kernel/workqueue.c:2412

Okay, that's weird. hid_reset is already running in a work queue
thread; it has no need to queue a request for a device reset. It can
just reset the device directly. Anyway, let's see where it gets called
from.

Alan Stern

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

Index: usb-devel/drivers/usb/core/hub.c
===================================================================
--- usb-devel.orig/drivers/usb/core/hub.c
+++ usb-devel/drivers/usb/core/hub.c
@@ -4440,6 +4440,7 @@ void usb_ep0_reinit(struct usb_device *u
usb_disable_endpoint(udev, 0 + USB_DIR_IN, true);
usb_disable_endpoint(udev, 0 + USB_DIR_OUT, true);
usb_enable_endpoint(udev, &udev->ep0, true);
+ udev->alan1 = 0;
}
EXPORT_SYMBOL_GPL(usb_ep0_reinit);

@@ -4471,6 +4472,7 @@ static int hub_set_address(struct usb_de
update_devnum(udev, devnum);
/* Device now using proper address. */
usb_set_device_state(udev, USB_STATE_ADDRESS);
+ udev->alan1 = 1;
usb_ep0_reinit(udev);
}
return retval;
@@ -4838,6 +4840,7 @@ hub_port_init(struct usb_hub *hub, struc
else
dev_warn(&udev->dev, "Using ep0 maxpacket: %d\n", i);
udev->ep0.desc.wMaxPacketSize = cpu_to_le16(i);
+ udev->alan1 = 2;
usb_ep0_reinit(udev);
}

@@ -5226,6 +5229,7 @@ static void hub_port_connect(struct usb_
loop_disable:
hub_port_disable(hub, port1, 1);
loop:
+ udev->alan1 = 3;
usb_ep0_reinit(udev);
release_devnum(udev);
hub_free_dev(udev);
@@ -5766,6 +5770,7 @@ static int usb_reset_and_verify_device(s

/* ep0 maxpacket size may change; let the HCD know about it.
* Other endpoints will be handled by re-enumeration. */
+ udev->alan1 = 4;
usb_ep0_reinit(udev);
ret = hub_port_init(parent_hub, udev, port1, i);
if (ret >= 0 || ret == -ENOTCONN || ret == -ENODEV)
@@ -6007,6 +6012,8 @@ EXPORT_SYMBOL_GPL(usb_reset_device);
*/
void usb_queue_reset_device(struct usb_interface *iface)
{
+ dev_WARN(&iface->dev, "Device reset\n");
+
if (schedule_work(&iface->reset_ws))
usb_get_intf(iface);
}
Index: usb-devel/drivers/usb/core/urb.c
===================================================================
--- usb-devel.orig/drivers/usb/core/urb.c
+++ usb-devel/drivers/usb/core/urb.c
@@ -204,8 +204,12 @@ int usb_urb_ep_type_check(const struct u
const struct usb_host_endpoint *ep;

ep = usb_pipe_endpoint(urb->dev, urb->pipe);
- if (!ep)
+ if (!ep) {
+ dev_info(&urb->dev->dev, "Ep %d disabled: %d\n",
+ usb_pipeendpoint(urb->pipe),
+ urb->dev->alan1);
return -EINVAL;
+ }
if (usb_pipetype(urb->pipe) != pipetypes[usb_endpoint_type(&ep->desc)])
return -EINVAL;
return 0;
Index: usb-devel/include/linux/usb.h
===================================================================
--- usb-devel.orig/include/linux/usb.h
+++ usb-devel/include/linux/usb.h
@@ -629,6 +629,7 @@ struct usb3_lpm_parameters {
* usb_set_device_state().
*/
struct usb_device {
+ int alan1;
int devnum;
char devpath[16];
u32 route;
Index: usb-devel/drivers/hid/usbhid/hid-core.c
===================================================================
--- usb-devel.orig/drivers/hid/usbhid/hid-core.c
+++ usb-devel/drivers/hid/usbhid/hid-core.c
@@ -135,7 +135,7 @@ static void hid_reset(struct work_struct

if (test_bit(HID_RESET_PENDING, &usbhid->iofl)) {
dev_dbg(&usbhid->intf->dev, "resetting device\n");
- usb_queue_reset_device(usbhid->intf);
+ usb_reset_device(interface_to_usbdev(usbhid->intf));
}
}

@@ -168,7 +168,7 @@ static void hid_io_error(struct hid_devi
/* Retries failed, so do a port reset unless we lack bandwidth*/
if (!test_bit(HID_NO_BANDWIDTH, &usbhid->iofl)
&& !test_and_set_bit(HID_RESET_PENDING, &usbhid->iofl)) {
-
+ dump_stack();
schedule_work(&usbhid->reset_work);
goto done;
}
@@ -299,6 +299,7 @@ static void hid_irq_in(struct urb *urb)
usbhid_mark_busy(usbhid);
clear_bit(HID_IN_RUNNING, &usbhid->iofl);
set_bit(HID_CLEAR_HALT, &usbhid->iofl);
+ dump_stack();
schedule_work(&usbhid->reset_work);
return;
case -ECONNRESET: /* unlink */
@@ -1438,8 +1439,10 @@ static void hid_restart_io(struct hid_de
clear_bit(HID_SUSPENDED, &usbhid->iofl);
usbhid_mark_busy(usbhid);

- if (clear_halt || reset_pending)
+ if (clear_halt || reset_pending) {
+ dump_stack();
schedule_work(&usbhid->reset_work);
+ }
usbhid->retry_delay = 0;
spin_unlock_irq(&usbhid->lock);


2020-04-24 15:36:43

by syzbot

[permalink] [raw]
Subject: Re: WARNING in usbhid_raw_request/usb_submit_urb (3)

Hello,

syzbot has tested the proposed patch but the reproducer still triggered crash:
INFO: task hung in usb_disable_device

INFO: task kworker/0:1:12 blocked for more than 143 seconds.
Not tainted 5.6.0-rc7-syzkaller #0
"echo 0 > /proc/sys/kernel/hung_task_timeout_secs" disables this message.
kworker/0:1 D23304 12 2 0x80004000
Workqueue: usb_hub_wq hub_event
Call Trace:
schedule+0xcd/0x2b0 kernel/sched/core.c:4154
schedule_preempt_disabled+0xc/0x20 kernel/sched/core.c:4213
__mutex_lock_common kernel/locking/mutex.c:1033 [inline]
__mutex_lock+0x87c/0x1360 kernel/locking/mutex.c:1103
device_lock include/linux/device.h:771 [inline]
device_del+0x9e/0xd30 drivers/base/core.c:2640
usb_disable_device+0x23d/0x7a0 drivers/usb/core/message.c:1238
usb_disconnect+0x2a5/0x920 drivers/usb/core/hub.c:2211
hub_port_connect drivers/usb/core/hub.c:5049 [inline]
hub_port_connect_change drivers/usb/core/hub.c:5339 [inline]
port_event drivers/usb/core/hub.c:5485 [inline]
hub_event+0x1a56/0x43b0 drivers/usb/core/hub.c:5567
process_one_work+0x94b/0x1620 kernel/workqueue.c:2266
worker_thread+0x96/0xe20 kernel/workqueue.c:2412
kthread+0x318/0x420 kernel/kthread.c:255
ret_from_fork+0x24/0x30 arch/x86/entry/entry_64.S:352
INFO: task kworker/1:0:17 blocked for more than 143 seconds.
Not tainted 5.6.0-rc7-syzkaller #0
"echo 0 > /proc/sys/kernel/hung_task_timeout_secs" disables this message.
kworker/1:0 D23392 17 2 0x80004000
Workqueue: usb_hub_wq hub_event
Call Trace:
schedule+0xcd/0x2b0 kernel/sched/core.c:4154
schedule_preempt_disabled+0xc/0x20 kernel/sched/core.c:4213
__mutex_lock_common kernel/locking/mutex.c:1033 [inline]
__mutex_lock+0x87c/0x1360 kernel/locking/mutex.c:1103
device_lock include/linux/device.h:771 [inline]
device_del+0x9e/0xd30 drivers/base/core.c:2640
usb_disable_device+0x23d/0x7a0 drivers/usb/core/message.c:1238
usb_disconnect+0x2a5/0x920 drivers/usb/core/hub.c:2211
hub_port_connect drivers/usb/core/hub.c:5049 [inline]
hub_port_connect_change drivers/usb/core/hub.c:5339 [inline]
port_event drivers/usb/core/hub.c:5485 [inline]
hub_event+0x1a56/0x43b0 drivers/usb/core/hub.c:5567
process_one_work+0x94b/0x1620 kernel/workqueue.c:2266
worker_thread+0x96/0xe20 kernel/workqueue.c:2412
kthread+0x318/0x420 kernel/kthread.c:255
ret_from_fork+0x24/0x30 arch/x86/entry/entry_64.S:352
INFO: task kworker/0:4:3216 blocked for more than 143 seconds.
Not tainted 5.6.0-rc7-syzkaller #0
"echo 0 > /proc/sys/kernel/hung_task_timeout_secs" disables this message.
kworker/0:4 D23696 3216 2 0x80004000
Workqueue: usb_hub_wq hub_event
Call Trace:
schedule+0xcd/0x2b0 kernel/sched/core.c:4154
schedule_preempt_disabled+0xc/0x20 kernel/sched/core.c:4213
__mutex_lock_common kernel/locking/mutex.c:1033 [inline]
__mutex_lock+0x87c/0x1360 kernel/locking/mutex.c:1103
__input_unregister_device+0x118/0x430 drivers/input/input.c:2087
input_unregister_device+0xb4/0xf0 drivers/input/input.c:2272
hidinput_disconnect+0x15e/0x3d0 drivers/hid/hid-input.c:1968
hid_disconnect+0x13f/0x1a0 drivers/hid/hid-core.c:2008
hid_hw_stop drivers/hid/hid-core.c:2055 [inline]
hid_device_remove+0x186/0x240 drivers/hid/hid-core.c:2298
__device_release_driver drivers/base/dd.c:1135 [inline]
device_release_driver_internal+0x231/0x500 drivers/base/dd.c:1168
bus_remove_device+0x2eb/0x5a0 drivers/base/bus.c:533
device_del+0x481/0xd30 drivers/base/core.c:2677
hid_remove_device drivers/hid/hid-core.c:2467 [inline]
hid_destroy_device+0xe1/0x150 drivers/hid/hid-core.c:2486
usbhid_disconnect+0x9f/0xe0 drivers/hid/usbhid/hid-core.c:1414
usb_unbind_interface+0x1c6/0x8a0 drivers/usb/core/driver.c:436
__device_release_driver drivers/base/dd.c:1137 [inline]
device_release_driver_internal+0x42f/0x500 drivers/base/dd.c:1168
bus_remove_device+0x2eb/0x5a0 drivers/base/bus.c:533
device_del+0x481/0xd30 drivers/base/core.c:2677
usb_disable_device+0x23d/0x7a0 drivers/usb/core/message.c:1238
usb_disconnect+0x2a5/0x920 drivers/usb/core/hub.c:2211
hub_port_connect drivers/usb/core/hub.c:5049 [inline]
hub_port_connect_change drivers/usb/core/hub.c:5339 [inline]
port_event drivers/usb/core/hub.c:5485 [inline]
hub_event+0x1a56/0x43b0 drivers/usb/core/hub.c:5567
process_one_work+0x94b/0x1620 kernel/workqueue.c:2266
worker_thread+0x96/0xe20 kernel/workqueue.c:2412
kthread+0x318/0x420 kernel/kthread.c:255
ret_from_fork+0x24/0x30 arch/x86/entry/entry_64.S:352
INFO: task kworker/0:5:3229 blocked for more than 144 seconds.
Not tainted 5.6.0-rc7-syzkaller #0
"echo 0 > /proc/sys/kernel/hung_task_timeout_secs" disables this message.
kworker/0:5 D27760 3229 2 0x80004000
Workqueue: events hid_reset
Call Trace:
schedule+0xcd/0x2b0 kernel/sched/core.c:4154
schedule_timeout+0x682/0xb20 kernel/time/timer.c:1871
do_wait_for_common kernel/sched/completion.c:83 [inline]
__wait_for_common kernel/sched/completion.c:104 [inline]
wait_for_common kernel/sched/completion.c:115 [inline]
wait_for_completion+0x26f/0x3c0 kernel/sched/completion.c:136
__flush_work+0x4f9/0xa80 kernel/workqueue.c:3043
__cancel_work_timer+0x32c/0x460 kernel/workqueue.c:3130
hid_cancel_delayed_stuff drivers/hid/usbhid/hid-core.c:1421 [inline]
usbhid_close+0x134/0x210 drivers/hid/usbhid/hid-core.c:750
hid_hw_close+0xa8/0xd0 drivers/hid/hid-core.c:2100
input_close_device+0x110/0x1a0 drivers/input/input.c:682
input_leds_disconnect+0xfd/0x160 drivers/input/input-leds.c:182
__input_unregister_device+0x1b0/0x430 drivers/input/input.c:2090
input_unregister_device+0xb4/0xf0 drivers/input/input.c:2272
hidinput_disconnect+0x15e/0x3d0 drivers/hid/hid-input.c:1968
hid_disconnect+0x13f/0x1a0 drivers/hid/hid-core.c:2008
hid_hw_stop drivers/hid/hid-core.c:2055 [inline]
hid_device_remove+0x186/0x240 drivers/hid/hid-core.c:2298
__device_release_driver drivers/base/dd.c:1135 [inline]
device_release_driver_internal+0x231/0x500 drivers/base/dd.c:1168
bus_remove_device+0x2eb/0x5a0 drivers/base/bus.c:533
device_del+0x481/0xd30 drivers/base/core.c:2677
hid_remove_device drivers/hid/hid-core.c:2467 [inline]
hid_destroy_device+0xe1/0x150 drivers/hid/hid-core.c:2486
usbhid_disconnect+0x9f/0xe0 drivers/hid/usbhid/hid-core.c:1414
usb_unbind_interface+0x1c6/0x8a0 drivers/usb/core/driver.c:436
__device_release_driver drivers/base/dd.c:1137 [inline]
device_release_driver_internal+0x42f/0x500 drivers/base/dd.c:1168
usb_driver_release_interface+0x102/0x180 drivers/usb/core/driver.c:609
usb_forced_unbind_intf+0x50/0xb0 drivers/usb/core/driver.c:1057
unbind_marked_interfaces.isra.0+0x15e/0x1c0 drivers/usb/core/driver.c:1080
usb_unbind_and_rebind_marked_interfaces+0x34/0x70 drivers/usb/core/driver.c:1141
usb_reset_device+0x742/0x960 drivers/usb/core/hub.c:5967
hid_reset+0x243/0x420 drivers/hid/usbhid/hid-core.c:138
process_one_work+0x94b/0x1620 kernel/workqueue.c:2266
worker_thread+0x96/0xe20 kernel/workqueue.c:2412
kthread+0x318/0x420 kernel/kthread.c:255
ret_from_fork+0x24/0x30 arch/x86/entry/entry_64.S:352
INFO: task kworker/0:6:3266 blocked for more than 144 seconds.
Not tainted 5.6.0-rc7-syzkaller #0
"echo 0 > /proc/sys/kernel/hung_task_timeout_secs" disables this message.
kworker/0:6 D27552 3266 2 0x80004000
Workqueue: events hid_reset
Call Trace:
schedule+0xcd/0x2b0 kernel/sched/core.c:4154
schedule_preempt_disabled+0xc/0x20 kernel/sched/core.c:4213
__mutex_lock_common kernel/locking/mutex.c:1033 [inline]
__mutex_lock+0x87c/0x1360 kernel/locking/mutex.c:1103
__input_unregister_device+0x118/0x430 drivers/input/input.c:2087
input_unregister_device+0xb4/0xf0 drivers/input/input.c:2272
hidinput_disconnect+0x15e/0x3d0 drivers/hid/hid-input.c:1968
hid_disconnect+0x13f/0x1a0 drivers/hid/hid-core.c:2008
hid_hw_stop drivers/hid/hid-core.c:2055 [inline]
hid_device_remove+0x186/0x240 drivers/hid/hid-core.c:2298
__device_release_driver drivers/base/dd.c:1135 [inline]
device_release_driver_internal+0x231/0x500 drivers/base/dd.c:1168
bus_remove_device+0x2eb/0x5a0 drivers/base/bus.c:533
device_del+0x481/0xd30 drivers/base/core.c:2677
hid_remove_device drivers/hid/hid-core.c:2467 [inline]
hid_destroy_device+0xe1/0x150 drivers/hid/hid-core.c:2486
usbhid_disconnect+0x9f/0xe0 drivers/hid/usbhid/hid-core.c:1414
usb_unbind_interface+0x1c6/0x8a0 drivers/usb/core/driver.c:436
__device_release_driver drivers/base/dd.c:1137 [inline]
device_release_driver_internal+0x42f/0x500 drivers/base/dd.c:1168
usb_driver_release_interface+0x102/0x180 drivers/usb/core/driver.c:609
usb_forced_unbind_intf+0x50/0xb0 drivers/usb/core/driver.c:1057
unbind_marked_interfaces.isra.0+0x15e/0x1c0 drivers/usb/core/driver.c:1080
usb_unbind_and_rebind_marked_interfaces+0x34/0x70 drivers/usb/core/driver.c:1141
usb_reset_device+0x742/0x960 drivers/usb/core/hub.c:5967
hid_reset+0x243/0x420 drivers/hid/usbhid/hid-core.c:138
process_one_work+0x94b/0x1620 kernel/workqueue.c:2266
worker_thread+0x96/0xe20 kernel/workqueue.c:2412
kthread+0x318/0x420 kernel/kthread.c:255
ret_from_fork+0x24/0x30 arch/x86/entry/entry_64.S:352
INFO: task kworker/1:4:3267 blocked for more than 144 seconds.
Not tainted 5.6.0-rc7-syzkaller #0
"echo 0 > /proc/sys/kernel/hung_task_timeout_secs" disables this message.
kworker/1:4 D28272 3267 2 0x80004000
Workqueue: events hid_reset
Call Trace:
schedule+0xcd/0x2b0 kernel/sched/core.c:4154
schedule_preempt_disabled+0xc/0x20 kernel/sched/core.c:4213
__mutex_lock_common kernel/locking/mutex.c:1033 [inline]
__mutex_lock+0x87c/0x1360 kernel/locking/mutex.c:1103
device_lock include/linux/device.h:771 [inline]
__device_attach+0x7b/0x390 drivers/base/dd.c:874
usb_rebind_intf drivers/usb/core/driver.c:1104 [inline]
rebind_marked_interfaces.isra.0+0x1ea/0x290 drivers/usb/core/driver.c:1127
usb_reset_device+0x742/0x960 drivers/usb/core/hub.c:5967
hid_reset+0x243/0x420 drivers/hid/usbhid/hid-core.c:138
process_one_work+0x94b/0x1620 kernel/workqueue.c:2266
worker_thread+0x96/0xe20 kernel/workqueue.c:2412
kthread+0x318/0x420 kernel/kthread.c:255
ret_from_fork+0x24/0x30 arch/x86/entry/entry_64.S:352

Showing all locks held in the system:
5 locks held by kworker/0:1/12:
#0: ffff8881d8944128 ((wq_completion)usb_hub_wq){+.+.}, at: __write_once_size include/linux/compiler.h:226 [inline]
#0: ffff8881d8944128 ((wq_completion)usb_hub_wq){+.+.}, at: arch_atomic64_set arch/x86/include/asm/atomic64_64.h:34 [inline]
#0: ffff8881d8944128 ((wq_completion)usb_hub_wq){+.+.}, at: atomic64_set include/asm-generic/atomic-instrumented.h:855 [inline]
#0: ffff8881d8944128 ((wq_completion)usb_hub_wq){+.+.}, at: atomic_long_set include/asm-generic/atomic-long.h:40 [inline]
#0: ffff8881d8944128 ((wq_completion)usb_hub_wq){+.+.}, at: set_work_data kernel/workqueue.c:615 [inline]
#0: ffff8881d8944128 ((wq_completion)usb_hub_wq){+.+.}, at: set_work_pool_and_clear_pending kernel/workqueue.c:642 [inline]
#0: ffff8881d8944128 ((wq_completion)usb_hub_wq){+.+.}, at: process_one_work+0x82a/0x1620 kernel/workqueue.c:2237
#1: ffff8881da227dd0 ((work_completion)(&hub->events)){+.+.}, at: process_one_work+0x85e/0x1620 kernel/workqueue.c:2241
#2: ffff8881d4d1e208 (&dev->mutex){....}, at: device_lock include/linux/device.h:771 [inline]
#2: ffff8881d4d1e208 (&dev->mutex){....}, at: hub_event+0x1be/0x43b0 drivers/usb/core/hub.c:5513
#3: ffff8881da2f3208 (&dev->mutex){....}, at: device_lock include/linux/device.h:771 [inline]
#3: ffff8881da2f3208 (&dev->mutex){....}, at: usb_disconnect+0xa3/0x920 drivers/usb/core/hub.c:2202
#4: ffff8881d8fac190 (&dev->mutex){....}, at: device_lock include/linux/device.h:771 [inline]
#4: ffff8881d8fac190 (&dev->mutex){....}, at: device_del+0x9e/0xd30 drivers/base/core.c:2640
5 locks held by kworker/1:0/17:
#0: ffff8881d8944128 ((wq_completion)usb_hub_wq){+.+.}, at: __write_once_size include/linux/compiler.h:226 [inline]
#0: ffff8881d8944128 ((wq_completion)usb_hub_wq){+.+.}, at: arch_atomic64_set arch/x86/include/asm/atomic64_64.h:34 [inline]
#0: ffff8881d8944128 ((wq_completion)usb_hub_wq){+.+.}, at: atomic64_set include/asm-generic/atomic-instrumented.h:855 [inline]
#0: ffff8881d8944128 ((wq_completion)usb_hub_wq){+.+.}, at: atomic_long_set include/asm-generic/atomic-long.h:40 [inline]
#0: ffff8881d8944128 ((wq_completion)usb_hub_wq){+.+.}, at: set_work_data kernel/workqueue.c:615 [inline]
#0: ffff8881d8944128 ((wq_completion)usb_hub_wq){+.+.}, at: set_work_pool_and_clear_pending kernel/workqueue.c:642 [inline]
#0: ffff8881d8944128 ((wq_completion)usb_hub_wq){+.+.}, at: process_one_work+0x82a/0x1620 kernel/workqueue.c:2237
#1: ffff8881da267dd0 ((work_completion)(&hub->events)){+.+.}, at: process_one_work+0x85e/0x1620 kernel/workqueue.c:2241
#2: ffff8881d4ccf208 (&dev->mutex){....}, at: device_lock include/linux/device.h:771 [inline]
#2: ffff8881d4ccf208 (&dev->mutex){....}, at: hub_event+0x1be/0x43b0 drivers/usb/core/hub.c:5513
#3: ffff8881c8d9e208 (&dev->mutex){....}, at: device_lock include/linux/device.h:771 [inline]
#3: ffff8881c8d9e208 (&dev->mutex){....}, at: usb_disconnect+0xa3/0x920 drivers/usb/core/hub.c:2202
#4: ffff8881d8fad190 (&dev->mutex){....}, at: device_lock include/linux/device.h:771 [inline]
#4: ffff8881d8fad190 (&dev->mutex){....}, at: device_del+0x9e/0xd30 drivers/base/core.c:2640
1 lock held by khungtaskd/23:
#0: ffffffff87108b60 (rcu_read_lock){....}, at: debug_show_all_locks+0x53/0x264 kernel/locking/lockdep.c:5331
7 locks held by kworker/1:2/83:
#0: ffff8881d8944128 ((wq_completion)usb_hub_wq){+.+.}, at: __write_once_size include/linux/compiler.h:226 [inline]
#0: ffff8881d8944128 ((wq_completion)usb_hub_wq){+.+.}, at: arch_atomic64_set arch/x86/include/asm/atomic64_64.h:34 [inline]
#0: ffff8881d8944128 ((wq_completion)usb_hub_wq){+.+.}, at: atomic64_set include/asm-generic/atomic-instrumented.h:855 [inline]
#0: ffff8881d8944128 ((wq_completion)usb_hub_wq){+.+.}, at: atomic_long_set include/asm-generic/atomic-long.h:40 [inline]
#0: ffff8881d8944128 ((wq_completion)usb_hub_wq){+.+.}, at: set_work_data kernel/workqueue.c:615 [inline]
#0: ffff8881d8944128 ((wq_completion)usb_hub_wq){+.+.}, at: set_work_pool_and_clear_pending kernel/workqueue.c:642 [inline]
#0: ffff8881d8944128 ((wq_completion)usb_hub_wq){+.+.}, at: process_one_work+0x82a/0x1620 kernel/workqueue.c:2237
#1: ffff8881d8a87dd0 ((work_completion)(&hub->events)){+.+.}, at: process_one_work+0x85e/0x1620 kernel/workqueue.c:2241
#2: ffff8881d4dc2208 (&dev->mutex){....}, at: device_lock include/linux/device.h:771 [inline]
#2: ffff8881d4dc2208 (&dev->mutex){....}, at: hub_event+0x1be/0x43b0 drivers/usb/core/hub.c:5513
#3: ffff8881d416f208 (&dev->mutex){....}, at: device_lock include/linux/device.h:771 [inline]
#3: ffff8881d416f208 (&dev->mutex){....}, at: __device_attach+0x7b/0x390 drivers/base/dd.c:874
#4: ffff8881d0a69190 (&dev->mutex){....}, at: device_lock include/linux/device.h:771 [inline]
#4: ffff8881d0a69190 (&dev->mutex){....}, at: __device_attach+0x7b/0x390 drivers/base/dd.c:874
#5: ffff8881c8219a88 (&dev->mutex){....}, at: device_lock include/linux/device.h:771 [inline]
#5: ffff8881c8219a88 (&dev->mutex){....}, at: __device_attach+0x7b/0x390 drivers/base/dd.c:874
#6: ffffffff877ca580 (input_mutex){+.+.}, at: input_register_device.cold+0x5c/0x316 drivers/input/input.c:2221
1 lock held by in:imklog/256:
#0: ffff8881c8ce1c60 (&f->f_pos_lock){+.+.}, at: __fdget_pos+0xe3/0x100 fs/file.c:826
7 locks held by kworker/0:4/3216:
#0: ffff8881d8944128 ((wq_completion)usb_hub_wq){+.+.}, at: __write_once_size include/linux/compiler.h:226 [inline]
#0: ffff8881d8944128 ((wq_completion)usb_hub_wq){+.+.}, at: arch_atomic64_set arch/x86/include/asm/atomic64_64.h:34 [inline]
#0: ffff8881d8944128 ((wq_completion)usb_hub_wq){+.+.}, at: atomic64_set include/asm-generic/atomic-instrumented.h:855 [inline]
#0: ffff8881d8944128 ((wq_completion)usb_hub_wq){+.+.}, at: atomic_long_set include/asm-generic/atomic-long.h:40 [inline]
#0: ffff8881d8944128 ((wq_completion)usb_hub_wq){+.+.}, at: set_work_data kernel/workqueue.c:615 [inline]
#0: ffff8881d8944128 ((wq_completion)usb_hub_wq){+.+.}, at: set_work_pool_and_clear_pending kernel/workqueue.c:642 [inline]
#0: ffff8881d8944128 ((wq_completion)usb_hub_wq){+.+.}, at: process_one_work+0x82a/0x1620 kernel/workqueue.c:2237
#1: ffff8881bd4d7dd0 ((work_completion)(&hub->events)){+.+.}, at: process_one_work+0x85e/0x1620 kernel/workqueue.c:2241
#2: ffff8881d4d4d208 (&dev->mutex){....}, at: device_lock include/linux/device.h:771 [inline]
#2: ffff8881d4d4d208 (&dev->mutex){....}, at: hub_event+0x1be/0x43b0 drivers/usb/core/hub.c:5513
#3: ffff8881d8eb3208 (&dev->mutex){....}, at: device_lock include/linux/device.h:771 [inline]
#3: ffff8881d8eb3208 (&dev->mutex){....}, at: usb_disconnect+0xa3/0x920 drivers/usb/core/hub.c:2202
#4: ffff8881d8fd0190 (&dev->mutex){....}, at: device_release_driver_internal+0x23/0x500 drivers/base/dd.c:1165
#5: ffff8881cd365a88 (&dev->mutex){....}, at: device_release_driver_internal+0x23/0x500 drivers/base/dd.c:1165
#6: ffffffff877ca580 (input_mutex){+.+.}, at: __input_unregister_device+0x118/0x430 drivers/input/input.c:2087
7 locks held by kworker/0:5/3229:
#0: ffff8881da028d28 ((wq_completion)events){+.+.}, at: __write_once_size include/linux/compiler.h:226 [inline]
#0: ffff8881da028d28 ((wq_completion)events){+.+.}, at: arch_atomic64_set arch/x86/include/asm/atomic64_64.h:34 [inline]
#0: ffff8881da028d28 ((wq_completion)events){+.+.}, at: atomic64_set include/asm-generic/atomic-instrumented.h:855 [inline]
#0: ffff8881da028d28 ((wq_completion)events){+.+.}, at: atomic_long_set include/asm-generic/atomic-long.h:40 [inline]
#0: ffff8881da028d28 ((wq_completion)events){+.+.}, at: set_work_data kernel/workqueue.c:615 [inline]
#0: ffff8881da028d28 ((wq_completion)events){+.+.}, at: set_work_pool_and_clear_pending kernel/workqueue.c:642 [inline]
#0: ffff8881da028d28 ((wq_completion)events){+.+.}, at: process_one_work+0x82a/0x1620 kernel/workqueue.c:2237
#1: ffff8881bd47fdd0 ((work_completion)(&usbhid->reset_work)){+.+.}, at: process_one_work+0x85e/0x1620 kernel/workqueue.c:2241
#2: ffff8881d8fad190 (&dev->mutex){....}, at: device_release_driver_internal+0x23/0x500 drivers/base/dd.c:1165
#3: ffff8881cd0c1a88 (&dev->mutex){....}, at: device_release_driver_internal+0x23/0x500 drivers/base/dd.c:1165
#4: ffffffff877ca580 (input_mutex){+.+.}, at: __input_unregister_device+0x118/0x430 drivers/input/input.c:2087
#5: ffff8881d8fa02a8 (&dev->mutex#2){+.+.}, at: input_close_device+0x42/0x1a0 drivers/input/input.c:673
#6: ffff8881cd0c1e50 (&hdev->ll_open_lock){+.+.}, at: hid_hw_close+0x1d/0xd0 drivers/hid/hid-core.c:2098
7 locks held by kworker/1:3/3231:
#0: ffff8881d8944128 ((wq_completion)usb_hub_wq){+.+.}, at: __write_once_size include/linux/compiler.h:226 [inline]
#0: ffff8881d8944128 ((wq_completion)usb_hub_wq){+.+.}, at: arch_atomic64_set arch/x86/include/asm/atomic64_64.h:34 [inline]
#0: ffff8881d8944128 ((wq_completion)usb_hub_wq){+.+.}, at: atomic64_set include/asm-generic/atomic-instrumented.h:855 [inline]
#0: ffff8881d8944128 ((wq_completion)usb_hub_wq){+.+.}, at: atomic_long_set include/asm-generic/atomic-long.h:40 [inline]
#0: ffff8881d8944128 ((wq_completion)usb_hub_wq){+.+.}, at: set_work_data kernel/workqueue.c:615 [inline]
#0: ffff8881d8944128 ((wq_completion)usb_hub_wq){+.+.}, at: set_work_pool_and_clear_pending kernel/workqueue.c:642 [inline]
#0: ffff8881d8944128 ((wq_completion)usb_hub_wq){+.+.}, at: process_one_work+0x82a/0x1620 kernel/workqueue.c:2237
#1: ffff8881cc797dd0 ((work_completion)(&hub->events)){+.+.}, at: process_one_work+0x85e/0x1620 kernel/workqueue.c:2241
#2: ffff8881d4da3208 (&dev->mutex){....}, at: device_lock include/linux/device.h:771 [inline]
#2: ffff8881d4da3208 (&dev->mutex){....}, at: hub_event+0x1be/0x43b0 drivers/usb/core/hub.c:5513
#3: ffff8881d0a6e208 (&dev->mutex){....}, at: device_lock include/linux/device.h:771 [inline]
#3: ffff8881d0a6e208 (&dev->mutex){....}, at: __device_attach+0x7b/0x390 drivers/base/dd.c:874
#4: ffff8881cffc8190 (&dev->mutex){....}, at: device_lock include/linux/device.h:771 [inline]
#4: ffff8881cffc8190 (&dev->mutex){....}, at: __device_attach+0x7b/0x390 drivers/base/dd.c:874
#5: ffff8881c81f9a88 (&dev->mutex){....}, at: device_lock include/linux/device.h:771 [inline]
#5: ffff8881c81f9a88 (&dev->mutex){....}, at: __device_attach+0x7b/0x390 drivers/base/dd.c:874
#6: ffffffff877ca580 (input_mutex){+.+.}, at: input_register_device.cold+0x5c/0x316 drivers/input/input.c:2221
5 locks held by kworker/0:6/3266:
#0: ffff8881da028d28 ((wq_completion)events){+.+.}, at: __write_once_size include/linux/compiler.h:226 [inline]
#0: ffff8881da028d28 ((wq_completion)events){+.+.}, at: arch_atomic64_set arch/x86/include/asm/atomic64_64.h:34 [inline]
#0: ffff8881da028d28 ((wq_completion)events){+.+.}, at: atomic64_set include/asm-generic/atomic-instrumented.h:855 [inline]
#0: ffff8881da028d28 ((wq_completion)events){+.+.}, at: atomic_long_set include/asm-generic/atomic-long.h:40 [inline]
#0: ffff8881da028d28 ((wq_completion)events){+.+.}, at: set_work_data kernel/workqueue.c:615 [inline]
#0: ffff8881da028d28 ((wq_completion)events){+.+.}, at: set_work_pool_and_clear_pending kernel/workqueue.c:642 [inline]
#0: ffff8881da028d28 ((wq_completion)events){+.+.}, at: process_one_work+0x82a/0x1620 kernel/workqueue.c:2237
#1: ffff8881c9ec7dd0 ((work_completion)(&usbhid->reset_work)){+.+.}, at: process_one_work+0x85e/0x1620 kernel/workqueue.c:2241
#2: ffff8881d8fac190 (&dev->mutex){....}, at: device_release_driver_internal+0x23/0x500 drivers/base/dd.c:1165
#3: ffff8881be6d1a88 (&dev->mutex){....}, at: device_release_driver_internal+0x23/0x500 drivers/base/dd.c:1165
#4: ffffffff877ca580 (input_mutex){+.+.}, at: __input_unregister_device+0x118/0x430 drivers/input/input.c:2087
3 locks held by kworker/1:4/3267:
#0: ffff8881da028d28 ((wq_completion)events){+.+.}, at: __write_once_size include/linux/compiler.h:226 [inline]
#0: ffff8881da028d28 ((wq_completion)events){+.+.}, at: arch_atomic64_set arch/x86/include/asm/atomic64_64.h:34 [inline]
#0: ffff8881da028d28 ((wq_completion)events){+.+.}, at: atomic64_set include/asm-generic/atomic-instrumented.h:855 [inline]
#0: ffff8881da028d28 ((wq_completion)events){+.+.}, at: atomic_long_set include/asm-generic/atomic-long.h:40 [inline]
#0: ffff8881da028d28 ((wq_completion)events){+.+.}, at: set_work_data kernel/workqueue.c:615 [inline]
#0: ffff8881da028d28 ((wq_completion)events){+.+.}, at: set_work_pool_and_clear_pending kernel/workqueue.c:642 [inline]
#0: ffff8881da028d28 ((wq_completion)events){+.+.}, at: process_one_work+0x82a/0x1620 kernel/workqueue.c:2237
#1: ffff8881bd6d7dd0 ((work_completion)(&usbhid->reset_work)){+.+.}, at: process_one_work+0x85e/0x1620 kernel/workqueue.c:2241
#2: ffff8881d8fd0190 (&dev->mutex){....}, at: device_lock include/linux/device.h:771 [inline]
#2: ffff8881d8fd0190 (&dev->mutex){....}, at: __device_attach+0x7b/0x390 drivers/base/dd.c:874
7 locks held by kworker/1:5/3268:
#0: ffff8881d8944128 ((wq_completion)usb_hub_wq){+.+.}, at: __write_once_size include/linux/compiler.h:226 [inline]
#0: ffff8881d8944128 ((wq_completion)usb_hub_wq){+.+.}, at: arch_atomic64_set arch/x86/include/asm/atomic64_64.h:34 [inline]
#0: ffff8881d8944128 ((wq_completion)usb_hub_wq){+.+.}, at: atomic64_set include/asm-generic/atomic-instrumented.h:855 [inline]
#0: ffff8881d8944128 ((wq_completion)usb_hub_wq){+.+.}, at: atomic_long_set include/asm-generic/atomic-long.h:40 [inline]
#0: ffff8881d8944128 ((wq_completion)usb_hub_wq){+.+.}, at: set_work_data kernel/workqueue.c:615 [inline]
#0: ffff8881d8944128 ((wq_completion)usb_hub_wq){+.+.}, at: set_work_pool_and_clear_pending kernel/workqueue.c:642 [inline]
#0: ffff8881d8944128 ((wq_completion)usb_hub_wq){+.+.}, at: process_one_work+0x82a/0x1620 kernel/workqueue.c:2237
#1: ffff8881bd6ffdd0 ((work_completion)(&hub->events)){+.+.}, at: process_one_work+0x85e/0x1620 kernel/workqueue.c:2241
#2: ffff8881d4d74208 (&dev->mutex){....}, at: device_lock include/linux/device.h:771 [inline]
#2: ffff8881d4d74208 (&dev->mutex){....}, at: hub_event+0x1be/0x43b0 drivers/usb/core/hub.c:5513
#3: ffff8881d9302208 (&dev->mutex){....}, at: device_lock include/linux/device.h:771 [inline]
#3: ffff8881d9302208 (&dev->mutex){....}, at: __device_attach+0x7b/0x390 drivers/base/dd.c:874
#4: ffff8881d9306190 (&dev->mutex){....}, at: device_lock include/linux/device.h:771 [inline]
#4: ffff8881d9306190 (&dev->mutex){....}, at: __device_attach+0x7b/0x390 drivers/base/dd.c:874
#5: ffff8881c9679a88 (&dev->mutex){....}, at: device_lock include/linux/device.h:771 [inline]
#5: ffff8881c9679a88 (&dev->mutex){....}, at: __device_attach+0x7b/0x390 drivers/base/dd.c:874
#6: ffffffff877ca580 (input_mutex){+.+.}, at: input_register_device.cold+0x5c/0x316 drivers/input/input.c:2221

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

NMI backtrace for cpu 0
CPU: 0 PID: 23 Comm: khungtaskd Not tainted 5.6.0-rc7-syzkaller #0
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+0xef/0x16e lib/dump_stack.c:118
nmi_cpu_backtrace.cold+0x70/0xb1 lib/nmi_backtrace.c:101
nmi_trigger_cpumask_backtrace+0x1db/0x207 lib/nmi_backtrace.c:62
trigger_all_cpu_backtrace include/linux/nmi.h:146 [inline]
check_hung_uninterruptible_tasks kernel/hung_task.c:205 [inline]
watchdog+0xa99/0xfd0 kernel/hung_task.c:289
kthread+0x318/0x420 kernel/kthread.c:255
ret_from_fork+0x24/0x30 arch/x86/entry/entry_64.S:352
Sending NMI from CPU 0 to CPUs 1:
NMI backtrace for cpu 1 skipped: idling at native_safe_halt arch/x86/include/asm/irqflags.h:60 [inline]
NMI backtrace for cpu 1 skipped: idling at arch_safe_halt arch/x86/include/asm/irqflags.h:103 [inline]
NMI backtrace for cpu 1 skipped: idling at default_idle+0x28/0x300 arch/x86/kernel/process.c:695


Tested on:

commit: 0fa84af8 Merge tag 'usb-serial-5.7-rc1' of https://git.ker..
git tree: https://github.com/google/kasan.git
console output: https://syzkaller.appspot.com/x/log.txt?x=13af50b0100000
kernel config: https://syzkaller.appspot.com/x/.config?x=6b9c154b0c23aecf
dashboard link: https://syzkaller.appspot.com/bug?extid=db339689b2101f6f6071
compiler: gcc (GCC) 9.0.0 20181231 (experimental)
patch: https://syzkaller.appspot.com/x/patch.diff?x=16e551cfe00000

2020-04-24 19:16:09

by Alan Stern

[permalink] [raw]
Subject: Re: WARNING in usbhid_raw_request/usb_submit_urb (3)

On Fri, 24 Apr 2020, syzbot wrote:

> Hello,
>
> syzbot has tested the proposed patch but the reproducer still triggered crash:
> INFO: task hung in usb_disable_device

That wasn't what I expected. Still, the important information was
present: The reset was instigated by hid_io_error(), because of some
sort of communication error.

Note that the hid_submit_out, hid_submit_ctrl, and so on don't test the
RESET_PENDING flag. At least, not with any proper synchronization.
That's why we got an URB submitted while the device was being reset.

Nevertheless, the USB core should be able to handle such things without
a big WARNing, particularly for ep0. The patch below tries to do this.

Alan Stern

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

Index: usb-devel/drivers/usb/core/urb.c
===================================================================
--- usb-devel.orig/drivers/usb/core/urb.c
+++ usb-devel/drivers/usb/core/urb.c
@@ -204,8 +204,17 @@ int usb_urb_ep_type_check(const struct u
const struct usb_host_endpoint *ep;

ep = usb_pipe_endpoint(urb->dev, urb->pipe);
- if (!ep)
- return -EINVAL;
+ if (!ep) {
+ /*
+ * Special case: The pointers for ep0 are temporarily cleared
+ * during device resets. We won't count this as an error;
+ * drivers can reasonably expect that ep0 always exists.
+ */
+ if (usb_pipeendpoint(urb->pipe) == 0)
+ ep = &urb->dev->ep0;
+ else
+ return -EINVAL;
+ }
if (usb_pipetype(urb->pipe) != pipetypes[usb_endpoint_type(&ep->desc)])
return -EINVAL;
return 0;

2020-04-24 19:36:36

by syzbot

[permalink] [raw]
Subject: Re: WARNING in usbhid_raw_request/usb_submit_urb (3)

Hello,

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

Reported-and-tested-by: [email protected]

Tested on:

commit: 0fa84af8 Merge tag 'usb-serial-5.7-rc1' of https://git.ker..
git tree: https://github.com/google/kasan.git
kernel config: https://syzkaller.appspot.com/x/.config?x=6b9c154b0c23aecf
dashboard link: https://syzkaller.appspot.com/bug?extid=db339689b2101f6f6071
compiler: gcc (GCC) 9.0.0 20181231 (experimental)
patch: https://syzkaller.appspot.com/x/patch.diff?x=17d37144100000

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

2020-04-25 20:28:01

by Alan Stern

[permalink] [raw]
Subject: Re: WARNING in usbhid_raw_request/usb_submit_urb (3)

On Fri, 24 Apr 2020, syzbot wrote:

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

Let's try a slightly different fix. This feels a little better; it
reflects the fact that ep0 always exists, even when a device is
unconfigured. Also, it's a smaller change.

Alan Stern

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

Index: usb-devel/drivers/usb/core/message.c
===================================================================
--- usb-devel.orig/drivers/usb/core/message.c
+++ usb-devel/drivers/usb/core/message.c
@@ -1143,11 +1143,11 @@ void usb_disable_endpoint(struct usb_dev

if (usb_endpoint_out(epaddr)) {
ep = dev->ep_out[epnum];
- if (reset_hardware)
+ if (reset_hardware && epnum != 0)
dev->ep_out[epnum] = NULL;
} else {
ep = dev->ep_in[epnum];
- if (reset_hardware)
+ if (reset_hardware && epnum != 0)
dev->ep_in[epnum] = NULL;
}
if (ep) {

2020-04-25 21:23:39

by syzbot

[permalink] [raw]
Subject: Re: WARNING in usbhid_raw_request/usb_submit_urb (3)

Hello,

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

Reported-and-tested-by: [email protected]

Tested on:

commit: 0fa84af8 Merge tag 'usb-serial-5.7-rc1' of https://git.ker..
git tree: https://github.com/google/kasan.git
kernel config: https://syzkaller.appspot.com/x/.config?x=6b9c154b0c23aecf
dashboard link: https://syzkaller.appspot.com/bug?extid=db339689b2101f6f6071
compiler: gcc (GCC) 9.0.0 20181231 (experimental)
patch: https://syzkaller.appspot.com/x/patch.diff?x=12437828100000

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

2020-04-29 20:15:32

by Alan Stern

[permalink] [raw]
Subject: Re: WARNING in usbhid_raw_request/usb_submit_urb (3)

On Sat, 25 Apr 2020, syzbot wrote:

> Hello,
>
> syzbot has tested the proposed patch and the reproducer did not trigger crash:
>
> Reported-and-tested-by: [email protected]

This is a two-pronged issue. The first problem is that usbhid does
something it shouldn't (it submits an URB while a device reset is in
progress). The second problem is that the USB core reports a driver
bug if a an URB is submitted for ep0 at the wrong time during a reset.

The patch tested above fixes the second issue but not the first. This
patch attempts to fix the first issue; it makes usbhid check for a
pending reset before submitting its URBs.

Alan Stern

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

Index: usb-devel/drivers/hid/usbhid/hid-core.c
===================================================================
--- usb-devel.orig/drivers/hid/usbhid/hid-core.c
+++ usb-devel/drivers/hid/usbhid/hid-core.c
@@ -87,6 +87,7 @@ static int hid_start_in(struct hid_devic
if (test_bit(HID_IN_POLLING, &usbhid->iofl) &&
!test_bit(HID_DISCONNECTED, &usbhid->iofl) &&
!test_bit(HID_SUSPENDED, &usbhid->iofl) &&
+ !test_bit(HID_RESET_PENDING, &usbhid->iofl) &&
!test_and_set_bit(HID_IN_RUNNING, &usbhid->iofl)) {
rc = usb_submit_urb(usbhid->urbin, GFP_ATOMIC);
if (rc != 0) {
@@ -273,6 +274,7 @@ static void hid_irq_in(struct urb *urb)
struct hid_device *hid = urb->context;
struct usbhid_device *usbhid = hid->driver_data;
int status;
+ unsigned long flags;

switch (urb->status) {
case 0: /* success */
@@ -319,7 +321,12 @@ static void hid_irq_in(struct urb *urb)
urb->status);
}

- status = usb_submit_urb(urb, GFP_ATOMIC);
+ spin_lock_irqsave(&usbhid->lock, flags);
+ if (test_bit(HID_RESET_PENDING, &usbhid->iofl))
+ status = -EPERM;
+ else
+ status = usb_submit_urb(urb, GFP_ATOMIC);
+ spin_unlock_irqrestore(&usbhid->lock, flags);
if (status) {
clear_bit(HID_IN_RUNNING, &usbhid->iofl);
if (status != -EPERM) {
@@ -339,6 +346,9 @@ static int hid_submit_out(struct hid_dev
struct usbhid_device *usbhid = hid->driver_data;
int r;

+ if (test_bit(HID_RESET_PENDING, &usbhid->iofl))
+ return -EAGAIN;
+
report = usbhid->out[usbhid->outtail].report;
raw_report = usbhid->out[usbhid->outtail].raw_report;

@@ -370,6 +380,9 @@ static int hid_submit_ctrl(struct hid_de
int len, r;
struct usbhid_device *usbhid = hid->driver_data;

+ if (test_bit(HID_RESET_PENDING, &usbhid->iofl))
+ return -EAGAIN;
+
report = usbhid->ctrl[usbhid->ctrltail].report;
raw_report = usbhid->ctrl[usbhid->ctrltail].raw_report;
dir = usbhid->ctrl[usbhid->ctrltail].dir;

2020-04-29 20:31:59

by syzbot

[permalink] [raw]
Subject: Re: WARNING in usbhid_raw_request/usb_submit_urb (3)

Hello,

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

Reported-and-tested-by: [email protected]

Tested on:

commit: 0fa84af8 Merge tag 'usb-serial-5.7-rc1' of https://git.ker..
git tree: https://github.com/google/kasan.git
kernel config: https://syzkaller.appspot.com/x/.config?x=6b9c154b0c23aecf
dashboard link: https://syzkaller.appspot.com/bug?extid=db339689b2101f6f6071
compiler: gcc (GCC) 9.0.0 20181231 (experimental)
patch: https://syzkaller.appspot.com/x/patch.diff?x=14a84ac4100000

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

2020-04-29 23:43:50

by Alan Stern

[permalink] [raw]
Subject: Re: WARNING in usbhid_raw_request/usb_submit_urb (3)

On Wed, 29 Apr 2020, syzbot wrote:

> Hello,
>
> syzbot has tested the proposed patch and the reproducer did not trigger crash:
>
> Reported-and-tested-by: [email protected]

Good. Here's an improved version of the patch.

Alan Stern

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

Index: usb-devel/drivers/hid/usbhid/hid-core.c
===================================================================
--- usb-devel.orig/drivers/hid/usbhid/hid-core.c
+++ usb-devel/drivers/hid/usbhid/hid-core.c
@@ -87,6 +87,7 @@ static int hid_start_in(struct hid_devic
if (test_bit(HID_IN_POLLING, &usbhid->iofl) &&
!test_bit(HID_DISCONNECTED, &usbhid->iofl) &&
!test_bit(HID_SUSPENDED, &usbhid->iofl) &&
+ !test_bit(HID_RESET_PENDING, &usbhid->iofl) &&
!test_and_set_bit(HID_IN_RUNNING, &usbhid->iofl)) {
rc = usb_submit_urb(usbhid->urbin, GFP_ATOMIC);
if (rc != 0) {
@@ -339,6 +340,9 @@ static int hid_submit_out(struct hid_dev
struct usbhid_device *usbhid = hid->driver_data;
int r;

+ if (test_bit(HID_RESET_PENDING, &usbhid->iofl))
+ return -EAGAIN;
+
report = usbhid->out[usbhid->outtail].report;
raw_report = usbhid->out[usbhid->outtail].raw_report;

@@ -370,6 +374,9 @@ static int hid_submit_ctrl(struct hid_de
int len, r;
struct usbhid_device *usbhid = hid->driver_data;

+ if (test_bit(HID_RESET_PENDING, &usbhid->iofl))
+ return -EAGAIN;
+
report = usbhid->ctrl[usbhid->ctrltail].report;
raw_report = usbhid->ctrl[usbhid->ctrltail].raw_report;
dir = usbhid->ctrl[usbhid->ctrltail].dir;

2020-04-30 00:00:57

by syzbot

[permalink] [raw]
Subject: Re: WARNING in usbhid_raw_request/usb_submit_urb (3)

Hello,

syzbot has tested the proposed patch but the reproducer still triggered crash:
WARNING in usbhid_raw_request/usb_submit_urb

------------[ cut here ]------------
usb 3-1: BOGUS urb xfer, pipe 2 != type 2
WARNING: CPU: 1 PID: 5096 at drivers/usb/core/urb.c:478 usb_submit_urb+0x1188/0x1460 drivers/usb/core/urb.c:478
Kernel panic - not syncing: panic_on_warn set ...
CPU: 1 PID: 5096 Comm: syz-executor.2 Not tainted 5.6.0-rc7-syzkaller #0
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+0xef/0x16e lib/dump_stack.c:118
panic+0x2aa/0x6e1 kernel/panic.c:221
__warn.cold+0x2f/0x30 kernel/panic.c:582
report_bug+0x27b/0x2f0 lib/bug.c:195
fixup_bug arch/x86/kernel/traps.c:174 [inline]
fixup_bug arch/x86/kernel/traps.c:169 [inline]
do_error_trap+0x12b/0x1e0 arch/x86/kernel/traps.c:267
do_invalid_op+0x32/0x40 arch/x86/kernel/traps.c:286
invalid_op+0x23/0x30 arch/x86/entry/entry_64.S:1027
RIP: 0010:usb_submit_urb+0x1188/0x1460 drivers/usb/core/urb.c:478
Code: 4d 85 ed 74 46 e8 68 87 dd fd 4c 89 f7 e8 90 57 17 ff 41 89 d8 44 89 e1 4c 89 ea 48 89 c6 48 c7 c7 80 dd 3b 86 e8 10 18 b2 fd <0f> 0b e9 20 f4 ff ff e8 3c 87 dd fd 0f 1f 44 00 00 e8 32 87 dd fd
RSP: 0018:ffff8881c83c7b38 EFLAGS: 00010286
RAX: 0000000000000000 RBX: 0000000000000002 RCX: 0000000000000000
RDX: 0000000000000000 RSI: ffffffff812974dd RDI: ffffed1039078f59
RBP: 0000000000000000 R08: ffff8881d5bbe200 R09: ffffed103b666248
R10: ffffed103b666247 R11: ffff8881db33123f R12: 0000000000000002
R13: ffff8881d8dc2480 R14: ffff8881c5ad80a0 R15: ffff8881d9a7ab00
usb_start_wait_urb+0x108/0x4c0 drivers/usb/core/message.c:58
usb_internal_control_msg drivers/usb/core/message.c:102 [inline]
usb_control_msg+0x31c/0x4a0 drivers/usb/core/message.c:153
usbhid_set_raw_report drivers/hid/usbhid/hid-core.c:924 [inline]
usbhid_raw_request+0x21f/0x640 drivers/hid/usbhid/hid-core.c:1272
hid_hw_raw_request include/linux/hid.h:1079 [inline]
hidraw_send_report+0x296/0x500 drivers/hid/hidraw.c:151
hidraw_ioctl+0x620/0xaf0 drivers/hid/hidraw.c:422
vfs_ioctl fs/ioctl.c:47 [inline]
ksys_ioctl+0x11a/0x180 fs/ioctl.c:763
__do_sys_ioctl fs/ioctl.c:772 [inline]
__se_sys_ioctl fs/ioctl.c:770 [inline]
__x64_sys_ioctl+0x6f/0xb0 fs/ioctl.c:770
do_syscall_64+0xb6/0x5a0 arch/x86/entry/common.c:294
entry_SYSCALL_64_after_hwframe+0x49/0xbe
RIP: 0033:0x45c849
Code: ad b6 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 7b b6 fb ff c3 66 2e 0f 1f 84 00 00 00 00
RSP: 002b:00007ff1fff1bc78 EFLAGS: 00000246 ORIG_RAX: 0000000000000010
RAX: ffffffffffffffda RBX: 00007ff1fff1c6d4 RCX: 000000000045c849
RDX: 00000000200000c0 RSI: 0000000080404806 RDI: 0000000000000006
RBP: 000000000076bfa0 R08: 0000000000000000 R09: 0000000000000000
R10: 0000000000000000 R11: 0000000000000246 R12: 00000000ffffffff
R13: 0000000000000335 R14: 00000000004c59df R15: 000000000076bfac
Kernel Offset: disabled
Rebooting in 86400 seconds..


Tested on:

commit: 0fa84af8 Merge tag 'usb-serial-5.7-rc1' of https://git.ker..
git tree: https://github.com/google/kasan.git
console output: https://syzkaller.appspot.com/x/log.txt?x=14d839dfe00000
kernel config: https://syzkaller.appspot.com/x/.config?x=6b9c154b0c23aecf
dashboard link: https://syzkaller.appspot.com/bug?extid=db339689b2101f6f6071
compiler: gcc (GCC) 9.0.0 20181231 (experimental)
patch: https://syzkaller.appspot.com/x/patch.diff?x=13204440100000

2020-04-30 15:01:55

by Alan Stern

[permalink] [raw]
Subject: Re: WARNING in usbhid_raw_request/usb_submit_urb (3)

On Wed, 29 Apr 2020, syzbot wrote:

> Hello,
>
> syzbot has tested the proposed patch but the reproducer still triggered crash:
> WARNING in usbhid_raw_request/usb_submit_urb

Ugh. I forgot that usbhid also uses usb_control_msg() and
usb_interrupt_msg(). Those things can't be protected by a spinlock;
I'll have to add a semaphore. (A mutex won't work either, because it
has to be locked in hid_pre_reset() and unlocked in hid_post_reset().)

Alan Stern

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

Index: usb-devel/drivers/hid/usbhid/hid-core.c
===================================================================
--- usb-devel.orig/drivers/hid/usbhid/hid-core.c
+++ usb-devel/drivers/hid/usbhid/hid-core.c
@@ -87,6 +87,7 @@ static int hid_start_in(struct hid_devic
if (test_bit(HID_IN_POLLING, &usbhid->iofl) &&
!test_bit(HID_DISCONNECTED, &usbhid->iofl) &&
!test_bit(HID_SUSPENDED, &usbhid->iofl) &&
+ !test_bit(HID_RESET_PENDING, &usbhid->iofl) &&
!test_and_set_bit(HID_IN_RUNNING, &usbhid->iofl)) {
rc = usb_submit_urb(usbhid->urbin, GFP_ATOMIC);
if (rc != 0) {
@@ -339,6 +340,9 @@ static int hid_submit_out(struct hid_dev
struct usbhid_device *usbhid = hid->driver_data;
int r;

+ if (test_bit(HID_RESET_PENDING, &usbhid->iofl))
+ return -EAGAIN;
+
report = usbhid->out[usbhid->outtail].report;
raw_report = usbhid->out[usbhid->outtail].raw_report;

@@ -370,6 +374,9 @@ static int hid_submit_ctrl(struct hid_de
int len, r;
struct usbhid_device *usbhid = hid->driver_data;

+ if (test_bit(HID_RESET_PENDING, &usbhid->iofl))
+ return -EAGAIN;
+
report = usbhid->ctrl[usbhid->ctrltail].report;
raw_report = usbhid->ctrl[usbhid->ctrltail].raw_report;
dir = usbhid->ctrl[usbhid->ctrltail].dir;
@@ -654,26 +661,37 @@ static int usbhid_wait_io(struct hid_dev
return 0;
}

-static int hid_set_idle(struct usb_device *dev, int ifnum, int report, int idle)
+static int hid_set_idle(struct hid_device *hid,
+ struct usb_device *dev, int ifnum, int report, int idle)
{
- return usb_control_msg(dev, usb_sndctrlpipe(dev, 0),
+ struct usbhid_device *usbhid = hid->driver_data;
+ int rc;
+
+ down(&usbhid->sem);
+ rc = usb_control_msg(dev, usb_sndctrlpipe(dev, 0),
HID_REQ_SET_IDLE, USB_TYPE_CLASS | USB_RECIP_INTERFACE, (idle << 8) | report,
ifnum, NULL, 0, USB_CTRL_SET_TIMEOUT);
+ up(&usbhid->sem);
+ return rc;
}

-static int hid_get_class_descriptor(struct usb_device *dev, int ifnum,
+static int hid_get_class_descriptor(struct hid_device *hid,
+ struct usb_device *dev, int ifnum,
unsigned char type, void *buf, int size)
{
+ struct usbhid_device *usbhid = hid->driver_data;
int result, retries = 4;

memset(buf, 0, size);

+ down(&usbhid->sem);
do {
result = usb_control_msg(dev, usb_rcvctrlpipe(dev, 0),
USB_REQ_GET_DESCRIPTOR, USB_RECIP_INTERFACE | USB_DIR_IN,
(type << 8), ifnum, buf, size, USB_CTRL_GET_TIMEOUT);
retries--;
} while (result < size && retries);
+ up(&usbhid->sem);
return result;
}

@@ -682,16 +700,21 @@ static int usbhid_open(struct hid_device
struct usbhid_device *usbhid = hid->driver_data;
int res;

+ mutex_lock(&usbhid->mutex);
+
set_bit(HID_OPENED, &usbhid->iofl);

- if (hid->quirks & HID_QUIRK_ALWAYS_POLL)
- return 0;
+ if (hid->quirks & HID_QUIRK_ALWAYS_POLL) {
+ res = 0;
+ goto Done;
+ }

res = usb_autopm_get_interface(usbhid->intf);
/* the device must be awake to reliably request remote wakeup */
if (res < 0) {
clear_bit(HID_OPENED, &usbhid->iofl);
- return -EIO;
+ res = -EIO;
+ goto Done;
}

usbhid->intf->needs_remote_wakeup = 1;
@@ -725,6 +748,9 @@ static int usbhid_open(struct hid_device
msleep(50);

clear_bit(HID_RESUME_RUNNING, &usbhid->iofl);
+
+ Done:
+ mutex_unlock(&usbhid->mutex);
return res;
}

@@ -732,6 +758,8 @@ static void usbhid_close(struct hid_devi
{
struct usbhid_device *usbhid = hid->driver_data;

+ mutex_lock(&usbhid->mutex);
+
/*
* Make sure we don't restart data acquisition due to
* a resumption we no longer care about by avoiding racing
@@ -743,12 +771,13 @@ static void usbhid_close(struct hid_devi
clear_bit(HID_IN_POLLING, &usbhid->iofl);
spin_unlock_irq(&usbhid->lock);

- if (hid->quirks & HID_QUIRK_ALWAYS_POLL)
- return;
+ if (!(hid->quirks & HID_QUIRK_ALWAYS_POLL)) {
+ hid_cancel_delayed_stuff(usbhid);
+ usb_kill_urb(usbhid->urbin);
+ usbhid->intf->needs_remote_wakeup = 0;
+ }

- hid_cancel_delayed_stuff(usbhid);
- usb_kill_urb(usbhid->urbin);
- usbhid->intf->needs_remote_wakeup = 0;
+ mutex_unlock(&usbhid->mutex);
}

/*
@@ -877,12 +906,14 @@ static int usbhid_get_raw_report(struct
count--;
skipped_report_id = 1;
}
+ down(&usbhid->sem);
ret = usb_control_msg(dev, usb_rcvctrlpipe(dev, 0),
HID_REQ_GET_REPORT,
USB_DIR_IN | USB_TYPE_CLASS | USB_RECIP_INTERFACE,
((report_type + 1) << 8) | report_number,
interface->desc.bInterfaceNumber, buf, count,
USB_CTRL_SET_TIMEOUT);
+ up(&usbhid->sem);

/* count also the report id */
if (ret > 0 && skipped_report_id)
@@ -914,12 +945,14 @@ static int usbhid_set_raw_report(struct
skipped_report_id = 1;
}

+ down(&usbhid->sem);
ret = usb_control_msg(dev, usb_sndctrlpipe(dev, 0),
HID_REQ_SET_REPORT,
USB_DIR_OUT | USB_TYPE_CLASS | USB_RECIP_INTERFACE,
((rtype + 1) << 8) | reportnum,
interface->desc.bInterfaceNumber, buf, count,
USB_CTRL_SET_TIMEOUT);
+ up(&usbhid->sem);
/* count also the report id, if this was a numbered report. */
if (ret > 0 && skipped_report_id)
ret++;
@@ -943,9 +976,11 @@ static int usbhid_output_report(struct h
skipped_report_id = 1;
}

+ down(&usbhid->sem);
ret = usb_interrupt_msg(dev, usbhid->urbout->pipe,
buf, count, &actual_length,
USB_CTRL_SET_TIMEOUT);
+ up(&usbhid->sem);
/* return the number of bytes transferred */
if (ret == 0) {
ret = actual_length;
@@ -1024,9 +1059,10 @@ static int usbhid_parse(struct hid_devic
if (!rdesc)
return -ENOMEM;

- hid_set_idle(dev, interface->desc.bInterfaceNumber, 0, 0);
+ hid_set_idle(hid, dev, interface->desc.bInterfaceNumber, 0, 0);

- ret = hid_get_class_descriptor(dev, interface->desc.bInterfaceNumber,
+ ret = hid_get_class_descriptor(hid, dev,
+ interface->desc.bInterfaceNumber,
HID_DT_REPORT, rdesc, rsize);
if (ret < 0) {
dbg_hid("reading report descriptor failed\n");
@@ -1057,6 +1093,8 @@ static int usbhid_start(struct hid_devic
unsigned int n, insize = 0;
int ret;

+ mutex_lock(&usbhid->mutex);
+
clear_bit(HID_DISCONNECTED, &usbhid->iofl);

usbhid->bufsize = HID_MIN_BUFFER_SIZE;
@@ -1177,6 +1215,8 @@ static int usbhid_start(struct hid_devic
usbhid_set_leds(hid);
device_set_wakeup_enable(&dev->dev, 1);
}
+
+ mutex_unlock(&usbhid->mutex);
return 0;

fail:
@@ -1187,6 +1227,7 @@ fail:
usbhid->urbout = NULL;
usbhid->urbctrl = NULL;
hid_free_buffers(dev, hid);
+ mutex_unlock(&usbhid->mutex);
return ret;
}

@@ -1202,6 +1243,8 @@ static void usbhid_stop(struct hid_devic
usbhid->intf->needs_remote_wakeup = 0;
}

+ mutex_lock(&usbhid->mutex);
+
clear_bit(HID_STARTED, &usbhid->iofl);
spin_lock_irq(&usbhid->lock); /* Sync with error and led handlers */
set_bit(HID_DISCONNECTED, &usbhid->iofl);
@@ -1222,6 +1265,8 @@ static void usbhid_stop(struct hid_devic
usbhid->urbout = NULL;

hid_free_buffers(hid_to_usb_dev(hid), hid);
+
+ mutex_unlock(&usbhid->mutex);
}

static int usbhid_power(struct hid_device *hid, int lvl)
@@ -1279,7 +1324,7 @@ static int usbhid_idle(struct hid_device
if (reqtype != HID_REQ_SET_IDLE)
return -EINVAL;

- return hid_set_idle(dev, ifnum, report, idle);
+ return hid_set_idle(hid, dev, ifnum, report, idle);
}

struct hid_ll_driver usb_hid_driver = {
@@ -1382,6 +1427,8 @@ static int usbhid_probe(struct usb_inter
INIT_WORK(&usbhid->reset_work, hid_reset);
timer_setup(&usbhid->io_retry, hid_retry_timeout, 0);
spin_lock_init(&usbhid->lock);
+ mutex_init(&usbhid->mutex);
+ sema_init(&usbhid->sem, 1);

ret = hid_add_device(hid);
if (ret) {
@@ -1470,6 +1517,8 @@ static int hid_pre_reset(struct usb_inte
spin_unlock_irq(&usbhid->lock);
hid_cease_io(usbhid);

+ down(&usbhid->sem); /* released in hid_post_reset */
+
return 0;
}

@@ -1483,6 +1532,8 @@ static int hid_post_reset(struct usb_int
int status;
char *rdesc;

+ up(&usbhid->sem); /* acquired in hid_pre_reset */
+
/* Fetch and examine the HID report descriptor. If this
* has changed, then rebind. Since usbcore's check of the
* configuration descriptors passed, we already know that
@@ -1492,7 +1543,7 @@ static int hid_post_reset(struct usb_int
if (!rdesc)
return -ENOMEM;

- status = hid_get_class_descriptor(dev,
+ status = hid_get_class_descriptor(hid, dev,
interface->desc.bInterfaceNumber,
HID_DT_REPORT, rdesc, hid->dev_rsize);
if (status < 0) {
@@ -1512,7 +1563,7 @@ static int hid_post_reset(struct usb_int
clear_bit(HID_RESET_PENDING, &usbhid->iofl);
clear_bit(HID_CLEAR_HALT, &usbhid->iofl);
spin_unlock_irq(&usbhid->lock);
- hid_set_idle(dev, intf->cur_altsetting->desc.bInterfaceNumber, 0, 0);
+ hid_set_idle(hid, dev, intf->cur_altsetting->desc.bInterfaceNumber, 0, 0);

hid_restart_io(hid);

Index: usb-devel/drivers/hid/usbhid/usbhid.h
===================================================================
--- usb-devel.orig/drivers/hid/usbhid/usbhid.h
+++ usb-devel/drivers/hid/usbhid/usbhid.h
@@ -19,6 +19,7 @@
#include <linux/wait.h>
#include <linux/workqueue.h>
#include <linux/input.h>
+#include <linux/semaphore.h>

/* API provided by hid-core.c for USB HID drivers */
void usbhid_init_reports(struct hid_device *hid);
@@ -80,6 +81,9 @@ struct usbhid_device {
dma_addr_t outbuf_dma; /* Output buffer dma */
unsigned long last_out; /* record of last output for timeouts */

+ struct mutex mutex; /* start/stop/open/close */
+ struct semaphore sem; /* submit/pre_reset/post_reset */
+
spinlock_t lock; /* fifo spinlock */
unsigned long iofl; /* I/O flags (CTRL_RUNNING, OUT_RUNNING) */
struct timer_list io_retry; /* Retry timer */

2020-04-30 15:20:08

by syzbot

[permalink] [raw]
Subject: Re: WARNING in usbhid_raw_request/usb_submit_urb (3)

Hello,

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

Reported-and-tested-by: [email protected]

Tested on:

commit: 0fa84af8 Merge tag 'usb-serial-5.7-rc1' of https://git.ker..
git tree: https://github.com/google/kasan.git
kernel config: https://syzkaller.appspot.com/x/.config?x=6b9c154b0c23aecf
dashboard link: https://syzkaller.appspot.com/bug?extid=db339689b2101f6f6071
compiler: gcc (GCC) 9.0.0 20181231 (experimental)
patch: https://syzkaller.appspot.com/x/patch.diff?x=12a35540100000

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