[PATCH v2 0/5] KVM: nSVM: Disallow bad L1 EFER for KVM_SET_NESTED_STATE

[PATCH v2 0/5] KVM: nSVM: Disallow bad L1 EFER for KVM_SET_NESTED_STATE

[Sean Christopherson]

Fix a bug where KVM allows userspace to set an impossible EFER for L1 via
KVM_SET_NESTED_STATE, which ultimately can lead to KVM misconfiguring L2's
MMU (yay, NPT!) and overflowing the guest_walker arrays.  Then, harden the
MMU against similar bugs (hopefully it works this time; nVMX also had a
similar bug, but the "NPT uses L1's EFER/CR4" wrinkle rendered the existing
hardening useless).

v2:
 - Check walker->max_level, not w->cpu_role.base.level, to play nice with PAE
   paging on 32-bit hosts. [Sashiko]
 - Force EFER.LMA=0 in nested_vmcb02_prepare_save() if EFER.LME=0 to avoid
   confusing MMU code.

v1: https://lore.kernel.org/all/20260826211844.884951-2-seanjc@google.com

Sean Christopherson (5):
  KVM: nSVM: Reject KVM_SET_NESTED_STATE if L1 has EFER.LMA=1 &&
    EFER.LME=0
  KVM: nSVM: Ignore EFER.LMA if EFER.LME=0 when preparing L2 state
  KVM: x86/mmu: Bug the VM if KVM attempts to walk more levels than the
    MMU has
  KVM: x86/mmu: Bug the VM if KVM calcs a CPU role with EFER.LMA=1 &&
    CR4.PAE=0
  KVM: x86/mmu: Convert MMU walker's bounds check from BUG_ON() to
    KVM_BUG_ON()

 arch/x86/kvm/mmu/mmu.c         |  3 +++
 arch/x86/kvm/mmu/paging_tmpl.h | 17 ++++++++++-------
 arch/x86/kvm/svm/nested.c      |  5 +++++
 3 files changed, 18 insertions(+), 7 deletions(-)


base-commit: 76671054f9a1ff6abb976583cd8da37650acdc97
-- 
2.55.0.970.g62bdec98f9-goog

[PATCH v2 1/5] KVM: nSVM: Reject KVM_SET_NESTED_STATE if L1 has EFER.LMA=1 && EFER.LME=0

[Sean Christopherson]

Reject KVM_SET_NESTED_STATE if the incoming L1 host state has what is
effectively an impossible EFER combination of LMA=1 but LME=0, i.e. if the
state says long mode is active but not enabled.  Unlike VMX, SVM doesn't
have an explicit consistent check for the illegal combination; presumably
hardware simply ignores EFER.LMA if EFER.LME=0.

Unfortunately, KVM doesn't ignore EFER.LMA in this case and consumes the
illegal state when constructing the shadow MMU for L2.  E.g. if userspace
also clears CR4.PAE, then kvm_calc_cpu_role() will compute a role with 4 or
5 levels of paging, but shadow_mmu_init_context() will wire up the MMU to
use the paging32 template, which maxes out its levels at 2.

Note, the "real badness" is effectively the same as what happened with the
nVMX bug fixed by commit 112e66017bff ("KVM: nVMX: add missing consistency
checks for CR0 and CR4").  Unfortunately, the sanity check added by commit
72e2fb24a0b0 ("KVM: x86/mmu: Bug the VM if a vCPU ends up in long mode
without PAE enabled") doesn't work for this case, since L2 state is active
at the time of the page fault, but it's L1 that has the bad state.

Fixes: cc440cdad5b7 ("KVM: nSVM: implement KVM_GET_NESTED_STATE and KVM_SET_NESTED_STATE")
Cc: stable@vger.kernel.org
Cc: Yosry Ahmed <yosry@kernel.org>
Reported-by: Stefan Teodorescu <fane@google.com>
Signed-off-by: Sean Christopherson <seanjc@google.com>
---
 arch/x86/kvm/svm/nested.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/x86/kvm/svm/nested.c b/arch/x86/kvm/svm/nested.c
index 73f37b050d0a..49fb10ad1f9f 100644
--- a/arch/x86/kvm/svm/nested.c
+++ b/arch/x86/kvm/svm/nested.c
@@ -2028,6 +2028,7 @@ static int svm_set_nested_state(struct kvm_vcpu *vcpu,
 	if (!(save->cr0 & X86_CR0_PG) ||
 	    !(save->cr0 & X86_CR0_PE) ||
 	    (save->rflags & X86_EFLAGS_VM) ||
+	    ((save->efer & EFER_LMA) && !(save->efer & EFER_LME)) ||
 	    !nested_vmcb_check_save(vcpu, &save_cached, false))
 		goto out_free;
 
-- 
2.55.0.970.g62bdec98f9-goog

[PATCH v2 2/5] KVM: nSVM: Ignore EFER.LMA if EFER.LME=0 when preparing L2 state

[Sean Christopherson]

Force EFER.LMA=0 if EFER.LME=0 when preparing L2 state for VMRUN, i.e.
mimic real hardware's behavior of ignoring EFER.LMA if EFER.LME=0.  VMRUN
unfortunately allows the nonsensical combination, i.e. doesn't fail, but
KVM itself has an invariant EFER.LMA can be set et if and only if EFER.LME
is set.   Breaking that invariant can lead to a variety of issue,
particularly in MMU code that keys off EFER.LMA when determining whether to
emulate/virtualization 4/5-level paging versus PAE paging.

Cc: stable@vger.kernel.org
Cc: Yosry Ahmed <yosry@kernel.org>
Signed-off-by: Sean Christopherson <seanjc@google.com>
---
 arch/x86/kvm/svm/nested.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/arch/x86/kvm/svm/nested.c b/arch/x86/kvm/svm/nested.c
index 49fb10ad1f9f..23d29597d6bf 100644
--- a/arch/x86/kvm/svm/nested.c
+++ b/arch/x86/kvm/svm/nested.c
@@ -789,6 +789,10 @@ static void nested_vmcb02_prepare_save(struct vcpu_svm *svm)
 
 	kvm_set_rflags(vcpu, save->rflags | X86_EFLAGS_FIXED);
 
+	/* SVM ignores EFER.LMA if EFER.LME=0 (instead of failing VMRUN). */
+	if (!(svm->nested.save.efer & EFER_LME))
+		svm->nested.save.efer &= ~EFER_LMA;
+
 	svm_set_efer(vcpu, svm->nested.save.efer);
 
 	svm_set_cr0(vcpu, svm->nested.save.cr0);
-- 
2.55.0.970.g62bdec98f9-goog

Re: [PATCH v2 2/5] KVM: nSVM: Ignore EFER.LMA if EFER.LME=0 when preparing L2 state

[Yosry Ahmed]

On Wed, Sep 2, 2026 at 4:20 PM Sean Christopherson <seanjc@google.com> wrote:
>
> Force EFER.LMA=0 if EFER.LME=0 when preparing L2 state for VMRUN, i.e.
> mimic real hardware's behavior of ignoring EFER.LMA if EFER.LME=0.  VMRUN
> unfortunately allows the nonsensical combination, i.e. doesn't fail, but
> KVM itself has an invariant EFER.LMA can be set et if and only if EFER.LME
> is set.   Breaking that invariant can lead to a variety of issue,
> particularly in MMU code that keys off EFER.LMA when determining whether to
> emulate/virtualization 4/5-level paging versus PAE paging.
>
> Cc: stable@vger.kernel.org
> Cc: Yosry Ahmed <yosry@kernel.org>
> Signed-off-by: Sean Christopherson <seanjc@google.com>
> ---
>  arch/x86/kvm/svm/nested.c | 4 ++++
>  1 file changed, 4 insertions(+)
>
> diff --git a/arch/x86/kvm/svm/nested.c b/arch/x86/kvm/svm/nested.c
> index 49fb10ad1f9f..23d29597d6bf 100644
> --- a/arch/x86/kvm/svm/nested.c
> +++ b/arch/x86/kvm/svm/nested.c
> @@ -789,6 +789,10 @@ static void nested_vmcb02_prepare_save(struct vcpu_svm *svm)
>
>         kvm_set_rflags(vcpu, save->rflags | X86_EFLAGS_FIXED);
>
> +       /* SVM ignores EFER.LMA if EFER.LME=0 (instead of failing VMRUN). */
> +       if (!(svm->nested.save.efer & EFER_LME))
> +               svm->nested.save.efer &= ~EFER_LMA;

We sanitize control fields in __nested_copy_vmcb_control_to_cache().
Should we similarly sanitize this in
__nested_copy_vmcb_save_to_cache()?


> +
>         svm_set_efer(vcpu, svm->nested.save.efer);
>
>         svm_set_cr0(vcpu, svm->nested.save.cr0);
> --
> 2.55.0.970.g62bdec98f9-goog
>

Re: [PATCH v2 2/5] KVM: nSVM: Ignore EFER.LMA if EFER.LME=0 when preparing L2 state

[Sean Christopherson]

On Wed, Sep 02, 2026, Yosry Ahmed wrote:
> On Wed, Sep 2, 2026 at 4:20 PM Sean Christopherson <seanjc@google.com> wrote:
> >
> > Force EFER.LMA=0 if EFER.LME=0 when preparing L2 state for VMRUN, i.e.
> > mimic real hardware's behavior of ignoring EFER.LMA if EFER.LME=0.  VMRUN
> > unfortunately allows the nonsensical combination, i.e. doesn't fail, but
> > KVM itself has an invariant EFER.LMA can be set et if and only if EFER.LME
> > is set.   Breaking that invariant can lead to a variety of issue,
> > particularly in MMU code that keys off EFER.LMA when determining whether to
> > emulate/virtualization 4/5-level paging versus PAE paging.
> >
> > Cc: stable@vger.kernel.org
> > Cc: Yosry Ahmed <yosry@kernel.org>
> > Signed-off-by: Sean Christopherson <seanjc@google.com>
> > ---
> >  arch/x86/kvm/svm/nested.c | 4 ++++
> >  1 file changed, 4 insertions(+)
> >
> > diff --git a/arch/x86/kvm/svm/nested.c b/arch/x86/kvm/svm/nested.c
> > index 49fb10ad1f9f..23d29597d6bf 100644
> > --- a/arch/x86/kvm/svm/nested.c
> > +++ b/arch/x86/kvm/svm/nested.c
> > @@ -789,6 +789,10 @@ static void nested_vmcb02_prepare_save(struct vcpu_svm *svm)
> >
> >         kvm_set_rflags(vcpu, save->rflags | X86_EFLAGS_FIXED);
> >
> > +       /* SVM ignores EFER.LMA if EFER.LME=0 (instead of failing VMRUN). */
> > +       if (!(svm->nested.save.efer & EFER_LME))
> > +               svm->nested.save.efer &= ~EFER_LMA;
> 
> We sanitize control fields in __nested_copy_vmcb_control_to_cache().
> Should we similarly sanitize this in __nested_copy_vmcb_save_to_cache()?

Ideally, yes?  In practice, it doesn't work because svm_set_nested_state() loads
state from "save", not from "save_cached".  And even if we fixed that, it would
then allow userspace to pass in garbage (that is then ignored), i.e. would undo
patch 1, and I don't want to do that.

Re: [PATCH v2 2/5] KVM: nSVM: Ignore EFER.LMA if EFER.LME=0 when preparing L2 state

[Yosry Ahmed]

On Wed, Sep 2, 2026 at 5:02 PM Sean Christopherson <seanjc@google.com> wrote:
>
> On Wed, Sep 02, 2026, Yosry Ahmed wrote:
> > On Wed, Sep 2, 2026 at 4:20 PM Sean Christopherson <seanjc@google.com> wrote:
> > >
> > > Force EFER.LMA=0 if EFER.LME=0 when preparing L2 state for VMRUN, i.e.
> > > mimic real hardware's behavior of ignoring EFER.LMA if EFER.LME=0.  VMRUN
> > > unfortunately allows the nonsensical combination, i.e. doesn't fail, but
> > > KVM itself has an invariant EFER.LMA can be set et if and only if EFER.LME
> > > is set.   Breaking that invariant can lead to a variety of issue,
> > > particularly in MMU code that keys off EFER.LMA when determining whether to
> > > emulate/virtualization 4/5-level paging versus PAE paging.
> > >
> > > Cc: stable@vger.kernel.org
> > > Cc: Yosry Ahmed <yosry@kernel.org>
> > > Signed-off-by: Sean Christopherson <seanjc@google.com>
> > > ---
> > >  arch/x86/kvm/svm/nested.c | 4 ++++
> > >  1 file changed, 4 insertions(+)
> > >
> > > diff --git a/arch/x86/kvm/svm/nested.c b/arch/x86/kvm/svm/nested.c
> > > index 49fb10ad1f9f..23d29597d6bf 100644
> > > --- a/arch/x86/kvm/svm/nested.c
> > > +++ b/arch/x86/kvm/svm/nested.c
> > > @@ -789,6 +789,10 @@ static void nested_vmcb02_prepare_save(struct vcpu_svm *svm)
> > >
> > >         kvm_set_rflags(vcpu, save->rflags | X86_EFLAGS_FIXED);
> > >
> > > +       /* SVM ignores EFER.LMA if EFER.LME=0 (instead of failing VMRUN). */
> > > +       if (!(svm->nested.save.efer & EFER_LME))
> > > +               svm->nested.save.efer &= ~EFER_LMA;
> >
> > We sanitize control fields in __nested_copy_vmcb_control_to_cache().
> > Should we similarly sanitize this in __nested_copy_vmcb_save_to_cache()?
>
> Ideally, yes?  In practice, it doesn't work because svm_set_nested_state() loads
> state from "save", not from "save_cached".

I was gonna say let's just pass save_cached to svm_copy_vmrun_state()
instead of "save", but then I realized the SMM Code also uses it. We
can make it work for both (macro time!), but maybe that's not worth
doing. Sigh.

> And even if we fixed that, it would
> then allow userspace to pass in garbage (that is then ignored), i.e. would undo
> patch 1, and I don't want to do that.

I don't think so, patch 1 specifically performs the check on "save".

Re: [PATCH v2 2/5] KVM: nSVM: Ignore EFER.LMA if EFER.LME=0 when preparing L2 state

[Sean Christopherson]

On Wed, Sep 02, 2026, Yosry Ahmed wrote:
> On Wed, Sep 2, 2026 at 5:02 PM Sean Christopherson <seanjc@google.com> wrote:
> >
> > On Wed, Sep 02, 2026, Yosry Ahmed wrote:
> > > On Wed, Sep 2, 2026 at 4:20 PM Sean Christopherson <seanjc@google.com> wrote:
> > > >
> > > > Force EFER.LMA=0 if EFER.LME=0 when preparing L2 state for VMRUN, i.e.
> > > > mimic real hardware's behavior of ignoring EFER.LMA if EFER.LME=0.  VMRUN
> > > > unfortunately allows the nonsensical combination, i.e. doesn't fail, but
> > > > KVM itself has an invariant EFER.LMA can be set et if and only if EFER.LME
> > > > is set.   Breaking that invariant can lead to a variety of issue,
> > > > particularly in MMU code that keys off EFER.LMA when determining whether to
> > > > emulate/virtualization 4/5-level paging versus PAE paging.
> > > >
> > > > Cc: stable@vger.kernel.org
> > > > Cc: Yosry Ahmed <yosry@kernel.org>
> > > > Signed-off-by: Sean Christopherson <seanjc@google.com>
> > > > ---
> > > >  arch/x86/kvm/svm/nested.c | 4 ++++
> > > >  1 file changed, 4 insertions(+)
> > > >
> > > > diff --git a/arch/x86/kvm/svm/nested.c b/arch/x86/kvm/svm/nested.c
> > > > index 49fb10ad1f9f..23d29597d6bf 100644
> > > > --- a/arch/x86/kvm/svm/nested.c
> > > > +++ b/arch/x86/kvm/svm/nested.c
> > > > @@ -789,6 +789,10 @@ static void nested_vmcb02_prepare_save(struct vcpu_svm *svm)
> > > >
> > > >         kvm_set_rflags(vcpu, save->rflags | X86_EFLAGS_FIXED);
> > > >
> > > > +       /* SVM ignores EFER.LMA if EFER.LME=0 (instead of failing VMRUN). */
> > > > +       if (!(svm->nested.save.efer & EFER_LME))
> > > > +               svm->nested.save.efer &= ~EFER_LMA;
> > >
> > > We sanitize control fields in __nested_copy_vmcb_control_to_cache().
> > > Should we similarly sanitize this in __nested_copy_vmcb_save_to_cache()?
> >
> > Ideally, yes?  In practice, it doesn't work because svm_set_nested_state() loads
> > state from "save", not from "save_cached".
> 
> I was gonna say let's just pass save_cached to svm_copy_vmrun_state()
> instead of "save", but then I realized the SMM Code also uses it. We
> can make it work for both (macro time!), but maybe that's not worth
> doing. Sigh.

IMO, not worth doing.  At least, not for an immediate fix.

> > And even if we fixed that, it would
> > then allow userspace to pass in garbage (that is then ignored), i.e. would undo
> > patch 1, and I don't want to do that.
> 
> I don't think so, patch 1 specifically performs the check on "save".

True, I was thinking that we'd want to omit the massaged/sanitized state, but
that's obviously not a requirement.

Re: [PATCH v2 2/5] KVM: nSVM: Ignore EFER.LMA if EFER.LME=0 when preparing L2 state

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] The patch incompletely mimics VMRUN hardware behavior for EFER.LMA, allowing a malicious guest to desynchronize KVM's MMU state from the physical CPU. While the patch forces EFER.LMA=0 when EFER.LME=0, it fails to set EFER.LMA=1 when EFER.LME=1 and CR0.PG=1.
--

commit 0061c7315461e9b92e5efb9b382e2eecda3b30bb
Author: Sean Christopherson <seanjc@google.com>

KVM: nSVM: Ignore EFER.LMA if EFER.LME=0 when preparing L2 state

This commit forces EFER.LMA=0 if EFER.LME=0 when preparing L2 state for
VMRUN, mimicking hardware behavior. It addresses issues in MMU code that
key off EFER.LMA when determining whether to emulate 4/5-level paging
versus PAE paging.

> diff --git a/arch/x86/kvm/svm/nested.c b/arch/x86/kvm/svm/nested.c
> index 49fb10ad1f9f6..23d29597d6bf2 100644
> --- a/arch/x86/kvm/svm/nested.c
> +++ b/arch/x86/kvm/svm/nested.c
> @@ -789,6 +789,10 @@ static void nested_vmcb02_prepare_save(struct vcpu_svm *svm)
>  
>  	kvm_set_rflags(vcpu, save->rflags | X86_EFLAGS_FIXED);
>  
> +	/* SVM ignores EFER.LMA if EFER.LME=0 (instead of failing VMRUN). */
> +	if (!(svm->nested.save.efer & EFER_LME))
> +		svm->nested.save.efer &= ~EFER_LMA;
> +
>  	svm_set_efer(vcpu, svm->nested.save.efer);
>  
>  	svm_set_cr0(vcpu, svm->nested.save.cr0);

[Severity: High]
This is a pre-existing issue, but does this logic only implement half of the
hardware VMRUN rule? 

While this correctly fixes the invariant for EFER.LME=0, could a malicious
guest desynchronize KVM's MMU state from the physical CPU when EFER.LME=1?

If an L1 guest creates a VMCB12 with EFER.LME=1, CR0.PG=1, but EFER.LMA=0,
and executes VMRUN, nested_vmcb02_prepare_save() skips the LMA update
because LME=1, passing LMA=0 to svm_set_efer().

Then, when we reach svm_set_cr0(), it fails to correct EFER.LMA because it
expects an edge transition (!is_paging(vcpu)), which is always false since
L1 must already be paging to execute VMRUN:

arch/x86/kvm/svm/svm.c:svm_set_cr0() {
    ...
    if (vcpu->arch.efer & EFER_LME) {
        if (!is_paging(vcpu) && (cr0 & X86_CR0_PG)) {
            vcpu->arch.efer |= EFER_LMA;
    ...
}

Would this allow the guest to trick KVM's instruction emulator into fetching
arbitrary instructions from a different physical page than what the guest
actually executed?

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

Re: [PATCH v2 2/5] KVM: nSVM: Ignore EFER.LMA if EFER.LME=0 when preparing L2 state

[Sean Christopherson]

On Wed, Sep 02, 2026, sashiko-bot@kernel.org wrote:
> > diff --git a/arch/x86/kvm/svm/nested.c b/arch/x86/kvm/svm/nested.c
> > index 49fb10ad1f9f6..23d29597d6bf2 100644
> > --- a/arch/x86/kvm/svm/nested.c
> > +++ b/arch/x86/kvm/svm/nested.c
> > @@ -789,6 +789,10 @@ static void nested_vmcb02_prepare_save(struct vcpu_svm *svm)
> >  
> >  	kvm_set_rflags(vcpu, save->rflags | X86_EFLAGS_FIXED);
> >  
> > +	/* SVM ignores EFER.LMA if EFER.LME=0 (instead of failing VMRUN). */
> > +	if (!(svm->nested.save.efer & EFER_LME))
> > +		svm->nested.save.efer &= ~EFER_LMA;
> > +
> >  	svm_set_efer(vcpu, svm->nested.save.efer);
> >  
> >  	svm_set_cr0(vcpu, svm->nested.save.cr0);
> 
> [Severity: High]
> This is a pre-existing issue, but does this logic only implement half of the
> hardware VMRUN rule? 
> 
> While this correctly fixes the invariant for EFER.LME=0, could a malicious
> guest desynchronize KVM's MMU state from the physical CPU when EFER.LME=1?
> 
> If an L1 guest creates a VMCB12 with EFER.LME=1, CR0.PG=1, but EFER.LMA=0,
> and executes VMRUN, nested_vmcb02_prepare_save() skips the LMA update
> because LME=1, passing LMA=0 to svm_set_efer().
> 
> Then, when we reach svm_set_cr0(), it fails to correct EFER.LMA because it
> expects an edge transition (!is_paging(vcpu)), which is always false since
> L1 must already be paging to execute VMRUN:
> 
> arch/x86/kvm/svm/svm.c:svm_set_cr0() {
>     ...
>     if (vcpu->arch.efer & EFER_LME) {
>         if (!is_paging(vcpu) && (cr0 & X86_CR0_PG)) {
>             vcpu->arch.efer |= EFER_LMA;
>     ...
> }
> 
> Would this allow the guest to trick KVM's instruction emulator into fetching
> arbitrary instructions from a different physical page than what the guest
> actually executed?

Maybe?  But unless it negatively affects the host, I don't think we care?

[PATCH v2 3/5] KVM: x86/mmu: Bug the VM if KVM attempts to walk more levels than the MMU has

[Sean Christopherson]

Extend the "EFER.LMA && !CR4.PAE" check, which exists largely to guard
against KVM configuring a paging32 MMU with more than 2 levels of paging,
with a very explicit check for exactly that: that KVM isn't trying to walk
more levels of paging than the MMU template provides.  I.e. harden KVM
against all bugs that would cause KVM to generates accesses beyond the
bounds of guest_walker's arrays, regardless of how KVM ended up with the
misconfigured MMU.

Note, don't use w->cpu_role.base.level directly as the paging64 template
only provides two levels of page tables for PAE paging on 32-bit hosts, and
handles the third level by manually emulating the PDPTR access.

Cc: stable@vger.kernel.org
Signed-off-by: Sean Christopherson <seanjc@google.com>
---
 arch/x86/kvm/mmu/paging_tmpl.h | 13 +++++++------
 1 file changed, 7 insertions(+), 6 deletions(-)

diff --git a/arch/x86/kvm/mmu/paging_tmpl.h b/arch/x86/kvm/mmu/paging_tmpl.h
index 27427e7f22fa..f925b11d76dd 100644
--- a/arch/x86/kvm/mmu/paging_tmpl.h
+++ b/arch/x86/kvm/mmu/paging_tmpl.h
@@ -368,13 +368,14 @@ static int FNAME(walk_addr_generic)(struct guest_walker *walker,
 	pte_access = ~0;
 
 	/*
-	 * Queue a page fault for injection if this assertion fails, as callers
-	 * assume that walker.fault contains sane info on a walk failure.  I.e.
-	 * avoid making the situation worse by inducing even worse badness
-	 * between when the assertion fails and when KVM kicks the vCPU out to
-	 * userspace (because the VM is bugged).
+	 * Queue a page fault for injection if any of the below assertions fail,
+	 * as callers assume that walker.fault contains sane info on a walk
+	 * failure.  I.e. avoid making the situation worse by inducing even
+	 * worse badness between when the assertion fails and when KVM kicks
+	 * the vCPU out to userspace (because the VM is bugged).
 	 */
-	if (KVM_BUG_ON(is_long_mode(vcpu) && !is_pae(vcpu), vcpu->kvm))
+	if (KVM_BUG_ON(is_long_mode(vcpu) && !is_pae(vcpu), vcpu->kvm) ||
+	    KVM_BUG_ON(walker->max_level > PT_MAX_FULL_LEVELS, vcpu->kvm))
 		goto error;
 
 	++walker->level;
-- 
2.55.0.970.g62bdec98f9-goog

[PATCH v2 4/5] KVM: x86/mmu: Bug the VM if KVM calcs a CPU role with EFER.LMA=1 && CR4.PAE=0

[Sean Christopherson]

Bug the VM if KVM attempts to construct a CPU role with the should-be-
impossible combination of long mode being active without PAE paging being
enabled.  KVM's MMU construction assumes that EFER.LMA can be set if and
only CR4.PAE is set, and will create a completely invalid MMU if that
assumption fails.  FNAME(walk_addr_generic) already has sanity checks to
try and mitigate the fallout, but attempt to catch such bugs earlier, as
this is (at least) the second time KVM has had bugs that escaped into
FNAME(walk_addr_generic), and it's entirely possible the bad state could
cause problems elsewhere.

Cc: stable@vger.kernel.org
Signed-off-by: Sean Christopherson <seanjc@google.com>
---
 arch/x86/kvm/mmu/mmu.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/arch/x86/kvm/mmu/mmu.c b/arch/x86/kvm/mmu/mmu.c
index 064ecc33b926..81c30e2c74f3 100644
--- a/arch/x86/kvm/mmu/mmu.c
+++ b/arch/x86/kvm/mmu/mmu.c
@@ -5910,6 +5910,9 @@ static union kvm_cpu_role kvm_calc_cpu_role(struct kvm_vcpu *vcpu,
 		return role;
 	}
 
+	if (KVM_BUG_ON(____is_efer_lma(regs) && !____is_cr4_pae(regs), vcpu->kvm))
+		*(u64 *)&regs->efer &= ~EFER_LMA;
+
 	role.base.efer_nx = ____is_efer_nx(regs);
 	role.base.cr0_wp = ____is_cr0_wp(regs);
 	role.base.cr4_smep = ____is_cr4_smep(regs);
-- 
2.55.0.970.g62bdec98f9-goog

Re: [PATCH v2 4/5] KVM: x86/mmu: Bug the VM if KVM calcs a CPU role with EFER.LMA=1 && CR4.PAE=0

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Modifying a const-qualified struct member via pointer cast invokes undefined behavior, and the compiler may optimize away subsequent checks, defeating the bug mitigation.
--

commit a1c4dd28227e88b096d9ee6618a2543aab000da4
Author: Sean Christopherson <seanjc@google.com>

KVM: x86/mmu: Bug the VM if KVM calcs a CPU role with EFER.LMA=1 && CR4.PAE=0

This commit adds a KVM_BUG_ON check to catch invalid CPU roles where long
mode is active but PAE paging is not. It attempts to mitigate the fallout by
clearing the EFER_LMA bit in the provided registers before continuing MMU
construction.

> diff --git a/arch/x86/kvm/mmu/mmu.c b/arch/x86/kvm/mmu/mmu.c
> index 064ecc33b9267..81c30e2c74f3c 100644
> --- a/arch/x86/kvm/mmu/mmu.c
> +++ b/arch/x86/kvm/mmu/mmu.c
> @@ -5910,6 +5910,9 @@ static union kvm_cpu_role kvm_calc_cpu_role(struct kvm_vcpu *vcpu,
>  		return role;
>  	}
>  
> +	if (KVM_BUG_ON(____is_efer_lma(regs) && !____is_cr4_pae(regs), vcpu->kvm))
> +		*(u64 *)&regs->efer &= ~EFER_LMA;

[Severity: High]
Does modifying a const-qualified struct member via a pointer cast invoke
undefined behavior here?

Looking at the definition of struct kvm_mmu_role_regs:

struct kvm_mmu_role_regs {
	const unsigned long cr0;
	const unsigned long cr4;
	const u64 efer;
};

Since the efer member is explicitly declared as const, could the compiler
assume it never changes after initialization and optimize away the clearing 
of EFER_LMA for the subsequent checks in kvm_calc_cpu_role()? 

If the compiler uses a cached value for the remaining efer checks, it seems
this would bypass the intended bug mitigation entirely and still construct a
completely invalid MMU role.

> +
>  	role.base.efer_nx = ____is_efer_nx(regs);
>  	role.base.cr0_wp = ____is_cr0_wp(regs);
>  	role.base.cr4_smep = ____is_cr4_smep(regs);

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

[PATCH v2 5/5] KVM: x86/mmu: Convert MMU walker's bounds check from BUG_ON() to KVM_BUG_ON()

[Sean Christopherson]

Bug the VM, not the host, if KVM's sanity check that walking guest PTEs
doesn't underflow the walker's level fires.  Bugging the host while holding
mmu_lock is all but guaranteed to panic the host, KVM hasn't _yet_ consumed
the out-of-bounds level (i.e. hasn't corrupted memory), and KVM is already
committed to bugging the VM and synthesizing a guest page fault if a fatal
MMU error occurs while walking guest PTEs.  I.e. there's no reason to keep
the BUG_ON() at this point.

Signed-off-by: Sean Christopherson <seanjc@google.com>
---
 arch/x86/kvm/mmu/paging_tmpl.h | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/arch/x86/kvm/mmu/paging_tmpl.h b/arch/x86/kvm/mmu/paging_tmpl.h
index f925b11d76dd..c8ec47b09264 100644
--- a/arch/x86/kvm/mmu/paging_tmpl.h
+++ b/arch/x86/kvm/mmu/paging_tmpl.h
@@ -392,7 +392,9 @@ static int FNAME(walk_addr_generic)(struct guest_walker *walker,
 		offset    = index * sizeof(pt_element_t);
 		pte_gpa   = gfn_to_gpa(table_gfn) + offset;
 
-		BUG_ON(walker->level < 1);
+		if (KVM_BUG_ON(walker->level < 1, vcpu->kvm))
+			goto error;
+
 		walker->table_gfn[walker->level - 1] = table_gfn;
 		walker->pte_gpa[walker->level - 1] = pte_gpa;
 
-- 
2.55.0.970.g62bdec98f9-goog