possible deadlock in mon_bin_vma_fault

15 views
Skip to first unread message

syzbot

unread,
Sep 3, 2018, 6:01:04 PM9/3/18
to ar...@arndb.de, gre...@linuxfoundation.org, jrdr....@gmail.com, kees...@chromium.org, kste...@linuxfoundation.org, linux-...@vger.kernel.org, linu...@vger.kernel.org, syzkall...@googlegroups.com, tg...@linutronix.de, vi...@zeniv.linux.org.uk, zai...@redhat.com
Hello,

syzbot found the following crash on:

HEAD commit: 58c3f14f86c9 Merge tag 'riscv-for-linus-4.19-rc2' of git:/..
git tree: upstream
console output: https://syzkaller.appspot.com/x/log.txt?x=12fc6f0e400000
kernel config: https://syzkaller.appspot.com/x/.config?x=531a917630d2a492
dashboard link: https://syzkaller.appspot.com/bug?extid=56f9673bb4cdcbeb0e92
compiler: gcc (GCC) 8.0.1 20180413 (experimental)
syz repro: https://syzkaller.appspot.com/x/repro.syz?x=13dca13e400000
C reproducer: https://syzkaller.appspot.com/x/repro.c?x=17cbe492400000

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

vhci_hcd: default hub control req: 0000 v0000 i0000 l0
vhci_hcd: default hub control req: 0000 v0000 i0000 l0
vhci_hcd: default hub control req: 0000 v0000 i0000 l0

======================================================
WARNING: possible circular locking dependency detected
4.19.0-rc1+ #215 Not tainted
vhci_hcd: default hub control req: 0000 v0000 i0000 l0
------------------------------------------------------
vhci_hcd: default hub control req: 0000 v0000 i0000 l0
syz-executor605/4648 is trying to acquire lock:
vhci_hcd: default hub control req: 0000 v0000 i0000 l0
00000000a9171a30 (&rp->fetch_lock){+.+.}, at: mon_bin_vma_fault+0xdc/0x4a0
drivers/usb/mon/mon_bin.c:1237

but task is already holding lock:
0000000069e9aac4 (&mm->mmap_sem){++++}, at: __mm_populate+0x31a/0x4d0
mm/gup.c:1250
vhci_hcd: default hub control req: 0000 v0000 i0000 l0

which lock already depends on the new lock.

vhci_hcd: default hub control req: 0000 v0000 i0000 l0

the existing dependency chain (in reverse order) is:
vhci_hcd: default hub control req: 0000 v0000 i0000 l0

-> #1 (&mm->mmap_sem){++++}:
__might_fault+0x155/0x1e0 mm/memory.c:4578
vhci_hcd: default hub control req: 0000 v0000 i0000 l0
_copy_to_user+0x30/0x110 lib/usercopy.c:25
vhci_hcd: default hub control req: 0000 v0000 i0000 l0
copy_to_user include/linux/uaccess.h:155 [inline]
mon_bin_read+0x334/0x650 drivers/usb/mon/mon_bin.c:825
vhci_hcd: default hub control req: 0000 v0000 i0000 l0
__vfs_read+0x117/0x9b0 fs/read_write.c:416
vhci_hcd: default hub control req: 0000 v0000 i0000 l0
vfs_read+0x17f/0x3c0 fs/read_write.c:452
ksys_read+0x101/0x260 fs/read_write.c:578
__do_sys_read fs/read_write.c:588 [inline]
__se_sys_read fs/read_write.c:586 [inline]
__x64_sys_read+0x73/0xb0 fs/read_write.c:586
do_syscall_64+0x1b9/0x820 arch/x86/entry/common.c:290
vhci_hcd: default hub control req: 0000 v0000 i0000 l0
entry_SYSCALL_64_after_hwframe+0x49/0xbe
vhci_hcd: default hub control req: 0000 v0000 i0000 l0

-> #0 (&rp->fetch_lock){+.+.}:
vhci_hcd: default hub control req: 0000 v0000 i0000 l0
lock_acquire+0x1e4/0x4f0 kernel/locking/lockdep.c:3901
__mutex_lock_common kernel/locking/mutex.c:925 [inline]
__mutex_lock+0x171/0x1700 kernel/locking/mutex.c:1073
mutex_lock_nested+0x16/0x20 kernel/locking/mutex.c:1088
vhci_hcd: default hub control req: 0000 v0000 i0000 l0
mon_bin_vma_fault+0xdc/0x4a0 drivers/usb/mon/mon_bin.c:1237
vhci_hcd: default hub control req: 0000 v0000 i0000 l0
__do_fault+0xee/0x450 mm/memory.c:3240
vhci_hcd: default hub control req: 0000 v0000 i0000 l0
do_read_fault mm/memory.c:3652 [inline]
do_fault mm/memory.c:3752 [inline]
handle_pte_fault mm/memory.c:3983 [inline]
__handle_mm_fault+0x2b4a/0x4350 mm/memory.c:4107
vhci_hcd: default hub control req: 0000 v0000 i0000 l0
handle_mm_fault+0x53e/0xc80 mm/memory.c:4144
faultin_page mm/gup.c:518 [inline]
__get_user_pages+0x823/0x1b50 mm/gup.c:718
populate_vma_page_range+0x2db/0x3d0 mm/gup.c:1222
__mm_populate+0x286/0x4d0 mm/gup.c:1270
vhci_hcd: default hub control req: 0000 v0000 i0000 l0
mm_populate include/linux/mm.h:2307 [inline]
vm_mmap_pgoff+0x27f/0x2c0 mm/util.c:362
vhci_hcd: default hub control req: 0000 v0000 i0000 l0
ksys_mmap_pgoff+0x4da/0x660 mm/mmap.c:1585
vhci_hcd: default hub control req: 0000 v0000 i0000 l0
__do_sys_mmap arch/x86/kernel/sys_x86_64.c:100 [inline]
__se_sys_mmap arch/x86/kernel/sys_x86_64.c:91 [inline]
__x64_sys_mmap+0xe9/0x1b0 arch/x86/kernel/sys_x86_64.c:91
vhci_hcd: default hub control req: 0000 v0000 i0000 l0
do_syscall_64+0x1b9/0x820 arch/x86/entry/common.c:290
entry_SYSCALL_64_after_hwframe+0x49/0xbe

other info that might help us debug this:

vhci_hcd: default hub control req: 0000 v0000 i0000 l0
Possible unsafe locking scenario:

vhci_hcd: default hub control req: 0000 v0000 i0000 l0
CPU0 CPU1
vhci_hcd: default hub control req: 0000 v0000 i0000 l0
---- ----
vhci_hcd: default hub control req: 0000 v0000 i0000 l0
lock(&mm->mmap_sem);
lock(&rp->fetch_lock);
lock(&mm->mmap_sem);
lock(&rp->fetch_lock);
vhci_hcd: default hub control req: 0000 v0000 i0000 l0

*** DEADLOCK ***

vhci_hcd: default hub control req: 0000 v0000 i0000 l0
1 lock held by syz-executor605/4648:
vhci_hcd: default hub control req: 0000 v0000 i0000 l0
#0: 0000000069e9aac4 (&mm->mmap_sem){++++}, at: __mm_populate+0x31a/0x4d0
mm/gup.c:1250
vhci_hcd: default hub control req: 0000 v0000 i0000 l0

stack backtrace:
CPU: 1 PID: 4648 Comm: syz-executor605 Not tainted 4.19.0-rc1+ #215
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS
Google 01/01/2011
vhci_hcd: default hub control req: 0000 v0000 i0000 l0
Call Trace:
vhci_hcd: default hub control req: 0000 v0000 i0000 l0
__dump_stack lib/dump_stack.c:77 [inline]
dump_stack+0x1c9/0x2b4 lib/dump_stack.c:113
vhci_hcd: default hub control req: 0000 v0000 i0000 l0
vhci_hcd: default hub control req: 0000 v0000 i0000 l0
print_circular_bug.isra.34.cold.55+0x1bd/0x27d
kernel/locking/lockdep.c:1222
check_prev_add kernel/locking/lockdep.c:1862 [inline]
check_prevs_add kernel/locking/lockdep.c:1975 [inline]
validate_chain kernel/locking/lockdep.c:2416 [inline]
__lock_acquire+0x3449/0x5020 kernel/locking/lockdep.c:3412
vhci_hcd: default hub control req: 0000 v0000 i0000 l0
vhci_hcd: default hub control req: 0000 v0000 i0000 l0
vhci_hcd: default hub control req: 0000 v0000 i0000 l0
vhci_hcd: default hub control req: 0000 v0000 i0000 l0
vhci_hcd: default hub control req: 0000 v0000 i0000 l0
vhci_hcd: default hub control req: 0000 v0000 i0000 l0
vhci_hcd: default hub control req: 0000 v0000 i0000 l0
vhci_hcd: default hub control req: 0000 v0000 i0000 l0
vhci_hcd: default hub control req: 0000 v0000 i0000 l0
lock_acquire+0x1e4/0x4f0 kernel/locking/lockdep.c:3901
vhci_hcd: default hub control req: 0000 v0000 i0000 l0
vhci_hcd: default hub control req: 0000 v0000 i0000 l0
vhci_hcd: default hub control req: 0000 v0000 i0000 l0
__mutex_lock_common kernel/locking/mutex.c:925 [inline]
__mutex_lock+0x171/0x1700 kernel/locking/mutex.c:1073
vhci_hcd: default hub control req: 0000 v0000 i0000 l0
vhci_hcd: default hub control req: 0000 v0000 i0000 l0
vhci_hcd: default hub control req: 0000 v0000 i0000 l0
vhci_hcd: default hub control req: 0000 v0000 i0000 l0
vhci_hcd: default hub control req: 0000 v0000 i0000 l0
vhci_hcd: default hub control req: 0000 v0000 i0000 l0
vhci_hcd: default hub control req: 0000 v0000 i0000 l0
vhci_hcd: default hub control req: 0000 v0000 i0000 l0
vhci_hcd: default hub control req: 0000 v0000 i0000 l0
vhci_hcd: default hub control req: 0000 v0000 i0000 l0
vhci_hcd: default hub control req: 0000 v0000 i0000 l0
mutex_lock_nested+0x16/0x20 kernel/locking/mutex.c:1088
vhci_hcd: default hub control req: 0000 v0000 i0000 l0
mon_bin_vma_fault+0xdc/0x4a0 drivers/usb/mon/mon_bin.c:1237
vhci_hcd: default hub control req: 0000 v0000 i0000 l0
vhci_hcd: default hub control req: 0000 v0000 i0000 l0
vhci_hcd: default hub control req: 0000 v0000 i0000 l0
vhci_hcd: default hub control req: 0000 v0000 i0000 l0
__do_fault+0xee/0x450 mm/memory.c:3240
vhci_hcd: default hub control req: 0000 v0000 i0000 l0
vhci_hcd: default hub control req: 0000 v0000 i0000 l0
vhci_hcd: default hub control req: 0000 v0000 i0000 l0
do_read_fault mm/memory.c:3652 [inline]
do_fault mm/memory.c:3752 [inline]
handle_pte_fault mm/memory.c:3983 [inline]
__handle_mm_fault+0x2b4a/0x4350 mm/memory.c:4107
vhci_hcd: default hub control req: 0000 v0000 i0000 l0
vhci_hcd: default hub control req: 0000 v0000 i0000 l0
vhci_hcd: default hub control req: 0000 v0000 i0000 l0
vhci_hcd: default hub control req: 0000 v0000 i0000 l0
vhci_hcd: default hub control req: 0000 v0000 i0000 l0
vhci_hcd: default hub control req: 0000 v0000 i0000 l0
handle_mm_fault+0x53e/0xc80 mm/memory.c:4144
vhci_hcd: default hub control req: 0000 v0000 i0000 l0
vhci_hcd: default hub control req: 0000 v0000 i0000 l0
vhci_hcd: default hub control req: 0000 v0000 i0000 l0
vhci_hcd: default hub control req: 0000 v0000 i0000 l0
faultin_page mm/gup.c:518 [inline]
__get_user_pages+0x823/0x1b50 mm/gup.c:718
vhci_hcd: default hub control req: 0000 v0000 i0000 l0
vhci_hcd: default hub control req: 0000 v0000 i0000 l0
populate_vma_page_range+0x2db/0x3d0 mm/gup.c:1222
vhci_hcd: default hub control req: 0000 v0000 i0000 l0
__mm_populate+0x286/0x4d0 mm/gup.c:1270
vhci_hcd: default hub control req: 0000 v0000 i0000 l0
vhci_hcd: default hub control req: 0000 v0000 i0000 l0
vhci_hcd: default hub control req: 0000 v0000 i0000 l0
mm_populate include/linux/mm.h:2307 [inline]
vm_mmap_pgoff+0x27f/0x2c0 mm/util.c:362
vhci_hcd: default hub control req: 0000 v0000 i0000 l0
ksys_mmap_pgoff+0x4da/0x660 mm/mmap.c:1585
vhci_hcd: default hub control req: 0000 v0000 i0000 l0
vhci_hcd: default hub control req: 0000 v0000 i0000 l0
vhci_hcd: default hub control req: 0000 v0000 i0000 l0
__do_sys_mmap arch/x86/kernel/sys_x86_64.c:100 [inline]
__se_sys_mmap arch/x86/kernel/sys_x86_64.c:91 [inline]
__x64_sys_mmap+0xe9/0x1b0 arch/x86/kernel/sys_x86_64.c:91
do_syscall_64+0x1b9/0x820 arch/x86/entry/common.c:290
vhci_hcd: default hub control req: 0000 v0000 i0000 l0
vhci_hcd: default hub control req: 0000 v0000 i0000 l0
vhci_hcd: default hub control req: 0000 v0000 i0000 l0
vhci_hcd: default hub control req: 0000 v0000 i0000 l0
vhci_hcd: default hub control req: 0000 v0000 i0000 l0
entry_SYSCALL_64_after_hwframe+0x49/0xbe
vhci_hcd: default hub control req: 0000 v0000 i0000 l0
RIP: 0033:0x444c09
vhci_hcd: default hub control req: 0000 v0000 i0000 l0
Code: e8 ac e8 ff ff 48 83 c4 18 c3 0f 1f 80 00 00 00 00 48 89 f8 48 89 f7
48 89 d6 48 89 ca 4d 89 c2 4d 89 c8 4c 8b 4c 24 08 0f 05 <48> 3d 01 f0 ff
ff 0f 83 2b ce fb ff c3 66 2e 0f 1f 84 00 00 00 00
vhci_hcd: default hub control req: 0000 v0000 i0000 l0
RSP: 002b:00007ffc31ed6738 EFLAGS: 00000216 ORIG_RAX: 0000000000000009
RAX: ffffffffffffffda RBX: 0000000000000000 RCX: 0000000000444c09
RDX: 0000000001fffffd RSI: 0000000000400000 RDI: 0000000020a05000
vhci_hcd: default hub control req: 0000 v0000 i0000 l0
RBP: 0000000000000000 R08: 0000000000000005 R09: 0000000000000000
vhci_hcd: default hub control req: 0000 v0000 i0000 l0
R10: 0000000000008011 R11: 0000000000000216 R12: 0000000000401ec0
vhci_hcd: default hub control req: 0000 v0000 i0000 l0
R13: 0000000000401f50 R14: 0000000000000000 R15: 0000000000000000
vhci_hcd: default hub control req: 0000 v0000 i0000 l0
vhci_hcd: default hub control req: 0000 v0000 i0000 l0
vhci_hcd: default hub control req: 0000 v0000 i0000 l0
vhci_hcd: default hub control req: 0000 v0000 i0000 l0
vhci_hcd: default hub control req: 0000 v0000 i0000 l0
vhci_hcd: default hub control req: 0000 v0000 i0000 l0
vhci_hcd: default hub control req: 0000 v0000 i0000 l0
vhci_hcd: default hub control req: 0000 v0000 i0000 l0
vhci_hcd: default hub control req: 0000 v0000 i0000 l0
vhci_hcd: default hub control req: 0000 v0000 i0000 l0
vhci_hcd: default hub control req: 0000 v0000 i0000 l0
vhci_hcd: default hub control req: 0000 v0000 i0000 l0
vhci_hcd: default hub control req: 0000 v0000 i0000 l0
vhci_hcd: default hub control req: 0000 v0000 i0000 l0
vhci_hcd: default hub control req: 0000 v0000 i0000 l0
vhci_hcd: default hub control req: 0000 v0000 i0000 l0
vhci_hcd: default hub control req: 0000 v0000 i0000 l0
vhci_hcd: default hub control req: 0000 v0000 i0000 l0
vhci_hcd: default hub control req: 0000 v0000 i0000 l0
vhci_hcd: default hub control req: 0000 v0000 i0000 l0
vhci_hcd: default hub control req: 0000 v0000 i0000 l0
vhci_hcd: default hub control req: 0000 v0000 i0000 l0
vhci_hcd: default hub control req: 0000 v0000 i0000 l0
vhci_hcd: default hub control req: 0000 v0000 i0000 l0
vhci_hcd: default hub control req: 0000 v0000 i0000 l0
vhci_hcd: default hub control req: 0000 v0000 i0000 l0
vhci_hcd: default hub control req: 0000 v0000 i0000 l0
vhci_hcd: default hub control req: 0000 v0000 i0000 l0
vhci_hcd: default hub control req: 0000 v0000 i0000 l0
vhci_hcd: default hub control req: 0000 v0000 i0000 l0
vhci_hcd: default hub control req: 0000 v0000 i0000 l0
vhci_hcd: default hub control req: 0000 v0000 i0000 l0
vhci_hcd: default hub control req: 0000 v0000 i0000 l0
vhci_hcd: default hub control req: 0000 v0000 i0000 l0
vhci_hcd: default hub control req: 0000 v0000 i0000 l0
vhci_hcd: default hub control req: 0000 v0000 i0000 l0
vhci_hcd: default hub control req: 0000 v0000 i0000 l0
vhci_hcd: default hub control req: 0000 v0000 i0000 l0
vhci_hcd: default hub control req: 0000 v0000 i0000 l0
vhci_hcd: default hub control req: 0000 v0000 i0000 l0
vhci_hcd: default hub control req: 0000 v0000 i0000 l0
vhci_hcd: default hub control req: 0000 v0000 i0000 l0
vhci_hcd: default hub control req: 0000 v0000 i0000 l0
vhci_hcd: default hub control req: 0000 v0000 i0000 l0
vhci_hcd: default hub control req: 0000 v0000 i0000 l0
vhci_hcd: default hub control req: 0000 v0000 i0000 l0
vhci_hcd: default hub control req: 0000 v0000 i0000 l0
vhci_hcd: default hub control req: 0000 v0000 i0000 l0
vhci_hcd: default hub control req: 0000 v0000 i0000 l0
vhci_hcd: default hub control req: 0000 v0000 i0000 l0
vhci_hcd: default hub control req: 0000 v0000 i0000 l0
vhci_hcd: default hub control req: 0000 v0000 i0000 l0
vhci_hcd: default hub control req: 0000 v0000 i0000 l0


---
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 syzk...@googlegroups.com.

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

syzbot

unread,
Nov 20, 2019, 7:01:01 AM11/20/19
to ar...@arndb.de, gre...@linuxfoundation.org, jrdr....@gmail.com, kees...@chromium.org, kste...@linuxfoundation.org, linux-...@vger.kernel.org, linu...@vger.kernel.org, syzkall...@googlegroups.com, tg...@linutronix.de, vi...@zeniv.linux.org.uk, zai...@redhat.com
syzbot has bisected this bug to:

commit 46eb14a6e1585d99c1b9f58d0e7389082a5f466b
Author: Pete Zaitcev <zai...@redhat.com>
Date: Mon Jan 8 21:46:41 2018 +0000

USB: fix usbmon BUG trigger

bisection log: https://syzkaller.appspot.com/x/bisect.txt?x=14deb012e00000
start commit: 58c3f14f Merge tag 'riscv-for-linus-4.19-rc2' of git://git..
git tree: upstream
final crash: https://syzkaller.appspot.com/x/report.txt?x=16deb012e00000
console output: https://syzkaller.appspot.com/x/log.txt?x=12deb012e00000
Reported-by: syzbot+56f967...@syzkaller.appspotmail.com
Fixes: 46eb14a6e158 ("USB: fix usbmon BUG trigger")

For information about bisection process see: https://goo.gl/tpsmEJ#bisection

Alan Stern

unread,
Nov 20, 2019, 11:14:07 AM11/20/19
to Pete Zaitcev, syzbot, ar...@arndb.de, gre...@linuxfoundation.org, jrdr....@gmail.com, kees...@chromium.org, kste...@linuxfoundation.org, Kernel development list, USB list, syzkall...@googlegroups.com, tg...@linutronix.de, vi...@zeniv.linux.org.uk
On Wed, 20 Nov 2019, syzbot wrote:

> syzbot has bisected this bug to:
>
> commit 46eb14a6e1585d99c1b9f58d0e7389082a5f466b
> Author: Pete Zaitcev <zai...@redhat.com>
> Date: Mon Jan 8 21:46:41 2018 +0000
>
> USB: fix usbmon BUG trigger

Here's part of the commit description:

USB: fix usbmon BUG trigger

Automated tests triggered this by opening usbmon and accessing the
mmap while simultaneously resizing the buffers. This bug was with
us since 2006, because typically applications only size the buffers
once and thus avoid racing. Reported by Kirill A. Shutemov.

As it happens, I spent a little time investigating this bug report just
yesterday. It seems to me that the easiest fix would be to disallow
resizing the buffer while it is mapped by any users. (Besides,
allowing that seems like a bad idea in any case.)

Pete, does that seem reasonable to you?

Alan Stern

Pete Zaitcev

unread,
Nov 20, 2019, 12:12:41 PM11/20/19
to Alan Stern, syzbot, ar...@arndb.de, gre...@linuxfoundation.org, jrdr....@gmail.com, kees...@chromium.org, kste...@linuxfoundation.org, Kernel development list, USB list, syzkall...@googlegroups.com, tg...@linutronix.de, vi...@zeniv.linux.org.uk, zai...@redhat.com
On Wed, 20 Nov 2019 11:14:05 -0500 (EST)
Alan Stern <st...@rowland.harvard.edu> wrote:

> As it happens, I spent a little time investigating this bug report just
> yesterday. It seems to me that the easiest fix would be to disallow
> resizing the buffer while it is mapped by any users. (Besides,
> allowing that seems like a bad idea in any case.)
>
> Pete, does that seem reasonable to you?

Yes, it does seem reasonable.

I think I understand it now. My fallacy was thinking that since everything
is nailed down as long as fetch_lock is held, it was okay to grab whatever
page from our pagemap. What happens later is an attempt to get pages of the
new buffer while looking at them through the old VMA, in mon_bin_vma_fault.

It seems to me that the use counter, mmap_active, is correct and sufficient
to check in the ioctl.

-- Pete

P.S. One thing that vaguely bothers me on this is that the bot
bisected to the commit that clearly fixed worse issues.

P.P.S. Like this?

diff --git a/drivers/usb/mon/mon_bin.c b/drivers/usb/mon/mon_bin.c
index ac2b4fcc265f..e27d99606adb 100644
--- a/drivers/usb/mon/mon_bin.c
+++ b/drivers/usb/mon/mon_bin.c
@@ -1020,6 +1020,9 @@ static long mon_bin_ioctl(struct file *file, unsigned int cmd, unsigned long arg
int size;
struct mon_pgmap *vec;

+ if (rp->mmap_active)
+ return -EBUSY;
+
if (arg < BUFF_MIN || arg > BUFF_MAX)
return -EINVAL;


Pete Zaitcev

unread,
Nov 20, 2019, 12:33:21 PM11/20/19
to Alan Stern, syzbot, ar...@arndb.de, gre...@linuxfoundation.org, jrdr....@gmail.com, kees...@chromium.org, kste...@linuxfoundation.org, Kernel development list, USB list, syzkall...@googlegroups.com, tg...@linutronix.de, vi...@zeniv.linux.org.uk, zai...@redhat.com
On Wed, 20 Nov 2019 11:14:05 -0500 (EST)
Alan Stern <st...@rowland.harvard.edu> wrote:

> As it happens, I spent a little time investigating this bug report just
> yesterday. It seems to me that the easiest fix would be to disallow
> resizing the buffer while it is mapped by any users. (Besides,
> allowing that seems like a bad idea in any case.)
>
> Pete, does that seem reasonable to you?

Actually, please hold on a little, I think to think more about this.
The deadlock is between mon_bin_read and mon_bin_vma_fault.
To disallow resizing isn't going to fix _that_.

-- Pete

Alan Stern

unread,
Nov 20, 2019, 1:18:08 PM11/20/19
to Pete Zaitcev, syzbot, ar...@arndb.de, gre...@linuxfoundation.org, jrdr....@gmail.com, kees...@chromium.org, kste...@linuxfoundation.org, Kernel development list, USB list, syzkall...@googlegroups.com, tg...@linutronix.de, vi...@zeniv.linux.org.uk
As I understand it (and my understanding is pretty limited, since I
only started to look seriously at the code one day ago), the reason why
mon_bin_vma_fault acquires fetch_lock is to prevent a resize from
happening while the fault is being handled. Is there another reason?

If you disallow resizing while the buffer is mapped then
mon_bin_vma_fault won't need to hold fetch_lock at all. That would fix
the deadlock, right?

Alan Stern

Alan Stern

unread,
Nov 20, 2019, 1:47:02 PM11/20/19
to Pete Zaitcev, syzbot, ar...@arndb.de, gre...@linuxfoundation.org, jrdr....@gmail.com, kees...@chromium.org, kste...@linuxfoundation.org, Kernel development list, USB list, syzkall...@googlegroups.com, tg...@linutronix.de, vi...@zeniv.linux.org.uk
On Wed, 20 Nov 2019, Pete Zaitcev wrote:

Like that, yes, but the test has to be made while fetch_lock is held.
Otherwise there's still a race: One thread could pass the test and then
do the resize, and in between another thread could map the buffer and
incur a fault.

Incidentally, the comment for fetch_lock says that it protects b_read
and b_out, but mon_bin_vma_fault doesn't use either of those fields.

Alan Stern

Pete Zaitcev

unread,
Nov 21, 2019, 9:48:48 AM11/21/19
to Alan Stern, syzbot, ar...@arndb.de, gre...@linuxfoundation.org, jrdr....@gmail.com, kees...@chromium.org, kste...@linuxfoundation.org, Kernel development list, USB list, syzkall...@googlegroups.com, tg...@linutronix.de, vi...@zeniv.linux.org.uk, zai...@redhat.com
On Wed, 20 Nov 2019 13:47:00 -0500 (EST)
Alan Stern <st...@rowland.harvard.edu> wrote:

> > + if (rp->mmap_active)
> > + return -EBUSY;

> Like that, yes, but the test has to be made while fetch_lock is held.

Certainly, thanks. I was rushing just to add a postscriptum.

> Incidentally, the comment for fetch_lock says that it protects b_read
> and b_out, but mon_bin_vma_fault doesn't use either of those fields.

I probably should change that comment to "protect the integrity of the
circular buffer, such as b_out".

Anyway... If you are looking at it too, what do you think about not using
any locks in mon_bin_vma_fault() at all? Isn't it valid? I think I tried
to be "safe", but it only uses things that are constants unless we're
opening and closing; a process cannot make page faults unless it has
some thing mapped; and that is only possible if device is open and stays
open. Can you find a hole in this reasoning?

-- Pete

Alan Stern

unread,
Nov 21, 2019, 11:20:21 AM11/21/19
to Pete Zaitcev, syzbot, ar...@arndb.de, gre...@linuxfoundation.org, jrdr....@gmail.com, kees...@chromium.org, kste...@linuxfoundation.org, Kernel development list, USB list, syzkall...@googlegroups.com, tg...@linutronix.de, vi...@zeniv.linux.org.uk
On Thu, 21 Nov 2019, Pete Zaitcev wrote:

> Anyway... If you are looking at it too, what do you think about not using
> any locks in mon_bin_vma_fault() at all? Isn't it valid? I think I tried
> to be "safe", but it only uses things that are constants unless we're
> opening and closing; a process cannot make page faults unless it has
> some thing mapped; and that is only possible if device is open and stays
> open. Can you find a hole in this reasoning?

I think you're right. But one thing concerns me: What happens if the
same buffer is mapped by more than one process? Do you allow that? I
haven't read the code in enough detail to see.

Alan Stern

Pete Zaitcev

unread,
Nov 21, 2019, 11:46:59 AM11/21/19
to Alan Stern, syzbot, ar...@arndb.de, gre...@linuxfoundation.org, jrdr....@gmail.com, kees...@chromium.org, kste...@linuxfoundation.org, Kernel development list, USB list, syzkall...@googlegroups.com, tg...@linutronix.de, vi...@zeniv.linux.org.uk, zai...@redhat.com
Yes, we allow 2 processes reading from mmap in the same time.
They may miss events, but there should be no issue to the internal
consistency of any pointers in usbmon, and no crashes or deadlocks.
Also, we cannot prohibit that. Imagine a process that does open(),
mmap(), fork()/clone().

-- Pete

Pete Zaitcev

unread,
Nov 21, 2019, 6:38:32 PM11/21/19
to Alan Stern, syzbot, ar...@arndb.de, gre...@linuxfoundation.org, jrdr....@gmail.com, kees...@chromium.org, kste...@linuxfoundation.org, Kernel development list, USB list, syzkall...@googlegroups.com, tg...@linutronix.de, vi...@zeniv.linux.org.uk, zai...@redhat.com
On Thu, 21 Nov 2019 11:20:20 -0500 (EST)
Alan Stern <st...@rowland.harvard.edu> wrote:

> On Thu, 21 Nov 2019, Pete Zaitcev wrote:
>
> > Anyway... If you are looking at it too, what do you think about not using
> > any locks in mon_bin_vma_fault() at all? Isn't it valid? I think I tried
> > to be "safe", but it only uses things that are constants unless we're
> > opening and closing; a process cannot make page faults unless it has
> > some thing mapped; and that is only possible if device is open and stays
> > open. Can you find a hole in this reasoning?
>
> I think you're right. [...]

How about the appended patch, then? You like?

Do you happen to know how to refer to a syzbot report in a commit message?

-- Pete

commit 628f3bbf37eee21cce4cfbfaa6a796b129d7736d
Author: Pete Zaitcev <zai...@kotori.zaitcev.us>
Date: Thu Nov 21 17:24:00 2019 -0600

usb: Fix a deadlock in usbmon between mmap and read

Signed-off-by: Pete Zaitcev <zai...@redhat.com>

diff --git a/drivers/usb/mon/mon_bin.c b/drivers/usb/mon/mon_bin.c
index ac2b4fcc265f..fb7df9810bad 100644
--- a/drivers/usb/mon/mon_bin.c
+++ b/drivers/usb/mon/mon_bin.c
@@ -1039,12 +1039,18 @@ static long mon_bin_ioctl(struct file *file, unsigned int cmd, unsigned long arg

mutex_lock(&rp->fetch_lock);
spin_lock_irqsave(&rp->b_lock, flags);
- mon_free_buff(rp->b_vec, rp->b_size/CHUNK_SIZE);
- kfree(rp->b_vec);
- rp->b_vec = vec;
- rp->b_size = size;
- rp->b_read = rp->b_in = rp->b_out = rp->b_cnt = 0;
- rp->cnt_lost = 0;
+ if (rp->mmap_active) {
+ mon_free_buff(vec, size/CHUNK_SIZE);
+ kfree(vec);
+ ret = -EBUSY;
+ } else {
+ mon_free_buff(rp->b_vec, rp->b_size/CHUNK_SIZE);
+ kfree(rp->b_vec);
+ rp->b_vec = vec;
+ rp->b_size = size;
+ rp->b_read = rp->b_in = rp->b_out = rp->b_cnt = 0;
+ rp->cnt_lost = 0;
+ }
spin_unlock_irqrestore(&rp->b_lock, flags);
mutex_unlock(&rp->fetch_lock);
}
@@ -1093,11 +1099,11 @@ static long mon_bin_ioctl(struct file *file, unsigned int cmd, unsigned long arg
return ret;
if (put_user(ret, &uptr->nfetch))
return -EFAULT;
- ret = 0;
}
break;

- case MON_IOCG_STATS: {
+ case MON_IOCG_STATS:
+ {
struct mon_bin_stats __user *sp;
unsigned int nevents;
unsigned int ndropped;
@@ -1113,7 +1119,6 @@ static long mon_bin_ioctl(struct file *file, unsigned int cmd, unsigned long arg
return -EFAULT;
if (put_user(nevents, &sp->queued))
return -EFAULT;
-
}
break;

@@ -1216,13 +1221,21 @@ mon_bin_poll(struct file *file, struct poll_table_struct *wait)
static void mon_bin_vma_open(struct vm_area_struct *vma)
{
struct mon_reader_bin *rp = vma->vm_private_data;
+ unsigned long flags;
+
+ spin_lock_irqsave(&rp->b_lock, flags);
rp->mmap_active++;
+ spin_unlock_irqrestore(&rp->b_lock, flags);
}

static void mon_bin_vma_close(struct vm_area_struct *vma)
{
+ unsigned long flags;
+
struct mon_reader_bin *rp = vma->vm_private_data;
+ spin_lock_irqsave(&rp->b_lock, flags);
rp->mmap_active--;
+ spin_unlock_irqrestore(&rp->b_lock, flags);
}

/*
@@ -1234,16 +1247,12 @@ static vm_fault_t mon_bin_vma_fault(struct vm_fault *vmf)
unsigned long offset, chunk_idx;
struct page *pageptr;

- mutex_lock(&rp->fetch_lock);
offset = vmf->pgoff << PAGE_SHIFT;
- if (offset >= rp->b_size) {
- mutex_unlock(&rp->fetch_lock);
+ if (offset >= rp->b_size)
return VM_FAULT_SIGBUS;
- }
chunk_idx = offset / CHUNK_SIZE;
pageptr = rp->b_vec[chunk_idx].pg;
get_page(pageptr);
- mutex_unlock(&rp->fetch_lock);
vmf->page = pageptr;
return 0;
}

Dmitry Vyukov

unread,
Nov 22, 2019, 2:18:42 AM11/22/19
to Pete Zaitcev, Alan Stern, syzbot, Arnd Bergmann, Greg Kroah-Hartman, Souptick Joarder, Kees Cook, Kate Stewart, Kernel development list, USB list, syzkaller-bugs, Thomas Gleixner, Al Viro
On Fri, Nov 22, 2019 at 12:38 AM Pete Zaitcev <zai...@redhat.com> wrote:
>
> On Thu, 21 Nov 2019 11:20:20 -0500 (EST)
> Alan Stern <st...@rowland.harvard.edu> wrote:
>
> > On Thu, 21 Nov 2019, Pete Zaitcev wrote:
> >
> > > Anyway... If you are looking at it too, what do you think about not using
> > > any locks in mon_bin_vma_fault() at all? Isn't it valid? I think I tried
> > > to be "safe", but it only uses things that are constants unless we're
> > > opening and closing; a process cannot make page faults unless it has
> > > some thing mapped; and that is only possible if device is open and stays
> > > open. Can you find a hole in this reasoning?
> >
> > I think you're right. [...]
>
> How about the appended patch, then? You like?
>
> Do you happen to know how to refer to a syzbot report in a commit message?
>
> -- Pete
>
> commit 628f3bbf37eee21cce4cfbfaa6a796b129d7736d
> Author: Pete Zaitcev <zai...@kotori.zaitcev.us>
> Date: Thu Nov 21 17:24:00 2019 -0600
>
> usb: Fix a deadlock in usbmon between mmap and read
>
> Signed-off-by: Pete Zaitcev <zai...@redhat.com>

/\/\/\/\/\/\/\/\/\/\

Please don't forget the Reported-by
> --
> 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-bug...@googlegroups.com.
> To view this discussion on the web visit https://groups.google.com/d/msgid/syzkaller-bugs/20191121173825.1527c3a5%40suzdal.zaitcev.lan.

Alan Stern

unread,
Nov 22, 2019, 10:27:12 AM11/22/19
to Pete Zaitcev, syzbot, ar...@arndb.de, gre...@linuxfoundation.org, jrdr....@gmail.com, kees...@chromium.org, kste...@linuxfoundation.org, Kernel development list, USB list, syzkall...@googlegroups.com, tg...@linutronix.de, vi...@zeniv.linux.org.uk
On Thu, 21 Nov 2019, Pete Zaitcev wrote:

> On Thu, 21 Nov 2019 11:20:20 -0500 (EST)
> Alan Stern <st...@rowland.harvard.edu> wrote:
>
> > On Thu, 21 Nov 2019, Pete Zaitcev wrote:
> >
> > > Anyway... If you are looking at it too, what do you think about not using
> > > any locks in mon_bin_vma_fault() at all? Isn't it valid? I think I tried
> > > to be "safe", but it only uses things that are constants unless we're
> > > opening and closing; a process cannot make page faults unless it has
> > > some thing mapped; and that is only possible if device is open and stays
> > > open. Can you find a hole in this reasoning?
> >
> > I think you're right. [...]
>
> How about the appended patch, then? You like?
>
> Do you happen to know how to refer to a syzbot report in a commit message?

As Dmitry mentioned, you should put the Reported-by: line from the
original syzbot bug report (see
<https://marc.info/?l=linux-usb&m=153601206710985&w=2>) in the patch.

> -- Pete
>
> commit 628f3bbf37eee21cce4cfbfaa6a796b129d7736d
> Author: Pete Zaitcev <zai...@kotori.zaitcev.us>
> Date: Thu Nov 21 17:24:00 2019 -0600
>
> usb: Fix a deadlock in usbmon between mmap and read
>
> Signed-off-by: Pete Zaitcev <zai...@redhat.com>
>
> diff --git a/drivers/usb/mon/mon_bin.c b/drivers/usb/mon/mon_bin.c
> index ac2b4fcc265f..fb7df9810bad 100644
> --- a/drivers/usb/mon/mon_bin.c
> +++ b/drivers/usb/mon/mon_bin.c
> @@ -1039,12 +1039,18 @@ static long mon_bin_ioctl(struct file *file, unsigned int cmd, unsigned long arg
>
> mutex_lock(&rp->fetch_lock);
> spin_lock_irqsave(&rp->b_lock, flags);
> - mon_free_buff(rp->b_vec, rp->b_size/CHUNK_SIZE);
> - kfree(rp->b_vec);
> - rp->b_vec = vec;
> - rp->b_size = size;
> - rp->b_read = rp->b_in = rp->b_out = rp->b_cnt = 0;
> - rp->cnt_lost = 0;
> + if (rp->mmap_active) {
> + mon_free_buff(vec, size/CHUNK_SIZE);
> + kfree(vec);
> + ret = -EBUSY;

It would be more elegant to do the rp->mmap_active test before calling
kcalloc and mon_alloc_buf. But of course that's a pretty minor thing.

> + } else {
> + mon_free_buff(rp->b_vec, rp->b_size/CHUNK_SIZE);
> + kfree(rp->b_vec);
> + rp->b_vec = vec;
> + rp->b_size = size;
> + rp->b_read = rp->b_in = rp->b_out = rp->b_cnt = 0;
> + rp->cnt_lost = 0;
> + }
> spin_unlock_irqrestore(&rp->b_lock, flags);
> mutex_unlock(&rp->fetch_lock);
> }
> @@ -1093,11 +1099,11 @@ static long mon_bin_ioctl(struct file *file, unsigned int cmd, unsigned long arg
> return ret;
> if (put_user(ret, &uptr->nfetch))
> return -EFAULT;
> - ret = 0;

What's the reason for this change?

> }
> break;
>
> - case MON_IOCG_STATS: {
> + case MON_IOCG_STATS:
> + {

And this one? This disagrees with the usual kernel style.
Apart from the items mentioned above, this looks right to me.

Alan Stern

Pete Zaitcev

unread,
Nov 22, 2019, 3:52:52 PM11/22/19
to Alan Stern, syzbot, ar...@arndb.de, gre...@linuxfoundation.org, jrdr....@gmail.com, kees...@chromium.org, kste...@linuxfoundation.org, Kernel development list, USB list, syzkall...@googlegroups.com, tg...@linutronix.de, vi...@zeniv.linux.org.uk, zai...@redhat.com
On Fri, 22 Nov 2019 10:27:10 -0500 (EST)
Alan Stern <st...@rowland.harvard.edu> wrote:

> As Dmitry mentioned, you should put the Reported-by: line from the
> original syzbot bug report (see
> <https://marc.info/?l=linux-usb&m=153601206710985&w=2>) in the patch.

Thanks, got it. I also dropped all the cosmetic changes.

> > mutex_lock(&rp->fetch_lock);
> > spin_lock_irqsave(&rp->b_lock, flags);
> > - mon_free_buff(rp->b_vec, rp->b_size/CHUNK_SIZE);
> > - kfree(rp->b_vec);
> > - rp->b_vec = vec;
> > - rp->b_size = size;
> > - rp->b_read = rp->b_in = rp->b_out = rp->b_cnt = 0;
> > - rp->cnt_lost = 0;
> > + if (rp->mmap_active) {
> > + mon_free_buff(vec, size/CHUNK_SIZE);
> > + kfree(vec);
> > + ret = -EBUSY;
>
> It would be more elegant to do the rp->mmap_active test before calling
> kcalloc and mon_alloc_buf. But of course that's a pretty minor thing.

Indeed it feels wrong that so much work gets discarded. However, memory
allocations can block, right? In the same time, our main objective here is
to make sure that when a page fault happens, we fill in the page that VMA
is intended to refer, and not one that was re-allocated. Therefore, I'm
trying to avoid a situation where:

1. thread A checks mmap_active, finds it at zero and proceeds into the
reallocation ioctl
2. thread A sleeps in get_free_page()
3. thread B runs mmap() and succeeds
4. thread A obtains its pages and proceeds to substitute the buffer
5. thread B (or any other) pagefaults and ends with the new, unexpected page

The code is not pretty, but I don't see an alternative. Heck, I would
love you to find more races if you can.

-- Pete

commit 5252eb4c8297fedbf1c5f1e67da44efe00e6ef6b
Author: Pete Zaitcev <zai...@kotori.zaitcev.us>
Date: Thu Nov 21 17:24:00 2019 -0600

usb: Fix a deadlock in usbmon between mmap and read

Signed-off-by: Pete Zaitcev <zai...@redhat.com>
Reported-by: syzbot+56f967...@syzkaller.appspotmail.com

diff --git a/drivers/usb/mon/mon_bin.c b/drivers/usb/mon/mon_bin.c
index ac2b4fcc265f..f48a23adbc35 100644
--- a/drivers/usb/mon/mon_bin.c
+++ b/drivers/usb/mon/mon_bin.c
@@ -1039,12 +1039,18 @@ static long mon_bin_ioctl(struct file *file, unsigned int cmd, unsigned long arg

mutex_lock(&rp->fetch_lock);
spin_lock_irqsave(&rp->b_lock, flags);
- mon_free_buff(rp->b_vec, rp->b_size/CHUNK_SIZE);
- kfree(rp->b_vec);
- rp->b_vec = vec;
- rp->b_size = size;
- rp->b_read = rp->b_in = rp->b_out = rp->b_cnt = 0;
- rp->cnt_lost = 0;
+ if (rp->mmap_active) {
+ mon_free_buff(vec, size/CHUNK_SIZE);
+ kfree(vec);
+ ret = -EBUSY;
+ } else {
+ mon_free_buff(rp->b_vec, rp->b_size/CHUNK_SIZE);
+ kfree(rp->b_vec);
+ rp->b_vec = vec;
+ rp->b_size = size;
+ rp->b_read = rp->b_in = rp->b_out = rp->b_cnt = 0;
+ rp->cnt_lost = 0;
+ }
spin_unlock_irqrestore(&rp->b_lock, flags);
mutex_unlock(&rp->fetch_lock);
}
@@ -1216,13 +1222,21 @@ mon_bin_poll(struct file *file, struct poll_table_struct *wait)
static void mon_bin_vma_open(struct vm_area_struct *vma)
{
struct mon_reader_bin *rp = vma->vm_private_data;
+ unsigned long flags;
+
+ spin_lock_irqsave(&rp->b_lock, flags);
rp->mmap_active++;
+ spin_unlock_irqrestore(&rp->b_lock, flags);
}

static void mon_bin_vma_close(struct vm_area_struct *vma)
{
+ unsigned long flags;
+
struct mon_reader_bin *rp = vma->vm_private_data;
+ spin_lock_irqsave(&rp->b_lock, flags);
rp->mmap_active--;
+ spin_unlock_irqrestore(&rp->b_lock, flags);
}

/*
@@ -1234,16 +1248,12 @@ static vm_fault_t mon_bin_vma_fault(struct vm_fault *vmf)

Alan Stern

unread,
Nov 22, 2019, 5:13:21 PM11/22/19
to Pete Zaitcev, syzbot, ar...@arndb.de, gre...@linuxfoundation.org, jrdr....@gmail.com, kees...@chromium.org, kste...@linuxfoundation.org, Kernel development list, USB list, syzkall...@googlegroups.com, tg...@linutronix.de, vi...@zeniv.linux.org.uk
On Fri, 22 Nov 2019, Pete Zaitcev wrote:

> > It would be more elegant to do the rp->mmap_active test before calling
> > kcalloc and mon_alloc_buf. But of course that's a pretty minor thing.
>
> Indeed it feels wrong that so much work gets discarded. However, memory
> allocations can block, right? In the same time, our main objective here is
> to make sure that when a page fault happens, we fill in the page that VMA
> is intended to refer, and not one that was re-allocated. Therefore, I'm
> trying to avoid a situation where:
>
> 1. thread A checks mmap_active, finds it at zero and proceeds into the
> reallocation ioctl
> 2. thread A sleeps in get_free_page()
> 3. thread B runs mmap() and succeeds
> 4. thread A obtains its pages and proceeds to substitute the buffer
> 5. thread B (or any other) pagefaults and ends with the new, unexpected page
>
> The code is not pretty, but I don't see an alternative. Heck, I would
> love you to find more races if you can.

The alternative is to have the routines for mmap() hold fetch_lock
instead of b_lock. mmap() is allowed to sleep, so that would be okay.
Then you would also hold fetch_lock while checking mmap_active and
doing the memory allocations. That would prevent any races -- in your
example above, thread A would acquire fetch_lock in step 1, so thread B
would block in step 3 until step 4 was finished. Hence B would end up
mapping the correct pages.

In practice, I don't see this being a routine problem. How often do
multiple threads independently try to mmap the same usbmon buffer?

Still, let's see syzbot reacts to your current patch. The line below
is how you ask syzbot to test a candidate patch.

Alan Stern

#syz test: linux-4.19.y f6e27dbb1afa

syzbot

unread,
Nov 22, 2019, 5:13:22 PM11/22/19
to Alan Stern, ar...@arndb.de, gre...@linuxfoundation.org, jrdr....@gmail.com, kees...@chromium.org, kste...@linuxfoundation.org, linux-...@vger.kernel.org, linu...@vger.kernel.org, st...@rowland.harvard.edu, syzkall...@googlegroups.com, tg...@linutronix.de, vi...@zeniv.linux.org.uk, zai...@redhat.com
"linux-4.19.y" does not look like a valid git repo address.

syzbot

unread,
Nov 22, 2019, 5:13:23 PM11/22/19
to Alan Stern, ar...@arndb.de, gre...@linuxfoundation.org, jrdr....@gmail.com, kees...@chromium.org, kste...@linuxfoundation.org, linux-...@vger.kernel.org, linu...@vger.kernel.org, st...@rowland.harvard.edu, syzkall...@googlegroups.com, tg...@linutronix.de, vi...@zeniv.linux.org.uk, zai...@redhat.com
"linux-4.19.y" does not look like a valid git repo address.


Alan Stern

unread,
Nov 23, 2019, 12:18:15 PM11/23/19
to syzbot, ar...@arndb.de, gre...@linuxfoundation.org, jrdr....@gmail.com, kees...@chromium.org, kste...@linuxfoundation.org, linux-...@vger.kernel.org, linu...@vger.kernel.org, syzkall...@googlegroups.com, tg...@linutronix.de, vi...@zeniv.linux.org.uk, zai...@redhat.com
On Fri, 22 Nov 2019, syzbot wrote:

> > #syz test: linux-4.19.y f6e27dbb1afa
>
> "linux-4.19.y" does not look like a valid git repo address.

Let's try again. The "git tree" value in the original bug report was
"upstream", so I'll use that even though it doesn't look like a valid
git repo address either.

Alan Stern

#syz test: upstream f6e27dbb1afa

syzbot

unread,
Nov 23, 2019, 12:18:16 PM11/23/19
to Alan Stern, ar...@arndb.de, gre...@linuxfoundation.org, jrdr....@gmail.com, kees...@chromium.org, kste...@linuxfoundation.org, linux-...@vger.kernel.org, linu...@vger.kernel.org, st...@rowland.harvard.edu, syzkall...@googlegroups.com, tg...@linutronix.de, vi...@zeniv.linux.org.uk, zai...@redhat.com
> On Fri, 22 Nov 2019, syzbot wrote:

>> > #syz test: linux-4.19.y f6e27dbb1afa

>> "linux-4.19.y" does not look like a valid git repo address.

> Let's try again. The "git tree" value in the original bug report was
> "upstream", so I'll use that even though it doesn't look like a valid
> git repo address either.

> Alan Stern

> #syz test: upstream f6e27dbb1afa

"upstream" does not look like a valid git repo address.

syzbot

unread,
Nov 23, 2019, 12:18:17 PM11/23/19
to Alan Stern, ar...@arndb.de, gre...@linuxfoundation.org, jrdr....@gmail.com, kees...@chromium.org, kste...@linuxfoundation.org, linux-...@vger.kernel.org, linu...@vger.kernel.org, st...@rowland.harvard.edu, syzkall...@googlegroups.com, tg...@linutronix.de, vi...@zeniv.linux.org.uk, zai...@redhat.com
> On Fri, 22 Nov 2019, syzbot wrote:

>> > #syz test: linux-4.19.y f6e27dbb1afa

>> "linux-4.19.y" does not look like a valid git repo address.

> Let's try again. The "git tree" value in the original bug report was
> "upstream", so I'll use that even though it doesn't look like a valid
> git repo address either.

> Alan Stern

> #syz test: upstream f6e27dbb1afa

"upstream" does not look like a valid git repo address.

Alan Stern

unread,
Nov 24, 2019, 10:59:44 AM11/24/19
to syzbot, Andrey Konovalov, ar...@arndb.de, gre...@linuxfoundation.org, jrdr....@gmail.com, kees...@chromium.org, kste...@linuxfoundation.org, Kernel development list, USB list, syzkall...@googlegroups.com, tg...@linutronix.de, vi...@zeniv.linux.org.uk, zai...@redhat.com
On Sat, 23 Nov 2019, syzbot wrote:

> > On Fri, 22 Nov 2019, syzbot wrote:
>
> >> > #syz test: linux-4.19.y f6e27dbb1afa
>
> >> "linux-4.19.y" does not look like a valid git repo address.
>
> > Let's try again. The "git tree" value in the original bug report was
> > "upstream", so I'll use that even though it doesn't look like a valid
> > git repo address either.
>
> > Alan Stern
>
> > #syz test: upstream f6e27dbb1afa
>
> "upstream" does not look like a valid git repo address.

Andrey, can you do something about that? It would be a lot nicer if
_all_ the syzbot output and records included an actual git repo address
in the appropriate places.

Alan Stern

#syz test: git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git v5.3

syzbot

unread,
Nov 24, 2019, 2:10:02 PM11/24/19
to andre...@google.com, ar...@arndb.de, gre...@linuxfoundation.org, jrdr....@gmail.com, kees...@chromium.org, kste...@linuxfoundation.org, linux-...@vger.kernel.org, linu...@vger.kernel.org, st...@rowland.harvard.edu, syzkall...@googlegroups.com, tg...@linutronix.de, vi...@zeniv.linux.org.uk, zai...@redhat.com
Hello,

syzbot tried to test the proposed patch but build/boot failed:

failed to apply patch:
checking file drivers/usb/mon/mon_bin.c
patch: **** unexpected end of file in patch



Tested on:

commit: 4d856f72 Linux 5.3
git tree:
git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git v5.3
dashboard link: https://syzkaller.appspot.com/bug?extid=56f9673bb4cdcbeb0e92
compiler: gcc (GCC) 9.0.0 20181231 (experimental)
patch: https://syzkaller.appspot.com/x/patch.diff?x=17a22f16e00000

Alan Stern

unread,
Nov 24, 2019, 3:55:31 PM11/24/19
to syzbot, andre...@google.com, ar...@arndb.de, gre...@linuxfoundation.org, jrdr....@gmail.com, kees...@chromium.org, kste...@linuxfoundation.org, linux-...@vger.kernel.org, linu...@vger.kernel.org, syzkall...@googlegroups.com, tg...@linutronix.de, vi...@zeniv.linux.org.uk, zai...@redhat.com
On Sun, 24 Nov 2019, syzbot wrote:

> Hello,
>
> syzbot tried to test the proposed patch but build/boot failed:
>
> failed to apply patch:
> checking file drivers/usb/mon/mon_bin.c
> patch: **** unexpected end of file in patch

One more try...

syzbot

unread,
Nov 24, 2019, 6:24:01 PM11/24/19
to andre...@google.com, ar...@arndb.de, gre...@linuxfoundation.org, jrdr....@gmail.com, kees...@chromium.org, kste...@linuxfoundation.org, linux-...@vger.kernel.org, linu...@vger.kernel.org, st...@rowland.harvard.edu, syzkall...@googlegroups.com, tg...@linutronix.de, vi...@zeniv.linux.org.uk, zai...@redhat.com
Hello,

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

Reported-and-tested-by:
syzbot+56f967...@syzkaller.appspotmail.com

Tested on:

commit: 4d856f72 Linux 5.3
git tree:
git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git v5.3
kernel config: https://syzkaller.appspot.com/x/.config?x=86071634b2594991
dashboard link: https://syzkaller.appspot.com/bug?extid=56f9673bb4cdcbeb0e92
compiler: gcc (GCC) 9.0.0 20181231 (experimental)
patch: https://syzkaller.appspot.com/x/patch.diff?x=11ff3eeee00000

Note: testing is done by a robot and is best-effort only.
Reply all
Reply to author
Forward
0 new messages