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

[Yosry Ahmed <yosry@kernel.org>]

On Mon, Nov 09, 2026 at 04:40:37PM -0800, Jim Mattson wrote:
> Per the APM, "Canonicalization and Consistency Checks,"
> 
>   [t]he following conditions are considered illegal state combinations:
> 
>    - CR4.CET=1 when CR0.WP=0
>    - Any reserved bit set in S_CET
>    - CR4.CET=1 and U_CET.SS=1 when EFLAGS.VM=1
> 
> Check these conditions in software before trying to VMRUN vmcb02, so that
> the illegal states result in a #VMEXIT to L1 rather than an exit to
> userspace with KVM_EXIT_FAIL_ENTRY.
> 
> Fixes: c5ba49458513 ("KVM: nSVM: Save/load CET Shadow Stack state to/from vmcb12/vmcb02")
> Signed-off-by: Jim Mattson <jmattson@google.com>
> ---
>  arch/x86/kvm/svm/nested.c | 15 +++++++++++++++
>  1 file changed, 15 insertions(+)
> 
> diff --git a/arch/x86/kvm/svm/nested.c b/arch/x86/kvm/svm/nested.c
> index af0ef678433d..20ede403b62d 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;

Why aren't we checking X86_FEATURE_SHSTK here as well? Do we know if
older CPUs that didn't support CET had the consistency check?

> +
> +	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) &&

If X86_FEATURE_SHSTK is not enumerated by the vCPU, then whether or not
the consistency check is done depends on ignore_msr. If ignore_msr=0,
the MSR read will fail and we won't do the consistency check, but if
ignore_msr=1 the MSR read will silently return 0, so the consistency
check will be done but will always succeed.

Instead of this implicit dependency, should we check for
X86_FEATURE_SHSTK explicitly?

Also, if X86_FEATURE_SHSTK is enumberated by the vCPU, and the MSR read
fails, the consistency check will silently be skipped. But I don't think
this can/should happen. Maybe we should WARN in this case.

One last question, in the restore path, the MSR may have not been
restored yet, right? Can the consistency check mistakenly fail in this
case?

> +		    CC(u_cet & CET_SHSTK_EN))
> +			return false;
> +	}
> +
>  	if (CC(!kvm_valid_efer(vcpu, save->efer)))
>  		return false;
>  
> -- 
> 2.56.0.506.g506a7eabc5-goog
>