Re: [PATCH] KVM: nSVM: Add missing CET VMRUN consistency checks

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found no new issues
in this patch, but noticed 1 related pre-existing issue it does not
introduce:
- [High] KVM: nSVM: vCPU left in guest mode when enter_svm_guest_mode()
  fails in RSM [1]
--

commit b87b2411f4552cf28fa62d1f5b9a2e342f4694d4
Author: Jim Mattson <jmattson@google.com>

KVM: nSVM: Add missing CET VMRUN consistency checks

This commit adds software VMRUN consistency checks for illegal CET state
combinations in nested_vmcb_check_save() so that invalid states trigger a
nested #VMEXIT to L1 instead of KVM_EXIT_FAIL_ENTRY.

> diff --git a/arch/x86/kvm/svm/nested.c b/arch/x86/kvm/svm/nested.c
> index f9090b601efaa..a3affdbc15c24 100644
> --- a/arch/x86/kvm/svm/nested.c
> +++ b/arch/x86/kvm/svm/nested.c
> @@ -469,6 +469,21 @@ static bool nested_vmcb_check_save(struct kvm_vcpu *vcpu,
>  	if (CC(!__kvm_is_valid_cr4(vcpu, save->cr4)))
>  		return false;
>  
> +	if (CC((save->cr4 & X86_CR4_CET) && !(save->cr0 & X86_CR0_WP)))
> +		return false;
> +
> +	if (guest_cpu_cap_has(vcpu, X86_FEATURE_SHSTK) &&
> +	    CC(!kvm_is_valid_u_s_cet(vcpu, save->s_cet)))
> +		return false;
> +
> +	if ((save->cr4 & X86_CR4_CET) && (save->rflags & X86_EFLAGS_VM)) {
> +		u64 u_cet = 0;
> +
> +		if (!__kvm_emulate_msr_read(vcpu, MSR_IA32_U_CET, &u_cet) &&
> +		    CC(u_cet & CET_SHSTK_EN))
> +			return false;
> +	}
> +
>  	if (CC(!kvm_valid_efer(vcpu, save->efer)))
>  		return false;

[Severity: High]
This problem wasn't introduced by this patch, but can nested_vmcb_check_save()
allow an illegal save->cr3 through on RSM and leave the vCPU stranded in
guest mode [1]?

nested_vmcb_check_save() only checks save->cr3 with kvm_vcpu_is_legal_cr3()
when both EFER_LME and X86_CR0_PG are set.  When resuming an L2 guest from
SMM with either bit clear and save->cr3 bits set beyond MAXPHYADDR,
svm_leave_smm() passes nested_svm_check_cached_vmcb12() and calls
enter_svm_guest_mode():

arch/x86/kvm/svm/nested.c:enter_svm_guest_mode() {
	...
	svm_switch_vmcb(svm, &svm->nested.vmcb02);
	nested_vmcb02_prepare_control(svm);
	nested_vmcb02_prepare_save(svm);

	ret = nested_svm_load_cr3(&svm->vcpu, svm->nested.save.cr3,
				  nested_npt_enabled(svm), from_vmrun);
	if (ret)
		return ret;
	...
}

nested_svm_load_cr3() unconditionally checks kvm_vcpu_is_legal_cr3() and
returns -EINVAL before kvm_init_mmu() runs, after the vCPU has already
switched to vmcb02, set HF_GUEST_MASK, and pointed vcpu->arch.mmu at
guest_mmu.  Because emulator_leave_smm() returns X86EMUL_UNHANDLEABLE
immediately when leave_smm() fails, it skips the kvm_leave_nested() cleanup
at the end of the function, leaving the subsequent KVM_REQ_TRIPLE_FAULT to
run in guest mode with an uninitialized guest_mmu and un-restored SMM state.

[1] https://sashiko.dev/#/bug/linux-c0e90666-2e32-44be-b404-c06898a02416

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261110004037.125692-1-jmattson@google.com?part=1