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

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

[Jim Mattson]

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;
+
+	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;
 
-- 
2.56.0.506.g506a7eabc5-goog

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

[Yosry Ahmed]

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
> 

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

[Jim Mattson]

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

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.

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

[Yosry Ahmed]

On Tue, Nov 10, 2026 at 11:18 AM Jim Mattson <jmattson@google.com> wrote:
>
> On Tue, Nov 10, 2026 at 9:56 AM Yosry Ahmed <yosry@kernel.org> 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@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?
>
> 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.

Oh I missed this, thanks.

>
> > > +
> > > +     if (guest_cpu_cap_has(vcpu, X86_FEATURE_SHSTK) &&
> > > +         CC(!kvm_is_valid_u_s_cet(vcpu, save->s_cet)))
> > > +             return false;

I wonder if we should clear the cached s_cet based on
X86_FEATURE_SHSTK, similar to how we sanitize some cached control
fields in __nested_copy_vmcb_control_to_cache(). Mostly orthogonal to
this patch tho.

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

Ack.

> > Instead of this implicit dependency, should we check for
> > X86_FEATURE_SHSTK explicitly?
>
> That seems superfluous.

Given the above, yes.

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

Oh that's subtle, but yes, makes sense. I was going to suggest a
comment, but I can't find a good place for it :)

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

[Jim Mattson]

On Tue, Nov 10, 2026 at 11:30 AM Yosry Ahmed <yosry@kernel.org> wrote:
>
> On Tue, Nov 10, 2026 at 11:18 AM Jim Mattson <jmattson@google.com> wrote:
> >
> > On Tue, Nov 10, 2026 at 9:56 AM Yosry Ahmed <yosry@kernel.org> wrote:
> > >
[ ... ]
> > > 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.
>
> Oh that's subtle, but yes, makes sense. I was going to suggest a
> comment, but I can't find a good place for it :)

Saved by subtlety!