linux-usb.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* divide error in dummy_timer
@ 2019-10-24 13:58 syzbot
  2019-10-24 15:04 ` Alan Stern
  0 siblings, 1 reply; 14+ messages in thread
From: syzbot @ 2019-10-24 13:58 UTC (permalink / raw)
  To: Jacky.Cao, andreyknvl, balbi, chunfeng.yun, gregkh, linux-kernel,
	linux-usb, stern, syzkaller-bugs

Hello,

syzbot found the following crash on:

HEAD commit:    22be26f7 usb-fuzzer: main usb gadget fuzzer driver
git tree:       https://github.com/google/kasan.git usb-fuzzer
console output: https://syzkaller.appspot.com/x/log.txt?x=11e2fda7600000
kernel config:  https://syzkaller.appspot.com/x/.config?x=5fe29bc39eff9627
dashboard link: https://syzkaller.appspot.com/bug?extid=8ab8bf161038a8768553
compiler:       gcc (GCC) 9.0.0 20181231 (experimental)
syz repro:      https://syzkaller.appspot.com/x/repro.syz?x=14f664e4e00000
C reproducer:   https://syzkaller.appspot.com/x/repro.c?x=14674000e00000

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

divide error: 0000 [#1] SMP KASAN
CPU: 0 PID: 0 Comm: swapper/0 Not tainted 5.4.0-rc3+ #0
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS  
Google 01/01/2011
RIP: 0010:transfer drivers/usb/gadget/udc/dummy_hcd.c:1426 [inline]
RIP: 0010:dummy_timer+0xad5/0x2fa2 drivers/usb/gadget/udc/dummy_hcd.c:1950
Code: 0f 84 f5 fd ff ff e8 8a 55 ab fd 89 de 44 89 e7 e8 70 56 ab fd 41 39  
dc 0f 82 b0 08 00 00 e8 72 55 ab fd 44 89 e0 31 d2 31 ff <f7> f3 89 d6 89  
94 24 c0 00 00 00 e8 cb 56 ab fd 8b 94 24 c0 00 00
RSP: 0018:ffff8881db209b20 EFLAGS: 00010046
RAX: 0000000000000040 RBX: 0000000000000000 RCX: ffffffff8392c330
RDX: 0000000000000000 RSI: ffffffff8392c33e RDI: 0000000000000000
RBP: 0000000000000000 R08: ffffffff86c2b200 R09: ffffed103b641353
R10: ffffed103b641352 R11: 0000000000000003 R12: 0000000000000040
R13: ffff8881d58c0000 R14: dffffc0000000000 R15: ffff8881d78d5a00
FS:  0000000000000000(0000) GS:ffff8881db200000(0000) knlGS:0000000000000000
CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
CR2: 00007fb771a61000 CR3: 00000001d2295000 CR4: 00000000001406f0
DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
Call Trace:
  <IRQ>
  call_timer_fn+0x179/0x650 kernel/time/timer.c:1404
  expire_timers kernel/time/timer.c:1449 [inline]
  __run_timers kernel/time/timer.c:1773 [inline]
  __run_timers kernel/time/timer.c:1740 [inline]
  run_timer_softirq+0x5e3/0x1490 kernel/time/timer.c:1786
  __do_softirq+0x221/0x912 kernel/softirq.c:292
  invoke_softirq kernel/softirq.c:373 [inline]
  irq_exit+0x178/0x1a0 kernel/softirq.c:413
  exiting_irq arch/x86/include/asm/apic.h:536 [inline]
  smp_apic_timer_interrupt+0x12f/0x500 arch/x86/kernel/apic/apic.c:1137
  apic_timer_interrupt+0xf/0x20 arch/x86/entry/entry_64.S:830
  </IRQ>
RIP: 0010:default_idle+0x28/0x2e0 arch/x86/kernel/process.c:581
Code: 90 90 41 56 41 55 65 44 8b 2d f4 00 92 7a 41 54 55 53 0f 1f 44 00 00  
e8 c6 b2 d3 fb e9 07 00 00 00 0f 00 2d ea a5 52 00 fb f4 <65> 44 8b 2d d0  
00 92 7a 0f 1f 44 00 00 5b 5d 41 5c 41 5d 41 5e c3
RSP: 0018:ffffffff86c07da8 EFLAGS: 00000246 ORIG_RAX: ffffffffffffff13
RAX: 0000000000000007 RBX: ffffffff86c2b200 RCX: 0000000000000000
RDX: 0000000000000000 RSI: 0000000000000006 RDI: ffffffff86c2ba4c
RBP: fffffbfff0d85640 R08: ffffffff86c2b200 R09: 0000000000000000
R10: 0000000000000000 R11: 0000000000000000 R12: 0000000000000000
R13: 0000000000000000 R14: 0000000000000000 R15: 0000000000000000
  cpuidle_idle_call kernel/sched/idle.c:154 [inline]
  do_idle+0x3b6/0x500 kernel/sched/idle.c:263
  cpu_startup_entry+0x14/0x20 kernel/sched/idle.c:355
  start_kernel+0x82a/0x864 init/main.c:784
  secondary_startup_64+0xa4/0xb0 arch/x86/kernel/head_64.S:241
Modules linked in:
---[ end trace 02e2d9c7c53d85a5 ]---
RIP: 0010:transfer drivers/usb/gadget/udc/dummy_hcd.c:1426 [inline]
RIP: 0010:dummy_timer+0xad5/0x2fa2 drivers/usb/gadget/udc/dummy_hcd.c:1950
Code: 0f 84 f5 fd ff ff e8 8a 55 ab fd 89 de 44 89 e7 e8 70 56 ab fd 41 39  
dc 0f 82 b0 08 00 00 e8 72 55 ab fd 44 89 e0 31 d2 31 ff <f7> f3 89 d6 89  
94 24 c0 00 00 00 e8 cb 56 ab fd 8b 94 24 c0 00 00
RSP: 0018:ffff8881db209b20 EFLAGS: 00010046
RAX: 0000000000000040 RBX: 0000000000000000 RCX: ffffffff8392c330
RDX: 0000000000000000 RSI: ffffffff8392c33e RDI: 0000000000000000
RBP: 0000000000000000 R08: ffffffff86c2b200 R09: ffffed103b641353
R10: ffffed103b641352 R11: 0000000000000003 R12: 0000000000000040
R13: ffff8881d58c0000 R14: dffffc0000000000 R15: ffff8881d78d5a00
FS:  0000000000000000(0000) GS:ffff8881db200000(0000) knlGS:0000000000000000
CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
CR2: 00007fb771a61000 CR3: 00000001d2295000 CR4: 00000000001406f0
DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400


---
This bug is generated by a bot. It may contain errors.
See https://goo.gl/tpsmEJ for more information about syzbot.
syzbot engineers can be reached at syzkaller@googlegroups.com.

syzbot will keep track of this bug report. See:
https://goo.gl/tpsmEJ#status for how to communicate with syzbot.
syzbot can test patches for this bug, for details see:
https://goo.gl/tpsmEJ#testing-patches

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: divide error in dummy_timer
  2019-10-24 13:58 divide error in dummy_timer syzbot
@ 2019-10-24 15:04 ` Alan Stern
  2019-10-24 15:22   ` syzbot
  2019-10-24 17:05   ` Andrey Konovalov
  0 siblings, 2 replies; 14+ messages in thread
From: Alan Stern @ 2019-10-24 15:04 UTC (permalink / raw)
  To: syzbot
  Cc: Jacky.Cao, andreyknvl, balbi, chunfeng.yun, gregkh, linux-kernel,
	linux-usb, syzkaller-bugs

On Thu, 24 Oct 2019, syzbot wrote:

> Hello,
> 
> syzbot found the following crash on:
> 
> HEAD commit:    22be26f7 usb-fuzzer: main usb gadget fuzzer driver
> git tree:       https://github.com/google/kasan.git usb-fuzzer
> console output: https://syzkaller.appspot.com/x/log.txt?x=11e2fda7600000
> kernel config:  https://syzkaller.appspot.com/x/.config?x=5fe29bc39eff9627
> dashboard link: https://syzkaller.appspot.com/bug?extid=8ab8bf161038a8768553
> compiler:       gcc (GCC) 9.0.0 20181231 (experimental)
> syz repro:      https://syzkaller.appspot.com/x/repro.syz?x=14f664e4e00000
> C reproducer:   https://syzkaller.appspot.com/x/repro.c?x=14674000e00000
> 
> IMPORTANT: if you fix the bug, please add the following tag to the commit:
> Reported-by: syzbot+8ab8bf161038a8768553@syzkaller.appspotmail.com
> 
> divide error: 0000 [#1] SMP KASAN
> CPU: 0 PID: 0 Comm: swapper/0 Not tainted 5.4.0-rc3+ #0
> Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS  
> Google 01/01/2011
> RIP: 0010:transfer drivers/usb/gadget/udc/dummy_hcd.c:1426 [inline]
> RIP: 0010:dummy_timer+0xad5/0x2fa2 drivers/usb/gadget/udc/dummy_hcd.c:1950
> Code: 0f 84 f5 fd ff ff e8 8a 55 ab fd 89 de 44 89 e7 e8 70 56 ab fd 41 39  
> dc 0f 82 b0 08 00 00 e8 72 55 ab fd 44 89 e0 31 d2 31 ff <f7> f3 89 d6 89  
> 94 24 c0 00 00 00 e8 cb 56 ab fd 8b 94 24 c0 00 00
> RSP: 0018:ffff8881db209b20 EFLAGS: 00010046
> RAX: 0000000000000040 RBX: 0000000000000000 RCX: ffffffff8392c330
> RDX: 0000000000000000 RSI: ffffffff8392c33e RDI: 0000000000000000
> RBP: 0000000000000000 R08: ffffffff86c2b200 R09: ffffed103b641353
> R10: ffffed103b641352 R11: 0000000000000003 R12: 0000000000000040
> R13: ffff8881d58c0000 R14: dffffc0000000000 R15: ffff8881d78d5a00
> FS:  0000000000000000(0000) GS:ffff8881db200000(0000) knlGS:0000000000000000
> CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> CR2: 00007fb771a61000 CR3: 00000001d2295000 CR4: 00000000001406f0
> DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
> DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
> Call Trace:
>   <IRQ>
>   call_timer_fn+0x179/0x650 kernel/time/timer.c:1404
>   expire_timers kernel/time/timer.c:1449 [inline]
>   __run_timers kernel/time/timer.c:1773 [inline]
>   __run_timers kernel/time/timer.c:1740 [inline]
>   run_timer_softirq+0x5e3/0x1490 kernel/time/timer.c:1786
>   __do_softirq+0x221/0x912 kernel/softirq.c:292
>   invoke_softirq kernel/softirq.c:373 [inline]
>   irq_exit+0x178/0x1a0 kernel/softirq.c:413
>   exiting_irq arch/x86/include/asm/apic.h:536 [inline]
>   smp_apic_timer_interrupt+0x12f/0x500 arch/x86/kernel/apic/apic.c:1137
>   apic_timer_interrupt+0xf/0x20 arch/x86/entry/entry_64.S:830
>   </IRQ>
> RIP: 0010:default_idle+0x28/0x2e0 arch/x86/kernel/process.c:581
> Code: 90 90 41 56 41 55 65 44 8b 2d f4 00 92 7a 41 54 55 53 0f 1f 44 00 00  
> e8 c6 b2 d3 fb e9 07 00 00 00 0f 00 2d ea a5 52 00 fb f4 <65> 44 8b 2d d0  
> 00 92 7a 0f 1f 44 00 00 5b 5d 41 5c 41 5d 41 5e c3
> RSP: 0018:ffffffff86c07da8 EFLAGS: 00000246 ORIG_RAX: ffffffffffffff13
> RAX: 0000000000000007 RBX: ffffffff86c2b200 RCX: 0000000000000000
> RDX: 0000000000000000 RSI: 0000000000000006 RDI: ffffffff86c2ba4c
> RBP: fffffbfff0d85640 R08: ffffffff86c2b200 R09: 0000000000000000
> R10: 0000000000000000 R11: 0000000000000000 R12: 0000000000000000
> R13: 0000000000000000 R14: 0000000000000000 R15: 0000000000000000
>   cpuidle_idle_call kernel/sched/idle.c:154 [inline]
>   do_idle+0x3b6/0x500 kernel/sched/idle.c:263
>   cpu_startup_entry+0x14/0x20 kernel/sched/idle.c:355
>   start_kernel+0x82a/0x864 init/main.c:784
>   secondary_startup_64+0xa4/0xb0 arch/x86/kernel/head_64.S:241
> Modules linked in:
> ---[ end trace 02e2d9c7c53d85a5 ]---
> RIP: 0010:transfer drivers/usb/gadget/udc/dummy_hcd.c:1426 [inline]
> RIP: 0010:dummy_timer+0xad5/0x2fa2 drivers/usb/gadget/udc/dummy_hcd.c:1950
> Code: 0f 84 f5 fd ff ff e8 8a 55 ab fd 89 de 44 89 e7 e8 70 56 ab fd 41 39  
> dc 0f 82 b0 08 00 00 e8 72 55 ab fd 44 89 e0 31 d2 31 ff <f7> f3 89 d6 89  
> 94 24 c0 00 00 00 e8 cb 56 ab fd 8b 94 24 c0 00 00
> RSP: 0018:ffff8881db209b20 EFLAGS: 00010046
> RAX: 0000000000000040 RBX: 0000000000000000 RCX: ffffffff8392c330
> RDX: 0000000000000000 RSI: ffffffff8392c33e RDI: 0000000000000000
> RBP: 0000000000000000 R08: ffffffff86c2b200 R09: ffffed103b641353
> R10: ffffed103b641352 R11: 0000000000000003 R12: 0000000000000040
> R13: ffff8881d58c0000 R14: dffffc0000000000 R15: ffff8881d78d5a00
> FS:  0000000000000000(0000) GS:ffff8881db200000(0000) knlGS:0000000000000000
> CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> CR2: 00007fb771a61000 CR3: 00000001d2295000 CR4: 00000000001406f0
> DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
> DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
> 
> 
> ---
> This bug is generated by a bot. It may contain errors.
> See https://goo.gl/tpsmEJ for more information about syzbot.
> syzbot engineers can be reached at syzkaller@googlegroups.com.
> 
> syzbot will keep track of this bug report. See:
> https://goo.gl/tpsmEJ#status for how to communicate with syzbot.
> syzbot can test patches for this bug, for details see:
> https://goo.gl/tpsmEJ#testing-patches

Is this really the sort of thing we need to catch?  It isn't a bug in
any existing kernel code, as far as I know.  Maybe only gadgetfs and 
configfs need to worry about it.

Alan Stern

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

 drivers/usb/gadget/udc/dummy_hcd.c |    2 ++
 1 file changed, 2 insertions(+)

Index: usb-devel/drivers/usb/gadget/udc/dummy_hcd.c
===================================================================
--- usb-devel.orig/drivers/usb/gadget/udc/dummy_hcd.c
+++ usb-devel/drivers/usb/gadget/udc/dummy_hcd.c
@@ -522,6 +522,8 @@ static int dummy_enable(struct usb_ep *_
 	 * For SS devices the wMaxPacketSize is limited by 1024.
 	 */
 	max = usb_endpoint_maxp(desc);
+	if (max == 0)
+		return -EINVAL;
 
 	/* drivers must not request bad settings, since lower levels
 	 * (hardware or its drivers) may not check.  some endpoints


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: divide error in dummy_timer
  2019-10-24 15:04 ` Alan Stern
@ 2019-10-24 15:22   ` syzbot
  2019-10-24 17:05   ` Andrey Konovalov
  1 sibling, 0 replies; 14+ messages in thread
From: syzbot @ 2019-10-24 15:22 UTC (permalink / raw)
  To: Jacky.Cao, andreyknvl, balbi, chunfeng.yun, gregkh, linux-kernel,
	linux-usb, stern, syzkaller-bugs

Hello,

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

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

Tested on:

commit:         22be26f7 usb-fuzzer: main usb gadget fuzzer driver
git tree:       https://github.com/google/kasan.git
kernel config:  https://syzkaller.appspot.com/x/.config?x=5fe29bc39eff9627
dashboard link: https://syzkaller.appspot.com/bug?extid=8ab8bf161038a8768553
compiler:       gcc (GCC) 9.0.0 20181231 (experimental)
patch:          https://syzkaller.appspot.com/x/patch.diff?x=14f2fda7600000

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

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: divide error in dummy_timer
  2019-10-24 15:04 ` Alan Stern
  2019-10-24 15:22   ` syzbot
@ 2019-10-24 17:05   ` Andrey Konovalov
  2019-10-24 17:57     ` Alan Stern
  1 sibling, 1 reply; 14+ messages in thread
From: Andrey Konovalov @ 2019-10-24 17:05 UTC (permalink / raw)
  To: Alan Stern
  Cc: syzbot, Jacky . Cao @ sony . com, Felipe Balbi, Chunfeng Yun,
	Greg Kroah-Hartman, LKML, USB list, syzkaller-bugs

On Thu, Oct 24, 2019 at 5:04 PM Alan Stern <stern@rowland.harvard.edu> wrote:
>
> On Thu, 24 Oct 2019, syzbot wrote:
>
> > Hello,
> >
> > syzbot found the following crash on:
> >
> > HEAD commit:    22be26f7 usb-fuzzer: main usb gadget fuzzer driver
> > git tree:       https://github.com/google/kasan.git usb-fuzzer
> > console output: https://syzkaller.appspot.com/x/log.txt?x=11e2fda7600000
> > kernel config:  https://syzkaller.appspot.com/x/.config?x=5fe29bc39eff9627
> > dashboard link: https://syzkaller.appspot.com/bug?extid=8ab8bf161038a8768553
> > compiler:       gcc (GCC) 9.0.0 20181231 (experimental)
> > syz repro:      https://syzkaller.appspot.com/x/repro.syz?x=14f664e4e00000
> > C reproducer:   https://syzkaller.appspot.com/x/repro.c?x=14674000e00000
> >
> > IMPORTANT: if you fix the bug, please add the following tag to the commit:
> > Reported-by: syzbot+8ab8bf161038a8768553@syzkaller.appspotmail.com
> >
> > divide error: 0000 [#1] SMP KASAN
> > CPU: 0 PID: 0 Comm: swapper/0 Not tainted 5.4.0-rc3+ #0
> > Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS
> > Google 01/01/2011
> > RIP: 0010:transfer drivers/usb/gadget/udc/dummy_hcd.c:1426 [inline]
> > RIP: 0010:dummy_timer+0xad5/0x2fa2 drivers/usb/gadget/udc/dummy_hcd.c:1950
> > Code: 0f 84 f5 fd ff ff e8 8a 55 ab fd 89 de 44 89 e7 e8 70 56 ab fd 41 39
> > dc 0f 82 b0 08 00 00 e8 72 55 ab fd 44 89 e0 31 d2 31 ff <f7> f3 89 d6 89
> > 94 24 c0 00 00 00 e8 cb 56 ab fd 8b 94 24 c0 00 00
> > RSP: 0018:ffff8881db209b20 EFLAGS: 00010046
> > RAX: 0000000000000040 RBX: 0000000000000000 RCX: ffffffff8392c330
> > RDX: 0000000000000000 RSI: ffffffff8392c33e RDI: 0000000000000000
> > RBP: 0000000000000000 R08: ffffffff86c2b200 R09: ffffed103b641353
> > R10: ffffed103b641352 R11: 0000000000000003 R12: 0000000000000040
> > R13: ffff8881d58c0000 R14: dffffc0000000000 R15: ffff8881d78d5a00
> > FS:  0000000000000000(0000) GS:ffff8881db200000(0000) knlGS:0000000000000000
> > CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> > CR2: 00007fb771a61000 CR3: 00000001d2295000 CR4: 00000000001406f0
> > DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
> > DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
> > Call Trace:
> >   <IRQ>
> >   call_timer_fn+0x179/0x650 kernel/time/timer.c:1404
> >   expire_timers kernel/time/timer.c:1449 [inline]
> >   __run_timers kernel/time/timer.c:1773 [inline]
> >   __run_timers kernel/time/timer.c:1740 [inline]
> >   run_timer_softirq+0x5e3/0x1490 kernel/time/timer.c:1786
> >   __do_softirq+0x221/0x912 kernel/softirq.c:292
> >   invoke_softirq kernel/softirq.c:373 [inline]
> >   irq_exit+0x178/0x1a0 kernel/softirq.c:413
> >   exiting_irq arch/x86/include/asm/apic.h:536 [inline]
> >   smp_apic_timer_interrupt+0x12f/0x500 arch/x86/kernel/apic/apic.c:1137
> >   apic_timer_interrupt+0xf/0x20 arch/x86/entry/entry_64.S:830
> >   </IRQ>
> > RIP: 0010:default_idle+0x28/0x2e0 arch/x86/kernel/process.c:581
> > Code: 90 90 41 56 41 55 65 44 8b 2d f4 00 92 7a 41 54 55 53 0f 1f 44 00 00
> > e8 c6 b2 d3 fb e9 07 00 00 00 0f 00 2d ea a5 52 00 fb f4 <65> 44 8b 2d d0
> > 00 92 7a 0f 1f 44 00 00 5b 5d 41 5c 41 5d 41 5e c3
> > RSP: 0018:ffffffff86c07da8 EFLAGS: 00000246 ORIG_RAX: ffffffffffffff13
> > RAX: 0000000000000007 RBX: ffffffff86c2b200 RCX: 0000000000000000
> > RDX: 0000000000000000 RSI: 0000000000000006 RDI: ffffffff86c2ba4c
> > RBP: fffffbfff0d85640 R08: ffffffff86c2b200 R09: 0000000000000000
> > R10: 0000000000000000 R11: 0000000000000000 R12: 0000000000000000
> > R13: 0000000000000000 R14: 0000000000000000 R15: 0000000000000000
> >   cpuidle_idle_call kernel/sched/idle.c:154 [inline]
> >   do_idle+0x3b6/0x500 kernel/sched/idle.c:263
> >   cpu_startup_entry+0x14/0x20 kernel/sched/idle.c:355
> >   start_kernel+0x82a/0x864 init/main.c:784
> >   secondary_startup_64+0xa4/0xb0 arch/x86/kernel/head_64.S:241
> > Modules linked in:
> > ---[ end trace 02e2d9c7c53d85a5 ]---
> > RIP: 0010:transfer drivers/usb/gadget/udc/dummy_hcd.c:1426 [inline]
> > RIP: 0010:dummy_timer+0xad5/0x2fa2 drivers/usb/gadget/udc/dummy_hcd.c:1950
> > Code: 0f 84 f5 fd ff ff e8 8a 55 ab fd 89 de 44 89 e7 e8 70 56 ab fd 41 39
> > dc 0f 82 b0 08 00 00 e8 72 55 ab fd 44 89 e0 31 d2 31 ff <f7> f3 89 d6 89
> > 94 24 c0 00 00 00 e8 cb 56 ab fd 8b 94 24 c0 00 00
> > RSP: 0018:ffff8881db209b20 EFLAGS: 00010046
> > RAX: 0000000000000040 RBX: 0000000000000000 RCX: ffffffff8392c330
> > RDX: 0000000000000000 RSI: ffffffff8392c33e RDI: 0000000000000000
> > RBP: 0000000000000000 R08: ffffffff86c2b200 R09: ffffed103b641353
> > R10: ffffed103b641352 R11: 0000000000000003 R12: 0000000000000040
> > R13: ffff8881d58c0000 R14: dffffc0000000000 R15: ffff8881d78d5a00
> > FS:  0000000000000000(0000) GS:ffff8881db200000(0000) knlGS:0000000000000000
> > CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> > CR2: 00007fb771a61000 CR3: 00000001d2295000 CR4: 00000000001406f0
> > DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
> > DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
> >
> >
> > ---
> > This bug is generated by a bot. It may contain errors.
> > See https://goo.gl/tpsmEJ for more information about syzbot.
> > syzbot engineers can be reached at syzkaller@googlegroups.com.
> >
> > syzbot will keep track of this bug report. See:
> > https://goo.gl/tpsmEJ#status for how to communicate with syzbot.
> > syzbot can test patches for this bug, for details see:
> > https://goo.gl/tpsmEJ#testing-patches
>
> Is this really the sort of thing we need to catch?  It isn't a bug in
> any existing kernel code, as far as I know.  Maybe only gadgetfs and
> configfs need to worry about it.

Hi Alan,

Do you mean that the gadget driver must ensure that the max packet
size in the endpoint descriptor is not zero? Do HCDs rely on that? I
can add this check into the driver we use for USB fuzzing.

Thanks!

>
> Alan Stern
>
> #syz test: https://github.com/google/kasan.git 22be26f7
>
>  drivers/usb/gadget/udc/dummy_hcd.c |    2 ++
>  1 file changed, 2 insertions(+)
>
> Index: usb-devel/drivers/usb/gadget/udc/dummy_hcd.c
> ===================================================================
> --- usb-devel.orig/drivers/usb/gadget/udc/dummy_hcd.c
> +++ usb-devel/drivers/usb/gadget/udc/dummy_hcd.c
> @@ -522,6 +522,8 @@ static int dummy_enable(struct usb_ep *_
>          * For SS devices the wMaxPacketSize is limited by 1024.
>          */
>         max = usb_endpoint_maxp(desc);
> +       if (max == 0)
> +               return -EINVAL;
>
>         /* drivers must not request bad settings, since lower levels
>          * (hardware or its drivers) may not check.  some endpoints
>

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: divide error in dummy_timer
  2019-10-24 17:05   ` Andrey Konovalov
@ 2019-10-24 17:57     ` Alan Stern
  2019-10-24 18:08       ` syzbot
  2019-10-24 18:55       ` divide error in dummy_timer Andrey Konovalov
  0 siblings, 2 replies; 14+ messages in thread
From: Alan Stern @ 2019-10-24 17:57 UTC (permalink / raw)
  To: Andrey Konovalov
  Cc: syzbot, Jacky . Cao @ sony . com, Felipe Balbi, Chunfeng Yun,
	Greg Kroah-Hartman, LKML, USB list, syzkaller-bugs

On Thu, 24 Oct 2019, Andrey Konovalov wrote:

> > Is this really the sort of thing we need to catch?  It isn't a bug in
> > any existing kernel code, as far as I know.  Maybe only gadgetfs and
> > configfs need to worry about it.
> 
> Hi Alan,
> 
> Do you mean that the gadget driver must ensure that the max packet
> size in the endpoint descriptor is not zero? Do HCDs rely on that? I
> can add this check into the driver we use for USB fuzzing.

Well, if there are any gadget drivers in the kernel which do set an
endpoint's maxpacket size to 0, they should be fixed.  I'm not aware of
any.

Of course, gadget drivers in userspace are always suspect.  That's why
I suggested having gadgetfs and configfs perform this check.  Even so
it's not really a _security_ risk, because only the superuser is
allowed to run a userspace gadget driver.  (Although obviously it is 
better to have a clean failure than to crash the system when a buggy 
program runs with superuser privileges.)

Yes, HCDs do depend on endpoints having reasonable maxpacket values.  I 
suppose the core should check for this.  Currently we check for values 
that are too large or invalid in other ways (like high-speed bulk 
endpoints with maxpacket != 512), but we don't check for 0.

In fact, that sounds like a much better solution to the problem
overall.  Let's see if this patch fixes the bug...

Alan Stern

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

 drivers/usb/core/config.c |    5 +++++
 1 file changed, 5 insertions(+)

Index: usb-devel/drivers/usb/core/config.c
===================================================================
--- usb-devel.orig/drivers/usb/core/config.c
+++ usb-devel/drivers/usb/core/config.c
@@ -348,6 +348,11 @@ static int usb_parse_endpoint(struct dev
 
 	/* Validate the wMaxPacketSize field */
 	maxp = usb_endpoint_maxp(&endpoint->desc);
+	if (maxp == 0) {
+		dev_warn(ddev, "config %d interface %d altsetting %d endpoint 0x%X has wMaxPacketSize 0, skipping\n",
+		    cfgno, inum, asnum, d->bEndpointAddress);
+		goto skip_to_next_endpoint_or_interface_descriptor;
+	}
 
 	/* Find the highest legal maxpacket size for this endpoint */
 	i = 0;		/* additional transactions per microframe */


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: divide error in dummy_timer
  2019-10-24 17:57     ` Alan Stern
@ 2019-10-24 18:08       ` syzbot
  2019-10-24 18:58         ` Alan Stern
  2019-10-24 18:55       ` divide error in dummy_timer Andrey Konovalov
  1 sibling, 1 reply; 14+ messages in thread
From: syzbot @ 2019-10-24 18:08 UTC (permalink / raw)
  To: Jacky.Cao, andreyknvl, balbi, chunfeng.yun, gregkh, linux-kernel,
	linux-usb, stern, syzkaller-bugs

Hello,

syzbot has tested the proposed patch but the reproducer still triggered  
crash:
divide error in dummy_timer

divide error: 0000 [#1] SMP KASAN
CPU: 1 PID: 0 Comm: swapper/1 Not tainted 5.4.0-rc3+ #0
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS  
Google 01/01/2011
RIP: 0010:transfer drivers/usb/gadget/udc/dummy_hcd.c:1426 [inline]
RIP: 0010:dummy_timer+0xad5/0x2fa2 drivers/usb/gadget/udc/dummy_hcd.c:1950
Code: 0f 84 f5 fd ff ff e8 2a 54 ab fd 89 de 44 89 e7 e8 10 55 ab fd 41 39  
dc 0f 82 b0 08 00 00 e8 12 54 ab fd 44 89 e0 31 d2 31 ff <f7> f3 89 d6 89  
94 24 c0 00 00 00 e8 6b 55 ab fd 8b 94 24 c0 00 00
RSP: 0018:ffff8881db309b20 EFLAGS: 00010046
RAX: 0000000000000040 RBX: 0000000000000000 RCX: ffffffff8392c490
RDX: 0000000000000000 RSI: ffffffff8392c49e RDI: 0000000000000000
RBP: 0000000000000000 R08: ffff8881da213000 R09: ffffed103b661353
R10: ffffed103b661352 R11: 0000000000000003 R12: 0000000000000040
R13: ffff8881d4712a00 R14: dffffc0000000000 R15: ffff8881d20a2200
FS:  0000000000000000(0000) GS:ffff8881db300000(0000) knlGS:0000000000000000
CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
CR2: 00007fb31aad7000 CR3: 00000001d4b0e000 CR4: 00000000001406e0
DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
Call Trace:
  <IRQ>
  call_timer_fn+0x179/0x650 kernel/time/timer.c:1404
  expire_timers kernel/time/timer.c:1449 [inline]
  __run_timers kernel/time/timer.c:1773 [inline]
  __run_timers kernel/time/timer.c:1740 [inline]
  run_timer_softirq+0x5e3/0x1490 kernel/time/timer.c:1786
  __do_softirq+0x221/0x912 kernel/softirq.c:292
  invoke_softirq kernel/softirq.c:373 [inline]
  irq_exit+0x178/0x1a0 kernel/softirq.c:413
  exiting_irq arch/x86/include/asm/apic.h:536 [inline]
  smp_apic_timer_interrupt+0x12f/0x500 arch/x86/kernel/apic/apic.c:1137
  apic_timer_interrupt+0xf/0x20 arch/x86/entry/entry_64.S:830
  </IRQ>
RIP: 0010:default_idle+0x28/0x2e0 arch/x86/kernel/process.c:581
Code: 90 90 41 56 41 55 65 44 8b 2d f4 00 92 7a 41 54 55 53 0f 1f 44 00 00  
e8 c6 b2 d3 fb e9 07 00 00 00 0f 00 2d ea a5 52 00 fb f4 <65> 44 8b 2d d0  
00 92 7a 0f 1f 44 00 00 5b 5d 41 5c 41 5d 41 5e c3
RSP: 0018:ffff8881da22fdc8 EFLAGS: 00000246 ORIG_RAX: ffffffffffffff13
RAX: 0000000000000007 RBX: ffff8881da213000 RCX: 0000000000000000
RDX: 0000000000000000 RSI: 0000000000000006 RDI: ffff8881da21384c
RBP: ffffed103b442600 R08: ffff8881da213000 R09: 0000000000000000
R10: 0000000000000000 R11: 0000000000000000 R12: 0000000000000001
R13: 0000000000000001 R14: 0000000000000000 R15: 0000000000000000
  cpuidle_idle_call kernel/sched/idle.c:154 [inline]
  do_idle+0x3b6/0x500 kernel/sched/idle.c:263
  cpu_startup_entry+0x14/0x20 kernel/sched/idle.c:355
  start_secondary+0x27d/0x330 arch/x86/kernel/smpboot.c:264
  secondary_startup_64+0xa4/0xb0 arch/x86/kernel/head_64.S:241
Modules linked in:
---[ end trace e6cab2429e956502 ]---
RIP: 0010:transfer drivers/usb/gadget/udc/dummy_hcd.c:1426 [inline]
RIP: 0010:dummy_timer+0xad5/0x2fa2 drivers/usb/gadget/udc/dummy_hcd.c:1950
Code: 0f 84 f5 fd ff ff e8 2a 54 ab fd 89 de 44 89 e7 e8 10 55 ab fd 41 39  
dc 0f 82 b0 08 00 00 e8 12 54 ab fd 44 89 e0 31 d2 31 ff <f7> f3 89 d6 89  
94 24 c0 00 00 00 e8 6b 55 ab fd 8b 94 24 c0 00 00
RSP: 0018:ffff8881db309b20 EFLAGS: 00010046
RAX: 0000000000000040 RBX: 0000000000000000 RCX: ffffffff8392c490
RDX: 0000000000000000 RSI: ffffffff8392c49e RDI: 0000000000000000
RBP: 0000000000000000 R08: ffff8881da213000 R09: ffffed103b661353
R10: ffffed103b661352 R11: 0000000000000003 R12: 0000000000000040
R13: ffff8881d4712a00 R14: dffffc0000000000 R15: ffff8881d20a2200
FS:  0000000000000000(0000) GS:ffff8881db300000(0000) knlGS:0000000000000000
CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
CR2: 00007fb31aad7000 CR3: 00000001d4b0e000 CR4: 00000000001406e0
DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400


Tested on:

commit:         22be26f7 usb-fuzzer: main usb gadget fuzzer driver
git tree:       https://github.com/google/kasan.git
console output: https://syzkaller.appspot.com/x/log.txt?x=17b3e44ce00000
kernel config:  https://syzkaller.appspot.com/x/.config?x=5fe29bc39eff9627
dashboard link: https://syzkaller.appspot.com/bug?extid=8ab8bf161038a8768553
compiler:       gcc (GCC) 9.0.0 20181231 (experimental)
patch:          https://syzkaller.appspot.com/x/patch.diff?x=15ea0a97600000


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: divide error in dummy_timer
  2019-10-24 17:57     ` Alan Stern
  2019-10-24 18:08       ` syzbot
@ 2019-10-24 18:55       ` Andrey Konovalov
  2019-10-24 19:17         ` Alan Stern
  1 sibling, 1 reply; 14+ messages in thread
From: Andrey Konovalov @ 2019-10-24 18:55 UTC (permalink / raw)
  To: Alan Stern
  Cc: syzbot, Jacky . Cao @ sony . com, Felipe Balbi, Chunfeng Yun,
	Greg Kroah-Hartman, LKML, USB list, syzkaller-bugs

On Thu, Oct 24, 2019 at 7:57 PM Alan Stern <stern@rowland.harvard.edu> wrote:
>
> On Thu, 24 Oct 2019, Andrey Konovalov wrote:
>
> > > Is this really the sort of thing we need to catch?  It isn't a bug in
> > > any existing kernel code, as far as I know.  Maybe only gadgetfs and
> > > configfs need to worry about it.
> >
> > Hi Alan,
> >
> > Do you mean that the gadget driver must ensure that the max packet
> > size in the endpoint descriptor is not zero? Do HCDs rely on that? I
> > can add this check into the driver we use for USB fuzzing.
>
> Well, if there are any gadget drivers in the kernel which do set an
> endpoint's maxpacket size to 0, they should be fixed.  I'm not aware of
> any.
>
> Of course, gadget drivers in userspace are always suspect.  That's why
> I suggested having gadgetfs and configfs perform this check.  Even so
> it's not really a _security_ risk, because only the superuser is
> allowed to run a userspace gadget driver.  (Although obviously it is
> better to have a clean failure than to crash the system when a buggy
> program runs with superuser privileges.)
>
> Yes, HCDs do depend on endpoints having reasonable maxpacket values.  I
> suppose the core should check for this.  Currently we check for values
> that are too large or invalid in other ways (like high-speed bulk
> endpoints with maxpacket != 512), but we don't check for 0.

Oh, I think I've confused the terms here. I meant to ask about UDCs.
The question is whether it's OK to try and emulate a gadget with
maxpacket = 0 on a board with a hardware UDC? Or can it cause issues?
The fact that HCDs must ensure correct maxpacket values of course
makes sense.

>
> In fact, that sounds like a much better solution to the problem
> overall.  Let's see if this patch fixes the bug...
>
> Alan Stern
>
> #syz test: https://github.com/google/kasan.git 22be26f7
>
>  drivers/usb/core/config.c |    5 +++++
>  1 file changed, 5 insertions(+)
>
> Index: usb-devel/drivers/usb/core/config.c
> ===================================================================
> --- usb-devel.orig/drivers/usb/core/config.c
> +++ usb-devel/drivers/usb/core/config.c
> @@ -348,6 +348,11 @@ static int usb_parse_endpoint(struct dev
>
>         /* Validate the wMaxPacketSize field */
>         maxp = usb_endpoint_maxp(&endpoint->desc);
> +       if (maxp == 0) {
> +               dev_warn(ddev, "config %d interface %d altsetting %d endpoint 0x%X has wMaxPacketSize 0, skipping\n",
> +                   cfgno, inum, asnum, d->bEndpointAddress);
> +               goto skip_to_next_endpoint_or_interface_descriptor;
> +       }
>
>         /* Find the highest legal maxpacket size for this endpoint */
>         i = 0;          /* additional transactions per microframe */
>

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: divide error in dummy_timer
  2019-10-24 18:08       ` syzbot
@ 2019-10-24 18:58         ` Alan Stern
  2019-10-24 19:16           ` syzbot
  0 siblings, 1 reply; 14+ messages in thread
From: Alan Stern @ 2019-10-24 18:58 UTC (permalink / raw)
  To: syzbot
  Cc: Jacky.Cao, andreyknvl, balbi, chunfeng.yun, gregkh,
	Kernel development list, USB list, syzkaller-bugs

On Thu, 24 Oct 2019, syzbot wrote:

> Hello,
> 
> syzbot has tested the proposed patch but the reproducer still triggered  
> crash:
> divide error in dummy_timer

> Tested on:
> 
> commit:         22be26f7 usb-fuzzer: main usb gadget fuzzer driver
> git tree:       https://github.com/google/kasan.git
> console output: https://syzkaller.appspot.com/x/log.txt?x=17b3e44ce00000
> kernel config:  https://syzkaller.appspot.com/x/.config?x=5fe29bc39eff9627
> dashboard link: https://syzkaller.appspot.com/bug?extid=8ab8bf161038a8768553
> compiler:       gcc (GCC) 9.0.0 20181231 (experimental)
> patch:          https://syzkaller.appspot.com/x/patch.diff?x=15ea0a97600000

Okay, this error has a couple of different aspects.  In particular, we
don't want endpoints to have maxpacket = 0 on either the host side or
the gadget side.  (Note that it _is_ possible for the two sides to
disagree about the maxpacket value, because the gadget driver can in
theory provide different endpoint descriptors to the UDC driver and to
the host.)

So yes, the core should check the value in the endpoint descriptor, but 
also we have to watch out for invalid values coming from userspace 
gadget drivers.  And not just in dummy-hcd; all UDCs are vulnerable.

So let's try the patch below to handle the gadget side of things.

Alan Stern

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

 drivers/usb/gadget/udc/core.c |   11 +++++++++++
 1 file changed, 11 insertions(+)

Index: usb-devel/drivers/usb/gadget/udc/core.c
===================================================================
--- usb-devel.orig/drivers/usb/gadget/udc/core.c
+++ usb-devel/drivers/usb/gadget/udc/core.c
@@ -98,6 +98,17 @@ int usb_ep_enable(struct usb_ep *ep)
 	if (ep->enabled)
 		goto out;
 
+	/* UDC drivers can't handle endpoints with maxpacket size 0 */
+	if (usb_endpoint_maxp(ep->desc) == 0) {
+		/*
+		 * We should log an error message here, but we can't call
+		 * dev_err() because there's no way to find the gadget
+		 * given only ep.
+		 */
+		ret = -EINVAL;
+		goto out;
+	}
+
 	ret = ep->ops->enable(ep, ep->desc);
 	if (ret)
 		goto out;


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: divide error in dummy_timer
  2019-10-24 18:58         ` Alan Stern
@ 2019-10-24 19:16           ` syzbot
  2019-10-28 14:52             ` [PATCH] USB: Skip endpoints with 0 maxpacket length Alan Stern
  2019-10-28 14:54             ` [PATCH] USB: gadget: Reject endpoints with 0 maxpacket value Alan Stern
  0 siblings, 2 replies; 14+ messages in thread
From: syzbot @ 2019-10-24 19:16 UTC (permalink / raw)
  To: Jacky.Cao, andreyknvl, balbi, chunfeng.yun, gregkh, linux-kernel,
	linux-usb, stern, syzkaller-bugs

Hello,

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

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

Tested on:

commit:         22be26f7 usb-fuzzer: main usb gadget fuzzer driver
git tree:       https://github.com/google/kasan.git
kernel config:  https://syzkaller.appspot.com/x/.config?x=5fe29bc39eff9627
dashboard link: https://syzkaller.appspot.com/bug?extid=8ab8bf161038a8768553
compiler:       gcc (GCC) 9.0.0 20181231 (experimental)
patch:          https://syzkaller.appspot.com/x/patch.diff?x=17f50728e00000

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

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: divide error in dummy_timer
  2019-10-24 18:55       ` divide error in dummy_timer Andrey Konovalov
@ 2019-10-24 19:17         ` Alan Stern
  0 siblings, 0 replies; 14+ messages in thread
From: Alan Stern @ 2019-10-24 19:17 UTC (permalink / raw)
  To: Andrey Konovalov
  Cc: syzbot, Jacky . Cao @ sony . com, Felipe Balbi, Chunfeng Yun,
	Greg Kroah-Hartman, LKML, USB list, syzkaller-bugs

On Thu, 24 Oct 2019, Andrey Konovalov wrote:

> On Thu, Oct 24, 2019 at 7:57 PM Alan Stern <stern@rowland.harvard.edu> wrote:
> >
> > On Thu, 24 Oct 2019, Andrey Konovalov wrote:
> >
> > > > Is this really the sort of thing we need to catch?  It isn't a bug in
> > > > any existing kernel code, as far as I know.  Maybe only gadgetfs and
> > > > configfs need to worry about it.
> > >
> > > Hi Alan,
> > >
> > > Do you mean that the gadget driver must ensure that the max packet
> > > size in the endpoint descriptor is not zero? Do HCDs rely on that? I
> > > can add this check into the driver we use for USB fuzzing.
> >
> > Well, if there are any gadget drivers in the kernel which do set an
> > endpoint's maxpacket size to 0, they should be fixed.  I'm not aware of
> > any.
> >
> > Of course, gadget drivers in userspace are always suspect.  That's why
> > I suggested having gadgetfs and configfs perform this check.  Even so
> > it's not really a _security_ risk, because only the superuser is
> > allowed to run a userspace gadget driver.  (Although obviously it is
> > better to have a clean failure than to crash the system when a buggy
> > program runs with superuser privileges.)
> >
> > Yes, HCDs do depend on endpoints having reasonable maxpacket values.  I
> > suppose the core should check for this.  Currently we check for values
> > that are too large or invalid in other ways (like high-speed bulk
> > endpoints with maxpacket != 512), but we don't check for 0.
> 
> Oh, I think I've confused the terms here. I meant to ask about UDCs.
> The question is whether it's OK to try and emulate a gadget with
> maxpacket = 0 on a board with a hardware UDC? Or can it cause issues?
> The fact that HCDs must ensure correct maxpacket values of course
> makes sense.

It doesn't make any sense to have an endpoint with maxpacket = 0 --
either real or emulated.  The USB spec doesn't prohibit them (probably
an oversight), but such endpoints would be useless since it would not
be possible to transfer any data to/from them.

And as you surmised, it wouldn't be at all surprising for UDC drivers
to crash (much like dummy-hcd does) when faced with an endpoint having
maxpacket = 0.  Best to rule out the possibility entirely.

Alan Stern


^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PATCH] USB: Skip endpoints with 0 maxpacket length
  2019-10-24 19:16           ` syzbot
@ 2019-10-28 14:52             ` Alan Stern
  2019-10-28 14:54             ` [PATCH] USB: gadget: Reject endpoints with 0 maxpacket value Alan Stern
  1 sibling, 0 replies; 14+ messages in thread
From: Alan Stern @ 2019-10-28 14:52 UTC (permalink / raw)
  To: Greg KH
  Cc: Jacky.Cao, andreyknvl, balbi, chunfeng.yun, USB list, syzkaller-bugs

Endpoints with a maxpacket length of 0 are probably useless.  They
can't transfer any data, and it's not at all unlikely that an HCD will
crash or hang when trying to handle an URB for such an endpoint.

Currently the USB core does not check for endpoints having a maxpacket
value of 0.  This patch adds a check, printing a warning and skipping
over any endpoints it catches.

Now, the USB spec does not rule out endpoints having maxpacket = 0.
But since they wouldn't have any practical use, there doesn't seem to
be any good reason for us to accept them.

Signed-off-by: Alan Stern <stern@rowland.harvard.edu>

---


[as1924]


 drivers/usb/core/config.c |    5 +++++
 1 file changed, 5 insertions(+)

Index: usb-devel/drivers/usb/core/config.c
===================================================================
--- usb-devel.orig/drivers/usb/core/config.c
+++ usb-devel/drivers/usb/core/config.c
@@ -348,6 +348,11 @@ static int usb_parse_endpoint(struct dev
 
 	/* Validate the wMaxPacketSize field */
 	maxp = usb_endpoint_maxp(&endpoint->desc);
+	if (maxp == 0) {
+		dev_warn(ddev, "config %d interface %d altsetting %d endpoint 0x%X has wMaxPacketSize 0, skipping\n",
+		    cfgno, inum, asnum, d->bEndpointAddress);
+		goto skip_to_next_endpoint_or_interface_descriptor;
+	}
 
 	/* Find the highest legal maxpacket size for this endpoint */
 	i = 0;		/* additional transactions per microframe */


^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PATCH] USB: gadget: Reject endpoints with 0 maxpacket value
  2019-10-24 19:16           ` syzbot
  2019-10-28 14:52             ` [PATCH] USB: Skip endpoints with 0 maxpacket length Alan Stern
@ 2019-10-28 14:54             ` Alan Stern
  2019-10-28 16:08               ` Greg KH
  1 sibling, 1 reply; 14+ messages in thread
From: Alan Stern @ 2019-10-28 14:54 UTC (permalink / raw)
  To: Felipe Balbi
  Cc: Jacky.Cao, andreyknvl, chunfeng.yun, USB list, syzkaller-bugs

Endpoints with a maxpacket length of 0 are probably useless.  They
can't transfer any data, and it's not at all unlikely that a UDC will
crash or hang when trying to handle a non-zero-length usb_request for
such an endpoint.  Indeed, dummy-hcd gets a divide error when trying
to calculate the remainder of a transfer length by the maxpacket
value, as discovered by the syzbot fuzzer.

Currently the gadget core does not check for endpoints having a
maxpacket value of 0.  This patch adds a check to usb_ep_enable(),
preventing such endpoints from being used.

As far as I know, none of the gadget drivers in the kernel tries to
create an endpoint with maxpacket = 0, but until now there has been
nothing to prevent userspace programs under gadgetfs or configfs from
doing it.

Signed-off-by: Alan Stern <stern@rowland.harvard.edu>
Reported-and-tested-by: syzbot+8ab8bf161038a8768553@syzkaller.appspotmail.com
CC: <stable@vger.kernel.org>

---


[as1925]


 drivers/usb/gadget/udc/core.c |   11 +++++++++++
 1 file changed, 11 insertions(+)

Index: usb-devel/drivers/usb/gadget/udc/core.c
===================================================================
--- usb-devel.orig/drivers/usb/gadget/udc/core.c
+++ usb-devel/drivers/usb/gadget/udc/core.c
@@ -98,6 +98,17 @@ int usb_ep_enable(struct usb_ep *ep)
 	if (ep->enabled)
 		goto out;
 
+	/* UDC drivers can't handle endpoints with maxpacket size 0 */
+	if (usb_endpoint_maxp(ep->desc) == 0) {
+		/*
+		 * We should log an error message here, but we can't call
+		 * dev_err() because there's no way to find the gadget
+		 * given only ep.
+		 */
+		ret = -EINVAL;
+		goto out;
+	}
+
 	ret = ep->ops->enable(ep, ep->desc);
 	if (ret)
 		goto out;


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH] USB: gadget: Reject endpoints with 0 maxpacket value
  2019-10-28 14:54             ` [PATCH] USB: gadget: Reject endpoints with 0 maxpacket value Alan Stern
@ 2019-10-28 16:08               ` Greg KH
  2019-10-29  8:38                 ` Felipe Balbi
  0 siblings, 1 reply; 14+ messages in thread
From: Greg KH @ 2019-10-28 16:08 UTC (permalink / raw)
  To: Alan Stern, Felipe Balbi
  Cc: Jacky.Cao, andreyknvl, chunfeng.yun, USB list, syzkaller-bugs

On Mon, Oct 28, 2019 at 10:54:26AM -0400, Alan Stern wrote:
> Endpoints with a maxpacket length of 0 are probably useless.  They
> can't transfer any data, and it's not at all unlikely that a UDC will
> crash or hang when trying to handle a non-zero-length usb_request for
> such an endpoint.  Indeed, dummy-hcd gets a divide error when trying
> to calculate the remainder of a transfer length by the maxpacket
> value, as discovered by the syzbot fuzzer.
> 
> Currently the gadget core does not check for endpoints having a
> maxpacket value of 0.  This patch adds a check to usb_ep_enable(),
> preventing such endpoints from being used.
> 
> As far as I know, none of the gadget drivers in the kernel tries to
> create an endpoint with maxpacket = 0, but until now there has been
> nothing to prevent userspace programs under gadgetfs or configfs from
> doing it.
> 
> Signed-off-by: Alan Stern <stern@rowland.harvard.edu>
> Reported-and-tested-by: syzbot+8ab8bf161038a8768553@syzkaller.appspotmail.com
> CC: <stable@vger.kernel.org>
> 
> ---
> 
> 
> [as1925]
> 
> 
>  drivers/usb/gadget/udc/core.c |   11 +++++++++++
>  1 file changed, 11 insertions(+)
> 
> Index: usb-devel/drivers/usb/gadget/udc/core.c
> ===================================================================
> --- usb-devel.orig/drivers/usb/gadget/udc/core.c
> +++ usb-devel/drivers/usb/gadget/udc/core.c
> @@ -98,6 +98,17 @@ int usb_ep_enable(struct usb_ep *ep)
>  	if (ep->enabled)
>  		goto out;
>  
> +	/* UDC drivers can't handle endpoints with maxpacket size 0 */
> +	if (usb_endpoint_maxp(ep->desc) == 0) {
> +		/*
> +		 * We should log an error message here, but we can't call
> +		 * dev_err() because there's no way to find the gadget
> +		 * given only ep.
> +		 */
> +		ret = -EINVAL;
> +		goto out;
> +	}
> +
>  	ret = ep->ops->enable(ep, ep->desc);
>  	if (ret)
>  		goto out;
> 

Felipe, I can take this now in my tree if I can get an ack.

thanks,

greg k-h

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH] USB: gadget: Reject endpoints with 0 maxpacket value
  2019-10-28 16:08               ` Greg KH
@ 2019-10-29  8:38                 ` Felipe Balbi
  0 siblings, 0 replies; 14+ messages in thread
From: Felipe Balbi @ 2019-10-29  8:38 UTC (permalink / raw)
  To: Greg KH, Alan Stern
  Cc: Jacky.Cao, andreyknvl, chunfeng.yun, USB list, syzkaller-bugs

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


Hi,

Greg KH <gregkh@linuxfoundation.org> writes:
> On Mon, Oct 28, 2019 at 10:54:26AM -0400, Alan Stern wrote:
>> Endpoints with a maxpacket length of 0 are probably useless.  They
>> can't transfer any data, and it's not at all unlikely that a UDC will
>> crash or hang when trying to handle a non-zero-length usb_request for
>> such an endpoint.  Indeed, dummy-hcd gets a divide error when trying
>> to calculate the remainder of a transfer length by the maxpacket
>> value, as discovered by the syzbot fuzzer.
>> 
>> Currently the gadget core does not check for endpoints having a
>> maxpacket value of 0.  This patch adds a check to usb_ep_enable(),
>> preventing such endpoints from being used.
>> 
>> As far as I know, none of the gadget drivers in the kernel tries to
>> create an endpoint with maxpacket = 0, but until now there has been
>> nothing to prevent userspace programs under gadgetfs or configfs from
>> doing it.
>> 
>> Signed-off-by: Alan Stern <stern@rowland.harvard.edu>
>> Reported-and-tested-by: syzbot+8ab8bf161038a8768553@syzkaller.appspotmail.com
>> CC: <stable@vger.kernel.org>
>> 
>> ---
>> 
>> 
>> [as1925]
>> 
>> 
>>  drivers/usb/gadget/udc/core.c |   11 +++++++++++
>>  1 file changed, 11 insertions(+)
>> 
>> Index: usb-devel/drivers/usb/gadget/udc/core.c
>> ===================================================================
>> --- usb-devel.orig/drivers/usb/gadget/udc/core.c
>> +++ usb-devel/drivers/usb/gadget/udc/core.c
>> @@ -98,6 +98,17 @@ int usb_ep_enable(struct usb_ep *ep)
>>  	if (ep->enabled)
>>  		goto out;
>>  
>> +	/* UDC drivers can't handle endpoints with maxpacket size 0 */
>> +	if (usb_endpoint_maxp(ep->desc) == 0) {
>> +		/*
>> +		 * We should log an error message here, but we can't call
>> +		 * dev_err() because there's no way to find the gadget
>> +		 * given only ep.
>> +		 */
>> +		ret = -EINVAL;
>> +		goto out;
>> +	}
>> +
>>  	ret = ep->ops->enable(ep, ep->desc);
>>  	if (ret)
>>  		goto out;
>> 
>
> Felipe, I can take this now in my tree if I can get an ack.

Definitely:

Acked-by: Felipe Balbi <balbi@kernel.org>

-- 
balbi

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 832 bytes --]

^ permalink raw reply	[flat|nested] 14+ messages in thread

end of thread, other threads:[~2019-10-29  8:39 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-10-24 13:58 divide error in dummy_timer syzbot
2019-10-24 15:04 ` Alan Stern
2019-10-24 15:22   ` syzbot
2019-10-24 17:05   ` Andrey Konovalov
2019-10-24 17:57     ` Alan Stern
2019-10-24 18:08       ` syzbot
2019-10-24 18:58         ` Alan Stern
2019-10-24 19:16           ` syzbot
2019-10-28 14:52             ` [PATCH] USB: Skip endpoints with 0 maxpacket length Alan Stern
2019-10-28 14:54             ` [PATCH] USB: gadget: Reject endpoints with 0 maxpacket value Alan Stern
2019-10-28 16:08               ` Greg KH
2019-10-29  8:38                 ` Felipe Balbi
2019-10-24 18:55       ` divide error in dummy_timer Andrey Konovalov
2019-10-24 19:17         ` Alan Stern

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).