[PATCH] KVM: nSVM: Validate L1 host save state restored on RSM from SMM

[PATCH] KVM: nSVM: Validate L1 host save state restored on RSM from SMM

[Abdelali Maddane]

When an SMI is taken while L2 is running, svm_enter_smm() saves L1's host
state into the HSAVE area (ordinary guest memory pointed to by
MSR_VM_HSAVE_PA).  On RSM, svm_leave_smm() copies that state back into
vmcb01 and builds the nested MMU from it, but unlike KVM_SET_NESTED_STATE
it does not validate the restored state.

Because the HSAVE area is guest memory, L1 can modify it while the vCPU is
in SMM so that RSM loads an impossible EFER.LMA=1 && EFER.LME=0 combination
(e.g. together with CR4.PAE=0).  kvm_calc_cpu_role() then computes a 4- or
5-level role while shadow_mmu_init_context() wires up the 2-level paging32
template, and FNAME(walk_addr_generic) indexes out of bounds of the
fixed-size guest_walker arrays.  On kernels carrying the KVM_BUG_ON()
backstop in kvm_calc_cpu_role() this is a guest-triggerable KVM_BUG_ON()
(dead VM, or a host panic with panic_on_warn); on older kernels it is an
out-of-bounds access in the page-table walker.

Validate the restored L1 host state in svm_leave_smm() with the same checks
KVM_SET_NESTED_STATE already applies to L1 host state, factored out into
nested_svm_check_l1_save().  A failed check fails RSM, which is the correct
response to an impossible saved state.

The patch, reproducer, and this changelog were developed with substantial
help from an AI assistant (Anthropic's Claude Code), directed and reviewed
by the submitter.  Tested by the submitter on an AMD EPYC host: without the
patch a reproducer triggers the guest-reachable KVM_BUG_ON() in
kvm_calc_cpu_role(); with the patch the reproducer is rejected, and the
smm_test and svm_nested_shutdown_test selftests still pass.

Fixes: 37be407b2ce8 ("KVM: nSVM: Fix L1 state corruption upon return from SMM")
Signed-off-by: Abdelali Maddane <ma.08@web.de>
---
Reproducer (a KVM selftest) available on request.

 arch/x86/kvm/svm/nested.c | 36 ++++++++++++++++++++++--------------
 arch/x86/kvm/svm/svm.c    |  7 +++++++
 arch/x86/kvm/svm/svm.h    |  1 +
 3 files changed, 30 insertions(+), 14 deletions(-)

diff --git a/arch/x86/kvm/svm/nested.c b/arch/x86/kvm/svm/nested.c
index af0ef6784..2295bd90a 100644
--- a/arch/x86/kvm/svm/nested.c
+++ b/arch/x86/kvm/svm/nested.c
@@ -616,6 +616,26 @@ void nested_copy_vmcb_save_to_cache(struct vcpu_svm *svm,
 	__nested_copy_vmcb_save_to_cache(&svm->nested.save, save);
 }
 
+/*
+ * Validate L1 host save state loaded on return to L1 (RSM from SMM, or via
+ * KVM_SET_NESTED_STATE).  g_pat is not checked: it may be clobbered by SMM and
+ * isn't loaded at emulated #VMEXIT.
+ */
+int nested_svm_check_l1_save(struct kvm_vcpu *vcpu, struct vmcb_save_area *save)
+{
+	struct vmcb_save_area_cached save_cached;
+
+	__nested_copy_vmcb_save_to_cache(&save_cached, save);
+	if (!(save->cr0 & X86_CR0_PG) ||
+	    !(save->cr0 & X86_CR0_PE) ||
+	    (save->rflags & X86_EFLAGS_VM) ||
+	    ((save->efer & EFER_LMA) && !(save->efer & EFER_LME)) ||
+	    !nested_vmcb_check_save(vcpu, &save_cached, false))
+		return -EINVAL;
+
+	return 0;
+}
+
 /*
  * Synchronize fields that are written by the processor, so that
  * they can be copied back into the vmcb12.
@@ -1968,7 +1988,6 @@ static int svm_set_nested_state(struct kvm_vcpu *vcpu,
 		&user_kvm_nested_state->data.svm[0];
 	struct vmcb_control_area *ctl;
 	struct vmcb_save_area *save;
-	struct vmcb_save_area_cached save_cached;
 	struct vmcb_ctrl_area_cached ctl_cached;
 	bool use_separate_l2_pat;
 	unsigned long cr0;
@@ -2033,19 +2052,8 @@ static int svm_set_nested_state(struct kvm_vcpu *vcpu,
         if (((cr0 & X86_CR0_CD) == 0) && (cr0 & X86_CR0_NW))
 		goto out_free;
 
-	/*
-	 * Validate host state saved from before VMRUN (see
-	 * nested_svm_check_permissions). Note that the g_pat field is not
-	 * validated, because (a) it may have been clobbered by SMM before
-	 * KVM_GET_NESTED_STATE, and (b) it is not loaded at emulated
-	 * #VMEXIT.
-	 */
-	__nested_copy_vmcb_save_to_cache(&save_cached, save);
-	if (!(save->cr0 & X86_CR0_PG) ||
-	    !(save->cr0 & X86_CR0_PE) ||
-	    (save->rflags & X86_EFLAGS_VM) ||
-	    ((save->efer & EFER_LMA) && !(save->efer & EFER_LME)) ||
-	    !nested_vmcb_check_save(vcpu, &save_cached, false))
+	/* Validate host state saved from before VMRUN. */
+	if (nested_svm_check_l1_save(vcpu, save))
 		goto out_free;
 
 	/*
diff --git a/arch/x86/kvm/svm/svm.c b/arch/x86/kvm/svm/svm.c
index 136069dfc..9de22fa92 100644
--- a/arch/x86/kvm/svm/svm.c
+++ b/arch/x86/kvm/svm/svm.c
@@ -5224,6 +5224,13 @@ static int svm_leave_smm(struct kvm_vcpu *vcpu, const union kvm_smram *smram)
 
 	svm_copy_vmrun_state(vcpu, &svm->vmcb01.ptr->save, m_save.map.hva + 0x400);
 
+	/*
+	 * The HSAVE area is guest memory; validate the restored L1 state before
+	 * it is used to build the nested MMU, same as KVM_SET_NESTED_STATE.
+	 */
+	if (nested_svm_check_l1_save(vcpu, &svm->vmcb01.ptr->save))
+		return 1;
+
 	/*
 	 * Enter the nested guest now
 	 */
diff --git a/arch/x86/kvm/svm/svm.h b/arch/x86/kvm/svm/svm.h
index 6d0242966..f7f0f3071 100644
--- a/arch/x86/kvm/svm/svm.h
+++ b/arch/x86/kvm/svm/svm.h
@@ -901,6 +901,7 @@ static inline void nested_svm_simple_vmexit(struct vcpu_svm *svm, u32 exit_code)
 int nested_svm_exit_handled(struct vcpu_svm *svm);
 int nested_svm_check_permissions(struct kvm_vcpu *vcpu);
 int nested_svm_check_cached_vmcb12(struct kvm_vcpu *vcpu);
+int nested_svm_check_l1_save(struct kvm_vcpu *vcpu, struct vmcb_save_area *save);
 int nested_svm_check_exception(struct vcpu_svm *svm, unsigned nr,
 			       bool has_error_code, u32 error_code);
 int nested_svm_exit_special(struct vcpu_svm *svm);

base-commit: dea4f6380f4ec96e8f3b0aaf2343764bd0d25ef4
-- 
2.56.0

Re: [PATCH] KVM: nSVM: Validate L1 host save state restored on RSM from SMM

[Abdelali Maddane]

Please disregard this patch, the same issue is already fixed by Sean's
"[PATCH 1/7] KVM: nSVM: Check validity of vmcs01 save area copied from
SMRAM on RSM" [1],
which I missed before sending. Sorry for the noise.

[1] https://lore.kernel.org/kvm/20261102231428.13523-1-pbonzini@redhat.com/

Thanks,
Abdelali