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

[Jim Mattson]

On Tue, Nov 10, 2026 at 9:56 AM Yosry Ahmed <yosry@xxxxxxxxxx> wrote:
>
> 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@xxxxxxxxxx>
> > ---
> >  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?

CR4.CET is reserved if an AMD CPU doesn't have X86_FEATURE_SHSTK.
Hence, older CPUs without CET support cannot fail 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.

If X86_FEATURE_SHSTK is not enumerated by the vCPU, then CR4.CET == 0
and this entire block is skipped.

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

That seems superfluous.

> 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.

You're right; failure to read the MSR should result in a failed VMRUN,
and a warning would help triage the failure.

> 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?

Not with a legal snapshot. Note that on the restore path, the 'save'
area holds L1 state. Hence, RFLAGS.VM must be clear, because VMRUN
raises #UD in V8086 mode.