[PATCH 0/2] KVM: x86: Reset MMU back to the root MMU when leaving guest mode

[PATCH 0/2] KVM: x86: Reset MMU back to the root MMU when leaving guest mode

[Paolo Bonzini]

Fix a bug where KVM fails to unwind vcpu->arch.mmu back to the root MMU after
leaving guest mode on a failed entry to L2 via RSM or KVM_SET_NETSED_STATE.
If KVM also frees nested state before the next entry to L1 (or maybe even
after?), trying to construct the shadow root will hit a NULL pointer deref
when trying to get vmcs12->eptp.

The only known instance of the bug is in patch 1; Sean's patch 2 avoids
the moving the reset to the place where the vCPU goes back to L1 for both
nSVM and nVMX.  However, it conflicts with pre-7.3 code due to the separation
of kvm_mmu and kvm_pagewalk; and it will also conflict with the stable-worthy
v3 of https://lore.kernel.org/kvm/20261011001303.723721-1-mlevitsk@redhat.com/,
so it will only be applied to kvm/next.

Thanks,

Paolo

Amirmohammad Eftekhar (1):
  KVM: nVMX: Reset MMU back to the root MMU if loading L2 state fails for !VM-Entry

Sean Christopherson (1):
  KVM: x86: Reset MMU back to the root MMU when leaving guest mode

 arch/x86/kvm/regs.h       | 2 ++
 arch/x86/kvm/svm/nested.c | 8 --------
 arch/x86/kvm/vmx/nested.c | 8 --------
 3 files changed, 2 insertions(+), 16 deletions(-)

-- 
2.56.0

[PATCH 1/2] KVM: nVMX: Reset MMU back to the root MMU if loading L2 state fails for !VM-Entry

[Paolo Bonzini]

From: Amirmohammad Eftekhar <amirmohammadeftekhar@gmail.com>

Reset the vCPU's MMU pointer back to the root MMU if loading L2 state fails
in the "not from VM-Entry" case, i.e. when emulating RSM or handling
KVM_SET_NESTED_STATE.  Failure to unwind the MMU pointer results in KVM
trying to run L1 with a nested MMU, and is especially problematic if the
vCPU manages to free its nested state before the VM crashes.  Constructing
the root for a nested EPT MMU goes through vmcs12, which is dynamically
allocated/freed when nVMX is enabled/disabled.  And so if the nested state
is freed with the MMU still pointing at the nested MMU, KVM will hit a NULL
pointer deref when attempting to get L1=>L2 EPTP.

  BUG: kernel NULL pointer dereference, address: 0000000000000078
  #PF: supervisor read access in kernel mode
  #PF: error_code(0x0000) - not-present page
  PGD 114a2d067 P4D 111743067 PUD 11141f067 PMD 0
  Oops: Oops: 0000 [#1] SMP
  CPU: 20 UID: 1000 PID: 1272 Comm: nested_tdp_faul Not tainted 7.3.0-rc4-8c89f11d1a99b-sink-vm #1914 PREEMPTLAZY
  Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS 0.0.0 02/06/2015
  RIP: 0010:nested_ept_get_eptp+0xb/0x10 [kvm_intel]
  Call Trace:
   <TASK>
   kvm_mmu_load+0xcd/0x870 [kvm]
   kvm_arch_vcpu_ioctl_run+0x18f5/0x1f80 [kvm]
   kvm_vcpu_ioctl+0x2c7/0x970 [kvm]
   __x64_sys_ioctl+0x90/0xd0
   do_syscall_64+0x9c/0x4d0
   entry_SYSCALL_64_after_hwframe+0x4b/0x53
  RIP: 0033:0x47491b
   </TASK>
  Modules linked in: kvm_intel kvm irqbypass
  CR2: 0000000000000078
  ---[ end trace 0000000000000000 ]---
  RIP: 0010:nested_ept_get_eptp+0xb/0x10 [kvm_intel]

Cc: David Woodhouse <dwmw2@infradead.org>
Fixes: 72e9cbdb4338 ("KVM: nVMX: fix SMI injection in guest mode")
Fixes: 8fcc4b5923af ("kvm: nVMX: Introduce KVM_CAP_NESTED_STATE")
Cc: stable@vger.kernel.org
Signed-off-by: Amirmohammad Eftekhar <amirmohammadeftekhar@gmail.com>
[sean: write changelog]
Signed-off-by: Sean Christopherson <seanjc@google.com>
Link: https://patch.msgid.link/20261001214356.3825100-2-seanjc@google.com
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 arch/x86/kvm/vmx/nested.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/arch/x86/kvm/vmx/nested.c b/arch/x86/kvm/vmx/nested.c
index b25216862740..5165275128fc 100644
--- a/arch/x86/kvm/vmx/nested.c
+++ b/arch/x86/kvm/vmx/nested.c
@@ -3770,8 +3770,10 @@ enum nvmx_vmentry_status nested_vmx_enter_non_root_mode(struct kvm_vcpu *vcpu,
 vmentry_fail_vmexit:
 	vmx_switch_vmcs(vcpu, &vmx->vmcs01);
 
-	if (!from_vmentry)
+	if (!from_vmentry) {
+		nested_ept_uninit_mmu_context(vcpu);
 		return NVMX_VMENTRY_VMEXIT;
+	}
 
 	nested_put_vmcs12_pages(vcpu);
 
-- 
2.56.0

[PATCH 2/2] KVM: x86: Reset MMU back to the root MMU when leaving guest mode

[Paolo Bonzini]

From: Sean Christopherson <seanjc@google.com>

Always reset a vCPU's MMU pointer back to the root MMU when leaving guest
mode, to ensure that KVM doesn't try to run L1 with a nested TDP MMU.
Dereferencing the MMU between putting the vCPU into L1 and restoring L1,
or even pivoting on mmu_is_nested(), would be a KVM bug, as the vCPU state
is guaranteed to be stale for one flow or another.

The risk of immediately resetting the MMU pointer is that doing so could
introduce a regression if either nSVM or nVMX has a correctness dependency
on buggy code, i.e. if KVM relies on the nested MMU being live between
leave_guest_mode() and the calls to nested{ept,svm}_uninit_mmu_context().
But for such a dependency to exist, KVM would either need to explicitly
check for nested TDP or make some very blind assumptions about the MMU.

A manual audit of the code confirmed that vcpu->arch.mmu isn't dereferenced
in the described window, and hacking KVM to poison the pointer and WARN if
KVM tries to pivot on the pointer with half-baked state didn't trigger any
splats while running KVM-Unit-Tests, KVM selftests, and Linux guests.  I.e.
running with these hacks didn't trigger any badness:

  static inline bool mmu_is_nested(struct kvm_vcpu *vcpu)
  {
	WARN_ON_ONCE(vcpu->arch.mmu == &vcpu->arch.guest_mmu &&
		     !is_guest_mode(vcpu));
	return vcpu->arch.mmu == &vcpu->arch.guest_mmu;
  }

  static inline void leave_guest_mode(struct kvm_vcpu *vcpu)
  {
	if (vcpu->arch.mmu != &vcpu->arch.root_mmu)
		vcpu->arch.mmu = LIST_POISON1;

	vcpu->arch.hflags &= ~HF_GUEST_MASK;

	if (vcpu->arch.load_eoi_exitmap_pending) {
		vcpu->arch.load_eoi_exitmap_pending = false;
		kvm_make_request(KVM_REQ_LOAD_EOI_EXITMAP, vcpu);
	}

	vcpu->stat.guest_mode = 0;
  }

Signed-off-by: Sean Christopherson <seanjc@google.com>
Link: https://patch.msgid.link/20261001214356.3825100-3-seanjc@google.com
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 arch/x86/kvm/regs.h       |  2 ++
 arch/x86/kvm/svm/nested.c |  8 --------
 arch/x86/kvm/vmx/nested.c | 12 +-----------
 3 files changed, 3 insertions(+), 19 deletions(-)

diff --git a/arch/x86/kvm/regs.h b/arch/x86/kvm/regs.h
index 447f0ec3e63e..b29dd24a4531 100644
--- a/arch/x86/kvm/regs.h
+++ b/arch/x86/kvm/regs.h
@@ -463,6 +463,8 @@ static inline void enter_guest_mode(struct kvm_vcpu *vcpu)
 
 static inline void leave_guest_mode(struct kvm_vcpu *vcpu)
 {
+	vcpu->arch.mmu = &vcpu->arch.root_mmu;
+
 	vcpu->arch.hflags &= ~HF_GUEST_MASK;
 
 	if (vcpu->arch.load_eoi_exitmap_pending) {
diff --git a/arch/x86/kvm/svm/nested.c b/arch/x86/kvm/svm/nested.c
index f9090b601efa..50f12cdeaa7c 100644
--- a/arch/x86/kvm/svm/nested.c
+++ b/arch/x86/kvm/svm/nested.c
@@ -119,11 +119,6 @@ static void nested_svm_init_mmu_context(struct kvm_vcpu *vcpu)
 	vcpu->arch.ngpa_walk.inject_page_fault = nested_svm_inject_npf_exit;
 }
 
-static void nested_svm_uninit_mmu_context(struct kvm_vcpu *vcpu)
-{
-	vcpu->arch.mmu = &vcpu->arch.root_mmu;
-}
-
 static bool nested_vmcb_needs_vls_intercept(struct vcpu_svm *svm)
 {
 	if (!guest_cpu_cap_has(&svm->vcpu, X86_FEATURE_V_VMSAVE_VMLOAD))
@@ -1432,8 +1427,6 @@ void nested_svm_vmexit(struct vcpu_svm *svm)
 
 	nested_svm_transition_tlb_flush(vcpu);
 
-	nested_svm_uninit_mmu_context(vcpu);
-
 	if (nested_svm_load_cr3(vcpu, vmcb01->save.cr3, false, true))
 		kvm_make_request(KVM_REQ_TRIPLE_FAULT, vcpu);
 
@@ -1549,7 +1542,6 @@ void svm_leave_nested(struct kvm_vcpu *vcpu)
 
 		svm_switch_vmcb(svm, &svm->vmcb01);
 
-		nested_svm_uninit_mmu_context(vcpu);
 		vmcb_mark_all_dirty(svm->vmcb);
 
 		svm_set_gif(svm, true);
diff --git a/arch/x86/kvm/vmx/nested.c b/arch/x86/kvm/vmx/nested.c
index 5165275128fc..6f18c236e3c6 100644
--- a/arch/x86/kvm/vmx/nested.c
+++ b/arch/x86/kvm/vmx/nested.c
@@ -525,11 +525,6 @@ static void nested_ept_init_mmu_context(struct kvm_vcpu *vcpu)
 	vcpu->arch.ngpa_walk.inject_page_fault = nested_ept_inject_page_fault;
 }
 
-static void nested_ept_uninit_mmu_context(struct kvm_vcpu *vcpu)
-{
-	vcpu->arch.mmu = &vcpu->arch.root_mmu;
-}
-
 static bool nested_vmx_is_page_fault_vmexit(struct vmcs12 *vmcs12,
 					    u16 error_code)
 {
@@ -3770,10 +3765,8 @@ enum nvmx_vmentry_status nested_vmx_enter_non_root_mode(struct kvm_vcpu *vcpu,
 vmentry_fail_vmexit:
 	vmx_switch_vmcs(vcpu, &vmx->vmcs01);
 
-	if (!from_vmentry) {
-		nested_ept_uninit_mmu_context(vcpu);
+	if (!from_vmentry)
 		return NVMX_VMENTRY_VMEXIT;
-	}
 
 	nested_put_vmcs12_pages(vcpu);
 
@@ -4833,8 +4826,6 @@ static void load_vmcs12_host_state(struct kvm_vcpu *vcpu,
 	vcpu->arch.cr4_guest_owned_bits = ~vmcs_readl(CR4_GUEST_HOST_MASK);
 	vmx_set_cr4(vcpu, vmcs12->host_cr4);
 
-	nested_ept_uninit_mmu_context(vcpu);
-
 	/*
 	 * Only PDPTE load can fail as the value of cr3 was checked on entry and
 	 * couldn't have changed.
@@ -4998,7 +4989,6 @@ static void nested_vmx_restore_host_state(struct kvm_vcpu *vcpu)
 	vcpu->arch.cr4_guest_owned_bits = ~vmcs_readl(CR4_GUEST_HOST_MASK);
 	vmx_set_cr4(vcpu, vmcs_readl(CR4_READ_SHADOW));
 
-	nested_ept_uninit_mmu_context(vcpu);
 	vcpu->arch.cr3 = vmx->nested.pre_vmenter_cr3;
 	kvm_register_mark_available(vcpu, VCPU_REG_CR3);
 
-- 
2.56.0