Re: kvm: GPF in kvm_lapic_set_tpr

[Paolo Bonzini]

On 21/06/2016 15:18, Dmitry Vyukov wrote:
>  [<ffffffff811269be>] kvm_lapic_set_tpr+0x5e/0xe0 arch/x86/kvm/lapic.c:1648
>  [<ffffffff8109496c>] kvm_set_cr8.part.135+0x2c/0xb0 arch/x86/kvm/x86.c:809
>  [<ffffffff81094a22>] kvm_set_cr8+0x32/0x50 arch/x86/kvm/x86.c:806
>  [<ffffffff81178dd4>] vmx_vcpu_reset+0x114/0xb60 arch/x86/kvm/vmx.c:4996
>  [<ffffffff810b91b0>] kvm_vcpu_reset+0x410/0x580 arch/x86/kvm/x86.c:7470
>  [<ffffffff810b9355>] kvm_arch_vcpu_setup+0x35/0x60 arch/x86/kvm/x86.c:7393
>  [<     inline     >] kvm_vm_ioctl_create_vcpu virt/kvm/kvm_main.c:2355
>  [<ffffffff8106b9f2>] kvm_vm_ioctl+0x582/0x10d0 virt/kvm/kvm_main.c:2839
>  [<     inline     >] vfs_ioctl fs/ioctl.c:43
>  [<ffffffff818510bc>] do_vfs_ioctl+0x18c/0xff0 fs/ioctl.c:674
>  [<     inline     >] SYSC_ioctl fs/ioctl.c:689
>  [<ffffffff81851faf>] SyS_ioctl+0x8f/0xc0 fs/ioctl.c:680
>  [<ffffffff86a96fc0>] entry_SYSCALL_64_fastpath+0x23/0xc1

The bug is not in KVM.  Modulo the shuffling of system calls from
threading, the program is equivalent to this:

long r[7];

int main(void)
{
        struct kvm_vcpu_events ve;
        r[0] = open("/dev/kvm", O_RDONLY);
        r[1] = ioctl(r[0], KVM_CREATE_VM, 0x0ul, 0, 0, 0);
        r[2] = ioctl(r[1], KVM_CREATE_VCPU, 0x1ul, 0, 0, 0);
        r[3] = ioctl(r[2], KVM_RUN, 0, 0, 0, 0);
        r[4] = ioctl(r[2], KVM_RUN, 0, 0, 0, 0);
        r[5] = ioctl(r[2], KVM_RUN, 0, 0, 0, 0);
        r[6] = ioctl(r[2], KVM_GET_VCPU_EVENTS, &ve, 0, 0, 0);
        return 0;
}

However, it is easy to see that there is a clear critical path in the
bug, so that the threading must be completely irrelevant:

1) r[1] was valid in KVM_CREATE_VCPU, so KVM_CREATE_VCPU started after
the assignment of KVM_CREATE_VM

2) r[0] was valid in KVM_CREATE_VM, so KVM_CREATE_VM started after the
assignment of open("/dev/kvm")

3) the assignment of r[2] must not have happened because of the
backtrace, so KVM_CREATE_VCPU has not finished and the other ioctls will
not get to KVM.

The GP _could_ happen because of a bug, because lapic_in_kernel attempts
to skip a NULL check using a static key.  But the static key is
incremented before the point that is in the backtrace:

kvm_vm_ioctl_create_vcpu (virt/kvm/kvm_main.c)
 kvm_arch_vcpu_create (arch/x86/kvm/x86.c)
  vmx_create_vcpu (arch/x86/kvm/vmx.c)
   kvm_vcpu_init (virt/kvm/kvm_main.c)
    kvm_arch_vcpu_init (arch/x86/kvm/x86.c)
     static_key_slow_inc
    <-
   <-
  <-
 <-
 kvm_arch_vcpu_setup (arch/x86/kvm/x86.c)
  kvm_vcpu_reset (arch/x86/kvm/x86.c)
   vmx_vcpu_reset (arch/x86/kvm/vmx.c)
    kvm_set_cr8 (arch/x86/kvm/x86.c)

*mumble mumble*

Ok, the bug shows up when you do two concurrent calls to
static_key_slow_inc and is caused by commit 706249c22.  Patch on its way.

Paolo