* 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 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
* [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
* 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: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
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).