WARNING: refcount bug in kvm_vm_ioctl

WARNING: refcount bug in kvm_vm_ioctl

[syzbot]

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

Re: WARNING: refcount bug in kvm_vm_ioctl

[Paolo Bonzini]

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)

Re: WARNING: refcount bug in kvm_vm_ioctl

[Dmitry Vyukov]

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.

Re: WARNING: refcount bug in kvm_vm_ioctl

[Dmitry Vyukov]

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)?

Re: WARNING: refcount bug in kvm_vm_ioctl

[Dmitry Vyukov]

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)

Re: WARNING: refcount bug in kvm_vm_ioctl

[Jann Horn]

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)

Re: WARNING: refcount bug in kvm_vm_ioctl

[Dmitry Vyukov]

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)

Re: WARNING: refcount bug in kvm_vm_ioctl

[Jann Horn]

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?

Re: WARNING: refcount bug in kvm_vm_ioctl

[Dmitry Vyukov]

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.

Re: WARNING: refcount bug in kvm_vm_ioctl

[Jann Horn]

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.

Re: WARNING: refcount bug in kvm_vm_ioctl

[Dmitry Vyukov]

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.