[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

[sashiko-bot]

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

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

[Jim Mattson]

On Mon, Nov 9, 2026 at 5:07 PM <sashiko-bot@kernel.org> wrote:
>
> 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.

Thank you for the review. I believe that Paolo is already addressing
this issue in https://lore.kernel.org/all/20261102174130.8779-2-pbonzini@redhat.com.

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!