Hello,
syzbot found the following issue on:
HEAD commit: dd9c7df94c1b Merge branch 'akpm' (patches from Andrew)
git tree: upstream
console output: https://syzkaller.appspot.com/x/log.txt?x=14a9f66a300000
kernel config: https://syzkaller.appspot.com/x/.config?x=f1b998c1afc13578
dashboard link: https://syzkaller.appspot.com/bug?extid=9b57a46bf1801ce2a2ca
userspace arch: i386
Unfortunately, I don't have any reproducer for this issue yet.
IMPORTANT: if you fix the issue, please add the following tag to the commit:
Reported-by: [email protected]
------------[ cut here ]------------
usb 7-1: BOGUS control dir, pipe 80000280 doesn't match bRequestType a1
WARNING: CPU: 0 PID: 15508 at drivers/usb/core/urb.c:410 usb_submit_urb+0x149d/0x18a0 drivers/usb/core/urb.c:410
Modules linked in:
CPU: 0 PID: 15508 Comm: syz-executor.2 Not tainted 5.14.0-rc1-syzkaller #0
Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS 1.14.0-2 04/01/2014
RIP: 0010:usb_submit_urb+0x149d/0x18a0 drivers/usb/core/urb.c:410
Code: 7c 24 40 e8 a5 e9 1f fc 48 8b 7c 24 40 e8 db 25 0c ff 45 89 e8 44 89 f1 4c 89 e2 48 89 c6 48 c7 c7 60 96 27 8a e8 e4 b2 91 03 <0f> 0b e9 a5 ee ff ff e8 77 e9 1f fc 0f b6 1d 37 2e 02 08 31 ff 41
RSP: 0018:ffffc900021cfb88 EFLAGS: 00010082
RAX: 0000000000000000 RBX: ffff8880786df058 RCX: 0000000000000000
RDX: 0000000000040000 RSI: ffffffff815d6855 RDI: fffff52000439f63
RBP: ffff88804a27bbe0 R08: 0000000000000000 R09: 0000000000000000
R10: ffffffff815d068e R11: 0000000000000000 R12: ffff8880168a42a8
R13: 00000000000000a1 R14: 0000000080000280 R15: ffff888029c86f00
FS: 0000000000000000(0000) GS:ffff88802ca00000(0063) knlGS:00000000f558cb40
CS: 0010 DS: 002b ES: 002b CR0: 0000000080050033
CR2: 00000000f5580db0 CR3: 0000000077c36000 CR4: 0000000000150ef0
DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
Call Trace:
hid_submit_ctrl+0x6ec/0xd80 drivers/hid/usbhid/hid-core.c:416
usbhid_restart_ctrl_queue.isra.0+0x244/0x3a0 drivers/hid/usbhid/hid-core.c:258
__usbhid_submit_report+0x6f0/0xd50 drivers/hid/usbhid/hid-core.c:603
usbhid_submit_report drivers/hid/usbhid/hid-core.c:640 [inline]
usbhid_init_reports+0x16e/0x3b0 drivers/hid/usbhid/hid-core.c:784
hiddev_ioctl+0x10d4/0x1630 drivers/hid/usbhid/hiddev.c:794
compat_ptr_ioctl+0x67/0x90 fs/ioctl.c:1105
__do_compat_sys_ioctl+0x1c7/0x290 fs/ioctl.c:1167
do_syscall_32_irqs_on arch/x86/entry/common.c:112 [inline]
__do_fast_syscall_32+0x65/0xf0 arch/x86/entry/common.c:178
do_fast_syscall_32+0x2f/0x70 arch/x86/entry/common.c:203
entry_SYSENTER_compat_after_hwframe+0x4d/0x5c
RIP: 0023:0xf7fb3549
Code: 03 74 c0 01 10 05 03 74 b8 01 10 06 03 74 b4 01 10 07 03 74 b0 01 10 08 03 74 d8 01 00 00 00 00 00 51 52 55 89 e5 0f 34 cd 80 <5d> 5a 59 c3 90 90 90 90 8d b4 26 00 00 00 00 8d b4 26 00 00 00 00
RSP: 002b:00000000f558c5fc EFLAGS: 00000296 ORIG_RAX: 0000000000000036
RAX: ffffffffffffffda RBX: 0000000000000003 RCX: 00000000c018480d
RDX: 0000000020000080 RSI: 0000000000000000 RDI: 0000000000000000
RBP: 0000000000000000 R08: 0000000000000000 R09: 0000000000000000
R10: 0000000000000000 R11: 0000000000000000 R12: 0000000000000000
R13: 0000000000000000 R14: 0000000000000000 R15: 0000000000000000
---
This report 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 issue. See:
https://goo.gl/tpsmEJ#status for how to communicate with syzbot.
syzbot has found a reproducer for the following issue on:
HEAD commit: 794c7931a242 Merge branch 'linus' of git://git.kernel.org/..
git tree: upstream
console output: https://syzkaller.appspot.com/x/log.txt?x=13af2205300000
kernel config: https://syzkaller.appspot.com/x/.config?x=96f0602203250753
dashboard link: https://syzkaller.appspot.com/bug?extid=9b57a46bf1801ce2a2ca
compiler: gcc (Debian 10.2.1-6) 10.2.1 20210110, GNU ld (GNU Binutils for Debian) 2.35.1
syz repro: https://syzkaller.appspot.com/x/repro.syz?x=11ae58ce300000
C reproducer: https://syzkaller.appspot.com/x/repro.c?x=11d71731300000
IMPORTANT: if you fix the issue, please add the following tag to the commit:
Reported-by: [email protected]
------------[ cut here ]------------
usb 1-1: BOGUS control dir, pipe 80000280 doesn't match bRequestType a1
WARNING: CPU: 0 PID: 8434 at drivers/usb/core/urb.c:410 usb_submit_urb+0x149d/0x18a0 drivers/usb/core/urb.c:410
Modules linked in:
CPU: 0 PID: 8434 Comm: syz-executor752 Not tainted 5.14.0-rc6-syzkaller #0
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 01/01/2011
RIP: 0010:usb_submit_urb+0x149d/0x18a0 drivers/usb/core/urb.c:410
Code: 7c 24 40 e8 45 64 1f fc 48 8b 7c 24 40 e8 4b fc 0b ff 45 89 e8 44 89 f1 4c 89 e2 48 89 c6 48 c7 c7 e0 b2 27 8a e8 01 fc 91 03 <0f> 0b e9 a5 ee ff ff e8 17 64 1f fc 0f b6 1d 19 ca 01 08 31 ff 41
RSP: 0018:ffffc90000effbd0 EFLAGS: 00010082
RAX: 0000000000000000 RBX: ffff888027944058 RCX: 0000000000000000
RDX: ffff8880235db880 RSI: ffffffff815d85c5 RDI: fffff520001dff6c
RBP: ffff888021618140 R08: 0000000000000000 R09: 0000000000000000
R10: ffffffff815d23fe R11: 0000000000000000 R12: ffff888018aff118
R13: 00000000000000a1 R14: 0000000080000280 R15: ffff888021900400
FS: 000000000223d300(0000) GS:ffff8880b9c00000(0000) knlGS:0000000000000000
CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
CR2: 00005614a6c2a160 CR3: 00000000222ca000 CR4: 0000000000350ef0
Call Trace:
hid_submit_ctrl+0x6ec/0xd80 drivers/hid/usbhid/hid-core.c:416
usbhid_restart_ctrl_queue.isra.0+0x244/0x3a0 drivers/hid/usbhid/hid-core.c:258
__usbhid_submit_report+0x6f0/0xd50 drivers/hid/usbhid/hid-core.c:603
usbhid_submit_report drivers/hid/usbhid/hid-core.c:640 [inline]
usbhid_init_reports+0xd7/0x3b0 drivers/hid/usbhid/hid-core.c:780
hiddev_ioctl+0xb27/0x1630 drivers/hid/usbhid/hiddev.c:689
vfs_ioctl fs/ioctl.c:51 [inline]
__do_sys_ioctl fs/ioctl.c:1069 [inline]
__se_sys_ioctl fs/ioctl.c:1055 [inline]
__x64_sys_ioctl+0x193/0x200 fs/ioctl.c:1055
do_syscall_x64 arch/x86/entry/common.c:50 [inline]
do_syscall_64+0x35/0xb0 arch/x86/entry/common.c:80
entry_SYSCALL_64_after_hwframe+0x44/0xae
RIP: 0033:0x444619
Code: 28 00 00 00 75 05 48 83 c4 28 c3 e8 21 14 00 00 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 73 01 c3 48 c7 c1 c0 ff ff ff f7 d8 64 89 01 48
RSP: 002b:00007ffe70eb96d8 EFLAGS: 00000246 ORIG_RAX: 0000000000000010
RAX: ffffffffffffffda RBX: 00000000004004a0 RCX: 0000000000444619
RDX: 0000000000000000 RSI: 0000000000004805 RDI: 0000000000000004
RBP: 0000000000403ea0 R08: 0000000000000001 R09: 00000000004004a0
R10: 000000000000001f R11: 0000000000000246 R12: 0000000000403f30
R13: 0000000000000000 R14: 00000000004b2018 R15: 00000000004004a0
----------------
Code disassembly (best guess):
0: 7c 24 jl 0x26
2: 40 e8 45 64 1f fc rex callq 0xfc1f644d
8: 48 8b 7c 24 40 mov 0x40(%rsp),%rdi
d: e8 4b fc 0b ff callq 0xff0bfc5d
12: 45 89 e8 mov %r13d,%r8d
15: 44 89 f1 mov %r14d,%ecx
18: 4c 89 e2 mov %r12,%rdx
1b: 48 89 c6 mov %rax,%rsi
1e: 48 c7 c7 e0 b2 27 8a mov $0xffffffff8a27b2e0,%rdi
25: e8 01 fc 91 03 callq 0x391fc2b
2a: 0f 0b ud2 <-- trapping instruction
2c: e9 a5 ee ff ff jmpq 0xffffeed6
31: e8 17 64 1f fc callq 0xfc1f644d
36: 0f b6 1d 19 ca 01 08 movzbl 0x801ca19(%rip),%ebx # 0x801ca56
3d: 31 ff xor %edi,%edi
3f: 41 rex.B
On Wed, Aug 18, 2021 at 02:14:23AM -0700, syzbot wrote:
> syzbot has found a reproducer for the following issue on:
>
> HEAD commit: 794c7931a242 Merge branch 'linus' of git://git.kernel.org/..
> git tree: upstream
> console output: https://syzkaller.appspot.com/x/log.txt?x=13af2205300000
> kernel config: https://syzkaller.appspot.com/x/.config?x=96f0602203250753
> dashboard link: https://syzkaller.appspot.com/bug?extid=9b57a46bf1801ce2a2ca
> compiler: gcc (Debian 10.2.1-6) 10.2.1 20210110, GNU ld (GNU Binutils for Debian) 2.35.1
> syz repro: https://syzkaller.appspot.com/x/repro.syz?x=11ae58ce300000
> C reproducer: https://syzkaller.appspot.com/x/repro.c?x=11d71731300000
>
> IMPORTANT: if you fix the issue, please add the following tag to the commit:
> Reported-by: [email protected]
>
> ------------[ cut here ]------------
> usb 1-1: BOGUS control dir, pipe 80000280 doesn't match bRequestType a1
> WARNING: CPU: 0 PID: 8434 at drivers/usb/core/urb.c:410 usb_submit_urb+0x149d/0x18a0 drivers/usb/core/urb.c:410
> Modules linked in:
> CPU: 0 PID: 8434 Comm: syz-executor752 Not tainted 5.14.0-rc6-syzkaller #0
> Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 01/01/2011
> RIP: 0010:usb_submit_urb+0x149d/0x18a0 drivers/usb/core/urb.c:410
> Code: 7c 24 40 e8 45 64 1f fc 48 8b 7c 24 40 e8 4b fc 0b ff 45 89 e8 44 89 f1 4c 89 e2 48 89 c6 48 c7 c7 e0 b2 27 8a e8 01 fc 91 03 <0f> 0b e9 a5 ee ff ff e8 17 64 1f fc 0f b6 1d 19 ca 01 08 31 ff 41
> RSP: 0018:ffffc90000effbd0 EFLAGS: 00010082
> RAX: 0000000000000000 RBX: ffff888027944058 RCX: 0000000000000000
> RDX: ffff8880235db880 RSI: ffffffff815d85c5 RDI: fffff520001dff6c
> RBP: ffff888021618140 R08: 0000000000000000 R09: 0000000000000000
> R10: ffffffff815d23fe R11: 0000000000000000 R12: ffff888018aff118
> R13: 00000000000000a1 R14: 0000000080000280 R15: ffff888021900400
> FS: 000000000223d300(0000) GS:ffff8880b9c00000(0000) knlGS:0000000000000000
> CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> CR2: 00005614a6c2a160 CR3: 00000000222ca000 CR4: 0000000000350ef0
> Call Trace:
> hid_submit_ctrl+0x6ec/0xd80 drivers/hid/usbhid/hid-core.c:416
> usbhid_restart_ctrl_queue.isra.0+0x244/0x3a0 drivers/hid/usbhid/hid-core.c:258
The problem is that syzbot has created a device with a report length of
zero. If we use the padded length instead of the actual length, the
error should vanish.
I believe this is fixed by Michal's patch, below.
Alan Stern
#syz test: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git 794c7931a242
--- a/drivers/hid/usbhid/hid-core.c
+++ b/drivers/hid/usbhid/hid-core.c
@@ -377,27 +377,26 @@ static int hid_submit_ctrl(struct hid_device *hid)
len = hid_report_len(report);
if (dir == USB_DIR_OUT) {
usbhid->urbctrl->pipe = usb_sndctrlpipe(hid_to_usb_dev(hid), 0);
- usbhid->urbctrl->transfer_buffer_length = len;
if (raw_report) {
memcpy(usbhid->ctrlbuf, raw_report, len);
kfree(raw_report);
usbhid->ctrl[usbhid->ctrltail].raw_report = NULL;
}
} else {
- int maxpacket, padlen;
+ int maxpacket;
usbhid->urbctrl->pipe = usb_rcvctrlpipe(hid_to_usb_dev(hid), 0);
maxpacket = usb_maxpacket(hid_to_usb_dev(hid),
usbhid->urbctrl->pipe, 0);
if (maxpacket > 0) {
- padlen = DIV_ROUND_UP(len, maxpacket);
- padlen *= maxpacket;
- if (padlen > usbhid->bufsize)
- padlen = usbhid->bufsize;
+ len = DIV_ROUND_UP(len, maxpacket);
+ len *= maxpacket;
+ if (len > usbhid->bufsize)
+ len = usbhid->bufsize;
} else
- padlen = 0;
- usbhid->urbctrl->transfer_buffer_length = padlen;
+ len = 0;
}
+ usbhid->urbctrl->transfer_buffer_length = len;
usbhid->urbctrl->dev = hid_to_usb_dev(hid);
usbhid->cr->bRequestType = USB_TYPE_CLASS | USB_RECIP_INTERFACE | dir;
On Wed, Aug 18, 2021 at 02:14:23AM -0700, syzbot wrote:
> syzbot has found a reproducer for the following issue on:
>
> HEAD commit: 794c7931a242 Merge branch 'linus' of git://git.kernel.org/..
> git tree: upstream
Dmitry:
Why does syzbot persist in reporting useless names like "upstream" for
the git tree being tested? How is anyone supposed to know what that
actually refers to? Why doesn't it put the real name (maybe with
"upstream" in parentheses as an additional comment)?
Furthermore, the last time I tried to submit a test patch with something
like "#syz test: upstream 794c7931a242", it didn't work because syzbot
itself didn't recognize the repository name!
This should already be fixed -- I can't remember when I first reported
the problem but it must have been at least two years ago.
If there isn't already a change request pending for this issue, can you
add one?
Thanks,
Alan Stern
syzbot has bisected this issue to:
commit 5cc59c418fde9d02859996707b9d5dfd2941c50b
Author: Alan Stern <[email protected]>
Date: Sat May 22 02:16:23 2021 +0000
USB: core: WARN if pipe direction != setup packet direction
bisection log: https://syzkaller.appspot.com/x/bisect.txt?x=126f1731300000
start commit: 794c7931a242 Merge branch 'linus' of git://git.kernel.org/..
git tree: upstream
final oops: https://syzkaller.appspot.com/x/report.txt?x=116f1731300000
console output: https://syzkaller.appspot.com/x/log.txt?x=166f1731300000
kernel config: https://syzkaller.appspot.com/x/.config?x=96f0602203250753
dashboard link: https://syzkaller.appspot.com/bug?extid=9b57a46bf1801ce2a2ca
syz repro: https://syzkaller.appspot.com/x/repro.syz?x=11ae58ce300000
C reproducer: https://syzkaller.appspot.com/x/repro.c?x=11d71731300000
Reported-by: [email protected]
Fixes: 5cc59c418fde ("USB: core: WARN if pipe direction != setup packet direction")
For information about bisection process see: https://goo.gl/tpsmEJ#bisection
Hello,
syzbot has tested the proposed patch but the reproducer is still triggering an issue:
WARNING in hid_submit_ctrl/usb_submit_urb
------------[ cut here ]------------
usb 1-1: BOGUS control dir, pipe 80000280 doesn't match bRequestType a1
WARNING: CPU: 1 PID: 10180 at drivers/usb/core/urb.c:410 usb_submit_urb+0x149d/0x18a0 drivers/usb/core/urb.c:410
Modules linked in:
CPU: 1 PID: 10180 Comm: syz-executor.0 Not tainted 5.14.0-rc6-syzkaller #0
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 01/01/2011
RIP: 0010:usb_submit_urb+0x149d/0x18a0 drivers/usb/core/urb.c:410
Code: 7c 24 40 e8 45 64 1f fc 48 8b 7c 24 40 e8 4b fc 0b ff 45 89 e8 44 89 f1 4c 89 e2 48 89 c6 48 c7 c7 e0 b2 27 8a e8 01 fc 91 03 <0f> 0b e9 a5 ee ff ff e8 17 64 1f fc 0f b6 1d 19 ca 01 08 31 ff 41
RSP: 0018:ffffc9000a68fbd0 EFLAGS: 00010082
RAX: 0000000000000000 RBX: ffff88802e22d058 RCX: 0000000000000000
RDX: ffff88801b2a1c40 RSI: ffffffff815d85c5 RDI: fffff520014d1f6c
RBP: ffff888018fcd910 R08: 0000000000000000 R09: 0000000000000000
R10: ffffffff815d23fe R11: 0000000000000000 R12: ffff8880155fb9d8
R13: 00000000000000a1 R14: 0000000080000280 R15: ffff88801c247600
FS: 00007fdbff87b700(0000) GS:ffff8880b9d00000(0000) knlGS:0000000000000000
CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
CR2: 000000000050eab0 CR3: 000000003d108000 CR4: 0000000000350ee0
Call Trace:
hid_submit_ctrl+0x6ff/0xde0 drivers/hid/usbhid/hid-core.c:415
usbhid_restart_ctrl_queue.isra.0+0x244/0x3a0 drivers/hid/usbhid/hid-core.c:258
__usbhid_submit_report+0x6f0/0xd50 drivers/hid/usbhid/hid-core.c:602
usbhid_submit_report drivers/hid/usbhid/hid-core.c:639 [inline]
usbhid_init_reports+0xd7/0x3b0 drivers/hid/usbhid/hid-core.c:779
hiddev_ioctl+0xb27/0x1630 drivers/hid/usbhid/hiddev.c:689
vfs_ioctl fs/ioctl.c:51 [inline]
__do_sys_ioctl fs/ioctl.c:1069 [inline]
__se_sys_ioctl fs/ioctl.c:1055 [inline]
__x64_sys_ioctl+0x193/0x200 fs/ioctl.c:1055
do_syscall_x64 arch/x86/entry/common.c:50 [inline]
do_syscall_64+0x35/0xb0 arch/x86/entry/common.c:80
entry_SYSCALL_64_after_hwframe+0x44/0xae
RIP: 0033:0x4665e9
Code: ff ff c3 66 2e 0f 1f 84 00 00 00 00 00 0f 1f 40 00 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 73 01 c3 48 c7 c1 bc ff ff ff f7 d8 64 89 01 48
RSP: 002b:00007fdbff87b188 EFLAGS: 00000246 ORIG_RAX: 0000000000000010
RAX: ffffffffffffffda RBX: 000000000056bf80 RCX: 00000000004665e9
RDX: 0000000000000000 RSI: 0000000000004805 RDI: 0000000000000004
RBP: 00000000004bfcc4 R08: 0000000000000000 R09: 0000000000000000
R10: 0000000000000000 R11: 0000000000000246 R12: 000000000056bf80
R13: 00007ffddbcdc2ff R14: 00007fdbff87b300 R15: 0000000000022000
----------------
Code disassembly (best guess):
0: 7c 24 jl 0x26
2: 40 e8 45 64 1f fc rex callq 0xfc1f644d
8: 48 8b 7c 24 40 mov 0x40(%rsp),%rdi
d: e8 4b fc 0b ff callq 0xff0bfc5d
12: 45 89 e8 mov %r13d,%r8d
15: 44 89 f1 mov %r14d,%ecx
18: 4c 89 e2 mov %r12,%rdx
1b: 48 89 c6 mov %rax,%rsi
1e: 48 c7 c7 e0 b2 27 8a mov $0xffffffff8a27b2e0,%rdi
25: e8 01 fc 91 03 callq 0x391fc2b
2a: 0f 0b ud2 <-- trapping instruction
2c: e9 a5 ee ff ff jmpq 0xffffeed6
31: e8 17 64 1f fc callq 0xfc1f644d
36: 0f b6 1d 19 ca 01 08 movzbl 0x801ca19(%rip),%ebx # 0x801ca56
3d: 31 ff xor %edi,%edi
3f: 41 rex.B
Tested on:
commit: 794c7931 Merge branch 'linus' of git://git.kernel.org/..
git tree: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
console output: https://syzkaller.appspot.com/x/log.txt?x=126c1765300000
kernel config: https://syzkaller.appspot.com/x/.config?x=96f0602203250753
dashboard link: https://syzkaller.appspot.com/bug?extid=9b57a46bf1801ce2a2ca
compiler: gcc (Debian 10.2.1-6) 10.2.1 20210110, GNU ld (GNU Binutils for Debian) 2.35.1
patch: https://syzkaller.appspot.com/x/patch.diff?x=152c3561300000
On Wed, Aug 18, 2021 at 01:13:06PM -0700, syzbot wrote:
> Hello,
>
> syzbot has tested the proposed patch but the reproducer is still triggering an issue:
> WARNING in hid_submit_ctrl/usb_submit_urb
>
> ------------[ cut here ]------------
> usb 1-1: BOGUS control dir, pipe 80000280 doesn't match bRequestType a1
> WARNING: CPU: 1 PID: 10180 at drivers/usb/core/urb.c:410 usb_submit_urb+0x149d/0x18a0 drivers/usb/core/urb.c:410
Looks like I was wrong. Let's see what's really happening.
Alan Stern
#syz test: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git 794c7931a242
--- usb-devel.orig/drivers/hid/usbhid/hid-core.c
+++ usb-devel/drivers/hid/usbhid/hid-core.c
@@ -397,6 +397,8 @@ static int hid_submit_ctrl(struct hid_de
} else
padlen = 0;
usbhid->urbctrl->transfer_buffer_length = padlen;
+ hid_err(hid, "submit_ctrl: maxpacket %d len %d padlen %d\n",
+ maxpacket, len, padlen);
}
usbhid->urbctrl->dev = hid_to_usb_dev(hid);
Hello,
syzbot has tested the proposed patch but the reproducer is still triggering an issue:
WARNING in hid_submit_ctrl/usb_submit_urb
cm6533_jd 0003:0D8C:0022.0001: submit_ctrl: maxpacket 64 len 0 padlen 0
------------[ cut here ]------------
usb 1-1: BOGUS control dir, pipe 80000280 doesn't match bRequestType a1
WARNING: CPU: 0 PID: 10203 at drivers/usb/core/urb.c:410 usb_submit_urb+0x149d/0x18a0 drivers/usb/core/urb.c:410
Modules linked in:
CPU: 0 PID: 10203 Comm: syz-executor.0 Not tainted 5.14.0-rc6-syzkaller #0
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 01/01/2011
RIP: 0010:usb_submit_urb+0x149d/0x18a0 drivers/usb/core/urb.c:410
Code: 7c 24 40 e8 45 64 1f fc 48 8b 7c 24 40 e8 4b fc 0b ff 45 89 e8 44 89 f1 4c 89 e2 48 89 c6 48 c7 c7 e0 b2 27 8a e8 01 ec 91 03 <0f> 0b e9 a5 ee ff ff e8 17 64 1f fc 0f b6 1d 59 ca 01 08 31 ff 41
RSP: 0018:ffffc9000a7f7bd0 EFLAGS: 00010082
RAX: 0000000000000000 RBX: ffff888033e83058 RCX: 0000000000000000
RDX: ffff88802bfd8000 RSI: ffffffff815d85c5 RDI: fffff520014fef6c
RBP: ffff8880168b2410 R08: 0000000000000000 R09: 0000000000000000
R10: ffffffff815d23fe R11: 0000000000000000 R12: ffff8880217a0230
R13: 00000000000000a1 R14: 0000000080000280 R15: ffff88801803aa00
FS: 00007f92577f8700(0000) GS:ffff8880b9c00000(0000) knlGS:0000000000000000
CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
CR2: 000000000050eab0 CR3: 000000003dd81000 CR4: 0000000000350ef0
Call Trace:
hid_submit_ctrl+0x6db/0xa70 drivers/hid/usbhid/hid-core.c:418
usbhid_restart_ctrl_queue.isra.0+0x244/0x3a0 drivers/hid/usbhid/hid-core.c:258
__usbhid_submit_report+0x6f0/0xd50 drivers/hid/usbhid/hid-core.c:605
usbhid_submit_report drivers/hid/usbhid/hid-core.c:642 [inline]
usbhid_init_reports+0xd7/0x3b0 drivers/hid/usbhid/hid-core.c:782
hiddev_ioctl+0xb27/0x1630 drivers/hid/usbhid/hiddev.c:689
vfs_ioctl fs/ioctl.c:51 [inline]
__do_sys_ioctl fs/ioctl.c:1069 [inline]
__se_sys_ioctl fs/ioctl.c:1055 [inline]
__x64_sys_ioctl+0x193/0x200 fs/ioctl.c:1055
do_syscall_x64 arch/x86/entry/common.c:50 [inline]
do_syscall_64+0x35/0xb0 arch/x86/entry/common.c:80
entry_SYSCALL_64_after_hwframe+0x44/0xae
RIP: 0033:0x4665e9
Code: ff ff c3 66 2e 0f 1f 84 00 00 00 00 00 0f 1f 40 00 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 73 01 c3 48 c7 c1 bc ff ff ff f7 d8 64 89 01 48
RSP: 002b:00007f92577f8188 EFLAGS: 00000246 ORIG_RAX: 0000000000000010
RAX: ffffffffffffffda RBX: 000000000056bf80 RCX: 00000000004665e9
RDX: 0000000000000000 RSI: 0000000000004805 RDI: 0000000000000004
RBP: 00000000004bfcc4 R08: 0000000000000000 R09: 0000000000000000
R10: 0000000000000000 R11: 0000000000000246 R12: 000000000056bf80
R13: 00007ffcad46f49f R14: 00007f92577f8300 R15: 0000000000022000
Tested on:
commit: 794c7931 Merge branch 'linus' of git://git.kernel.org/..
git tree: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
console output: https://syzkaller.appspot.com/x/log.txt?x=107e8c99300000
kernel config: https://syzkaller.appspot.com/x/.config?x=96f0602203250753
dashboard link: https://syzkaller.appspot.com/bug?extid=9b57a46bf1801ce2a2ca
compiler: gcc (Debian 10.2.1-6) 10.2.1 20210110, GNU ld (GNU Binutils for Debian) 2.35.1
patch: https://syzkaller.appspot.com/x/patch.diff?x=14dde499300000
On Thu, Aug 19, 2021 at 10:35:11AM -0700, syzbot wrote:
> Hello,
>
> syzbot has tested the proposed patch but the reproducer is still triggering an issue:
> WARNING in hid_submit_ctrl/usb_submit_urb
>
> cm6533_jd 0003:0D8C:0022.0001: submit_ctrl: maxpacket 64 len 0 padlen 0
> ------------[ cut here ]------------
> usb 1-1: BOGUS control dir, pipe 80000280 doesn't match bRequestType a1
Ah. The padding code doesn't add anything if the length is
already a multiple of the maxpacket value, and of course 0 is such
a multiple.
The following simplified variant of Michal's patch should fix the
problem.
Alan Stern
#syz test: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git 794c7931a242
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
@@ -377,27 +377,23 @@ static int hid_submit_ctrl(struct hid_de
len = hid_report_len(report);
if (dir == USB_DIR_OUT) {
usbhid->urbctrl->pipe = usb_sndctrlpipe(hid_to_usb_dev(hid), 0);
- usbhid->urbctrl->transfer_buffer_length = len;
if (raw_report) {
memcpy(usbhid->ctrlbuf, raw_report, len);
kfree(raw_report);
usbhid->ctrl[usbhid->ctrltail].raw_report = NULL;
}
} else {
- int maxpacket, padlen;
+ int maxpacket;
usbhid->urbctrl->pipe = usb_rcvctrlpipe(hid_to_usb_dev(hid), 0);
maxpacket = usb_maxpacket(hid_to_usb_dev(hid),
usbhid->urbctrl->pipe, 0);
- if (maxpacket > 0) {
- padlen = DIV_ROUND_UP(len, maxpacket);
- padlen *= maxpacket;
- if (padlen > usbhid->bufsize)
- padlen = usbhid->bufsize;
- } else
- padlen = 0;
- usbhid->urbctrl->transfer_buffer_length = padlen;
+ len += (len == 0); /* Don't allow 0-length reports */
+ len = round_up(len, maxpacket);
+ if (len > usbhid->bufsize)
+ len = usbhid->bufsize;
}
+ usbhid->urbctrl->transfer_buffer_length = len;
usbhid->urbctrl->dev = hid_to_usb_dev(hid);
usbhid->cr->bRequestType = USB_TYPE_CLASS | USB_RECIP_INTERFACE | dir;
Hello,
syzbot has tested the proposed patch and the reproducer did not trigger any issue:
Reported-and-tested-by: [email protected]
Tested on:
commit: 794c7931 Merge branch 'linus' of git://git.kernel.org/..
git tree: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
kernel config: https://syzkaller.appspot.com/x/.config?x=96f0602203250753
dashboard link: https://syzkaller.appspot.com/bug?extid=9b57a46bf1801ce2a2ca
compiler: gcc (Debian 10.2.1-6) 10.2.1 20210110, GNU ld (GNU Binutils for Debian) 2.35.1
patch: https://syzkaller.appspot.com/x/patch.diff?x=119cfb31300000
Note: testing is done by a robot and is best-effort only.
On Thu, Aug 19, 2021 at 05:40:07PM -0700, syzbot wrote:
> Hello,
>
> syzbot has tested the proposed patch and the reproducer did not trigger any issue:
That's good to know. Still, I suspect there's a better way of handling
this condition.
In particular, does it make sense to accept descriptors for input or
feature reports with length zero? I can't imagine what good such
reports would do.
On the other hand, I'm not familiar enough with the code to know the
right way to reject these descriptors and reports. It looks like the
HID subsystem was not designed with this sort of check in mind.
Benjamin and Jiri, what do you think? Is it okay to allow descriptors
for zero-length reports and just pretend they have length 1 (as the
patch tested by syzbot did), or should we instead reject them during
probing?
Alan Stern
On Thu, Aug 19, 2021 at 03:53:00PM -0400, Alan Stern wrote:
> The following simplified variant of Michal's patch should fix the
> problem.
>
> Alan Stern
>
> #syz test: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git 794c7931a242
>
> 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
> @@ -377,27 +377,23 @@ static int hid_submit_ctrl(struct hid_de
> len = hid_report_len(report);
> if (dir == USB_DIR_OUT) {
> usbhid->urbctrl->pipe = usb_sndctrlpipe(hid_to_usb_dev(hid), 0);
> - usbhid->urbctrl->transfer_buffer_length = len;
> if (raw_report) {
> memcpy(usbhid->ctrlbuf, raw_report, len);
> kfree(raw_report);
> usbhid->ctrl[usbhid->ctrltail].raw_report = NULL;
> }
> } else {
> - int maxpacket, padlen;
> + int maxpacket;
>
> usbhid->urbctrl->pipe = usb_rcvctrlpipe(hid_to_usb_dev(hid), 0);
> maxpacket = usb_maxpacket(hid_to_usb_dev(hid),
> usbhid->urbctrl->pipe, 0);
> - if (maxpacket > 0) {
> - padlen = DIV_ROUND_UP(len, maxpacket);
> - padlen *= maxpacket;
> - if (padlen > usbhid->bufsize)
> - padlen = usbhid->bufsize;
> - } else
> - padlen = 0;
> - usbhid->urbctrl->transfer_buffer_length = padlen;
> + len += (len == 0); /* Don't allow 0-length reports */
> + len = round_up(len, maxpacket);
> + if (len > usbhid->bufsize)
> + len = usbhid->bufsize;
> }
> + usbhid->urbctrl->transfer_buffer_length = len;
> usbhid->urbctrl->dev = hid_to_usb_dev(hid);
>
> usbhid->cr->bRequestType = USB_TYPE_CLASS | USB_RECIP_INTERFACE | dir;
I have also tested this patch on top of 5.14-rc7 on my system and it
does address the original issue (no error/warning messages in kernel log
and communication with the UPS works correctly). So if you are going to
submit the patch formally, feel free to use
Tested-by: Michal Kubecek <[email protected]>
Michal
On Fri, 20 Aug 2021, Alan Stern wrote:
> > syzbot has tested the proposed patch and the reproducer did not trigger any issue:
>
> That's good to know. Still, I suspect there's a better way of handling
> this condition.
>
> In particular, does it make sense to accept descriptors for input or
> feature reports with length zero? I can't imagine what good such
> reports would do.
I quickly went through drivers + some hidraw users, and can't spot any use
case for it.
> On the other hand, I'm not familiar enough with the code to know the
> right way to reject these descriptors and reports. It looks like the
> HID subsystem was not designed with this sort of check in mind.
>
> Benjamin and Jiri, what do you think? Is it okay to allow descriptors
> for zero-length reports and just pretend they have length 1 (as the
> patch tested by syzbot did), or should we instead reject them during
> probing?
I think it's a good band-aid for 5.14 (or 5.14-stable if we don't make
it), and if it turns out to break something (which I don't expect), than
we can look into rejecting already during probe.
Benjamin, is there a way to run this quickly through your HID regression
testing machinery?
Thanks,
--
Jiri Kosina
SUSE Labs
On Tue, Aug 24, 2021 at 1:54 PM Jiri Kosina <[email protected]> wrote:
>
> On Fri, 20 Aug 2021, Alan Stern wrote:
>
> > > syzbot has tested the proposed patch and the reproducer did not trigger any issue:
> >
> > That's good to know. Still, I suspect there's a better way of handling
> > this condition.
> >
> > In particular, does it make sense to accept descriptors for input or
> > feature reports with length zero? I can't imagine what good such
> > reports would do.
>
> I quickly went through drivers + some hidraw users, and can't spot any use
> case for it.
>
> > On the other hand, I'm not familiar enough with the code to know the
> > right way to reject these descriptors and reports. It looks like the
> > HID subsystem was not designed with this sort of check in mind.
> >
> > Benjamin and Jiri, what do you think? Is it okay to allow descriptors
> > for zero-length reports and just pretend they have length 1 (as the
> > patch tested by syzbot did), or should we instead reject them during
> > probing?
>
> I think it's a good band-aid for 5.14 (or 5.14-stable if we don't make
> it), and if it turns out to break something (which I don't expect), than
> we can look into rejecting already during probe.
>
> Benjamin, is there a way to run this quickly through your HID regression
> testing machinery?
[Sorry, on holidays since last week until the end of this one]
This patch addresses usbhid, so I don't have tests on this unless I
manually plug mice, keyboards or random input hardware :(
I can try to quickly get a logitech dj receiver test that should use
heavily control endpoints, and probably get a Wacom test. No guarantee
I can get it today though.
Cheers,
Benjamin
>
> Thanks,
>
> --
> Jiri Kosina
> SUSE Labs
>
Hello.
On Ätvrtek 19. srpna 2021 21:53:00 CEST Alan Stern wrote:
> On Thu, Aug 19, 2021 at 10:35:11AM -0700, syzbot wrote:
> > Hello,
> >
> > syzbot has tested the proposed patch but the reproducer is still
> > triggering an issue: WARNING in hid_submit_ctrl/usb_submit_urb
> >
> > cm6533_jd 0003:0D8C:0022.0001: submit_ctrl: maxpacket 64 len 0 padlen 0
> > ------------[ cut here ]------------
> > usb 1-1: BOGUS control dir, pipe 80000280 doesn't match bRequestType a1
>
> Ah. The padding code doesn't add anything if the length is
> already a multiple of the maxpacket value, and of course 0 is such
> a multiple.
>
> The following simplified variant of Michal's patch should fix the
> problem.
>
> Alan Stern
>
> #syz test:
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
> 794c7931a242
>
> 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
> @@ -377,27 +377,23 @@ static int hid_submit_ctrl(struct hid_de
> len = hid_report_len(report);
> if (dir == USB_DIR_OUT) {
> usbhid->urbctrl->pipe = usb_sndctrlpipe(hid_to_usb_dev(hid),
0);
> - usbhid->urbctrl->transfer_buffer_length = len;
> if (raw_report) {
> memcpy(usbhid->ctrlbuf, raw_report, len);
> kfree(raw_report);
> usbhid->ctrl[usbhid->ctrltail].raw_report = NULL;
> }
> } else {
> - int maxpacket, padlen;
> + int maxpacket;
>
> usbhid->urbctrl->pipe = usb_rcvctrlpipe(hid_to_usb_dev(hid),
0);
> maxpacket = usb_maxpacket(hid_to_usb_dev(hid),
> usbhid->urbctrl->pipe, 0);
> - if (maxpacket > 0) {
> - padlen = DIV_ROUND_UP(len, maxpacket);
> - padlen *= maxpacket;
> - if (padlen > usbhid->bufsize)
> - padlen = usbhid->bufsize;
> - } else
> - padlen = 0;
> - usbhid->urbctrl->transfer_buffer_length = padlen;
> + len += (len == 0); /* Don't allow 0-length reports */
> + len = round_up(len, maxpacket);
> + if (len > usbhid->bufsize)
> + len = usbhid->bufsize;
> }
> + usbhid->urbctrl->transfer_buffer_length = len;
> usbhid->urbctrl->dev = hid_to_usb_dev(hid);
>
> usbhid->cr->bRequestType = USB_TYPE_CLASS | USB_RECIP_INTERFACE | dir;
I've tried both Michal's patch as well as this one, and both work for me,
hence feel free to add this:
Tested-by: Oleksandr Natalenko <[email protected]>
once the fix is submitted.
Thanks!
--
Oleksandr Natalenko (post-factum)
On Tue, Aug 24, 2021 at 1:54 PM Jiri Kosina <[email protected]> wrote:
>
> On Fri, 20 Aug 2021, Alan Stern wrote:
>
> > > syzbot has tested the proposed patch and the reproducer did not trigger any issue:
> >
> > That's good to know. Still, I suspect there's a better way of handling
> > this condition.
> >
> > In particular, does it make sense to accept descriptors for input or
> > feature reports with length zero? I can't imagine what good such
> > reports would do.
>
> I quickly went through drivers + some hidraw users, and can't spot any use
> case for it.
>
> > On the other hand, I'm not familiar enough with the code to know the
> > right way to reject these descriptors and reports. It looks like the
> > HID subsystem was not designed with this sort of check in mind.
> >
> > Benjamin and Jiri, what do you think? Is it okay to allow descriptors
> > for zero-length reports and just pretend they have length 1 (as the
> > patch tested by syzbot did), or should we instead reject them during
> > probing?
>
> I think it's a good band-aid for 5.14 (or 5.14-stable if we don't make
> it), and if it turns out to break something (which I don't expect), than
> we can look into rejecting already during probe.
>
> Benjamin, is there a way to run this quickly through your HID regression
> testing machinery?
>
I have finally been able to test this patch:
- the testsuite is still passing (of course, this is not hid-core related)
- Logitech unify receivers are fine (according to the automated tests)
- Gaming mice with hidraw calls works (with libratbag in userspace)
- Wacom Intuos Pro still works (so the usbhid calls to enable the
tablet mode are still OK)
->
Tested-by: Benjamin Tissoires <[email protected]>
Acked-by: Benjamin Tissoires <[email protected]>
Alan, would you mind resending the patch with the various tags with a
commit description? (unless I missed it...)
Cheers,
Benjamin
On Tue, Aug 31, 2021 at 11:51:31AM +0200, Benjamin Tissoires wrote:
> On Tue, Aug 24, 2021 at 1:54 PM Jiri Kosina <[email protected]> wrote:
> >
> > On Fri, 20 Aug 2021, Alan Stern wrote:
> >
> > > > syzbot has tested the proposed patch and the reproducer did not trigger any issue:
> > >
> > > That's good to know. Still, I suspect there's a better way of handling
> > > this condition.
> > >
> > > In particular, does it make sense to accept descriptors for input or
> > > feature reports with length zero? I can't imagine what good such
> > > reports would do.
> >
> > I quickly went through drivers + some hidraw users, and can't spot any use
> > case for it.
> >
> > > On the other hand, I'm not familiar enough with the code to know the
> > > right way to reject these descriptors and reports. It looks like the
> > > HID subsystem was not designed with this sort of check in mind.
> > >
> > > Benjamin and Jiri, what do you think? Is it okay to allow descriptors
> > > for zero-length reports and just pretend they have length 1 (as the
> > > patch tested by syzbot did), or should we instead reject them during
> > > probing?
> >
> > I think it's a good band-aid for 5.14 (or 5.14-stable if we don't make
> > it), and if it turns out to break something (which I don't expect), than
> > we can look into rejecting already during probe.
> >
> > Benjamin, is there a way to run this quickly through your HID regression
> > testing machinery?
> >
>
> I have finally been able to test this patch:
> - the testsuite is still passing (of course, this is not hid-core related)
> - Logitech unify receivers are fine (according to the automated tests)
> - Gaming mice with hidraw calls works (with libratbag in userspace)
> - Wacom Intuos Pro still works (so the usbhid calls to enable the
> tablet mode are still OK)
>
> ->
> Tested-by: Benjamin Tissoires <[email protected]>
> Acked-by: Benjamin Tissoires <[email protected]>
>
> Alan, would you mind resending the patch with the various tags with a
> commit description? (unless I missed it...)
Will do. I'm rather busy today, so it may have to wait until tomorrow.
Alan Stern
On Tue, 31 Aug 2021, Alan Stern wrote:
> > Alan, would you mind resending the patch with the various tags with a
> > commit description? (unless I missed it...)
>
> Will do. I'm rather busy today, so it may have to wait until tomorrow.
Thanks. I'll wait with my pull request to Linus for tomorrow then, to make
sure we get the fix into -rc1 already.
--
Jiri Kosina
SUSE Labs
On Tue, Aug 31, 2021 at 11:51:31AM +0200, Benjamin Tissoires wrote:
> Tested-by: Benjamin Tissoires <[email protected]>
> Acked-by: Benjamin Tissoires <[email protected]>
>
> Alan, would you mind resending the patch with the various tags with a
> commit description? (unless I missed it...)
I plan to break this up into three patches, each doing a single thing. The
first patch in the series will be the one written by Michal. The second
will fix the problem found by syzbot, and the third will be a general
cleanup.
Michal, is it okay to add your Signed-off-by: tag to the first patch?
Alan Stern
On Wed, Sep 01, 2021 at 11:38:11AM -0400, Alan Stern wrote:
> On Tue, Aug 31, 2021 at 11:51:31AM +0200, Benjamin Tissoires wrote:
> > Tested-by: Benjamin Tissoires <[email protected]>
> > Acked-by: Benjamin Tissoires <[email protected]>
> >
> > Alan, would you mind resending the patch with the various tags with a
> > commit description? (unless I missed it...)
>
> I plan to break this up into three patches, each doing a single thing. The
> first patch in the series will be the one written by Michal. The second
> will fix the problem found by syzbot, and the third will be a general
> cleanup.
>
> Michal, is it okay to add your Signed-off-by: tag to the first patch?
Yes, sure.
Michal