[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

[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

[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

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.