* WARNING: refcount bug in kvm_vm_ioctl @ 2018-10-10 7:58 syzbot 2018-10-11 14:17 ` Paolo Bonzini 0 siblings, 1 reply; 11+ messages in thread From: syzbot @ 2018-10-10 7:58 UTC (permalink / raw) To: kvm, linux-kernel, pbonzini, rkrcmar, syzkaller-bugs Hello, syzbot found the following crash on: HEAD commit: 64c5e530ac2c Merge tag 'arc-4.19-rc8' of git://git.kernel... git tree: upstream console output: https://syzkaller.appspot.com/x/log.txt?x=16343ea1400000 kernel config: https://syzkaller.appspot.com/x/.config?x=88e9a8a39dc0be2d dashboard link: https://syzkaller.appspot.com/bug?extid=6df4d24a0a50fd5d1a08 compiler: gcc (GCC) 8.0.1 20180413 (experimental) syz repro: https://syzkaller.appspot.com/x/repro.syz?x=133eb685400000 IMPORTANT: if you fix the bug, please add the following tag to the commit: Reported-by: syzbot+6df4d24a0a50fd5d1a08@syzkaller.appspotmail.com 8021q: adding VLAN 0 to HW filter on device team0 L1TF CPU bug present and SMT on, data leak possible. See CVE-2018-3646 and https://www.kernel.org/doc/html/latest/admin-guide/l1tf.html for details. hrtimer: interrupt took 30857 ns ------------[ cut here ]------------ refcount_t: increment on 0; use-after-free. WARNING: CPU: 1 PID: 8299 at lib/refcount.c:153 refcount_inc_checked+0x5d/0x70 lib/refcount.c:153 Kernel panic - not syncing: panic_on_warn set ... CPU: 1 PID: 8299 Comm: syz-executor2 Not tainted 4.19.0-rc7+ #275 kobject: 'kvm' (00000000705acfae): kobject_uevent_env Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 01/01/2011 Call Trace: __dump_stack lib/dump_stack.c:77 [inline] dump_stack+0x1c4/0x2b4 lib/dump_stack.c:113 panic+0x238/0x4e7 kernel/panic.c:184 kobject: 'kvm' (00000000705acfae): fill_kobj_path: path = '/devices/virtual/misc/kvm' __warn.cold.8+0x163/0x1ba kernel/panic.c:536 report_bug+0x254/0x2d0 lib/bug.c:186 fixup_bug arch/x86/kernel/traps.c:178 [inline] do_error_trap+0x1fc/0x4d0 arch/x86/kernel/traps.c:296 kobject: 'kvm' (00000000705acfae): kobject_uevent_env kobject: 'kvm' (00000000705acfae): fill_kobj_path: path = '/devices/virtual/misc/kvm' kobject: 'kvm' (00000000705acfae): kobject_uevent_env do_invalid_op+0x1b/0x20 arch/x86/kernel/traps.c:316 invalid_op+0x14/0x20 arch/x86/entry/entry_64.S:993 RIP: 0010:refcount_inc_checked+0x5d/0x70 lib/refcount.c:153 kobject: 'kvm' (00000000705acfae): fill_kobj_path: path = '/devices/virtual/misc/kvm' Code: 1d 2b 7d 62 06 31 ff 89 de e8 1f bc f2 fd 84 db 75 df e8 46 bb f2 fd 48 c7 c7 80 72 40 88 c6 05 0b 7d 62 06 01 e8 23 8e bc fd <0f> 0b eb c3 0f 1f 44 00 00 66 2e 0f 1f 84 00 00 00 00 00 55 48 89 RSP: 0018:ffff8801c1647708 EFLAGS: 00010286 RAX: 0000000000000000 RBX: 0000000000000000 RCX: 0000000000000000 RDX: 0000000000000000 RSI: ffffffff81650405 RDI: 0000000000000005 RBP: ffff8801c1647710 R08: ffff8801c2b98300 R09: ffffed003b5e4fe8 R10: ffffed003b5e4fe8 R11: ffff8801daf27f47 R12: ffff8801c16477c0 R13: ffff8801c4225800 R14: ffffffff892b6ec0 R15: ffffc9000497b000 kvm_get_kvm arch/x86/kvm/../../../virt/kvm/kvm_main.c:766 [inline] kvm_ioctl_create_device arch/x86/kvm/../../../virt/kvm/kvm_main.c:2924 [inline] kvm_vm_ioctl+0xed7/0x1d40 arch/x86/kvm/../../../virt/kvm/kvm_main.c:3114 vfs_ioctl fs/ioctl.c:46 [inline] file_ioctl fs/ioctl.c:501 [inline] do_vfs_ioctl+0x1de/0x1720 fs/ioctl.c:685 ksys_ioctl+0xa9/0xd0 fs/ioctl.c:702 __do_sys_ioctl fs/ioctl.c:709 [inline] __se_sys_ioctl fs/ioctl.c:707 [inline] __x64_sys_ioctl+0x73/0xb0 fs/ioctl.c:707 do_syscall_64+0x1b9/0x820 arch/x86/entry/common.c:290 entry_SYSCALL_64_after_hwframe+0x49/0xbe RIP: 0033:0x457579 Code: 1d b4 fb ff c3 66 2e 0f 1f 84 00 00 00 00 00 66 90 48 89 f8 48 89 f7 48 89 d6 48 89 ca 4d 89 c2 4d 89 c8 4c 8b 4c 24 08 0f 05 <48> 3d 01 f0 ff ff 0f 83 eb b3 fb ff c3 66 2e 0f 1f 84 00 00 00 00 RSP: 002b:00007f5511e6bc78 EFLAGS: 00000246 ORIG_RAX: 0000000000000010 RAX: ffffffffffffffda RBX: 0000000000000003 RCX: 0000000000457579 RDX: 00000000200002c0 RSI: 00000000c00caee0 RDI: 0000000000000009 RBP: 000000000072bfa0 R08: 0000000000000000 R09: 0000000000000000 R10: 0000000000000000 R11: 0000000000000246 R12: 00007f5511e6c6d4 R13: 00000000004bfbd1 R14: 00000000004cfc58 R15: 00000000ffffffff Kernel Offset: disabled Rebooting in 86400 seconds.. --- 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#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 ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: WARNING: refcount bug in kvm_vm_ioctl 2018-10-10 7:58 WARNING: refcount bug in kvm_vm_ioctl syzbot @ 2018-10-11 14:17 ` Paolo Bonzini 2018-10-11 18:23 ` Dmitry Vyukov 2019-02-15 15:39 ` Dmitry Vyukov 0 siblings, 2 replies; 11+ messages in thread From: Paolo Bonzini @ 2018-10-11 14:17 UTC (permalink / raw) To: syzbot, kvm, linux-kernel, rkrcmar, syzkaller-bugs, Christoffer Dall, Janosch Frank, Christian Borntraeger On 10/10/2018 09:58, syzbot wrote: > do_invalid_op+0x1b/0x20 arch/x86/kernel/traps.c:316 > invalid_op+0x14/0x20 arch/x86/entry/entry_64.S:993 > RIP: 0010:refcount_inc_checked+0x5d/0x70 lib/refcount.c:153 > kvm_get_kvm arch/x86/kvm/../../../virt/kvm/kvm_main.c:766 [inline] > kvm_ioctl_create_device arch/x86/kvm/../../../virt/kvm/kvm_main.c:2924 > kvm_vm_ioctl+0xed7/0x1d40 arch/x86/kvm/../../../virt/kvm/kvm_main.c:3114 > vfs_ioctl fs/ioctl.c:46 [inline] > file_ioctl fs/ioctl.c:501 [inline] > do_vfs_ioctl+0x1de/0x1720 fs/ioctl.c:685 > ksys_ioctl+0xa9/0xd0 fs/ioctl.c:702 > __do_sys_ioctl fs/ioctl.c:709 [inline] > __se_sys_ioctl fs/ioctl.c:707 [inline] > __x64_sys_ioctl+0x73/0xb0 fs/ioctl.c:707 > do_syscall_64+0x1b9/0x820 arch/x86/entry/common.c:290 > entry_SYSCALL_64_after_hwframe+0x49/0xbe The trace here is fairly simple, but I don't understand how this could happen. The kvm_get_kvm is done within kvm_ioctl_create_device, which is called from ioctl; the last reference cannot disappear inside a ioctl, because: 1) kvm_ioctl is called from vfs_ioctl, which does fdget and holds the fd reference until after kvm_vm_ioctl returns 2) the file descriptor holds one reference to the struct kvm*, and this reference is not released until kvm_vm_release is called by the last fput (which could be fdput's call to fput if the process has exited in the meanwhile) 3) for completeness, in case anon_inode_getfd fails, put_unused_fd will not invoke the file descriptor's ->release callback (in this case kvm_device_release). CCing some random people to get their opinion... Paolo > # See https://goo.gl/kgGztJ for information about syzkaller reproducers. > #{"threaded":true,"collide":true,"repeat":true,"procs":6,"sandbox":"none","fault_call":-1,"tun":true,"tmpdir":true,"cgroups":true,"netdev":true,"resetnet":true,"segv":true} > r0 = openat$kvm(0xffffffffffffff9c, &(0x7f0000000380)='/dev/kvm\x00', 0x0, 0x0) > r1 = syz_open_dev$dspn(&(0x7f0000000100)='/dev/dsp#\x00', 0x3fe, 0x400) > r2 = ioctl$KVM_CREATE_VM(r0, 0xae01, 0x0) > perf_event_open(&(0x7f0000000040)={0x1, 0x70, 0x0, 0x0, 0x0, 0x0, 0x0, 0x50d}, 0x0, 0xffffffffffffffff, 0xffffffffffffffff, 0x0) > mincore(&(0x7f0000ffc000/0x1000)=nil, 0x1000, &(0x7f00000003c0)=""/4096) > setrlimit(0x0, &(0x7f0000000000)) > readahead(r1, 0x3, 0x9a6) > ioctl$KVM_CREATE_DEVICE(r2, 0xc00caee0, &(0x7f00000002c0)={0x4}) > setsockopt$inet_sctp6_SCTP_FRAGMENT_INTERLEAVE(r1, 0x84, 0x12, &(0x7f00000001c0)=0x9, 0x4) ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: WARNING: refcount bug in kvm_vm_ioctl 2018-10-11 14:17 ` Paolo Bonzini @ 2018-10-11 18:23 ` Dmitry Vyukov 2018-10-11 18:39 ` Dmitry Vyukov 2019-02-15 15:39 ` Dmitry Vyukov 1 sibling, 1 reply; 11+ messages in thread From: Dmitry Vyukov @ 2018-10-11 18:23 UTC (permalink / raw) To: Paolo Bonzini Cc: syzbot, KVM list, LKML, Radim Krčmář, syzkaller-bugs, Christoffer Dall, Janosch Frank, Christian Borntraeger On Thu, Oct 11, 2018 at 4:17 PM, Paolo Bonzini <pbonzini@redhat.com> wrote: > On 10/10/2018 09:58, syzbot wrote: >> do_invalid_op+0x1b/0x20 arch/x86/kernel/traps.c:316 >> invalid_op+0x14/0x20 arch/x86/entry/entry_64.S:993 >> RIP: 0010:refcount_inc_checked+0x5d/0x70 lib/refcount.c:153 >> kvm_get_kvm arch/x86/kvm/../../../virt/kvm/kvm_main.c:766 [inline] >> kvm_ioctl_create_device arch/x86/kvm/../../../virt/kvm/kvm_main.c:2924 >> kvm_vm_ioctl+0xed7/0x1d40 arch/x86/kvm/../../../virt/kvm/kvm_main.c:3114 >> vfs_ioctl fs/ioctl.c:46 [inline] >> file_ioctl fs/ioctl.c:501 [inline] >> do_vfs_ioctl+0x1de/0x1720 fs/ioctl.c:685 >> ksys_ioctl+0xa9/0xd0 fs/ioctl.c:702 >> __do_sys_ioctl fs/ioctl.c:709 [inline] >> __se_sys_ioctl fs/ioctl.c:707 [inline] >> __x64_sys_ioctl+0x73/0xb0 fs/ioctl.c:707 >> do_syscall_64+0x1b9/0x820 arch/x86/entry/common.c:290 >> entry_SYSCALL_64_after_hwframe+0x49/0xbe > > The trace here is fairly simple, but I don't understand how this could > happen. > > The kvm_get_kvm is done within kvm_ioctl_create_device, which is called > from ioctl; the last reference cannot disappear inside a ioctl, because: > > 1) kvm_ioctl is called from vfs_ioctl, which does fdget and holds the fd > reference until after kvm_vm_ioctl returns > > 2) the file descriptor holds one reference to the struct kvm*, and this > reference is not released until kvm_vm_release is called by the last > fput (which could be fdput's call to fput if the process has exited in > the meanwhile) > > 3) for completeness, in case anon_inode_getfd fails, put_unused_fd will > not invoke the file descriptor's ->release callback (in this case > kvm_device_release). > > CCing some random people to get their opinion... > > Paolo > >> # See https://goo.gl/kgGztJ for information about syzkaller reproducers. >> #{"threaded":true,"collide":true,"repeat":true,"procs":6,"sandbox":"none","fault_call":-1,"tun":true,"tmpdir":true,"cgroups":true,"netdev":true,"resetnet":true,"segv":true} >> r0 = openat$kvm(0xffffffffffffff9c, &(0x7f0000000380)='/dev/kvm\x00', 0x0, 0x0) >> r1 = syz_open_dev$dspn(&(0x7f0000000100)='/dev/dsp#\x00', 0x3fe, 0x400) >> r2 = ioctl$KVM_CREATE_VM(r0, 0xae01, 0x0) >> perf_event_open(&(0x7f0000000040)={0x1, 0x70, 0x0, 0x0, 0x0, 0x0, 0x0, 0x50d}, 0x0, 0xffffffffffffffff, 0xffffffffffffffff, 0x0) >> mincore(&(0x7f0000ffc000/0x1000)=nil, 0x1000, &(0x7f00000003c0)=""/4096) >> setrlimit(0x0, &(0x7f0000000000)) >> readahead(r1, 0x3, 0x9a6) >> ioctl$KVM_CREATE_DEVICE(r2, 0xc00caee0, &(0x7f00000002c0)={0x4}) >> setsockopt$inet_sctp6_SCTP_FRAGMENT_INTERLEAVE(r1, 0x84, 0x12, &(0x7f00000001c0)=0x9, 0x4) Looking at crash rate and dates here: https://syzkaller.appspot.com/bug?id=2b9fab00a235b50a34adbc35adc236f7dbe490fd https://syzkaller.appspot.com/bug?id=d6e4dd59a9b708895738b9cc59e6bdcb3a43ff14 It looks like something that reached upstream tree yesterday. However, there is one outliner: 2018/09/30 08:58. So either that one crash was something else, or syzkaller learned how to reproduce it more often... But maybe if you know some recent suspect patch that touched that area, that may be the easiest way to localize the bug. ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: WARNING: refcount bug in kvm_vm_ioctl 2018-10-11 18:23 ` Dmitry Vyukov @ 2018-10-11 18:39 ` Dmitry Vyukov 0 siblings, 0 replies; 11+ messages in thread From: Dmitry Vyukov @ 2018-10-11 18:39 UTC (permalink / raw) To: Paolo Bonzini Cc: syzbot, KVM list, LKML, Radim Krčmář, syzkaller-bugs, Janosch Frank, Christian Borntraeger On Thu, Oct 11, 2018 at 8:23 PM, Dmitry Vyukov <dvyukov@google.com> wrote: > On Thu, Oct 11, 2018 at 4:17 PM, Paolo Bonzini <pbonzini@redhat.com> wrote: >> On 10/10/2018 09:58, syzbot wrote: >>> do_invalid_op+0x1b/0x20 arch/x86/kernel/traps.c:316 >>> invalid_op+0x14/0x20 arch/x86/entry/entry_64.S:993 >>> RIP: 0010:refcount_inc_checked+0x5d/0x70 lib/refcount.c:153 >>> kvm_get_kvm arch/x86/kvm/../../../virt/kvm/kvm_main.c:766 [inline] >>> kvm_ioctl_create_device arch/x86/kvm/../../../virt/kvm/kvm_main.c:2924 >>> kvm_vm_ioctl+0xed7/0x1d40 arch/x86/kvm/../../../virt/kvm/kvm_main.c:3114 >>> vfs_ioctl fs/ioctl.c:46 [inline] >>> file_ioctl fs/ioctl.c:501 [inline] >>> do_vfs_ioctl+0x1de/0x1720 fs/ioctl.c:685 >>> ksys_ioctl+0xa9/0xd0 fs/ioctl.c:702 >>> __do_sys_ioctl fs/ioctl.c:709 [inline] >>> __se_sys_ioctl fs/ioctl.c:707 [inline] >>> __x64_sys_ioctl+0x73/0xb0 fs/ioctl.c:707 >>> do_syscall_64+0x1b9/0x820 arch/x86/entry/common.c:290 >>> entry_SYSCALL_64_after_hwframe+0x49/0xbe >> >> The trace here is fairly simple, but I don't understand how this could >> happen. >> >> The kvm_get_kvm is done within kvm_ioctl_create_device, which is called >> from ioctl; the last reference cannot disappear inside a ioctl, because: >> >> 1) kvm_ioctl is called from vfs_ioctl, which does fdget and holds the fd >> reference until after kvm_vm_ioctl returns >> >> 2) the file descriptor holds one reference to the struct kvm*, and this >> reference is not released until kvm_vm_release is called by the last >> fput (which could be fdput's call to fput if the process has exited in >> the meanwhile) >> >> 3) for completeness, in case anon_inode_getfd fails, put_unused_fd will >> not invoke the file descriptor's ->release callback (in this case >> kvm_device_release). >> >> CCing some random people to get their opinion... >> >> Paolo >> >>> # See https://goo.gl/kgGztJ for information about syzkaller reproducers. >>> #{"threaded":true,"collide":true,"repeat":true,"procs":6,"sandbox":"none","fault_call":-1,"tun":true,"tmpdir":true,"cgroups":true,"netdev":true,"resetnet":true,"segv":true} >>> r0 = openat$kvm(0xffffffffffffff9c, &(0x7f0000000380)='/dev/kvm\x00', 0x0, 0x0) >>> r1 = syz_open_dev$dspn(&(0x7f0000000100)='/dev/dsp#\x00', 0x3fe, 0x400) >>> r2 = ioctl$KVM_CREATE_VM(r0, 0xae01, 0x0) >>> perf_event_open(&(0x7f0000000040)={0x1, 0x70, 0x0, 0x0, 0x0, 0x0, 0x0, 0x50d}, 0x0, 0xffffffffffffffff, 0xffffffffffffffff, 0x0) >>> mincore(&(0x7f0000ffc000/0x1000)=nil, 0x1000, &(0x7f00000003c0)=""/4096) >>> setrlimit(0x0, &(0x7f0000000000)) >>> readahead(r1, 0x3, 0x9a6) >>> ioctl$KVM_CREATE_DEVICE(r2, 0xc00caee0, &(0x7f00000002c0)={0x4}) >>> setsockopt$inet_sctp6_SCTP_FRAGMENT_INTERLEAVE(r1, 0x84, 0x12, &(0x7f00000001c0)=0x9, 0x4) > > > Looking at crash rate and dates here: > https://syzkaller.appspot.com/bug?id=2b9fab00a235b50a34adbc35adc236f7dbe490fd > https://syzkaller.appspot.com/bug?id=d6e4dd59a9b708895738b9cc59e6bdcb3a43ff14 > > It looks like something that reached upstream tree yesterday. > However, there is one outliner: 2018/09/30 08:58. So either that one > crash was something else, or syzkaller learned how to reproduce it > more often... > But maybe if you know some recent suspect patch that touched that > area, that may be the easiest way to localize the bug. I can't reproduce it locally qemu on top of 4.17.17 host kernel, at least easily. What are the chances this is provoked by L0 kernel (syzbot also runs in VMs)? ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: WARNING: refcount bug in kvm_vm_ioctl 2018-10-11 14:17 ` Paolo Bonzini 2018-10-11 18:23 ` Dmitry Vyukov @ 2019-02-15 15:39 ` Dmitry Vyukov 2019-02-15 16:02 ` Jann Horn 1 sibling, 1 reply; 11+ messages in thread From: Dmitry Vyukov @ 2019-02-15 15:39 UTC (permalink / raw) To: Paolo Bonzini, Jann Horn Cc: syzbot, KVM list, LKML, Radim Krčmář, syzkaller-bugs, Christoffer Dall, Janosch Frank, Christian Borntraeger On Thu, Oct 11, 2018 at 4:18 PM Paolo Bonzini <pbonzini@redhat.com> wrote: > > On 10/10/2018 09:58, syzbot wrote: > > do_invalid_op+0x1b/0x20 arch/x86/kernel/traps.c:316 > > invalid_op+0x14/0x20 arch/x86/entry/entry_64.S:993 > > RIP: 0010:refcount_inc_checked+0x5d/0x70 lib/refcount.c:153 > > kvm_get_kvm arch/x86/kvm/../../../virt/kvm/kvm_main.c:766 [inline] > > kvm_ioctl_create_device arch/x86/kvm/../../../virt/kvm/kvm_main.c:2924 > > kvm_vm_ioctl+0xed7/0x1d40 arch/x86/kvm/../../../virt/kvm/kvm_main.c:3114 > > vfs_ioctl fs/ioctl.c:46 [inline] > > file_ioctl fs/ioctl.c:501 [inline] > > do_vfs_ioctl+0x1de/0x1720 fs/ioctl.c:685 > > ksys_ioctl+0xa9/0xd0 fs/ioctl.c:702 > > __do_sys_ioctl fs/ioctl.c:709 [inline] > > __se_sys_ioctl fs/ioctl.c:707 [inline] > > __x64_sys_ioctl+0x73/0xb0 fs/ioctl.c:707 > > do_syscall_64+0x1b9/0x820 arch/x86/entry/common.c:290 > > entry_SYSCALL_64_after_hwframe+0x49/0xbe > > The trace here is fairly simple, but I don't understand how this could > happen. > > The kvm_get_kvm is done within kvm_ioctl_create_device, which is called > from ioctl; the last reference cannot disappear inside a ioctl, because: > > 1) kvm_ioctl is called from vfs_ioctl, which does fdget and holds the fd > reference until after kvm_vm_ioctl returns > > 2) the file descriptor holds one reference to the struct kvm*, and this > reference is not released until kvm_vm_release is called by the last > fput (which could be fdput's call to fput if the process has exited in > the meanwhile) > > 3) for completeness, in case anon_inode_getfd fails, put_unused_fd will > not invoke the file descriptor's ->release callback (in this case > kvm_device_release). > > CCing some random people to get their opinion... > > Paolo Jann, is it what you fixed in "kvm: fix kvm_ioctl_create_device() reference counting (CVE-2019-6974)"? If so, we need to close the syzbot bug. > > # See https://goo.gl/kgGztJ for information about syzkaller reproducers. > > #{"threaded":true,"collide":true,"repeat":true,"procs":6,"sandbox":"none","fault_call":-1,"tun":true,"tmpdir":true,"cgroups":true,"netdev":true,"resetnet":true,"segv":true} > > r0 = openat$kvm(0xffffffffffffff9c, &(0x7f0000000380)='/dev/kvm\x00', 0x0, 0x0) > > r1 = syz_open_dev$dspn(&(0x7f0000000100)='/dev/dsp#\x00', 0x3fe, 0x400) > > r2 = ioctl$KVM_CREATE_VM(r0, 0xae01, 0x0) > > perf_event_open(&(0x7f0000000040)={0x1, 0x70, 0x0, 0x0, 0x0, 0x0, 0x0, 0x50d}, 0x0, 0xffffffffffffffff, 0xffffffffffffffff, 0x0) > > mincore(&(0x7f0000ffc000/0x1000)=nil, 0x1000, &(0x7f00000003c0)=""/4096) > > setrlimit(0x0, &(0x7f0000000000)) > > readahead(r1, 0x3, 0x9a6) > > ioctl$KVM_CREATE_DEVICE(r2, 0xc00caee0, &(0x7f00000002c0)={0x4}) > > setsockopt$inet_sctp6_SCTP_FRAGMENT_INTERLEAVE(r1, 0x84, 0x12, &(0x7f00000001c0)=0x9, 0x4) ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: WARNING: refcount bug in kvm_vm_ioctl 2019-02-15 15:39 ` Dmitry Vyukov @ 2019-02-15 16:02 ` Jann Horn 2019-02-15 16:45 ` Dmitry Vyukov 0 siblings, 1 reply; 11+ messages in thread From: Jann Horn @ 2019-02-15 16:02 UTC (permalink / raw) To: Dmitry Vyukov Cc: Paolo Bonzini, syzbot, KVM list, LKML, Radim Krčmář, syzkaller-bugs, Christoffer Dall, Janosch Frank, Christian Borntraeger On Fri, Feb 15, 2019 at 4:40 PM Dmitry Vyukov <dvyukov@google.com> wrote: > On Thu, Oct 11, 2018 at 4:18 PM Paolo Bonzini <pbonzini@redhat.com> wrote: > > On 10/10/2018 09:58, syzbot wrote: > > > do_invalid_op+0x1b/0x20 arch/x86/kernel/traps.c:316 > > > invalid_op+0x14/0x20 arch/x86/entry/entry_64.S:993 > > > RIP: 0010:refcount_inc_checked+0x5d/0x70 lib/refcount.c:153 > > > kvm_get_kvm arch/x86/kvm/../../../virt/kvm/kvm_main.c:766 [inline] > > > kvm_ioctl_create_device arch/x86/kvm/../../../virt/kvm/kvm_main.c:2924 > > > kvm_vm_ioctl+0xed7/0x1d40 arch/x86/kvm/../../../virt/kvm/kvm_main.c:3114 > > > vfs_ioctl fs/ioctl.c:46 [inline] > > > file_ioctl fs/ioctl.c:501 [inline] > > > do_vfs_ioctl+0x1de/0x1720 fs/ioctl.c:685 > > > ksys_ioctl+0xa9/0xd0 fs/ioctl.c:702 > > > __do_sys_ioctl fs/ioctl.c:709 [inline] > > > __se_sys_ioctl fs/ioctl.c:707 [inline] > > > __x64_sys_ioctl+0x73/0xb0 fs/ioctl.c:707 > > > do_syscall_64+0x1b9/0x820 arch/x86/entry/common.c:290 > > > entry_SYSCALL_64_after_hwframe+0x49/0xbe > > > > The trace here is fairly simple, but I don't understand how this could > > happen. > > > > The kvm_get_kvm is done within kvm_ioctl_create_device, which is called > > from ioctl; the last reference cannot disappear inside a ioctl, because: > > > > 1) kvm_ioctl is called from vfs_ioctl, which does fdget and holds the fd > > reference until after kvm_vm_ioctl returns > > > > 2) the file descriptor holds one reference to the struct kvm*, and this > > reference is not released until kvm_vm_release is called by the last > > fput (which could be fdput's call to fput if the process has exited in > > the meanwhile) > > > > 3) for completeness, in case anon_inode_getfd fails, put_unused_fd will > > not invoke the file descriptor's ->release callback (in this case > > kvm_device_release). > > > > CCing some random people to get their opinion... > > > > Paolo > > > Jann, is it what you fixed in "kvm: fix kvm_ioctl_create_device() > reference counting (CVE-2019-6974)"? > If so, we need to close the syzbot bug. > > > > > # See https://goo.gl/kgGztJ for information about syzkaller reproducers. > > > #{"threaded":true,"collide":true,"repeat":true,"procs":6,"sandbox":"none","fault_call":-1,"tun":true,"tmpdir":true,"cgroups":true,"netdev":true,"resetnet":true,"segv":true} > > > r0 = openat$kvm(0xffffffffffffff9c, &(0x7f0000000380)='/dev/kvm\x00', 0x0, 0x0) > > > r1 = syz_open_dev$dspn(&(0x7f0000000100)='/dev/dsp#\x00', 0x3fe, 0x400) > > > r2 = ioctl$KVM_CREATE_VM(r0, 0xae01, 0x0) Here we create a VM fd... > > > perf_event_open(&(0x7f0000000040)={0x1, 0x70, 0x0, 0x0, 0x0, 0x0, 0x0, 0x50d}, 0x0, 0xffffffffffffffff, 0xffffffffffffffff, 0x0) > > > mincore(&(0x7f0000ffc000/0x1000)=nil, 0x1000, &(0x7f00000003c0)=""/4096) > > > setrlimit(0x0, &(0x7f0000000000)) > > > readahead(r1, 0x3, 0x9a6) > > > ioctl$KVM_CREATE_DEVICE(r2, 0xc00caee0, &(0x7f00000002c0)={0x4}) ... and here we do the KVM_CREATE_DEVICE ioctl with type==KVM_DEV_TYPE_VFIO. So that far it looks exactly like CVE-2019-6974. But CVE-2019-6974 also requires that someone calls close() on the file descriptor of the newly created device very quickly, before the ioctl is able to increment the refcount further, and I don't see anything like that here. Is there a chance that syzkaller called close() on a file descriptor while the ioctl() was still running without saying so here (potentially through dup2() or something like that)? It would be helpful if we could see the backtrace of how the refcount was dropped to zero... > > > setsockopt$inet_sctp6_SCTP_FRAGMENT_INTERLEAVE(r1, 0x84, 0x12, &(0x7f00000001c0)=0x9, 0x4) ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: WARNING: refcount bug in kvm_vm_ioctl 2019-02-15 16:02 ` Jann Horn @ 2019-02-15 16:45 ` Dmitry Vyukov 2019-02-15 17:10 ` Jann Horn 0 siblings, 1 reply; 11+ messages in thread From: Dmitry Vyukov @ 2019-02-15 16:45 UTC (permalink / raw) To: Jann Horn Cc: Paolo Bonzini, syzbot, KVM list, LKML, Radim Krčmář, syzkaller-bugs, Christoffer Dall, Janosch Frank, Christian Borntraeger On Fri, Feb 15, 2019 at 5:03 PM Jann Horn <jannh@google.com> wrote: > > On Fri, Feb 15, 2019 at 4:40 PM Dmitry Vyukov <dvyukov@google.com> wrote: > > On Thu, Oct 11, 2018 at 4:18 PM Paolo Bonzini <pbonzini@redhat.com> wrote: > > > On 10/10/2018 09:58, syzbot wrote: > > > > do_invalid_op+0x1b/0x20 arch/x86/kernel/traps.c:316 > > > > invalid_op+0x14/0x20 arch/x86/entry/entry_64.S:993 > > > > RIP: 0010:refcount_inc_checked+0x5d/0x70 lib/refcount.c:153 > > > > kvm_get_kvm arch/x86/kvm/../../../virt/kvm/kvm_main.c:766 [inline] > > > > kvm_ioctl_create_device arch/x86/kvm/../../../virt/kvm/kvm_main.c:2924 > > > > kvm_vm_ioctl+0xed7/0x1d40 arch/x86/kvm/../../../virt/kvm/kvm_main.c:3114 > > > > vfs_ioctl fs/ioctl.c:46 [inline] > > > > file_ioctl fs/ioctl.c:501 [inline] > > > > do_vfs_ioctl+0x1de/0x1720 fs/ioctl.c:685 > > > > ksys_ioctl+0xa9/0xd0 fs/ioctl.c:702 > > > > __do_sys_ioctl fs/ioctl.c:709 [inline] > > > > __se_sys_ioctl fs/ioctl.c:707 [inline] > > > > __x64_sys_ioctl+0x73/0xb0 fs/ioctl.c:707 > > > > do_syscall_64+0x1b9/0x820 arch/x86/entry/common.c:290 > > > > entry_SYSCALL_64_after_hwframe+0x49/0xbe > > > > > > The trace here is fairly simple, but I don't understand how this could > > > happen. > > > > > > The kvm_get_kvm is done within kvm_ioctl_create_device, which is called > > > from ioctl; the last reference cannot disappear inside a ioctl, because: > > > > > > 1) kvm_ioctl is called from vfs_ioctl, which does fdget and holds the fd > > > reference until after kvm_vm_ioctl returns > > > > > > 2) the file descriptor holds one reference to the struct kvm*, and this > > > reference is not released until kvm_vm_release is called by the last > > > fput (which could be fdput's call to fput if the process has exited in > > > the meanwhile) > > > > > > 3) for completeness, in case anon_inode_getfd fails, put_unused_fd will > > > not invoke the file descriptor's ->release callback (in this case > > > kvm_device_release). > > > > > > CCing some random people to get their opinion... > > > > > > Paolo > > > > > > Jann, is it what you fixed in "kvm: fix kvm_ioctl_create_device() > > reference counting (CVE-2019-6974)"? > > If so, we need to close the syzbot bug. > > > > > > > > # See https://goo.gl/kgGztJ for information about syzkaller reproducers. > > > > #{"threaded":true,"collide":true,"repeat":true,"procs":6,"sandbox":"none","fault_call":-1,"tun":true,"tmpdir":true,"cgroups":true,"netdev":true,"resetnet":true,"segv":true} > > > > r0 = openat$kvm(0xffffffffffffff9c, &(0x7f0000000380)='/dev/kvm\x00', 0x0, 0x0) > > > > r1 = syz_open_dev$dspn(&(0x7f0000000100)='/dev/dsp#\x00', 0x3fe, 0x400) > > > > r2 = ioctl$KVM_CREATE_VM(r0, 0xae01, 0x0) > > Here we create a VM fd... > > > > > perf_event_open(&(0x7f0000000040)={0x1, 0x70, 0x0, 0x0, 0x0, 0x0, 0x0, 0x50d}, 0x0, 0xffffffffffffffff, 0xffffffffffffffff, 0x0) > > > > mincore(&(0x7f0000ffc000/0x1000)=nil, 0x1000, &(0x7f00000003c0)=""/4096) > > > > setrlimit(0x0, &(0x7f0000000000)) > > > > readahead(r1, 0x3, 0x9a6) > > > > ioctl$KVM_CREATE_DEVICE(r2, 0xc00caee0, &(0x7f00000002c0)={0x4}) > > ... and here we do the KVM_CREATE_DEVICE ioctl with type==KVM_DEV_TYPE_VFIO. > > So that far it looks exactly like CVE-2019-6974. But CVE-2019-6974 > also requires that someone calls close() on the file descriptor of the > newly created device very quickly, before the ioctl is able to > increment the refcount further, and I don't see anything like that > here. Is there a chance that syzkaller called close() on a file > descriptor while the ioctl() was still running without saying so here > (potentially through dup2() or something like that)? Yes, all fd's are closed at the end of the test: https://github.com/google/syzkaller/blob/master/executor/common_linux.h#L2561-L2568 > > It would be helpful if we could see the backtrace of how the refcount > was dropped to zero... > > > > > setsockopt$inet_sctp6_SCTP_FRAGMENT_INTERLEAVE(r1, 0x84, 0x12, &(0x7f00000001c0)=0x9, 0x4) ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: WARNING: refcount bug in kvm_vm_ioctl 2019-02-15 16:45 ` Dmitry Vyukov @ 2019-02-15 17:10 ` Jann Horn 2019-02-15 17:12 ` Dmitry Vyukov 0 siblings, 1 reply; 11+ messages in thread From: Jann Horn @ 2019-02-15 17:10 UTC (permalink / raw) To: Dmitry Vyukov Cc: Paolo Bonzini, syzbot, KVM list, LKML, Radim Krčmář, syzkaller-bugs, Christoffer Dall, Janosch Frank, Christian Borntraeger On Fri, Feb 15, 2019 at 5:45 PM Dmitry Vyukov <dvyukov@google.com> wrote: > > On Fri, Feb 15, 2019 at 5:03 PM Jann Horn <jannh@google.com> wrote: > > > > On Fri, Feb 15, 2019 at 4:40 PM Dmitry Vyukov <dvyukov@google.com> wrote: > > > On Thu, Oct 11, 2018 at 4:18 PM Paolo Bonzini <pbonzini@redhat.com> wrote: > > > > On 10/10/2018 09:58, syzbot wrote: > > > > > do_invalid_op+0x1b/0x20 arch/x86/kernel/traps.c:316 > > > > > invalid_op+0x14/0x20 arch/x86/entry/entry_64.S:993 > > > > > RIP: 0010:refcount_inc_checked+0x5d/0x70 lib/refcount.c:153 > > > > > kvm_get_kvm arch/x86/kvm/../../../virt/kvm/kvm_main.c:766 [inline] > > > > > kvm_ioctl_create_device arch/x86/kvm/../../../virt/kvm/kvm_main.c:2924 > > > > > kvm_vm_ioctl+0xed7/0x1d40 arch/x86/kvm/../../../virt/kvm/kvm_main.c:3114 > > > > > vfs_ioctl fs/ioctl.c:46 [inline] > > > > > file_ioctl fs/ioctl.c:501 [inline] > > > > > do_vfs_ioctl+0x1de/0x1720 fs/ioctl.c:685 > > > > > ksys_ioctl+0xa9/0xd0 fs/ioctl.c:702 > > > > > __do_sys_ioctl fs/ioctl.c:709 [inline] > > > > > __se_sys_ioctl fs/ioctl.c:707 [inline] > > > > > __x64_sys_ioctl+0x73/0xb0 fs/ioctl.c:707 > > > > > do_syscall_64+0x1b9/0x820 arch/x86/entry/common.c:290 > > > > > entry_SYSCALL_64_after_hwframe+0x49/0xbe > > > > > > > > The trace here is fairly simple, but I don't understand how this could > > > > happen. > > > > > > > > The kvm_get_kvm is done within kvm_ioctl_create_device, which is called > > > > from ioctl; the last reference cannot disappear inside a ioctl, because: > > > > > > > > 1) kvm_ioctl is called from vfs_ioctl, which does fdget and holds the fd > > > > reference until after kvm_vm_ioctl returns > > > > > > > > 2) the file descriptor holds one reference to the struct kvm*, and this > > > > reference is not released until kvm_vm_release is called by the last > > > > fput (which could be fdput's call to fput if the process has exited in > > > > the meanwhile) > > > > > > > > 3) for completeness, in case anon_inode_getfd fails, put_unused_fd will > > > > not invoke the file descriptor's ->release callback (in this case > > > > kvm_device_release). > > > > > > > > CCing some random people to get their opinion... > > > > > > > > Paolo > > > > > > > > > Jann, is it what you fixed in "kvm: fix kvm_ioctl_create_device() > > > reference counting (CVE-2019-6974)"? > > > If so, we need to close the syzbot bug. > > > > > > > > > > > # See https://goo.gl/kgGztJ for information about syzkaller reproducers. > > > > > #{"threaded":true,"collide":true,"repeat":true,"procs":6,"sandbox":"none","fault_call":-1,"tun":true,"tmpdir":true,"cgroups":true,"netdev":true,"resetnet":true,"segv":true} > > > > > r0 = openat$kvm(0xffffffffffffff9c, &(0x7f0000000380)='/dev/kvm\x00', 0x0, 0x0) > > > > > r1 = syz_open_dev$dspn(&(0x7f0000000100)='/dev/dsp#\x00', 0x3fe, 0x400) > > > > > r2 = ioctl$KVM_CREATE_VM(r0, 0xae01, 0x0) > > > > Here we create a VM fd... > > > > > > > perf_event_open(&(0x7f0000000040)={0x1, 0x70, 0x0, 0x0, 0x0, 0x0, 0x0, 0x50d}, 0x0, 0xffffffffffffffff, 0xffffffffffffffff, 0x0) > > > > > mincore(&(0x7f0000ffc000/0x1000)=nil, 0x1000, &(0x7f00000003c0)=""/4096) > > > > > setrlimit(0x0, &(0x7f0000000000)) > > > > > readahead(r1, 0x3, 0x9a6) > > > > > ioctl$KVM_CREATE_DEVICE(r2, 0xc00caee0, &(0x7f00000002c0)={0x4}) > > > > ... and here we do the KVM_CREATE_DEVICE ioctl with type==KVM_DEV_TYPE_VFIO. > > > > So that far it looks exactly like CVE-2019-6974. But CVE-2019-6974 > > also requires that someone calls close() on the file descriptor of the > > newly created device very quickly, before the ioctl is able to > > increment the refcount further, and I don't see anything like that > > here. Is there a chance that syzkaller called close() on a file > > descriptor while the ioctl() was still running without saying so here > > (potentially through dup2() or something like that)? > > Yes, all fd's are closed at the end of the test: > https://github.com/google/syzkaller/blob/master/executor/common_linux.h#L2561-L2568 Can that happen before the ioctl() has finished? ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: WARNING: refcount bug in kvm_vm_ioctl 2019-02-15 17:10 ` Jann Horn @ 2019-02-15 17:12 ` Dmitry Vyukov 2019-02-15 17:22 ` Jann Horn 0 siblings, 1 reply; 11+ messages in thread From: Dmitry Vyukov @ 2019-02-15 17:12 UTC (permalink / raw) To: Jann Horn Cc: Paolo Bonzini, syzbot, KVM list, LKML, Radim Krčmář, syzkaller-bugs, Christoffer Dall, Janosch Frank, Christian Borntraeger On Fri, Feb 15, 2019 at 6:10 PM Jann Horn <jannh@google.com> wrote: > > On Fri, Feb 15, 2019 at 5:45 PM Dmitry Vyukov <dvyukov@google.com> wrote: > > > > On Fri, Feb 15, 2019 at 5:03 PM Jann Horn <jannh@google.com> wrote: > > > > > > On Fri, Feb 15, 2019 at 4:40 PM Dmitry Vyukov <dvyukov@google.com> wrote: > > > > On Thu, Oct 11, 2018 at 4:18 PM Paolo Bonzini <pbonzini@redhat.com> wrote: > > > > > On 10/10/2018 09:58, syzbot wrote: > > > > > > do_invalid_op+0x1b/0x20 arch/x86/kernel/traps.c:316 > > > > > > invalid_op+0x14/0x20 arch/x86/entry/entry_64.S:993 > > > > > > RIP: 0010:refcount_inc_checked+0x5d/0x70 lib/refcount.c:153 > > > > > > kvm_get_kvm arch/x86/kvm/../../../virt/kvm/kvm_main.c:766 [inline] > > > > > > kvm_ioctl_create_device arch/x86/kvm/../../../virt/kvm/kvm_main.c:2924 > > > > > > kvm_vm_ioctl+0xed7/0x1d40 arch/x86/kvm/../../../virt/kvm/kvm_main.c:3114 > > > > > > vfs_ioctl fs/ioctl.c:46 [inline] > > > > > > file_ioctl fs/ioctl.c:501 [inline] > > > > > > do_vfs_ioctl+0x1de/0x1720 fs/ioctl.c:685 > > > > > > ksys_ioctl+0xa9/0xd0 fs/ioctl.c:702 > > > > > > __do_sys_ioctl fs/ioctl.c:709 [inline] > > > > > > __se_sys_ioctl fs/ioctl.c:707 [inline] > > > > > > __x64_sys_ioctl+0x73/0xb0 fs/ioctl.c:707 > > > > > > do_syscall_64+0x1b9/0x820 arch/x86/entry/common.c:290 > > > > > > entry_SYSCALL_64_after_hwframe+0x49/0xbe > > > > > > > > > > The trace here is fairly simple, but I don't understand how this could > > > > > happen. > > > > > > > > > > The kvm_get_kvm is done within kvm_ioctl_create_device, which is called > > > > > from ioctl; the last reference cannot disappear inside a ioctl, because: > > > > > > > > > > 1) kvm_ioctl is called from vfs_ioctl, which does fdget and holds the fd > > > > > reference until after kvm_vm_ioctl returns > > > > > > > > > > 2) the file descriptor holds one reference to the struct kvm*, and this > > > > > reference is not released until kvm_vm_release is called by the last > > > > > fput (which could be fdput's call to fput if the process has exited in > > > > > the meanwhile) > > > > > > > > > > 3) for completeness, in case anon_inode_getfd fails, put_unused_fd will > > > > > not invoke the file descriptor's ->release callback (in this case > > > > > kvm_device_release). > > > > > > > > > > CCing some random people to get their opinion... > > > > > > > > > > Paolo > > > > > > > > > > > > Jann, is it what you fixed in "kvm: fix kvm_ioctl_create_device() > > > > reference counting (CVE-2019-6974)"? > > > > If so, we need to close the syzbot bug. > > > > > > > > > > > > > > # See https://goo.gl/kgGztJ for information about syzkaller reproducers. > > > > > > #{"threaded":true,"collide":true,"repeat":true,"procs":6,"sandbox":"none","fault_call":-1,"tun":true,"tmpdir":true,"cgroups":true,"netdev":true,"resetnet":true,"segv":true} > > > > > > r0 = openat$kvm(0xffffffffffffff9c, &(0x7f0000000380)='/dev/kvm\x00', 0x0, 0x0) > > > > > > r1 = syz_open_dev$dspn(&(0x7f0000000100)='/dev/dsp#\x00', 0x3fe, 0x400) > > > > > > r2 = ioctl$KVM_CREATE_VM(r0, 0xae01, 0x0) > > > > > > Here we create a VM fd... > > > > > > > > > perf_event_open(&(0x7f0000000040)={0x1, 0x70, 0x0, 0x0, 0x0, 0x0, 0x0, 0x50d}, 0x0, 0xffffffffffffffff, 0xffffffffffffffff, 0x0) > > > > > > mincore(&(0x7f0000ffc000/0x1000)=nil, 0x1000, &(0x7f00000003c0)=""/4096) > > > > > > setrlimit(0x0, &(0x7f0000000000)) > > > > > > readahead(r1, 0x3, 0x9a6) > > > > > > ioctl$KVM_CREATE_DEVICE(r2, 0xc00caee0, &(0x7f00000002c0)={0x4}) > > > > > > ... and here we do the KVM_CREATE_DEVICE ioctl with type==KVM_DEV_TYPE_VFIO. > > > > > > So that far it looks exactly like CVE-2019-6974. But CVE-2019-6974 > > > also requires that someone calls close() on the file descriptor of the > > > newly created device very quickly, before the ioctl is able to > > > increment the refcount further, and I don't see anything like that > > > here. Is there a chance that syzkaller called close() on a file > > > descriptor while the ioctl() was still running without saying so here > > > (potentially through dup2() or something like that)? > > > > Yes, all fd's are closed at the end of the test: > > https://github.com/google/syzkaller/blob/master/executor/common_linux.h#L2561-L2568 > > Can that happen before the ioctl() has finished? Yes, ioctl runs in a separate thread. ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: WARNING: refcount bug in kvm_vm_ioctl 2019-02-15 17:12 ` Dmitry Vyukov @ 2019-02-15 17:22 ` Jann Horn 2019-02-16 6:28 ` Dmitry Vyukov 0 siblings, 1 reply; 11+ messages in thread From: Jann Horn @ 2019-02-15 17:22 UTC (permalink / raw) To: Dmitry Vyukov Cc: Paolo Bonzini, syzbot, KVM list, LKML, Radim Krčmář, syzkaller-bugs, Christoffer Dall, Janosch Frank, Christian Borntraeger On Fri, Feb 15, 2019 at 6:13 PM Dmitry Vyukov <dvyukov@google.com> wrote: > On Fri, Feb 15, 2019 at 6:10 PM Jann Horn <jannh@google.com> wrote: > > On Fri, Feb 15, 2019 at 5:45 PM Dmitry Vyukov <dvyukov@google.com> wrote: > > > On Fri, Feb 15, 2019 at 5:03 PM Jann Horn <jannh@google.com> wrote: > > > > On Fri, Feb 15, 2019 at 4:40 PM Dmitry Vyukov <dvyukov@google.com> wrote: > > > > > On Thu, Oct 11, 2018 at 4:18 PM Paolo Bonzini <pbonzini@redhat.com> wrote: > > > > > > On 10/10/2018 09:58, syzbot wrote: > > > > > > > do_invalid_op+0x1b/0x20 arch/x86/kernel/traps.c:316 > > > > > > > invalid_op+0x14/0x20 arch/x86/entry/entry_64.S:993 > > > > > > > RIP: 0010:refcount_inc_checked+0x5d/0x70 lib/refcount.c:153 > > > > > > > kvm_get_kvm arch/x86/kvm/../../../virt/kvm/kvm_main.c:766 [inline] > > > > > > > kvm_ioctl_create_device arch/x86/kvm/../../../virt/kvm/kvm_main.c:2924 > > > > > > > kvm_vm_ioctl+0xed7/0x1d40 arch/x86/kvm/../../../virt/kvm/kvm_main.c:3114 > > > > > > > vfs_ioctl fs/ioctl.c:46 [inline] > > > > > > > file_ioctl fs/ioctl.c:501 [inline] > > > > > > > do_vfs_ioctl+0x1de/0x1720 fs/ioctl.c:685 > > > > > > > ksys_ioctl+0xa9/0xd0 fs/ioctl.c:702 > > > > > > > __do_sys_ioctl fs/ioctl.c:709 [inline] > > > > > > > __se_sys_ioctl fs/ioctl.c:707 [inline] > > > > > > > __x64_sys_ioctl+0x73/0xb0 fs/ioctl.c:707 > > > > > > > do_syscall_64+0x1b9/0x820 arch/x86/entry/common.c:290 > > > > > > > entry_SYSCALL_64_after_hwframe+0x49/0xbe > > > > > > > > > > > > The trace here is fairly simple, but I don't understand how this could > > > > > > happen. > > > > > > > > > > > > The kvm_get_kvm is done within kvm_ioctl_create_device, which is called > > > > > > from ioctl; the last reference cannot disappear inside a ioctl, because: > > > > > > > > > > > > 1) kvm_ioctl is called from vfs_ioctl, which does fdget and holds the fd > > > > > > reference until after kvm_vm_ioctl returns > > > > > > > > > > > > 2) the file descriptor holds one reference to the struct kvm*, and this > > > > > > reference is not released until kvm_vm_release is called by the last > > > > > > fput (which could be fdput's call to fput if the process has exited in > > > > > > the meanwhile) > > > > > > > > > > > > 3) for completeness, in case anon_inode_getfd fails, put_unused_fd will > > > > > > not invoke the file descriptor's ->release callback (in this case > > > > > > kvm_device_release). > > > > > > > > > > > > CCing some random people to get their opinion... > > > > > > > > > > > > Paolo > > > > > > > > > > > > > > > Jann, is it what you fixed in "kvm: fix kvm_ioctl_create_device() > > > > > reference counting (CVE-2019-6974)"? > > > > > If so, we need to close the syzbot bug. > > > > > > > > > > > > > > > > > # See https://goo.gl/kgGztJ for information about syzkaller reproducers. > > > > > > > #{"threaded":true,"collide":true,"repeat":true,"procs":6,"sandbox":"none","fault_call":-1,"tun":true,"tmpdir":true,"cgroups":true,"netdev":true,"resetnet":true,"segv":true} > > > > > > > r0 = openat$kvm(0xffffffffffffff9c, &(0x7f0000000380)='/dev/kvm\x00', 0x0, 0x0) > > > > > > > r1 = syz_open_dev$dspn(&(0x7f0000000100)='/dev/dsp#\x00', 0x3fe, 0x400) > > > > > > > r2 = ioctl$KVM_CREATE_VM(r0, 0xae01, 0x0) > > > > > > > > Here we create a VM fd... > > > > > > > > > > > perf_event_open(&(0x7f0000000040)={0x1, 0x70, 0x0, 0x0, 0x0, 0x0, 0x0, 0x50d}, 0x0, 0xffffffffffffffff, 0xffffffffffffffff, 0x0) > > > > > > > mincore(&(0x7f0000ffc000/0x1000)=nil, 0x1000, &(0x7f00000003c0)=""/4096) > > > > > > > setrlimit(0x0, &(0x7f0000000000)) > > > > > > > readahead(r1, 0x3, 0x9a6) > > > > > > > ioctl$KVM_CREATE_DEVICE(r2, 0xc00caee0, &(0x7f00000002c0)={0x4}) > > > > > > > > ... and here we do the KVM_CREATE_DEVICE ioctl with type==KVM_DEV_TYPE_VFIO. > > > > > > > > So that far it looks exactly like CVE-2019-6974. But CVE-2019-6974 > > > > also requires that someone calls close() on the file descriptor of the > > > > newly created device very quickly, before the ioctl is able to > > > > increment the refcount further, and I don't see anything like that > > > > here. Is there a chance that syzkaller called close() on a file > > > > descriptor while the ioctl() was still running without saying so here > > > > (potentially through dup2() or something like that)? > > > > > > Yes, all fd's are closed at the end of the test: > > > https://github.com/google/syzkaller/blob/master/executor/common_linux.h#L2561-L2568 > > > > Can that happen before the ioctl() has finished? > > Yes, ioctl runs in a separate thread. Alright, then yes, it looks like the same bug. Since the root cause wasn't identified from the original syzkaller report, I wonder whether it would make sense to add infrastructure that makes it easier to identify the root cause from a syzkaller report; here are some random ideas: - A comment at the end of the syzkaller reproducer that lists interesting syscalls that are performed implicitly, in particular close(3..30). Without that information, the race isn't really visible here. - A config option that allows recording (subsets of) stacks, PIDs, and cpu numbers in every function that modifies a refcount, and can dump the last N such records when a refcount detects an error. This would probably be helpful for figuring out refcounting bugs, but I don't actually know how many of the bugs syzkaller finds are refcounting-related - if it's not a lot, this might not be worth the effort. ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: WARNING: refcount bug in kvm_vm_ioctl 2019-02-15 17:22 ` Jann Horn @ 2019-02-16 6:28 ` Dmitry Vyukov 0 siblings, 0 replies; 11+ messages in thread From: Dmitry Vyukov @ 2019-02-16 6:28 UTC (permalink / raw) To: Jann Horn Cc: Paolo Bonzini, syzbot, KVM list, LKML, Radim Krčmář, syzkaller-bugs, Christoffer Dall, Janosch Frank, Christian Borntraeger On Fri, Feb 15, 2019 at 6:22 PM Jann Horn <jannh@google.com> wrote: > > On Fri, Feb 15, 2019 at 6:13 PM Dmitry Vyukov <dvyukov@google.com> wrote: > > On Fri, Feb 15, 2019 at 6:10 PM Jann Horn <jannh@google.com> wrote: > > > On Fri, Feb 15, 2019 at 5:45 PM Dmitry Vyukov <dvyukov@google.com> wrote: > > > > On Fri, Feb 15, 2019 at 5:03 PM Jann Horn <jannh@google.com> wrote: > > > > > On Fri, Feb 15, 2019 at 4:40 PM Dmitry Vyukov <dvyukov@google.com> wrote: > > > > > > On Thu, Oct 11, 2018 at 4:18 PM Paolo Bonzini <pbonzini@redhat.com> wrote: > > > > > > > On 10/10/2018 09:58, syzbot wrote: > > > > > > > > do_invalid_op+0x1b/0x20 arch/x86/kernel/traps.c:316 > > > > > > > > invalid_op+0x14/0x20 arch/x86/entry/entry_64.S:993 > > > > > > > > RIP: 0010:refcount_inc_checked+0x5d/0x70 lib/refcount.c:153 > > > > > > > > kvm_get_kvm arch/x86/kvm/../../../virt/kvm/kvm_main.c:766 [inline] > > > > > > > > kvm_ioctl_create_device arch/x86/kvm/../../../virt/kvm/kvm_main.c:2924 > > > > > > > > kvm_vm_ioctl+0xed7/0x1d40 arch/x86/kvm/../../../virt/kvm/kvm_main.c:3114 > > > > > > > > vfs_ioctl fs/ioctl.c:46 [inline] > > > > > > > > file_ioctl fs/ioctl.c:501 [inline] > > > > > > > > do_vfs_ioctl+0x1de/0x1720 fs/ioctl.c:685 > > > > > > > > ksys_ioctl+0xa9/0xd0 fs/ioctl.c:702 > > > > > > > > __do_sys_ioctl fs/ioctl.c:709 [inline] > > > > > > > > __se_sys_ioctl fs/ioctl.c:707 [inline] > > > > > > > > __x64_sys_ioctl+0x73/0xb0 fs/ioctl.c:707 > > > > > > > > do_syscall_64+0x1b9/0x820 arch/x86/entry/common.c:290 > > > > > > > > entry_SYSCALL_64_after_hwframe+0x49/0xbe > > > > > > > > > > > > > > The trace here is fairly simple, but I don't understand how this could > > > > > > > happen. > > > > > > > > > > > > > > The kvm_get_kvm is done within kvm_ioctl_create_device, which is called > > > > > > > from ioctl; the last reference cannot disappear inside a ioctl, because: > > > > > > > > > > > > > > 1) kvm_ioctl is called from vfs_ioctl, which does fdget and holds the fd > > > > > > > reference until after kvm_vm_ioctl returns > > > > > > > > > > > > > > 2) the file descriptor holds one reference to the struct kvm*, and this > > > > > > > reference is not released until kvm_vm_release is called by the last > > > > > > > fput (which could be fdput's call to fput if the process has exited in > > > > > > > the meanwhile) > > > > > > > > > > > > > > 3) for completeness, in case anon_inode_getfd fails, put_unused_fd will > > > > > > > not invoke the file descriptor's ->release callback (in this case > > > > > > > kvm_device_release). > > > > > > > > > > > > > > CCing some random people to get their opinion... > > > > > > > > > > > > > > Paolo > > > > > > > > > > > > > > > > > > Jann, is it what you fixed in "kvm: fix kvm_ioctl_create_device() > > > > > > reference counting (CVE-2019-6974)"? > > > > > > If so, we need to close the syzbot bug. > > > > > > > > > > > > > > > > > > > > # See https://goo.gl/kgGztJ for information about syzkaller reproducers. > > > > > > > > #{"threaded":true,"collide":true,"repeat":true,"procs":6,"sandbox":"none","fault_call":-1,"tun":true,"tmpdir":true,"cgroups":true,"netdev":true,"resetnet":true,"segv":true} > > > > > > > > r0 = openat$kvm(0xffffffffffffff9c, &(0x7f0000000380)='/dev/kvm\x00', 0x0, 0x0) > > > > > > > > r1 = syz_open_dev$dspn(&(0x7f0000000100)='/dev/dsp#\x00', 0x3fe, 0x400) > > > > > > > > r2 = ioctl$KVM_CREATE_VM(r0, 0xae01, 0x0) > > > > > > > > > > Here we create a VM fd... > > > > > > > > > > > > > perf_event_open(&(0x7f0000000040)={0x1, 0x70, 0x0, 0x0, 0x0, 0x0, 0x0, 0x50d}, 0x0, 0xffffffffffffffff, 0xffffffffffffffff, 0x0) > > > > > > > > mincore(&(0x7f0000ffc000/0x1000)=nil, 0x1000, &(0x7f00000003c0)=""/4096) > > > > > > > > setrlimit(0x0, &(0x7f0000000000)) > > > > > > > > readahead(r1, 0x3, 0x9a6) > > > > > > > > ioctl$KVM_CREATE_DEVICE(r2, 0xc00caee0, &(0x7f00000002c0)={0x4}) > > > > > > > > > > ... and here we do the KVM_CREATE_DEVICE ioctl with type==KVM_DEV_TYPE_VFIO. > > > > > > > > > > So that far it looks exactly like CVE-2019-6974. But CVE-2019-6974 > > > > > also requires that someone calls close() on the file descriptor of the > > > > > newly created device very quickly, before the ioctl is able to > > > > > increment the refcount further, and I don't see anything like that > > > > > here. Is there a chance that syzkaller called close() on a file > > > > > descriptor while the ioctl() was still running without saying so here > > > > > (potentially through dup2() or something like that)? > > > > > > > > Yes, all fd's are closed at the end of the test: > > > > https://github.com/google/syzkaller/blob/master/executor/common_linux.h#L2561-L2568 > > > > > > Can that happen before the ioctl() has finished? > > > > Yes, ioctl runs in a separate thread. > > Alright, then yes, it looks like the same bug. Let's mark is as fixed then: #syz fix: kvm: fix kvm_ioctl_create_device() reference counting (CVE-2019-6974) > Since the root cause wasn't identified from the original syzkaller > report, I wonder whether it would make sense to add infrastructure > that makes it easier to identify the root cause from a syzkaller > report; here are some random ideas: > > - A comment at the end of the syzkaller reproducer that lists > interesting syscalls that are performed implicitly, in particular > close(3..30). Without that information, the race isn't really visible > here. It would definitely be useful to produce C reproducers in more cases. That's lingua franca of bug reporting. All info is in the repro, but I am not sure it's possible to compress it to something much more compact without losing information. That's several KLOC of complex C code, not just "close(3..30)", also all of syscall arguments and data and control flow, etc, which is naturally represented by the C code which is already available. Also this case is quite special, there is generally no other single report to match with. > - A config option that allows recording (subsets of) stacks, PIDs, > and cpu numbers in every function that modifies a refcount, and can > dump the last N such records when a refcount detects an error. This > would probably be helpful for figuring out refcounting bugs, but I > don't actually know how many of the bugs syzkaller finds are > refcounting-related - if it's not a lot, this might not be worth the > effort. ^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2019-02-16 6:28 UTC | newest] Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2018-10-10 7:58 WARNING: refcount bug in kvm_vm_ioctl syzbot 2018-10-11 14:17 ` Paolo Bonzini 2018-10-11 18:23 ` Dmitry Vyukov 2018-10-11 18:39 ` Dmitry Vyukov 2019-02-15 15:39 ` Dmitry Vyukov 2019-02-15 16:02 ` Jann Horn 2019-02-15 16:45 ` Dmitry Vyukov 2019-02-15 17:10 ` Jann Horn 2019-02-15 17:12 ` Dmitry Vyukov 2019-02-15 17:22 ` Jann Horn 2019-02-16 6:28 ` Dmitry Vyukov
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).