KVM: VMX: Fix crash due to uninitialized current_vmcs
authorAlexandru Matei <alexandru.matei@uipath.com>
Mon, 23 Jan 2023 22:12:08 +0000 (00:12 +0200)
committerSean Christopherson <seanjc@google.com>
Tue, 7 Feb 2023 17:02:50 +0000 (09:02 -0800)
KVM enables 'Enlightened VMCS' and 'Enlightened MSR Bitmap' when running as
a nested hypervisor on top of Hyper-V. When MSR bitmap is updated,
evmcs_touch_msr_bitmap function uses current_vmcs per-cpu variable to mark
that the msr bitmap was changed.

vmx_vcpu_create() modifies the msr bitmap via vmx_disable_intercept_for_msr
-> vmx_msr_bitmap_l01_changed which in the end calls this function. The
function checks for current_vmcs if it is null but the check is
insufficient because current_vmcs is not initialized. Because of this, the
code might incorrectly write to the structure pointed by current_vmcs value
left by another task. Preemption is not disabled, the current task can be
preempted and moved to another CPU while current_vmcs is accessed multiple
times from evmcs_touch_msr_bitmap() which leads to crash.

The manipulation of MSR bitmaps by callers happens only for vmcs01 so the
solution is to use vmx->vmcs01.vmcs instead of current_vmcs.

  BUG: kernel NULL pointer dereference, address: 0000000000000338
  PGD 4e1775067 P4D 0
  Oops: 0002 [#1] PREEMPT SMP NOPTI
  ...
  RIP: 0010:vmx_msr_bitmap_l01_changed+0x39/0x50 [kvm_intel]
  ...
  Call Trace:
   vmx_disable_intercept_for_msr+0x36/0x260 [kvm_intel]
   vmx_vcpu_create+0xe6/0x540 [kvm_intel]
   kvm_arch_vcpu_create+0x1d1/0x2e0 [kvm]
   kvm_vm_ioctl_create_vcpu+0x178/0x430 [kvm]
   kvm_vm_ioctl+0x53f/0x790 [kvm]
   __x64_sys_ioctl+0x8a/0xc0
   do_syscall_64+0x5c/0x90
   entry_SYSCALL_64_after_hwframe+0x63/0xcd

Fixes: ceef7d10dfb6 ("KVM: x86: VMX: hyper-v: Enlightened MSR-Bitmap support")
Cc: stable@vger.kernel.org
Suggested-by: Sean Christopherson <seanjc@google.com>
Signed-off-by: Alexandru Matei <alexandru.matei@uipath.com>
Link: https://lore.kernel.org/r/20230123221208.4964-1-alexandru.matei@uipath.com
Signed-off-by: Sean Christopherson <seanjc@google.com>
arch/x86/kvm/vmx/hyperv.h
arch/x86/kvm/vmx/vmx.c

index caf658726169a4df9343f55d085a4ec6d0d08e6d..78d17667e7ec282db12bed578a6c2ae8c5d04105 100644 (file)
@@ -250,16 +250,6 @@ static __always_inline u16 evmcs_read16(unsigned long field)
        return *(u16 *)((char *)current_evmcs + offset);
 }
 
-static inline void evmcs_touch_msr_bitmap(void)
-{
-       if (unlikely(!current_evmcs))
-               return;
-
-       if (current_evmcs->hv_enlightenments_control.msr_bitmap)
-               current_evmcs->hv_clean_fields &=
-                       ~HV_VMX_ENLIGHTENED_CLEAN_FIELD_MSR_BITMAP;
-}
-
 static inline void evmcs_load(u64 phys_addr)
 {
        struct hv_vp_assist_page *vp_ap =
@@ -280,7 +270,6 @@ static __always_inline u64 evmcs_read64(unsigned long field) { return 0; }
 static __always_inline u32 evmcs_read32(unsigned long field) { return 0; }
 static __always_inline u16 evmcs_read16(unsigned long field) { return 0; }
 static inline void evmcs_load(u64 phys_addr) {}
-static inline void evmcs_touch_msr_bitmap(void) {}
 #endif /* IS_ENABLED(CONFIG_HYPERV) */
 
 #define EVMPTR_INVALID (-1ULL)
index 8a9911ae1240f9a6ad124f1bca68640adcafc576..33614ee2cd673a9e5f4c14aa7b1381765f26e51b 100644 (file)
@@ -3936,8 +3936,13 @@ static void vmx_msr_bitmap_l01_changed(struct vcpu_vmx *vmx)
         * 'Enlightened MSR Bitmap' feature L0 needs to know that MSR
         * bitmap has changed.
         */
-       if (static_branch_unlikely(&enable_evmcs))
-               evmcs_touch_msr_bitmap();
+       if (IS_ENABLED(CONFIG_HYPERV) && static_branch_unlikely(&enable_evmcs)) {
+               struct hv_enlightened_vmcs *evmcs = (void *)vmx->vmcs01.vmcs;
+
+               if (evmcs->hv_enlightenments_control.msr_bitmap)
+                       evmcs->hv_clean_fields &=
+                               ~HV_VMX_ENLIGHTENED_CLEAN_FIELD_MSR_BITMAP;
+       }
 
        vmx->nested.force_msr_bitmap_recalc = true;
 }