* [syzbot] general protection fault in gadget_setup @ 2021-04-13 8:08 syzbot 2021-04-13 8:12 ` Dmitry Vyukov 0 siblings, 1 reply; 13+ messages in thread From: syzbot @ 2021-04-13 8:08 UTC (permalink / raw) To: andreyknvl, balbi, dan.carpenter, gregkh, linux-kernel, linux-usb, syzkaller-bugs Hello, syzbot found the following issue on: HEAD commit: 0f4498ce Merge tag 'for-5.12/dm-fixes-2' of git://git.kern.. git tree: upstream console output: https://syzkaller.appspot.com/x/log.txt?x=124adbf6d00000 kernel config: https://syzkaller.appspot.com/x/.config?x=daeff30c2474a60f dashboard link: https://syzkaller.appspot.com/bug?extid=eb4674092e6cc8d9e0bd userspace arch: i386 Unfortunately, I don't have any reproducer for this issue yet. IMPORTANT: if you fix the issue, please add the following tag to the commit: Reported-by: syzbot+eb4674092e6cc8d9e0bd@syzkaller.appspotmail.com general protection fault, probably for non-canonical address 0xdffffc0000000004: 0000 [#1] PREEMPT SMP KASAN KASAN: null-ptr-deref in range [0x0000000000000020-0x0000000000000027] CPU: 1 PID: 5016 Comm: systemd-udevd Not tainted 5.12.0-rc4-syzkaller #0 Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS 1.14.0-2 04/01/2014 RIP: 0010:__lock_acquire+0xcfe/0x54c0 kernel/locking/lockdep.c:4770 Code: 09 0e 41 bf 01 00 00 00 0f 86 8c 00 00 00 89 05 48 69 09 0e e9 81 00 00 00 48 b8 00 00 00 00 00 fc ff df 4c 89 f2 48 c1 ea 03 <80> 3c 02 00 0f 85 5b 31 00 00 49 81 3e c0 13 38 8f 0f 84 d0 f3 ff RSP: 0000:ffffc90000ce77d8 EFLAGS: 00010002 RAX: dffffc0000000000 RBX: 0000000000000000 RCX: 0000000000000000 RDX: 0000000000000004 RSI: 1ffff9200019cf0c RDI: 0000000000000020 RBP: 0000000000000000 R08: 0000000000000001 R09: 0000000000000001 R10: 0000000000000001 R11: 0000000000000006 R12: ffff88801295b880 R13: 0000000000000000 R14: 0000000000000020 R15: 0000000000000000 FS: 00007fcd745f98c0(0000) GS:ffff88802cb00000(0000) knlGS:0000000000000000 CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 CR2: 00007ffe279f7d87 CR3: 000000001c7d4000 CR4: 0000000000150ee0 DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000 DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400 Call Trace: lock_acquire kernel/locking/lockdep.c:5510 [inline] lock_acquire+0x1ab/0x740 kernel/locking/lockdep.c:5475 __raw_spin_lock_irqsave include/linux/spinlock_api_smp.h:110 [inline] _raw_spin_lock_irqsave+0x39/0x50 kernel/locking/spinlock.c:159 gadget_setup+0x4e/0x510 drivers/usb/gadget/legacy/raw_gadget.c:327 dummy_timer+0x1615/0x32a0 drivers/usb/gadget/udc/dummy_hcd.c:1903 call_timer_fn+0x1a5/0x6b0 kernel/time/timer.c:1431 expire_timers kernel/time/timer.c:1476 [inline] __run_timers.part.0+0x67c/0xa50 kernel/time/timer.c:1745 __run_timers kernel/time/timer.c:1726 [inline] run_timer_softirq+0xb3/0x1d0 kernel/time/timer.c:1758 __do_softirq+0x29b/0x9f6 kernel/softirq.c:345 invoke_softirq kernel/softirq.c:221 [inline] __irq_exit_rcu kernel/softirq.c:422 [inline] irq_exit_rcu+0x134/0x200 kernel/softirq.c:434 sysvec_apic_timer_interrupt+0x45/0xc0 arch/x86/kernel/apic/apic.c:1100 asm_sysvec_apic_timer_interrupt+0x12/0x20 arch/x86/include/asm/idtentry.h:632 RIP: 0033:0x560cfc4a02ed Code: 4c 39 c1 48 89 42 18 4c 89 52 08 4c 89 5a 10 48 89 1a 0f 87 7b ff ff ff 48 89 f8 48 f7 d0 48 01 c8 48 83 e0 f8 48 8d 7c 07 08 <48> 8d 0d 34 d9 02 00 48 63 04 b1 48 01 c8 ff e0 0f 1f 00 48 8d 0d RSP: 002b:00007ffe279f9dd0 EFLAGS: 00000246 RAX: 0000000000000000 RBX: 0000560cfcd88e40 RCX: 0000560cfcd72af0 RDX: 00007ffe279f9de0 RSI: 0000000000000007 RDI: 0000560cfcd72af0 RBP: 00007ffe279f9e70 R08: 0000000000000000 R09: 0000000000000020 R10: 0000560cfcd72af7 R11: 0000560cfcd73530 R12: 0000560cfcd72af0 R13: 0000000000000000 R14: 0000560cfcd72b10 R15: 0000000000000001 Modules linked in: ---[ end trace ab0f6632fdd289cf ]--- RIP: 0010:__lock_acquire+0xcfe/0x54c0 kernel/locking/lockdep.c:4770 Code: 09 0e 41 bf 01 00 00 00 0f 86 8c 00 00 00 89 05 48 69 09 0e e9 81 00 00 00 48 b8 00 00 00 00 00 fc ff df 4c 89 f2 48 c1 ea 03 <80> 3c 02 00 0f 85 5b 31 00 00 49 81 3e c0 13 38 8f 0f 84 d0 f3 ff RSP: 0000:ffffc90000ce77d8 EFLAGS: 00010002 RAX: dffffc0000000000 RBX: 0000000000000000 RCX: 0000000000000000 RDX: 0000000000000004 RSI: 1ffff9200019cf0c RDI: 0000000000000020 RBP: 0000000000000000 R08: 0000000000000001 R09: 0000000000000001 R10: 0000000000000001 R11: 0000000000000006 R12: ffff88801295b880 R13: 0000000000000000 R14: 0000000000000020 R15: 0000000000000000 FS: 00007fcd745f98c0(0000) GS:ffff88802cb00000(0000) knlGS:0000000000000000 CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 CR2: 00007ffe279f7d87 CR3: 000000001c7d4000 CR4: 0000000000150ee0 DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000 DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400 --- This report is generated by a bot. It may contain errors. See https://goo.gl/tpsmEJ for more information about syzbot. syzbot engineers can be reached at syzkaller@googlegroups.com. syzbot will keep track of this issue. See: https://goo.gl/tpsmEJ#status for how to communicate with syzbot. ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [syzbot] general protection fault in gadget_setup 2021-04-13 8:08 [syzbot] general protection fault in gadget_setup syzbot @ 2021-04-13 8:12 ` Dmitry Vyukov 2021-04-13 16:13 ` Alan Stern 0 siblings, 1 reply; 13+ messages in thread From: Dmitry Vyukov @ 2021-04-13 8:12 UTC (permalink / raw) To: syzbot Cc: Andrey Konovalov, Felipe Balbi, Dan Carpenter, Greg Kroah-Hartman, LKML, USB list, syzkaller-bugs On Tue, Apr 13, 2021 at 10:08 AM syzbot <syzbot+eb4674092e6cc8d9e0bd@syzkaller.appspotmail.com> wrote: > > Hello, > > syzbot found the following issue on: > > HEAD commit: 0f4498ce Merge tag 'for-5.12/dm-fixes-2' of git://git.kern.. > git tree: upstream > console output: https://syzkaller.appspot.com/x/log.txt?x=124adbf6d00000 > kernel config: https://syzkaller.appspot.com/x/.config?x=daeff30c2474a60f > dashboard link: https://syzkaller.appspot.com/bug?extid=eb4674092e6cc8d9e0bd > userspace arch: i386 > > Unfortunately, I don't have any reproducer for this issue yet. > > IMPORTANT: if you fix the issue, please add the following tag to the commit: > Reported-by: syzbot+eb4674092e6cc8d9e0bd@syzkaller.appspotmail.com I suspect that the raw gadget_unbind() can be called while the timer is still active. gadget_unbind() sets gadget data to NULL. But I am not sure which unbind call this is: usb_gadget_remove_driver() or right in udc_bind_to_driver() due to a start error. Also looking at the code, gadget_bind() resets data to NULL on this error path: https://elixir.bootlin.com/linux/v5.12-rc7/source/drivers/usb/gadget/legacy/raw_gadget.c#L283 but not on this error path: https://elixir.bootlin.com/linux/v5.12-rc7/source/drivers/usb/gadget/legacy/raw_gadget.c#L306 Should the second one also reset data to NULL? > general protection fault, probably for non-canonical address 0xdffffc0000000004: 0000 [#1] PREEMPT SMP KASAN > KASAN: null-ptr-deref in range [0x0000000000000020-0x0000000000000027] > CPU: 1 PID: 5016 Comm: systemd-udevd Not tainted 5.12.0-rc4-syzkaller #0 > Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS 1.14.0-2 04/01/2014 > RIP: 0010:__lock_acquire+0xcfe/0x54c0 kernel/locking/lockdep.c:4770 > Code: 09 0e 41 bf 01 00 00 00 0f 86 8c 00 00 00 89 05 48 69 09 0e e9 81 00 00 00 48 b8 00 00 00 00 00 fc ff df 4c 89 f2 48 c1 ea 03 <80> 3c 02 00 0f 85 5b 31 00 00 49 81 3e c0 13 38 8f 0f 84 d0 f3 ff > RSP: 0000:ffffc90000ce77d8 EFLAGS: 00010002 > RAX: dffffc0000000000 RBX: 0000000000000000 RCX: 0000000000000000 > RDX: 0000000000000004 RSI: 1ffff9200019cf0c RDI: 0000000000000020 > RBP: 0000000000000000 R08: 0000000000000001 R09: 0000000000000001 > R10: 0000000000000001 R11: 0000000000000006 R12: ffff88801295b880 > R13: 0000000000000000 R14: 0000000000000020 R15: 0000000000000000 > FS: 00007fcd745f98c0(0000) GS:ffff88802cb00000(0000) knlGS:0000000000000000 > CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 > CR2: 00007ffe279f7d87 CR3: 000000001c7d4000 CR4: 0000000000150ee0 > DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000 > DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400 > Call Trace: > lock_acquire kernel/locking/lockdep.c:5510 [inline] > lock_acquire+0x1ab/0x740 kernel/locking/lockdep.c:5475 > __raw_spin_lock_irqsave include/linux/spinlock_api_smp.h:110 [inline] > _raw_spin_lock_irqsave+0x39/0x50 kernel/locking/spinlock.c:159 > gadget_setup+0x4e/0x510 drivers/usb/gadget/legacy/raw_gadget.c:327 > dummy_timer+0x1615/0x32a0 drivers/usb/gadget/udc/dummy_hcd.c:1903 > call_timer_fn+0x1a5/0x6b0 kernel/time/timer.c:1431 > expire_timers kernel/time/timer.c:1476 [inline] > __run_timers.part.0+0x67c/0xa50 kernel/time/timer.c:1745 > __run_timers kernel/time/timer.c:1726 [inline] > run_timer_softirq+0xb3/0x1d0 kernel/time/timer.c:1758 > __do_softirq+0x29b/0x9f6 kernel/softirq.c:345 > invoke_softirq kernel/softirq.c:221 [inline] > __irq_exit_rcu kernel/softirq.c:422 [inline] > irq_exit_rcu+0x134/0x200 kernel/softirq.c:434 > sysvec_apic_timer_interrupt+0x45/0xc0 arch/x86/kernel/apic/apic.c:1100 > asm_sysvec_apic_timer_interrupt+0x12/0x20 arch/x86/include/asm/idtentry.h:632 > RIP: 0033:0x560cfc4a02ed > Code: 4c 39 c1 48 89 42 18 4c 89 52 08 4c 89 5a 10 48 89 1a 0f 87 7b ff ff ff 48 89 f8 48 f7 d0 48 01 c8 48 83 e0 f8 48 8d 7c 07 08 <48> 8d 0d 34 d9 02 00 48 63 04 b1 48 01 c8 ff e0 0f 1f 00 48 8d 0d > RSP: 002b:00007ffe279f9dd0 EFLAGS: 00000246 > RAX: 0000000000000000 RBX: 0000560cfcd88e40 RCX: 0000560cfcd72af0 > RDX: 00007ffe279f9de0 RSI: 0000000000000007 RDI: 0000560cfcd72af0 > RBP: 00007ffe279f9e70 R08: 0000000000000000 R09: 0000000000000020 > R10: 0000560cfcd72af7 R11: 0000560cfcd73530 R12: 0000560cfcd72af0 > R13: 0000000000000000 R14: 0000560cfcd72b10 R15: 0000000000000001 > Modules linked in: > ---[ end trace ab0f6632fdd289cf ]--- > RIP: 0010:__lock_acquire+0xcfe/0x54c0 kernel/locking/lockdep.c:4770 > Code: 09 0e 41 bf 01 00 00 00 0f 86 8c 00 00 00 89 05 48 69 09 0e e9 81 00 00 00 48 b8 00 00 00 00 00 fc ff df 4c 89 f2 48 c1 ea 03 <80> 3c 02 00 0f 85 5b 31 00 00 49 81 3e c0 13 38 8f 0f 84 d0 f3 ff > RSP: 0000:ffffc90000ce77d8 EFLAGS: 00010002 > RAX: dffffc0000000000 RBX: 0000000000000000 RCX: 0000000000000000 > RDX: 0000000000000004 RSI: 1ffff9200019cf0c RDI: 0000000000000020 > RBP: 0000000000000000 R08: 0000000000000001 R09: 0000000000000001 > R10: 0000000000000001 R11: 0000000000000006 R12: ffff88801295b880 > R13: 0000000000000000 R14: 0000000000000020 R15: 0000000000000000 > FS: 00007fcd745f98c0(0000) GS:ffff88802cb00000(0000) knlGS:0000000000000000 > CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 > CR2: 00007ffe279f7d87 CR3: 000000001c7d4000 CR4: 0000000000150ee0 > DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000 > DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400 > > > --- > This report is generated by a bot. It may contain errors. > See https://goo.gl/tpsmEJ for more information about syzbot. > syzbot engineers can be reached at syzkaller@googlegroups.com. > > syzbot will keep track of this issue. See: > https://goo.gl/tpsmEJ#status for how to communicate with syzbot. > > -- > You received this message because you are subscribed to the Google Groups "syzkaller-bugs" group. > To unsubscribe from this group and stop receiving emails from it, send an email to syzkaller-bugs+unsubscribe@googlegroups.com. > To view this discussion on the web visit https://groups.google.com/d/msgid/syzkaller-bugs/00000000000075c58405bfd6228c%40google.com. ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [syzbot] general protection fault in gadget_setup 2021-04-13 8:12 ` Dmitry Vyukov @ 2021-04-13 16:13 ` Alan Stern 2021-04-13 16:47 ` Dmitry Vyukov 2021-04-16 5:40 ` Anirudh Rayabharam 0 siblings, 2 replies; 13+ messages in thread From: Alan Stern @ 2021-04-13 16:13 UTC (permalink / raw) To: Dmitry Vyukov Cc: syzbot, Andrey Konovalov, Felipe Balbi, Dan Carpenter, Greg Kroah-Hartman, LKML, USB list, syzkaller-bugs On Tue, Apr 13, 2021 at 10:12:05AM +0200, Dmitry Vyukov wrote: > On Tue, Apr 13, 2021 at 10:08 AM syzbot > <syzbot+eb4674092e6cc8d9e0bd@syzkaller.appspotmail.com> wrote: > > > > Hello, > > > > syzbot found the following issue on: > > > > HEAD commit: 0f4498ce Merge tag 'for-5.12/dm-fixes-2' of git://git.kern.. > > git tree: upstream > > console output: https://syzkaller.appspot.com/x/log.txt?x=124adbf6d00000 > > kernel config: https://syzkaller.appspot.com/x/.config?x=daeff30c2474a60f > > dashboard link: https://syzkaller.appspot.com/bug?extid=eb4674092e6cc8d9e0bd > > userspace arch: i386 > > > > Unfortunately, I don't have any reproducer for this issue yet. > > > > IMPORTANT: if you fix the issue, please add the following tag to the commit: > > Reported-by: syzbot+eb4674092e6cc8d9e0bd@syzkaller.appspotmail.com > > I suspect that the raw gadget_unbind() can be called while the timer > is still active. gadget_unbind() sets gadget data to NULL. > But I am not sure which unbind call this is: > usb_gadget_remove_driver() or right in udc_bind_to_driver() due to a > start error. This certainly looks like a race between gadget_unbind and gadget_setup in raw_gadget. In theory, this race shouldn't matter. The gadget core is supposed to guarantee that there won't be any more callbacks to the gadget driver once the driver's unbind routine is called. That guarantee is enforced in usb_gadget_remove_driver as follows: usb_gadget_disconnect(udc->gadget); if (udc->gadget->irq) synchronize_irq(udc->gadget->irq); udc->driver->unbind(udc->gadget); usb_gadget_udc_stop(udc); usb_gadget_disconnect turns off the pullup resistor, telling the host that the gadget is no longer connected and preventing the transmission of any more USB packets. Any packets that have already been received are sure to processed by the UDC driver's interrupt handler by the time synchronize_irq returns. But this doesn't work with dummy_hcd, because dummy_hcd doesn't use interrupts; it uses a timer instead. It does have code to emulate the effect of synchronize_irq, but that code doesn't get invoked at the right time -- it currently runs in usb_gadget_udc_stop, after the unbind callback instead of before. Indeed, there's no way for usb_gadget_remove_driver to invoke this code before the unbind callback,. I thought the synchronize_irq emulation problem had been completely solved, but evidently it hasn't. It looks like the best solution is to add a call of the synchronize_irq emulation code in dummy_pullup. Maybe we can test this reasoning by putting a delay just before the call to dum->driver->setup. That runs in the timer handler, so it's not a good place to delay, but it may be okay just for testing purposes. Hopefully this patch will make the race a lot more likely to occur. Is there any way to tell syzkaller to test it, despite the fact that syzkaller doesn't think it has a reproducer for this issue? Alan Stern 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 @@ -1900,6 +1900,7 @@ restart: if (value > 0) { ++dum->callback_usage; spin_unlock(&dum->lock); + mdelay(5); value = dum->driver->setup(&dum->gadget, &setup); spin_lock(&dum->lock); ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [syzbot] general protection fault in gadget_setup 2021-04-13 16:13 ` Alan Stern @ 2021-04-13 16:47 ` Dmitry Vyukov 2021-04-13 16:57 ` Alan Stern 2021-04-16 5:40 ` Anirudh Rayabharam 1 sibling, 1 reply; 13+ messages in thread From: Dmitry Vyukov @ 2021-04-13 16:47 UTC (permalink / raw) To: Alan Stern Cc: syzbot, Andrey Konovalov, Felipe Balbi, Dan Carpenter, Greg Kroah-Hartman, LKML, USB list, syzkaller-bugs On Tue, Apr 13, 2021 at 6:13 PM Alan Stern <stern@rowland.harvard.edu> wrote: > > On Tue, Apr 13, 2021 at 10:12:05AM +0200, Dmitry Vyukov wrote: > > On Tue, Apr 13, 2021 at 10:08 AM syzbot > > <syzbot+eb4674092e6cc8d9e0bd@syzkaller.appspotmail.com> wrote: > > > > > > Hello, > > > > > > syzbot found the following issue on: > > > > > > HEAD commit: 0f4498ce Merge tag 'for-5.12/dm-fixes-2' of git://git.kern.. > > > git tree: upstream > > > console output: https://syzkaller.appspot.com/x/log.txt?x=124adbf6d00000 > > > kernel config: https://syzkaller.appspot.com/x/.config?x=daeff30c2474a60f > > > dashboard link: https://syzkaller.appspot.com/bug?extid=eb4674092e6cc8d9e0bd > > > userspace arch: i386 > > > > > > Unfortunately, I don't have any reproducer for this issue yet. > > > > > > IMPORTANT: if you fix the issue, please add the following tag to the commit: > > > Reported-by: syzbot+eb4674092e6cc8d9e0bd@syzkaller.appspotmail.com > > > > I suspect that the raw gadget_unbind() can be called while the timer > > is still active. gadget_unbind() sets gadget data to NULL. > > But I am not sure which unbind call this is: > > usb_gadget_remove_driver() or right in udc_bind_to_driver() due to a > > start error. > > This certainly looks like a race between gadget_unbind and gadget_setup > in raw_gadget. > > In theory, this race shouldn't matter. The gadget core is supposed to > guarantee that there won't be any more callbacks to the gadget driver > once the driver's unbind routine is called. That guarantee is enforced > in usb_gadget_remove_driver as follows: > > usb_gadget_disconnect(udc->gadget); > if (udc->gadget->irq) > synchronize_irq(udc->gadget->irq); > udc->driver->unbind(udc->gadget); > usb_gadget_udc_stop(udc); > > usb_gadget_disconnect turns off the pullup resistor, telling the host > that the gadget is no longer connected and preventing the transmission > of any more USB packets. Any packets that have already been received > are sure to processed by the UDC driver's interrupt handler by the time > synchronize_irq returns. > > But this doesn't work with dummy_hcd, because dummy_hcd doesn't use > interrupts; it uses a timer instead. It does have code to emulate the > effect of synchronize_irq, but that code doesn't get invoked at the > right time -- it currently runs in usb_gadget_udc_stop, after the unbind > callback instead of before. Indeed, there's no way for > usb_gadget_remove_driver to invoke this code before the unbind > callback,. > > I thought the synchronize_irq emulation problem had been completely > solved, but evidently it hasn't. It looks like the best solution is to > add a call of the synchronize_irq emulation code in dummy_pullup. > > Maybe we can test this reasoning by putting a delay just before the call > to dum->driver->setup. That runs in the timer handler, so it's not a > good place to delay, but it may be okay just for testing purposes. > > Hopefully this patch will make the race a lot more likely to occur. Is > there any way to tell syzkaller to test it, despite the fact that > syzkaller doesn't think it has a reproducer for this issue? If there is no reproducer the only way syzbot can test it is if it's in linux-next under CONFIG_DEBUG_AID_FOR_SYZBOT: http://bit.do/syzbot#no-custom-patches > Alan Stern > > > 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 > @@ -1900,6 +1900,7 @@ restart: > if (value > 0) { > ++dum->callback_usage; > spin_unlock(&dum->lock); > + mdelay(5); > value = dum->driver->setup(&dum->gadget, > &setup); > spin_lock(&dum->lock); ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [syzbot] general protection fault in gadget_setup 2021-04-13 16:47 ` Dmitry Vyukov @ 2021-04-13 16:57 ` Alan Stern 2021-04-13 17:11 ` Dmitry Vyukov 0 siblings, 1 reply; 13+ messages in thread From: Alan Stern @ 2021-04-13 16:57 UTC (permalink / raw) To: Dmitry Vyukov Cc: syzbot, Andrey Konovalov, Felipe Balbi, Dan Carpenter, Greg Kroah-Hartman, LKML, USB list, syzkaller-bugs On Tue, Apr 13, 2021 at 06:47:47PM +0200, Dmitry Vyukov wrote: > On Tue, Apr 13, 2021 at 6:13 PM Alan Stern <stern@rowland.harvard.edu> wrote: > > Hopefully this patch will make the race a lot more likely to occur. Is > > there any way to tell syzkaller to test it, despite the fact that > > syzkaller doesn't think it has a reproducer for this issue? > > If there is no reproducer the only way syzbot can test it is if it's > in linux-next under CONFIG_DEBUG_AID_FOR_SYZBOT: > http://bit.do/syzbot#no-custom-patches There _is_ a theoretical reproducer: the test that provoked syzkaller's original bug report. But syzkaller doesn't realize that it is (or may be) a reproducer. It ought to be possible to ask syzkaller to run a particular test that it has done before, with a patch applied -- and without having to add anything to linux-next. Alan Stern ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [syzbot] general protection fault in gadget_setup 2021-04-13 16:57 ` Alan Stern @ 2021-04-13 17:11 ` Dmitry Vyukov 2021-04-15 20:59 ` Alan Stern 0 siblings, 1 reply; 13+ messages in thread From: Dmitry Vyukov @ 2021-04-13 17:11 UTC (permalink / raw) To: Alan Stern Cc: syzbot, Andrey Konovalov, Felipe Balbi, Dan Carpenter, Greg Kroah-Hartman, LKML, USB list, syzkaller-bugs, syzkaller On Tue, Apr 13, 2021 at 6:57 PM Alan Stern <stern@rowland.harvard.edu> wrote: > > On Tue, Apr 13, 2021 at 06:47:47PM +0200, Dmitry Vyukov wrote: > > On Tue, Apr 13, 2021 at 6:13 PM Alan Stern <stern@rowland.harvard.edu> wrote: > > > Hopefully this patch will make the race a lot more likely to occur. Is > > > there any way to tell syzkaller to test it, despite the fact that > > > syzkaller doesn't think it has a reproducer for this issue? > > > > If there is no reproducer the only way syzbot can test it is if it's > > in linux-next under CONFIG_DEBUG_AID_FOR_SYZBOT: > > http://bit.do/syzbot#no-custom-patches > > There _is_ a theoretical reproducer: the test that provoked syzkaller's > original bug report. But syzkaller doesn't realize that it is (or may > be) a reproducer. > > It ought to be possible to ask syzkaller to run a particular test that > it has done before, with a patch applied -- and without having to add > anything to linux-next. Yes, this is possible: http://bit.do/syzbot#syzkaller-reproducers The log of tests executed before the crash is available under the "console output" link: console output: https://syzkaller.appspot.com/x/log.txt?x=124adbf6d00000 And this log can be replayed using syz-execprog utility. ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [syzbot] general protection fault in gadget_setup 2021-04-13 17:11 ` Dmitry Vyukov @ 2021-04-15 20:59 ` Alan Stern 2021-04-16 7:21 ` Dmitry Vyukov 0 siblings, 1 reply; 13+ messages in thread From: Alan Stern @ 2021-04-15 20:59 UTC (permalink / raw) To: Dmitry Vyukov Cc: syzbot, Andrey Konovalov, Felipe Balbi, Dan Carpenter, Greg Kroah-Hartman, LKML, USB list, syzkaller-bugs, syzkaller On Tue, Apr 13, 2021 at 07:11:11PM +0200, Dmitry Vyukov wrote: > On Tue, Apr 13, 2021 at 6:57 PM Alan Stern <stern@rowland.harvard.edu> wrote: > > > > On Tue, Apr 13, 2021 at 06:47:47PM +0200, Dmitry Vyukov wrote: > > > On Tue, Apr 13, 2021 at 6:13 PM Alan Stern <stern@rowland.harvard.edu> wrote: > > > > Hopefully this patch will make the race a lot more likely to occur. Is > > > > there any way to tell syzkaller to test it, despite the fact that > > > > syzkaller doesn't think it has a reproducer for this issue? > > > > > > If there is no reproducer the only way syzbot can test it is if it's > > > in linux-next under CONFIG_DEBUG_AID_FOR_SYZBOT: > > > http://bit.do/syzbot#no-custom-patches > > > > There _is_ a theoretical reproducer: the test that provoked syzkaller's > > original bug report. But syzkaller doesn't realize that it is (or may > > be) a reproducer. > > > > It ought to be possible to ask syzkaller to run a particular test that > > it has done before, with a patch applied -- and without having to add > > anything to linux-next. > > Yes, this is possible: > http://bit.do/syzbot#syzkaller-reproducers That's not really what I had in mind. I don't want to spend the time and effort installing syskaller on my own system; I want to tell syzbot to run a particular syzkaller program (the one that originally led to this bug report) on a patched kernel. The syzbot instructions say that it can test bugs with reproducers. The problem here is that there doesn't seem to be any way to tell it to use a particular syzkaller program as a reproducer. Alan Stern > The log of tests executed before the crash is available under the > "console output" link: > console output: https://syzkaller.appspot.com/x/log.txt?x=124adbf6d00000 > And this log can be replayed using syz-execprog utility. ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [syzbot] general protection fault in gadget_setup 2021-04-15 20:59 ` Alan Stern @ 2021-04-16 7:21 ` Dmitry Vyukov 2021-04-16 15:29 ` Alan Stern 0 siblings, 1 reply; 13+ messages in thread From: Dmitry Vyukov @ 2021-04-16 7:21 UTC (permalink / raw) To: Alan Stern Cc: syzbot, Andrey Konovalov, Felipe Balbi, Dan Carpenter, Greg Kroah-Hartman, LKML, USB list, syzkaller-bugs, syzkaller On Thu, Apr 15, 2021 at 10:59 PM Alan Stern <stern@rowland.harvard.edu> wrote: > > On Tue, Apr 13, 2021 at 07:11:11PM +0200, Dmitry Vyukov wrote: > > On Tue, Apr 13, 2021 at 6:57 PM Alan Stern <stern@rowland.harvard.edu> wrote: > > > > > > On Tue, Apr 13, 2021 at 06:47:47PM +0200, Dmitry Vyukov wrote: > > > > On Tue, Apr 13, 2021 at 6:13 PM Alan Stern <stern@rowland.harvard.edu> wrote: > > > > > Hopefully this patch will make the race a lot more likely to occur. Is > > > > > there any way to tell syzkaller to test it, despite the fact that > > > > > syzkaller doesn't think it has a reproducer for this issue? > > > > > > > > If there is no reproducer the only way syzbot can test it is if it's > > > > in linux-next under CONFIG_DEBUG_AID_FOR_SYZBOT: > > > > http://bit.do/syzbot#no-custom-patches > > > > > > There _is_ a theoretical reproducer: the test that provoked syzkaller's > > > original bug report. But syzkaller doesn't realize that it is (or may > > > be) a reproducer. > > > > > > It ought to be possible to ask syzkaller to run a particular test that > > > it has done before, with a patch applied -- and without having to add > > > anything to linux-next. > > > > Yes, this is possible: > > http://bit.do/syzbot#syzkaller-reproducers > > That's not really what I had in mind. I don't want to spend the time > and effort installing syskaller on my own system; I want to tell syzbot > to run a particular syzkaller program (the one that originally led to > this bug report) on a patched kernel. > > The syzbot instructions say that it can test bugs with reproducers. The > problem here is that there doesn't seem to be any way to tell it to use > a particular syzkaller program as a reproducer. Hi Alan, This makes sense and I've found an existing feature request: https://github.com/google/syzkaller/issues/1611 I've added a reference to this thread there. ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [syzbot] general protection fault in gadget_setup 2021-04-16 7:21 ` Dmitry Vyukov @ 2021-04-16 15:29 ` Alan Stern 0 siblings, 0 replies; 13+ messages in thread From: Alan Stern @ 2021-04-16 15:29 UTC (permalink / raw) To: Dmitry Vyukov Cc: syzbot, Andrey Konovalov, Felipe Balbi, Dan Carpenter, Greg Kroah-Hartman, LKML, USB list, syzkaller-bugs, syzkaller On Fri, Apr 16, 2021 at 09:21:12AM +0200, Dmitry Vyukov wrote: > On Thu, Apr 15, 2021 at 10:59 PM Alan Stern <stern@rowland.harvard.edu> wrote: > > > > On Tue, Apr 13, 2021 at 07:11:11PM +0200, Dmitry Vyukov wrote: > > > Yes, this is possible: > > > http://bit.do/syzbot#syzkaller-reproducers > > > > That's not really what I had in mind. I don't want to spend the time > > and effort installing syskaller on my own system; I want to tell syzbot > > to run a particular syzkaller program (the one that originally led to > > this bug report) on a patched kernel. > > > > The syzbot instructions say that it can test bugs with reproducers. The > > problem here is that there doesn't seem to be any way to tell it to use > > a particular syzkaller program as a reproducer. > > Hi Alan, > > This makes sense and I've found an existing feature request: > https://github.com/google/syzkaller/issues/1611 > I've added a reference to this thread there. Great! Thank you. Alan Stern ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [syzbot] general protection fault in gadget_setup 2021-04-13 16:13 ` Alan Stern 2021-04-13 16:47 ` Dmitry Vyukov @ 2021-04-16 5:40 ` Anirudh Rayabharam 2021-04-16 15:27 ` Alan Stern 1 sibling, 1 reply; 13+ messages in thread From: Anirudh Rayabharam @ 2021-04-16 5:40 UTC (permalink / raw) To: Alan Stern Cc: Dmitry Vyukov, syzbot, Andrey Konovalov, Felipe Balbi, Dan Carpenter, Greg Kroah-Hartman, LKML, USB list, syzkaller-bugs On Tue, Apr 13, 2021 at 12:13:11PM -0400, Alan Stern wrote: > On Tue, Apr 13, 2021 at 10:12:05AM +0200, Dmitry Vyukov wrote: > > On Tue, Apr 13, 2021 at 10:08 AM syzbot > > <syzbot+eb4674092e6cc8d9e0bd@syzkaller.appspotmail.com> wrote: > > > > > > Hello, > > > > > > syzbot found the following issue on: > > > > > > HEAD commit: 0f4498ce Merge tag 'for-5.12/dm-fixes-2' of git://git.kern.. > > > git tree: upstream > > > console output: https://syzkaller.appspot.com/x/log.txt?x=124adbf6d00000 > > > kernel config: https://syzkaller.appspot.com/x/.config?x=daeff30c2474a60f > > > dashboard link: https://syzkaller.appspot.com/bug?extid=eb4674092e6cc8d9e0bd > > > userspace arch: i386 > > > > > > Unfortunately, I don't have any reproducer for this issue yet. > > > > > > IMPORTANT: if you fix the issue, please add the following tag to the commit: > > > Reported-by: syzbot+eb4674092e6cc8d9e0bd@syzkaller.appspotmail.com > > > > I suspect that the raw gadget_unbind() can be called while the timer > > is still active. gadget_unbind() sets gadget data to NULL. > > But I am not sure which unbind call this is: > > usb_gadget_remove_driver() or right in udc_bind_to_driver() due to a > > start error. > > This certainly looks like a race between gadget_unbind and gadget_setup > in raw_gadget. > > In theory, this race shouldn't matter. The gadget core is supposed to > guarantee that there won't be any more callbacks to the gadget driver > once the driver's unbind routine is called. That guarantee is enforced > in usb_gadget_remove_driver as follows: > > usb_gadget_disconnect(udc->gadget); > if (udc->gadget->irq) > synchronize_irq(udc->gadget->irq); > udc->driver->unbind(udc->gadget); > usb_gadget_udc_stop(udc); > > usb_gadget_disconnect turns off the pullup resistor, telling the host > that the gadget is no longer connected and preventing the transmission > of any more USB packets. Any packets that have already been received > are sure to processed by the UDC driver's interrupt handler by the time > synchronize_irq returns. > > But this doesn't work with dummy_hcd, because dummy_hcd doesn't use > interrupts; it uses a timer instead. It does have code to emulate the > effect of synchronize_irq, but that code doesn't get invoked at the > right time -- it currently runs in usb_gadget_udc_stop, after the unbind > callback instead of before. Indeed, there's no way for > usb_gadget_remove_driver to invoke this code before the unbind > callback,. > > I thought the synchronize_irq emulation problem had been completely > solved, but evidently it hasn't. It looks like the best solution is to > add a call of the synchronize_irq emulation code in dummy_pullup. > > Maybe we can test this reasoning by putting a delay just before the call > to dum->driver->setup. That runs in the timer handler, so it's not a > good place to delay, but it may be okay just for testing purposes. > > Hopefully this patch will make the race a lot more likely to occur. Is Hi Alan, Indeed, I was able to reproduce this bug easily on my machine with your delay patch applied and using this syzkaller program: syz_usb_connect$cdc_ncm(0x1, 0x6e, &(0x7f0000000040)={{0x12, 0x1, 0x0, 0x2, 0x0, 0x0, 0x8, 0x525, 0xa4a1, 0x40, 0x1, 0x2, 0x3, 0x1, [{{0x9, 0x2, 0x5c, 0x2, 0x1, 0x0, 0x0, 0x0, {{0x9, 0x4, 0x0, 0x0, 0x1, 0x2, 0xd, 0x0, 0x0, {{0x5}, {0x5}, {0xd}, {0x6}}, {{0x9, 0x5, 0x81, 0x3, 0x200}}}}}}]}}, &(0x7f0000000480)={0x0, 0x0, 0x0, 0x0, 0x3, [{0x0, 0x0}, {0x0, 0x0}, {0x0, 0x0}]}) I also tested doing the synchronize_irq emulation in dummy_pullup and it fixed the issue. The patch is below. Thanks! - Anirudh. diff --git a/drivers/usb/gadget/udc/dummy_hcd.c b/drivers/usb/gadget/udc/dummy_hcd.c index ce24d4f28f2a..931d4612d859 100644 --- a/drivers/usb/gadget/udc/dummy_hcd.c +++ b/drivers/usb/gadget/udc/dummy_hcd.c @@ -903,6 +903,12 @@ static int dummy_pullup(struct usb_gadget *_gadget, int value) spin_lock_irqsave(&dum->lock, flags); dum->pullup = (value != 0); set_link_state(dum_hcd); + /* emulate synchronize_irq(): wait for callbacks to finish */\x1c + while (dum->callback_usage > 0) { + spin_unlock_irqrestore(&dum->lock, flags); + usleep_range(1000, 2000); + spin_lock_irqsave(&dum->lock, flags); + } spin_unlock_irqrestore(&dum->lock, flags); usb_hcd_poll_rh_status(dummy_hcd_to_hcd(dum_hcd)); @@ -1005,13 +1011,6 @@ static int dummy_udc_stop(struct usb_gadget *g) dum->ints_enabled = 0; stop_activity(dum); - /* emulate synchronize_irq(): wait for callbacks to finish */ - while (dum->callback_usage > 0) { - spin_unlock_irq(&dum->lock); - usleep_range(1000, 2000); - spin_lock_irq(&dum->lock); - } - dum->driver = NULL; spin_unlock_irq(&dum->lock); @@ -1900,6 +1899,7 @@ static void dummy_timer(struct timer_list *t) if (value > 0) { ++dum->callback_usage; spin_unlock(&dum->lock); + mdelay(5); value = dum->driver->setup(&dum->gadget, &setup); spin_lock(&dum->lock); ^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [syzbot] general protection fault in gadget_setup 2021-04-16 5:40 ` Anirudh Rayabharam @ 2021-04-16 15:27 ` Alan Stern 2021-04-16 17:05 ` Anirudh Rayabharam 0 siblings, 1 reply; 13+ messages in thread From: Alan Stern @ 2021-04-16 15:27 UTC (permalink / raw) To: Anirudh Rayabharam Cc: Dmitry Vyukov, syzbot, Andrey Konovalov, Felipe Balbi, Dan Carpenter, Greg Kroah-Hartman, LKML, USB list, syzkaller-bugs On Fri, Apr 16, 2021 at 11:10:35AM +0530, Anirudh Rayabharam wrote: > On Tue, Apr 13, 2021 at 12:13:11PM -0400, Alan Stern wrote: > > Maybe we can test this reasoning by putting a delay just before the call > > to dum->driver->setup. That runs in the timer handler, so it's not a > > good place to delay, but it may be okay just for testing purposes. > > > > Hopefully this patch will make the race a lot more likely to occur. Is > > Hi Alan, > > Indeed, I was able to reproduce this bug easily on my machine with your > delay patch applied and using this syzkaller program: > > syz_usb_connect$cdc_ncm(0x1, 0x6e, &(0x7f0000000040)={{0x12, 0x1, 0x0, 0x2, 0x0, 0x0, 0x8, 0x525, 0xa4a1, 0x40, 0x1, 0x2, 0x3, 0x1, [{{0x9, 0x2, 0x5c, 0x2, 0x1, 0x0, 0x0, 0x0, {{0x9, 0x4, 0x0, 0x0, 0x1, 0x2, 0xd, 0x0, 0x0, {{0x5}, {0x5}, {0xd}, {0x6}}, {{0x9, 0x5, 0x81, 0x3, 0x200}}}}}}]}}, &(0x7f0000000480)={0x0, 0x0, 0x0, 0x0, 0x3, [{0x0, 0x0}, {0x0, 0x0}, {0x0, 0x0}]}) > > I also tested doing the synchronize_irq emulation in dummy_pullup and it > fixed the issue. The patch is below. That's great! Thanks for testing. > Thanks! > > - Anirudh. > > diff --git a/drivers/usb/gadget/udc/dummy_hcd.c b/drivers/usb/gadget/udc/dummy_hcd.c > index ce24d4f28f2a..931d4612d859 100644 > --- a/drivers/usb/gadget/udc/dummy_hcd.c > +++ b/drivers/usb/gadget/udc/dummy_hcd.c > @@ -903,6 +903,12 @@ static int dummy_pullup(struct usb_gadget *_gadget, int value) > spin_lock_irqsave(&dum->lock, flags); > dum->pullup = (value != 0); > set_link_state(dum_hcd); > + /* emulate synchronize_irq(): wait for callbacks to finish */\x1c > + while (dum->callback_usage > 0) { > + spin_unlock_irqrestore(&dum->lock, flags); > + usleep_range(1000, 2000); > + spin_lock_irqsave(&dum->lock, flags); > + } We should do this only if value == 0. No synchronization is needed when the pullup is turned on. Also, there should be a comment explaining that this is necessary because there's no other place to emulate the call made to synchronize_irq() in core.c:usb_gadget_remove_driver(). > spin_unlock_irqrestore(&dum->lock, flags); > > usb_hcd_poll_rh_status(dummy_hcd_to_hcd(dum_hcd)); > @@ -1005,13 +1011,6 @@ static int dummy_udc_stop(struct usb_gadget *g) > dum->ints_enabled = 0; > stop_activity(dum); > > - /* emulate synchronize_irq(): wait for callbacks to finish */ > - while (dum->callback_usage > 0) { > - spin_unlock_irq(&dum->lock); > - usleep_range(1000, 2000); > - spin_lock_irq(&dum->lock); > - } > - > dum->driver = NULL; > spin_unlock_irq(&dum->lock); Actually, I wanted to move this emulation code into a new subroutine and then call that subroutine from _both_ places. Would you like to write and submit a patch that does this? Alan Stern ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [syzbot] general protection fault in gadget_setup 2021-04-16 15:27 ` Alan Stern @ 2021-04-16 17:05 ` Anirudh Rayabharam 2021-04-16 17:46 ` Alan Stern 0 siblings, 1 reply; 13+ messages in thread From: Anirudh Rayabharam @ 2021-04-16 17:05 UTC (permalink / raw) To: Alan Stern Cc: Dmitry Vyukov, syzbot, Andrey Konovalov, Felipe Balbi, Dan Carpenter, Greg Kroah-Hartman, LKML, USB list, syzkaller-bugs On Fri, Apr 16, 2021 at 11:27:34AM -0400, Alan Stern wrote: > On Fri, Apr 16, 2021 at 11:10:35AM +0530, Anirudh Rayabharam wrote: > > On Tue, Apr 13, 2021 at 12:13:11PM -0400, Alan Stern wrote: > > > Maybe we can test this reasoning by putting a delay just before the call > > > to dum->driver->setup. That runs in the timer handler, so it's not a > > > good place to delay, but it may be okay just for testing purposes. > > > > > > Hopefully this patch will make the race a lot more likely to occur. Is > > > > Hi Alan, > > > > Indeed, I was able to reproduce this bug easily on my machine with your > > delay patch applied and using this syzkaller program: > > > > syz_usb_connect$cdc_ncm(0x1, 0x6e, &(0x7f0000000040)={{0x12, 0x1, 0x0, 0x2, 0x0, 0x0, 0x8, 0x525, 0xa4a1, 0x40, 0x1, 0x2, 0x3, 0x1, [{{0x9, 0x2, 0x5c, 0x2, 0x1, 0x0, 0x0, 0x0, {{0x9, 0x4, 0x0, 0x0, 0x1, 0x2, 0xd, 0x0, 0x0, {{0x5}, {0x5}, {0xd}, {0x6}}, {{0x9, 0x5, 0x81, 0x3, 0x200}}}}}}]}}, &(0x7f0000000480)={0x0, 0x0, 0x0, 0x0, 0x3, [{0x0, 0x0}, {0x0, 0x0}, {0x0, 0x0}]}) > > > > I also tested doing the synchronize_irq emulation in dummy_pullup and it > > fixed the issue. The patch is below. > > That's great! Thanks for testing. > > > Thanks! > > > > - Anirudh. > > > > diff --git a/drivers/usb/gadget/udc/dummy_hcd.c b/drivers/usb/gadget/udc/dummy_hcd.c > > index ce24d4f28f2a..931d4612d859 100644 > > --- a/drivers/usb/gadget/udc/dummy_hcd.c > > +++ b/drivers/usb/gadget/udc/dummy_hcd.c > > @@ -903,6 +903,12 @@ static int dummy_pullup(struct usb_gadget *_gadget, int value) > > spin_lock_irqsave(&dum->lock, flags); > > dum->pullup = (value != 0); > > set_link_state(dum_hcd); > > + /* emulate synchronize_irq(): wait for callbacks to finish */ > > + while (dum->callback_usage > 0) { > > + spin_unlock_irqrestore(&dum->lock, flags); > > + usleep_range(1000, 2000); > > + spin_lock_irqsave(&dum->lock, flags); > > + } > > We should do this only if value == 0. No synchronization is needed when > the pullup is turned on. Oh right! My bad. > Also, there should be a comment explaining that this is necessary > because there's no other place to emulate the call made to > synchronize_irq() in core.c:usb_gadget_remove_driver(). Will do. > > spin_unlock_irqrestore(&dum->lock, flags); > > > > usb_hcd_poll_rh_status(dummy_hcd_to_hcd(dum_hcd)); > > @@ -1005,13 +1011,6 @@ static int dummy_udc_stop(struct usb_gadget *g) > > dum->ints_enabled = 0; > > stop_activity(dum); > > > > - /* emulate synchronize_irq(): wait for callbacks to finish */ > > - while (dum->callback_usage > 0) { > > - spin_unlock_irq(&dum->lock); > > - usleep_range(1000, 2000); > > - spin_lock_irq(&dum->lock); > > - } > > - > > dum->driver = NULL; > > spin_unlock_irq(&dum->lock); > > Actually, I wanted to move this emulation code into a new subroutine and > then call that subroutine from _both_ places. Would you like to write Does it really need to be called from both places? > and submit a patch that does this? Sure! I will do that. Thanks! - Anirudh. ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [syzbot] general protection fault in gadget_setup 2021-04-16 17:05 ` Anirudh Rayabharam @ 2021-04-16 17:46 ` Alan Stern 0 siblings, 0 replies; 13+ messages in thread From: Alan Stern @ 2021-04-16 17:46 UTC (permalink / raw) To: Anirudh Rayabharam Cc: Dmitry Vyukov, syzbot, Andrey Konovalov, Felipe Balbi, Dan Carpenter, Greg Kroah-Hartman, LKML, USB list, syzkaller-bugs On Fri, Apr 16, 2021 at 10:35:20PM +0530, Anirudh Rayabharam wrote: > On Fri, Apr 16, 2021 at 11:27:34AM -0400, Alan Stern wrote: > > Actually, I wanted to move this emulation code into a new subroutine and > > then call that subroutine from _both_ places. Would you like to write > > Does it really need to be called from both places? You know, I was going to say Yes, but now I think you're right; it's not needed in dummy_udc_stop. This is because core.c always calls usb_gadget_disconnect before usb_gadget_udc_stop. And we can rely on this behavior; it's obviously necessary to disconnect from the host before stopping the UDC driver. On the other hand, while checking that fact I noticed that soft_connect_store in core.c doesn't call synchronize_irq in between the other two, the way usb_gadget_remove_driver does. That seems like a bug -- if it's necessary to synchronize with the IRQ handler on one path, it should be necessary on the other path as well. But that's a matter for a separate patch. Alan Stern > > and submit a patch that does this? > > Sure! I will do that. > > Thanks! > > - Anirudh. ^ permalink raw reply [flat|nested] 13+ messages in thread
end of thread, other threads:[~2021-04-16 17:46 UTC | newest] Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2021-04-13 8:08 [syzbot] general protection fault in gadget_setup syzbot 2021-04-13 8:12 ` Dmitry Vyukov 2021-04-13 16:13 ` Alan Stern 2021-04-13 16:47 ` Dmitry Vyukov 2021-04-13 16:57 ` Alan Stern 2021-04-13 17:11 ` Dmitry Vyukov 2021-04-15 20:59 ` Alan Stern 2021-04-16 7:21 ` Dmitry Vyukov 2021-04-16 15:29 ` Alan Stern 2021-04-16 5:40 ` Anirudh Rayabharam 2021-04-16 15:27 ` Alan Stern 2021-04-16 17:05 ` Anirudh Rayabharam 2021-04-16 17:46 ` 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).