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

[PATCH 0/4] 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).

Sean Christopherson (4):
  KVM: nSVM: Reject KVM_SET_NESTED_STATE if L1 has EFER.LMA=1 &&
    EFER.LME=0
  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      |  1 +
 3 files changed, 14 insertions(+), 7 deletions(-)


base-commit: 76671054f9a1ff6abb976583cd8da37650acdc97
-- 
2.55.0.887.g758fc8c411-goog

[PATCH 1/4] 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.887.g758fc8c411-goog

[PATCH 2/4] 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.

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..46a0f7796e55 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(w->cpu_role.base.level > PT_MAX_FULL_LEVELS, vcpu->kvm))
 		goto error;
 
 	++walker->level;
-- 
2.55.0.887.g758fc8c411-goog

[PATCH 3/4] 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.887.g758fc8c411-goog

[PATCH 4/4] 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 46a0f7796e55..2cf5e733aba4 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.887.g758fc8c411-goog

Re: [PATCH 3/4] 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 structure field via a pointer cast invokes Undefined Behavior, allowing the compiler to optimize out the mitigation and leaving KVM vulnerable to processing corrupted MMU states.
--

commit 398614ee03891692ae2eecdde307edc043d7403e
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 attempts to mitigate invalid MMU roles by bugging the VM and
clearing EFER.LMA if long mode is active without PAE paging enabled.

> 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 casting away the const qualifier and modifying regs->efer here invoke
undefined behavior?

The regs pointer points to a struct kvm_mmu_role_regs, which defines the
efer field as const:

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

Modifying a const-qualified object invokes undefined behavior per C11 6.7.3p6.
Because the regs structure is typically allocated locally on the stack, for
example via vcpu_to_role_regs, and passed to kvm_calc_cpu_role, modern
compilers have full visibility into the object's definition and can cache the
initial const value of regs.efer in a register.

If the compiler evaluates subsequent checks like ____is_efer_lma(regs) using
this cached value, could this completely bypass the sanitization? 

If an untrusted guest attempts to set its vCPU state with EFER.LMA=1 but
CR4.PAE=0, this bypass could cause KVM to compute a corrupted MMU role, such
as configuring 64-bit levels but 32-bit PTE sizes, which then escapes into
the MMU configuration.

While KVM_BUG_ON flags the VM as bugged, the function continues executing
with the corrupted role, potentially leading to out-of-bounds accesses before
the VM is fully halted.

This issue defeats the purpose of the security mitigation and is not fixed by
the subsequent commit "KVM: x86/mmu: Convert MMU walker's bounds check from
BUG_ON() to KVM_BUG_ON()".

> +
>  	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/20260826211844.884951-1-seanjc@google.com?part=3

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

[sashiko-bot]

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

Pre-existing issues:
- [High] svm_set_nested_state() leaves the vCPU in an inconsistent nested state if nested_svm_load_cr3() fails.
--

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

KVM: nSVM: Reject KVM_SET_NESTED_STATE if L1 has EFER.LMA=1 && EFER.LME=0

Reject KVM_SET_NESTED_STATE if the incoming L1 host state has an
impossible EFER combination of LMA=1 but LME=0. This prevents KVM from
consuming the illegal state when constructing the shadow MMU for L2.

> diff --git a/arch/x86/kvm/svm/nested.c b/arch/x86/kvm/svm/nested.c
> index 73f37b050d0a0..49fb10ad1f9f6 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;
>  

[Severity: High]
This isn't a bug introduced by this patch, but looking further down in
svm_set_nested_state(), does this code leave the vCPU in an inconsistent
state if nested_svm_load_cr3() fails?

arch/x86/kvm/svm/nested.c:svm_set_nested_state() {
...
	ret = nested_svm_load_cr3(&svm->vcpu, vcpu->arch.cr3,
				  nested_npt_enabled(svm), false);
	if (ret)
		goto out_free;
...
}

If nested_svm_load_cr3() fails, the function jumps to out_free and returns
an error without unwinding the guest mode state transitions that occurred
earlier in the function.

The vCPU is left in guest mode with svm->vmcb pointing to vmcb02, but the
MMU is not re-initialized for the nested guest because it returns before
calling kvm_init_mmu().

Will a subsequent KVM_RUN ioctl attempt to execute the vCPU with mismatched
VMCB and MMU state?

Could an unprivileged host userspace process trigger this intentionally to
corrupt KVM internal state and crash the host?

For example, could userspace call KVM_SET_CPUID to set a high MAXPHYADDR,
call KVM_SET_SREGS with a CR3 near that limit, and then call KVM_SET_CPUID
again to lower MAXPHYADDR? This would make the previously set vcpu->arch.cr3
illegal, causing nested_svm_load_cr3() to fail its kvm_vcpu_is_legal_cr3()
check and return -EINVAL during KVM_SET_NESTED_STATE.

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

Re: [PATCH 2/4] KVM: x86/mmu: Bug the VM if KVM attempts to walk more levels than the MMU has

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] The new bounds check falsely triggers KVM_BUG_ON for valid PAE page walks on 32-bit KVM hosts.
--

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

KVM: x86/mmu: Bug the VM if KVM attempts to walk more levels than the MMU has

This commit extends the paging levels check to ensure KVM doesn't attempt to
walk more levels than the MMU template provides, preventing out-of-bounds
array accesses.

> diff --git a/arch/x86/kvm/mmu/paging_tmpl.h b/arch/x86/kvm/mmu/paging_tmpl.h
> index 27427e7f22fa0..46a0f7796e55a 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(w->cpu_role.base.level > PT_MAX_FULL_LEVELS, vcpu->kvm))

[Severity: High]
Does this new check falsely trigger KVM_BUG_ON for valid PAE page walks on
32-bit KVM hosts?

When a PAE guest runs on a 32-bit KVM host (!CONFIG_X86_64), KVM assigns
w->cpu_role.base.level = PT32E_ROOT_LEVEL (3). During a page walk,
walk_addr_generic() is compiled for PTTYPE == 64. In this context,
PT_MAX_FULL_LEVELS is defined as 2.

The check w->cpu_role.base.level > PT_MAX_FULL_LEVELS evaluates to 3 > 2,
which unconditionally bugs the VM.

The array accesses in the subsequent loop are safe because walker->level is
explicitly decremented from 3 to 2 in the PAE-specific block earlier in the
function, ensuring the max_level used for array indexing does not exceed 2.

Should this check use the adjusted walker->max_level instead of the original
unmodified w->cpu_role.base.level?

>  		goto error;
>  
>  	++walker->level;

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

Re: [PATCH 2/4] KVM: x86/mmu: Bug the VM if KVM attempts to walk more levels than the MMU has

[Sean Christopherson]

On Wed, Aug 26, 2026, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [High] The new bounds check falsely triggers KVM_BUG_ON for valid PAE page walks on 32-bit KVM hosts.
> --
> 
> commit c8c785606f8b5ac435db8c42d576eaf8f88c2fcd
> Author: Sean Christopherson <seanjc@google.com>
> 
> KVM: x86/mmu: Bug the VM if KVM attempts to walk more levels than the MMU has
> 
> This commit extends the paging levels check to ensure KVM doesn't attempt to
> walk more levels than the MMU template provides, preventing out-of-bounds
> array accesses.
> 
> > diff --git a/arch/x86/kvm/mmu/paging_tmpl.h b/arch/x86/kvm/mmu/paging_tmpl.h
> > index 27427e7f22fa0..46a0f7796e55a 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(w->cpu_role.base.level > PT_MAX_FULL_LEVELS, vcpu->kvm))
> 
> [Severity: High]
> Does this new check falsely trigger KVM_BUG_ON for valid PAE page walks on
> 32-bit KVM hosts?

Argh!  I missed/forgot that KVM only configures paging64 for 2 levels.

#if PTTYPE == 64
	#define pt_element_t u64
	#define guest_walker guest_walker64
	#define FNAME(name) paging##64_##name
	#define PT_LEVEL_BITS 9
	#define PT_GUEST_DIRTY_SHIFT PT_DIRTY_SHIFT
	#define PT_GUEST_ACCESSED_SHIFT PT_ACCESSED_SHIFT
	#define PT_HAVE_ACCESSED_DIRTY(mmu) true
	#ifdef CONFIG_X86_64
	#define PT_MAX_FULL_LEVELS PT64_ROOT_MAX_LEVEL
	#else
	#define PT_MAX_FULL_LEVELS 2   <=========================
	#endif

I tested a 32-bit PAE guest, but only on a 64-bit host.  I didn't test a 32-bit
PAE guest on a 32-bit host.  Lame.

  ------------[ cut here ]------------
  WARNING: arch/x86/kvm/mmu/paging_tmpl.h:378 at paging64_walk_addr_generic+0x422/0x950 [kvm], CPU#3: CPU 0/KVM/2511
  Modules linked in: vhost_net vhost vhost_iotlb tap kvm_intel kvm irqbypass
  CPU: 3 UID: 1000 PID: 2511 Comm: CPU 0/KVM Not tainted 7.2.0-rc7-cc46e34b6df0-x86_nested_npt_lma_pae-pae #7 PREEMPT 
  Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS 0.0.0 02/06/2015
  EIP: paging64_walk_addr_generic+0x422/0x950 [kvm]
  Call Trace:
   paging64_page_fault+0x51/0x870 [kvm]
   kvm_mmu_do_page_fault+0xfc/0x220 [kvm]
   kvm_mmu_page_fault+0xa2/0x770 [kvm]
   kvm_handle_page_fault+0x6c/0x130 [kvm]
   handle_exception_nmi+0x50f/0x610 [kvm_intel]
   vmx_handle_exit+0x18f/0x5b0 [kvm_intel]
   kvm_arch_vcpu_ioctl_run+0xc71/0x1c00 [kvm]
   kvm_vcpu_ioctl+0x2c5/0x960 [kvm]
   __ia32_sys_ioctl+0xff/0x960
   ia32_sys_call+0xad5/0xc30
   __do_fast_syscall_32+0x5b/0x500
   do_fast_syscall_32+0x2b/0x60
   do_SYSENTER_32+0x17/0x20
   entry_SYSENTER_32+0x98/0xf9
  ---[ end trace 0000000000000000 ]---

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

[Yosry Ahmed]

On Wed, Aug 26, 2026 at 2:18 PM Sean Christopherson <seanjc@google.com> wrote:
>
> 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.

Isn't the "right" thing to do what hardware (presumably) does and
ignore EFER.LMA if EFER.LME=0?

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

I just realized I have no idea why we check X86_CR0_PE and
X86_EFLAGS_VM here. Commit 6906e06db9b04 ("KVM: nSVM: Add missing
checks for reserved bits to svm_set_nested_state()") says it's to do
the same checks as VMRUN, but I don't think that's actually the case?
The checks here seem arbitrary to me?

>             !nested_vmcb_check_save(vcpu, &save_cached, false))
>                 goto out_free;
>
> --
> 2.55.0.887.g758fc8c411-goog
>

Re: [PATCH 2/4] KVM: x86/mmu: Bug the VM if KVM attempts to walk more levels than the MMU has

[Yosry Ahmed]

On Wed, Aug 26, 2026 at 2:18 PM Sean Christopherson <seanjc@google.com> wrote:
>
> 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,

Is there value in keeping the original check? I suppose yes as you
said it exists "largely" for this, but it would probably be nice to
spell it out (if we know).

> 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.
>
> 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..46a0f7796e55 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(w->cpu_role.base.level > PT_MAX_FULL_LEVELS, vcpu->kvm))
>                 goto error;
>
>         ++walker->level;
> --
> 2.55.0.887.g758fc8c411-goog
>

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

[Yosry Ahmed]

On Wed, Aug 26, 2026 at 2:18 PM Sean Christopherson <seanjc@google.com> wrote:
>
> 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))

Can we shove this into the existing if (____is_efer_lma(regs)) below?

> +               *(u64 *)&regs->efer &= ~EFER_LMA;

Why do this if we will crash the VM anyway (and Sashiko doesn't like it)?

> +
>         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.887.g758fc8c411-goog
>

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

[Yosry Ahmed]

On Wed, Aug 26, 2026 at 2:18 PM Sean Christopherson <seanjc@google.com> wrote:
>
> 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>

Reviewed-by: Yosry Ahmed <yosry@kernel.org>

> ---
>  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 46a0f7796e55..2cf5e733aba4 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.887.g758fc8c411-goog
>

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

[Sean Christopherson]

On Thu, Aug 27, 2026, Yosry Ahmed wrote:
> On Wed, Aug 26, 2026 at 2:18 PM Sean Christopherson <seanjc@google.com> wrote:
> >
> > 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.
> 
> Isn't the "right" thing to do what hardware (presumably) does and
> ignore EFER.LMA if EFER.LME=0?

No, because (a) this is KVM uAPI, not emulation of hardware, and (b) it's a check
on L1 state, not L2 state.  It should be impossible for L1 state to have this
combination through "natural" means, and so a snapshot provided by KVM should
never have this combo either, which there's zero reason to allow userspace to
provide garbage.

> > 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)) ||
> 
> I just realized I have no idea why we check X86_CR0_PE and
> X86_EFLAGS_VM here. Commit 6906e06db9b04 ("KVM: nSVM: Add missing
> checks for reserved bits to svm_set_nested_state()") says it's to do
> the same checks as VMRUN, but I don't think that's actually the case?
> The checks here seem arbitrary to me?

Again, this is L1 state when L2 is active (the !KVM_STATE_NESTED_GUEST_MODE path
has already bailed), and VMRUN "can only be executed in protected mode with SVM
enabled".  Amusingly, the APM says #VMEXIT "Forces CR0.PE = 1, RFLAGS.VM = 0.",
so I guess it means business.

If anything is wrong, it's the CR0.PG check.  Presumably that got carried forward
from commit c0725420cfdc ("KVM: SVM: Add helper functions for nested SVM").  I
don't see anything in the APM that requires paging to be enabled, and nothing in
that ancient series points at concrete documentation either.

Re: [PATCH 2/4] KVM: x86/mmu: Bug the VM if KVM attempts to walk more levels than the MMU has

[Sean Christopherson]

On Thu, Aug 27, 2026, Yosry Ahmed wrote:
> On Wed, Aug 26, 2026 at 2:18 PM Sean Christopherson <seanjc@google.com> wrote:
> >
> > 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,
> 
> Is there value in keeping the original check? I suppose yes as you
> said it exists "largely" for this, but it would probably be nice to
> spell it out (if we know).

Because LMA && !PAE is still an illegal combination.  The symptom we know about
is a misconfiguration of base.level vs. the paging template, but I also want to
guard against any other symptoms that stem from the bad state.

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

[Sean Christopherson]

On Thu, Aug 27, 2026, Yosry Ahmed wrote:
> On Wed, Aug 26, 2026 at 2:18 PM Sean Christopherson <seanjc@google.com> wrote:
> >
> > 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))
> 
> Can we shove this into the existing if (____is_efer_lma(regs)) below?

No, because there are three more checks on EFER.LMA:

	role.ext.cr4_pke = ____is_efer_lma(regs) && ____is_cr4_pke(regs);
	role.ext.cr4_la57 = ____is_efer_lma(regs) && ____is_cr4_la57(regs);
	role.ext.efer_lma = ____is_efer_lma(regs);

and I don't want to have to condition them all on something that shouldn't happen.

OMG, I hate SVM.  I resurrected the selftest hack I used to verify this bug, to
demonstrate that Sashiko's "technically that's undefined behavior and this is
useless" complaint is wrong, because even though it's undefined behavior and the
compiler *could* ignore the change, in practice the compiler probably won't ignore
the change.  And since this is defense-in-depth, it's "fine" if the paranoid
hardening only isn't guaranteed to kick in.

And in doing so managed to trip this KVM_BUG_ON() in *L0* when running the test
in L1, because as you kinda sorta noted in patch 1, KVM doesn't ignore EFER.LMA
when loading L2 state.

I had actually tried to do exactly that, by having nested_vmcb_check_save() clear
EFER.LMA if EFER.LME=0, but that doesn't work because svm_set_nested_state() uses
the "cache" only for the checks, not for the actual loading of state.  *sigh*

So in addition to patch 1, we also need this to guard against configuring L2's
walk_mmu with bad state.

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);

Anyways, back to Sashiko's "technically this is wrong" statement, I confirmed
that tweaking the code to do this does NOT trigger the KVM_BUG_ON() with at least
clang-21.  I.e. my assertion that clearing regs->efer.LMA could be useful holds
true.

diff --git a/arch/x86/kvm/mmu/mmu.c b/arch/x86/kvm/mmu/mmu.c
index b515a49c5e86..cab8690d0fa0 100644
--- a/arch/x86/kvm/mmu/mmu.c
+++ b/arch/x86/kvm/mmu/mmu.c
@@ -5932,8 +5932,10 @@ 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))
+	if (____is_efer_lma(regs) && !____is_cr4_pae(regs)) {
+		pr_warn("Forcing EFER.LMA=0 in calc CPU role\n");
 		*(u64 *)&regs->efer &= ~EFER_LMA;
+	}
 
 	role.base.efer_nx = ____is_efer_nx(regs);
 	role.base.cr0_wp = ____is_cr0_wp(regs);
@@ -5941,6 +5943,8 @@ static union kvm_cpu_role kvm_calc_cpu_role(struct kvm_vcpu *vcpu,
 	role.base.smap_andnot_wp = ____is_cr4_smap(regs) && !____is_cr0_wp(regs);
 	role.base.has_4_byte_gpte = !____is_cr4_pae(regs);
 
+	KVM_BUG_ON(____is_efer_lma(regs) && !____is_cr4_pae(regs), vcpu->kvm);
+
 	if (____is_efer_lma(regs))
 		role.base.level = ____is_cr4_la57(regs) ? PT64_ROOT_5LEVEL
 							: PT64_ROOT_4LEVEL;

Side topic, I also (inadvertantly) somewhat justified keeping the

	if (KVM_BUG_ON(is_long_mode(vcpu) && !is_pae(vcpu), vcpu->kvm) ||

check in FNAME(walk_addr_generic) when testing the above.  If KVM manages to
configure a sane MMU, but still has a vCPU with the above state, then we still
want to WARN and bail.

> > +               *(u64 *)&regs->efer &= ~EFER_LMA;
> 
> Why do this if we will crash the VM anyway (and Sashiko doesn't like it)?

Because there's a lot of code between here and checking KVM_VM_DEAD in
vcpu_enter_guest().  And has been proven far too many times this year, detecting
a flaw doesn't automagically mitigate true badness.

> > +
> >         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.887.g758fc8c411-goog
> >

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

[Yosry Ahmed]

On Thu, Aug 27, 2026 at 6:37 AM Sean Christopherson <seanjc@google.com> wrote:
>
> On Thu, Aug 27, 2026, Yosry Ahmed wrote:
> > On Wed, Aug 26, 2026 at 2:18 PM Sean Christopherson <seanjc@google.com> wrote:
> > >
> > > 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.
> >
> > Isn't the "right" thing to do what hardware (presumably) does and
> > ignore EFER.LMA if EFER.LME=0?
>
> No, because (a) this is KVM uAPI, not emulation of hardware, and (b) it's a check
> on L1 state, not L2 state.  It should be impossible for L1 state to have this
> combination through "natural" means, and so a snapshot provided by KVM should
> never have this combo either, which there's zero reason to allow userspace to
> provide garbage.

Right, I understand that KVM can do whatever it wants here. I was
wondering if we wanted to make the uAPI behavior match the  VMRUN
behavior, but I guess we're free to make it more strict.

>
> > > 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)) ||
> >
> > I just realized I have no idea why we check X86_CR0_PE and
> > X86_EFLAGS_VM here. Commit 6906e06db9b04 ("KVM: nSVM: Add missing
> > checks for reserved bits to svm_set_nested_state()") says it's to do
> > the same checks as VMRUN, but I don't think that's actually the case?
> > The checks here seem arbitrary to me?
>
> Again, this is L1 state when L2 is active (the !KVM_STATE_NESTED_GUEST_MODE path
> has already bailed), and VMRUN "can only be executed in protected mode with SVM
> enabled".  Amusingly, the APM says #VMEXIT "Forces CR0.PE = 1, RFLAGS.VM = 0.",
> so I guess it means business.
>
> If anything is wrong, it's the CR0.PG check.  Presumably that got carried forward
> from commit c0725420cfdc ("KVM: SVM: Add helper functions for nested SVM").  I
> don't see anything in the APM that requires paging to be enabled, and nothing in
> that ancient series points at concrete documentation either.

Oh yeah you're right, for some reason I thought it was paging not
protected mode. Well then, it seems like
nested_svm_check_permissions() is also incorrectly checking paging as
well, seems like both checks are incorrect? Also, I don't see anything
in the APM about checking RFLAGS.VM before VMRUN.

If the goal here is to keep the checks here consistent with
nested_svm_check_permissions(), aside from the new EFER check, then
maybe we should also check CPL here?

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

[Yosry Ahmed]

On Thu, Aug 27, 2026 at 7:57 AM Sean Christopherson <seanjc@google.com> wrote:
>
> On Thu, Aug 27, 2026, Yosry Ahmed wrote:
> > On Wed, Aug 26, 2026 at 2:18 PM Sean Christopherson <seanjc@google.com> wrote:
> > >
> > > 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))
> >
> > Can we shove this into the existing if (____is_efer_lma(regs)) below?
>
> No, because there are three more checks on EFER.LMA:
>
>         role.ext.cr4_pke = ____is_efer_lma(regs) && ____is_cr4_pke(regs);
>         role.ext.cr4_la57 = ____is_efer_lma(regs) && ____is_cr4_la57(regs);
>         role.ext.efer_lma = ____is_efer_lma(regs);
>
> and I don't want to have to condition them all on something that shouldn't happen.

Yeah I assumed that we don't care about the state anymore if we'll
KVM_BUG_ON(), but apparently that's not the case based on your comment
below.

>
> OMG, I hate SVM.  I resurrected the selftest hack I used to verify this bug, to
> demonstrate that Sashiko's "technically that's undefined behavior and this is
> useless" complaint is wrong, because even though it's undefined behavior and the
> compiler *could* ignore the change, in practice the compiler probably won't ignore
> the change.  And since this is defense-in-depth, it's "fine" if the paranoid
> hardening only isn't guaranteed to kick in.
>
> And in doing so managed to trip this KVM_BUG_ON() in *L0* when running the test
> in L1, because as you kinda sorta noted in patch 1, KVM doesn't ignore EFER.LMA
> when loading L2 state.
>
> I had actually tried to do exactly that, by having nested_vmcb_check_save() clear
> EFER.LMA if EFER.LME=0, but that doesn't work because svm_set_nested_state() uses
> the "cache" only for the checks, not for the actual loading of state.  *sigh*
>
> So in addition to patch 1, we also need this to guard against configuring L2's
> walk_mmu with bad state.

Hmm wouldn't it be simpler at this point to let KVM_SET_NESTED_STATE
and nested VMRUN have the invalid LMA/LME combination and just ignore
EFER.LMA if EFER.LME (or just always check EFER.LMA && EFER.LME)?

>
> 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);
>
> Anyways, back to Sashiko's "technically this is wrong" statement, I confirmed
> that tweaking the code to do this does NOT trigger the KVM_BUG_ON() with at least
> clang-21.  I.e. my assertion that clearing regs->efer.LMA could be useful holds
> true.
>
> diff --git a/arch/x86/kvm/mmu/mmu.c b/arch/x86/kvm/mmu/mmu.c
> index b515a49c5e86..cab8690d0fa0 100644
> --- a/arch/x86/kvm/mmu/mmu.c
> +++ b/arch/x86/kvm/mmu/mmu.c
> @@ -5932,8 +5932,10 @@ 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))
> +       if (____is_efer_lma(regs) && !____is_cr4_pae(regs)) {
> +               pr_warn("Forcing EFER.LMA=0 in calc CPU role\n");
>                 *(u64 *)&regs->efer &= ~EFER_LMA;
> +       }
>
>         role.base.efer_nx = ____is_efer_nx(regs);
>         role.base.cr0_wp = ____is_cr0_wp(regs);
> @@ -5941,6 +5943,8 @@ static union kvm_cpu_role kvm_calc_cpu_role(struct kvm_vcpu *vcpu,
>         role.base.smap_andnot_wp = ____is_cr4_smap(regs) && !____is_cr0_wp(regs);
>         role.base.has_4_byte_gpte = !____is_cr4_pae(regs);
>
> +       KVM_BUG_ON(____is_efer_lma(regs) && !____is_cr4_pae(regs), vcpu->kvm);
> +
>         if (____is_efer_lma(regs))
>                 role.base.level = ____is_cr4_la57(regs) ? PT64_ROOT_5LEVEL
>                                                         : PT64_ROOT_4LEVEL;
>
> Side topic, I also (inadvertantly) somewhat justified keeping the
>
>         if (KVM_BUG_ON(is_long_mode(vcpu) && !is_pae(vcpu), vcpu->kvm) ||
>
> check in FNAME(walk_addr_generic) when testing the above.  If KVM manages to
> configure a sane MMU, but still has a vCPU with the above state, then we still
> want to WARN and bail.

Good point.

>
> > > +               *(u64 *)&regs->efer &= ~EFER_LMA;
> >
> > Why do this if we will crash the VM anyway (and Sashiko doesn't like it)?
>
> Because there's a lot of code between here and checking KVM_VM_DEAD in
> vcpu_enter_guest().  And has been proven far too many times this year, detecting
> a flaw doesn't automagically mitigate true badness.

Interesting, I always assumed we can do whatever we want after KVM_BUG_ON() :P

>
> > > +
> > >         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.887.g758fc8c411-goog
> > >

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

[Sean Christopherson]

On Thu, Aug 27, 2026, Yosry Ahmed wrote:
> On Thu, Aug 27, 2026 at 7:57 AM Sean Christopherson <seanjc@google.com> wrote:
> > > Can we shove this into the existing if (____is_efer_lma(regs)) below?
> >
> > No, because there are three more checks on EFER.LMA:
> >
> >         role.ext.cr4_pke = ____is_efer_lma(regs) && ____is_cr4_pke(regs);
> >         role.ext.cr4_la57 = ____is_efer_lma(regs) && ____is_cr4_la57(regs);
> >         role.ext.efer_lma = ____is_efer_lma(regs);
> >
> > and I don't want to have to condition them all on something that shouldn't happen.
> 
> Yeah I assumed that we don't care about the state anymore if we'll
> KVM_BUG_ON(), but apparently that's not the case based on your comment below.

Ya, it's not an immediate "jump all the way back to userspace", though that would
be kinda cool/terrifying.

> > OMG, I hate SVM.  I resurrected the selftest hack I used to verify this bug, to
> > demonstrate that Sashiko's "technically that's undefined behavior and this is
> > useless" complaint is wrong, because even though it's undefined behavior and the
> > compiler *could* ignore the change, in practice the compiler probably won't ignore
> > the change.  And since this is defense-in-depth, it's "fine" if the paranoid
> > hardening only isn't guaranteed to kick in.
> >
> > And in doing so managed to trip this KVM_BUG_ON() in *L0* when running the test
> > in L1, because as you kinda sorta noted in patch 1, KVM doesn't ignore EFER.LMA
> > when loading L2 state.
> >
> > I had actually tried to do exactly that, by having nested_vmcb_check_save() clear
> > EFER.LMA if EFER.LME=0, but that doesn't work because svm_set_nested_state() uses
> > the "cache" only for the checks, not for the actual loading of state.  *sigh*
> >
> > So in addition to patch 1, we also need this to guard against configuring L2's
> > walk_mmu with bad state.
> 
> Hmm wouldn't it be simpler at this point to let KVM_SET_NESTED_STATE
> and nested VMRUN have the invalid LMA/LME combination and just ignore
> EFER.LMA if EFER.LME

Definitely not a straight "ignore", because that would end up being an even worse
game of whack-a-mole, because very path that checks vcpu->arch.efer would have to
account for that possibility.

We could forcefully sanitize EFER in flows that write EFER, but (a) that's still
a (must smaller) game of whack-a-mole and (b) it would actively hide KVM bugs for
flows that are supposed to reject the invalid state.  And if we WARNed to address
(b), we'll be right back where we are today: playing whack-a-mole to prevent the
WARN from being triggered.

> (or just always check EFER.LMA && EFER.LME)?

No can do, because we can't disallow the combination for L2 on VMRUN without
violating AMD's architecture.  And practically speaking, we *are* doing that,
just in a bunch of places because there's no one rule to rule them all.

> > Because there's a lot of code between here and checking KVM_VM_DEAD in
> > vcpu_enter_guest().  And has been proven far too many times this year, detecting
> > a flaw doesn't automagically mitigate true badness.
> 
> Interesting, I always assumed we can do whatever we want after KVM_BUG_ON() :P

Nope.  In addition to KVM_VM_DEAD not being checked until vcpu_enter_guest(),
more broadly it only kicks in for cross-task behaviors on the next ioctl.  E.g.
if KVM_BUG_ON() guards against bad VM state, as opposed to bad vCPU state, i.e.
if *other* vCPUs could consume the bad state, then it's especially important to
take evasive action.

KVM_BUG_ON() is as much about protecting the guest as it is about protecting the
host.  E.g. if KVM *knows* it fatally screwed up, then continuing to run the
guest risks corrupting guest state and thus causing far worse problems than DoSing
the guest.

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

[Sean Christopherson]

On Thu, Aug 27, 2026, Yosry Ahmed wrote:
> On Thu, Aug 27, 2026 at 6:37 AM Sean Christopherson <seanjc@google.com> wrote:
> >
> > On Thu, Aug 27, 2026, Yosry Ahmed wrote:
> > > On Wed, Aug 26, 2026 at 2:18 PM Sean Christopherson <seanjc@google.com> wrote:
> > > >
> > > > 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.
> > >
> > > Isn't the "right" thing to do what hardware (presumably) does and
> > > ignore EFER.LMA if EFER.LME=0?
> >
> > No, because (a) this is KVM uAPI, not emulation of hardware, and (b) it's a check
> > on L1 state, not L2 state.  It should be impossible for L1 state to have this
> > combination through "natural" means, and so a snapshot provided by KVM should
> > never have this combo either, which there's zero reason to allow userspace to
> > provide garbage.
> 
> Right, I understand that KVM can do whatever it wants here. I was wondering
> if we wanted to make the uAPI behavior match the  VMRUN behavior, but I guess
> we're free to make it more strict.

But again, this *does* match VMRUN behavior, because it's impossible for L1 to
have EFER.LMA=1, EFER.LME=0, and EFER.PAE=0 at the time of VMRUN.

> > > > 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)) ||
> > >
> > > I just realized I have no idea why we check X86_CR0_PE and
> > > X86_EFLAGS_VM here. Commit 6906e06db9b04 ("KVM: nSVM: Add missing
> > > checks for reserved bits to svm_set_nested_state()") says it's to do
> > > the same checks as VMRUN, but I don't think that's actually the case?
> > > The checks here seem arbitrary to me?
> >
> > Again, this is L1 state when L2 is active (the !KVM_STATE_NESTED_GUEST_MODE path
> > has already bailed), and VMRUN "can only be executed in protected mode with SVM
> > enabled".  Amusingly, the APM says #VMEXIT "Forces CR0.PE = 1, RFLAGS.VM = 0.",
> > so I guess it means business.
> >
> > If anything is wrong, it's the CR0.PG check.  Presumably that got carried forward
> > from commit c0725420cfdc ("KVM: SVM: Add helper functions for nested SVM").  I
> > don't see anything in the APM that requires paging to be enabled, and nothing in
> > that ancient series points at concrete documentation either.
> 
> Oh yeah you're right, for some reason I thought it was paging not
> protected mode. Well then, it seems like
> nested_svm_check_permissions() is also incorrectly checking paging as
> well, seems like both checks are incorrect? Also, I don't see anything
> in the APM about checking RFLAGS.VM before VMRUN.

Presumably it's covered by the !PROTECTED_MODE clause.

  IF ((MSR_EFER.SVME == 0) || (!PROTECTED_MODE))  // This instruction can only be executed in protected
    EXCEPTION [#UD]                               // mode with SVM enabled

Section "1.3.4 Legacy Modes" describes "Protected Mode" and "Virtual-8086 Mode"
as separate submodes.  And the tables for most instructions differentiate between
Real, Virtual 8086, and Protected modes when enumerating exceptions.

The APM weasels around 64-bit mode by also saying "Before enabling and activating
long mode, system software must first enable protected mode".  E.g. the table for
SYSENTER describes the #UD due to "This instruction is not recognized in long mode"
as scenario that's unique to Protected Mode.

Stating the obvious, it would be lovely if the APM explicitly stated what the
exact checks are, though that's about as likely as AMD gifting me a pony.

FWIW, if that reading is wrong (though I'm pretty sure it's not), then the only
issue is that KVM is synthesizing #UD instead of #GP, because Virtual 8086 always
runs at CPL=3.

  IF (CPL != 0)                                   // This instruction is only allowed at CPL 0
    EXCEPTION [#GP]


> If the goal here is to keep the checks here consistent with
> nested_svm_check_permissions(), aside from the new EFER check, then
> maybe we should also check CPL here?

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

[Yosry Ahmed]

> > > OMG, I hate SVM.  I resurrected the selftest hack I used to verify this bug, to
> > > demonstrate that Sashiko's "technically that's undefined behavior and this is
> > > useless" complaint is wrong, because even though it's undefined behavior and the
> > > compiler *could* ignore the change, in practice the compiler probably won't ignore
> > > the change.  And since this is defense-in-depth, it's "fine" if the paranoid
> > > hardening only isn't guaranteed to kick in.
> > >
> > > And in doing so managed to trip this KVM_BUG_ON() in *L0* when running the test
> > > in L1, because as you kinda sorta noted in patch 1, KVM doesn't ignore EFER.LMA
> > > when loading L2 state.
> > >
> > > I had actually tried to do exactly that, by having nested_vmcb_check_save() clear
> > > EFER.LMA if EFER.LME=0, but that doesn't work because svm_set_nested_state() uses
> > > the "cache" only for the checks, not for the actual loading of state.  *sigh*
> > >
> > > So in addition to patch 1, we also need this to guard against configuring L2's
> > > walk_mmu with bad state.
> >
> > Hmm wouldn't it be simpler at this point to let KVM_SET_NESTED_STATE
> > and nested VMRUN have the invalid LMA/LME combination and just ignore
> > EFER.LMA if EFER.LME
>
> Definitely not a straight "ignore", because that would end up being an even worse
> game of whack-a-mole, because very path that checks vcpu->arch.efer would have to
> account for that possibility.

All paths that check EFER.LMA/EFER.LME only, right?

Don't get me wrong, it's still a lot, but I was hoping it's something
a couple of helpers can help with. Looking at the code, maybe not so
easy because it's not just vcpu->arch.efer.

> We could forcefully sanitize EFER in flows that write EFER,

Hmm that's exactly what we're doing here, except that we sanitize when
creating the vmcb02 and reject in KVM_SET_NESTED_STATE.

> but (a) that's still
> a (must smaller) game of whack-a-mole and (b) it would actively hide KVM bugs for
> flows that are supposed to reject the invalid state.  And if we WARNed to address
> (b), we'll be right back where we are today: playing whack-a-mole to prevent the
> WARN from being triggered.

Either way it's a game of whack-a-mole, unfortunately, whether it's on
the write side or the read side. What I am hoping is that if we do
miss one case, we don't crash or corrupt the VM. Ideally, we can just
ignore EFER.LMA if EFER.LME is not set in these cases?

>
> > (or just always check EFER.LMA && EFER.LME)?
>
> No can do, because we can't disallow the combination for L2 on VMRUN without
> violating AMD's architecture.  And practically speaking, we *are* doing that,
> just in a bunch of places because there's no one rule to rule them all.

Well, "always" except for this one case :P

>
> > > Because there's a lot of code between here and checking KVM_VM_DEAD in
> > > vcpu_enter_guest().  And has been proven far too many times this year, detecting
> > > a flaw doesn't automagically mitigate true badness.
> >
> > Interesting, I always assumed we can do whatever we want after KVM_BUG_ON() :P
>
> Nope.  In addition to KVM_VM_DEAD not being checked until vcpu_enter_guest(),
> more broadly it only kicks in for cross-task behaviors on the next ioctl.  E.g.
> if KVM_BUG_ON() guards against bad VM state, as opposed to bad vCPU state, i.e.
> if *other* vCPUs could consume the bad state, then it's especially important to
> take evasive action.
>
> KVM_BUG_ON() is as much about protecting the guest as it is about protecting the
> host.  E.g. if KVM *knows* it fatally screwed up, then continuing to run the
> guest risks corrupting guest state and thus causing far worse problems than DoSing
> the guest.

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

[Yosry Ahmed]

On Thu, Aug 27, 2026 at 10:33 AM Sean Christopherson <seanjc@google.com> wrote:
>
> On Thu, Aug 27, 2026, Yosry Ahmed wrote:
> > On Thu, Aug 27, 2026 at 6:37 AM Sean Christopherson <seanjc@google.com> wrote:
> > >
> > > On Thu, Aug 27, 2026, Yosry Ahmed wrote:
> > > > On Wed, Aug 26, 2026 at 2:18 PM Sean Christopherson <seanjc@google.com> wrote:
> > > > >
> > > > > 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.
> > > >
> > > > Isn't the "right" thing to do what hardware (presumably) does and
> > > > ignore EFER.LMA if EFER.LME=0?
> > >
> > > No, because (a) this is KVM uAPI, not emulation of hardware, and (b) it's a check
> > > on L1 state, not L2 state.  It should be impossible for L1 state to have this
> > > combination through "natural" means, and so a snapshot provided by KVM should
> > > never have this combo either, which there's zero reason to allow userspace to
> > > provide garbage.
> >
> > Right, I understand that KVM can do whatever it wants here. I was wondering
> > if we wanted to make the uAPI behavior match the  VMRUN behavior, but I guess
> > we're free to make it more strict.
>
> But again, this *does* match VMRUN behavior, because it's impossible for L1 to
> have EFER.LMA=1, EFER.LME=0, and EFER.PAE=0 at the time of VMRUN.
>
> > > > > 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)) ||
> > > >
> > > > I just realized I have no idea why we check X86_CR0_PE and
> > > > X86_EFLAGS_VM here. Commit 6906e06db9b04 ("KVM: nSVM: Add missing
> > > > checks for reserved bits to svm_set_nested_state()") says it's to do
> > > > the same checks as VMRUN, but I don't think that's actually the case?
> > > > The checks here seem arbitrary to me?
> > >
> > > Again, this is L1 state when L2 is active (the !KVM_STATE_NESTED_GUEST_MODE path
> > > has already bailed), and VMRUN "can only be executed in protected mode with SVM
> > > enabled".  Amusingly, the APM says #VMEXIT "Forces CR0.PE = 1, RFLAGS.VM = 0.",
> > > so I guess it means business.
> > >
> > > If anything is wrong, it's the CR0.PG check.  Presumably that got carried forward
> > > from commit c0725420cfdc ("KVM: SVM: Add helper functions for nested SVM").  I
> > > don't see anything in the APM that requires paging to be enabled, and nothing in
> > > that ancient series points at concrete documentation either.
> >
> > Oh yeah you're right, for some reason I thought it was paging not
> > protected mode. Well then, it seems like
> > nested_svm_check_permissions() is also incorrectly checking paging as
> > well, seems like both checks are incorrect? Also, I don't see anything
> > in the APM about checking RFLAGS.VM before VMRUN.
>
> Presumably it's covered by the !PROTECTED_MODE clause.
>
>   IF ((MSR_EFER.SVME == 0) || (!PROTECTED_MODE))  // This instruction can only be executed in protected
>     EXCEPTION [#UD]                               // mode with SVM enabled
>
> Section "1.3.4 Legacy Modes" describes "Protected Mode" and "Virtual-8086 Mode"
> as separate submodes.  And the tables for most instructions differentiate between
> Real, Virtual 8086, and Protected modes when enumerating exceptions.
>
> The APM weasels around 64-bit mode by also saying "Before enabling and activating
> long mode, system software must first enable protected mode".  E.g. the table for
> SYSENTER describes the #UD due to "This instruction is not recognized in long mode"
> as scenario that's unique to Protected Mode.
>
> Stating the obvious, it would be lovely if the APM explicitly stated what the
> exact checks are, though that's about as likely as AMD gifting me a pony.
>
> FWIW, if that reading is wrong (though I'm pretty sure it's not), then the only
> issue is that KVM is synthesizing #UD instead of #GP, because Virtual 8086 always
> runs at CPL=3.
>
>   IF (CPL != 0)                                   // This instruction is only allowed at CPL 0
>     EXCEPTION [#GP]

I see, thanks for digging this up. Either way I think we want to keep
things consistent between here and nested_svm_check_permissions(). We
probably want a CPL check here, and it would be a superset of
X86_EFLAGS_VM as you mentioned. We probably wanna drop the CR0.PG
check from both places, and add an explicit X86_EFLAGS_VM check in
nested_svm_check_permissions() to #UD instead of #GP?

>
>
> > If the goal here is to keep the checks here consistent with
> > nested_svm_check_permissions(), aside from the new EFER check, then
> > maybe we should also check CPL here?

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

[Sean Christopherson]

On Thu, Aug 27, 2026, Yosry Ahmed wrote:
> > but (a) that's still
> > a (must smaller) game of whack-a-mole and (b) it would actively hide KVM bugs for
> > flows that are supposed to reject the invalid state.  And if we WARNed to address
> > (b), we'll be right back where we are today: playing whack-a-mole to prevent the
> > WARN from being triggered.
> 
> Either way it's a game of whack-a-mole, unfortunately, whether it's on
> the write side or the read side. What I am hoping is that if we do
> miss one case, we don't crash or corrupt the VM. Ideally, we can just
> ignore EFER.LMA if EFER.LME is not set in these cases?

No, because the context matters.  Did we end up with EFER.LMA=1 && EFER.LME=0
because L1 is running L2 with weird settings and wants EFER.LMA to be ignored?
Or did we end up with the impossible state because KVM screwed up?  In which case
crashing the guest because KVM runs it with long mode disabled when it expects to
have long mode enabled is the *best* case scenario, and the worst case scenario
is the guest limps along enough to corrupt its memory and persist the badness to
disk or whatever.

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

[Sean Christopherson]

On Thu, Aug 27, 2026, Yosry Ahmed wrote:
> On Thu, Aug 27, 2026 at 10:33 AM Sean Christopherson <seanjc@google.com> wrote:
> > > Oh yeah you're right, for some reason I thought it was paging not
> > > protected mode. Well then, it seems like
> > > nested_svm_check_permissions() is also incorrectly checking paging as
> > > well, seems like both checks are incorrect? Also, I don't see anything
> > > in the APM about checking RFLAGS.VM before VMRUN.
> >
> > Presumably it's covered by the !PROTECTED_MODE clause.
> >
> >   IF ((MSR_EFER.SVME == 0) || (!PROTECTED_MODE))  // This instruction can only be executed in protected
> >     EXCEPTION [#UD]                               // mode with SVM enabled
> >
> > Section "1.3.4 Legacy Modes" describes "Protected Mode" and "Virtual-8086 Mode"
> > as separate submodes.  And the tables for most instructions differentiate between
> > Real, Virtual 8086, and Protected modes when enumerating exceptions.
> >
> > The APM weasels around 64-bit mode by also saying "Before enabling and activating
> > long mode, system software must first enable protected mode".  E.g. the table for
> > SYSENTER describes the #UD due to "This instruction is not recognized in long mode"
> > as scenario that's unique to Protected Mode.
> >
> > Stating the obvious, it would be lovely if the APM explicitly stated what the
> > exact checks are, though that's about as likely as AMD gifting me a pony.
> >
> > FWIW, if that reading is wrong (though I'm pretty sure it's not), then the only
> > issue is that KVM is synthesizing #UD instead of #GP, because Virtual 8086 always
> > runs at CPL=3.
> >
> >   IF (CPL != 0)                                   // This instruction is only allowed at CPL 0
> >     EXCEPTION [#GP]
> 
> I see, thanks for digging this up. Either way I think we want to keep
> things consistent between here and nested_svm_check_permissions().

Yes, for sure.  nested_svm_check_permissions() is... odd.

> We probably want a CPL check here,

Yes?  Though FWIW, that risks breaking userspace because currently KVM completely
ignores save.cpl and forces it to '0' in svm_copy_vmrun_state().  I mean, this
change also risks breaking userspace, but at least in this case there's a very
good reason for doing so.  But I'm certainly not against trying to provide sanity.

> and it would be a superset of X86_EFLAGS_VM as you mentioned. 

It might not be?  Both CPL and VM8086 are bizarre, I wouldn't be surprised if
save.cpl could be '0' with VM8086 active.  E.g. on Intel, there is no explicit
CPL, it's derived from SS.DPL (because CPL isn't actualy its own thing without
virtualization, it's indeed a reflection of state).

P.S. vmx->rmode.vm86_active doesn't track that the guest is in VM8086, it tracks
     that KVM has put the vCPU into VM8086 in order to emulate Real Mode, which
     is why __vmx_get_cpl() returns '0', not '3'.

  static int __vmx_get_cpl(struct kvm_vcpu *vcpu, bool no_cache)
  {
	struct vcpu_vmx *vmx = to_vmx(vcpu);
	int ar;

	if (unlikely(vmx->rmode.vm86_active))
		return 0;

  }

> We probably wanna drop the CR0.PG check from both places, and add an explicit
> X86_EFLAGS_VM check in nested_svm_check_permissions() to #UD instead of #GP?

And add a CR0.PE check.  But to get it "right", we'll probably need to check
against real hardware, so practically speaking, I don't expect anyone to tackle
this anytime soon.

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

[Paolo Bonzini]

On 8/27/26 19:33, Sean Christopherson wrote:
>>> If anything is wrong, it's the CR0.PG check.  Presumably that got carried forward
>>> from commit c0725420cfdc ("KVM: SVM: Add helper functions for nested SVM").  I
>>> don't see anything in the APM that requires paging to be enabled, and nothing in
>>> that ancient series points at concrete documentation either.
>>
>> Oh yeah you're right, for some reason I thought it was paging not
>> protected mode. Well then, it seems like
>> nested_svm_check_permissions() is also incorrectly checking paging as
>> well, seems like both checks are incorrect? Also, I don't see anything
>> in the APM about checking RFLAGS.VM before VMRUN.
> 
> Presumably it's covered by the !PROTECTED_MODE clause.
> 
>    IF ((MSR_EFER.SVME == 0) || (!PROTECTED_MODE))  // This instruction can only be executed in protected
>      EXCEPTION [#UD]                               // mode with SVM enabled
> 
> Section "1.3.4 Legacy Modes" describes "Protected Mode" and "Virtual-8086 Mode"
> as separate submodes.  And the tables for most instructions differentiate between
> Real, Virtual 8086, and Protected modes when enumerating exceptions.

Right.

As to CR0.PG it does seem incorrect to check it entirely, however note 
that there is this too (15.25.3 Enabling Nested Paging):

	If VMRUN is executed with hCR0.PG cleared to zero and
	NP_ENABLE set to 1, VMRUN terminates with
	#VMEXIT(VMEXIT_INVALID)

which would have to be checked in nested_vmcb_check_controls().

>> If the goal here is to keep the checks here consistent with
>> nested_svm_check_permissions(), aside from the new EFER check, then
>> maybe we should also check CPL here?
Perhaps, but nested_svm_check_permissions() is not reached with CPL=0 
because the #GP overrides the interception (table 15-7, instruction 
intercepts).  The same should be true about EFLAGS.VM=1.

Paolo

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

[Yosry Ahmed]

On Thu, Aug 27, 2026 at 2:42 PM Paolo Bonzini <pbonzini@redhat.com> wrote:
>
> On 8/27/26 19:33, Sean Christopherson wrote:
> >>> If anything is wrong, it's the CR0.PG check.  Presumably that got carried forward
> >>> from commit c0725420cfdc ("KVM: SVM: Add helper functions for nested SVM").  I
> >>> don't see anything in the APM that requires paging to be enabled, and nothing in
> >>> that ancient series points at concrete documentation either.
> >>
> >> Oh yeah you're right, for some reason I thought it was paging not
> >> protected mode. Well then, it seems like
> >> nested_svm_check_permissions() is also incorrectly checking paging as
> >> well, seems like both checks are incorrect? Also, I don't see anything
> >> in the APM about checking RFLAGS.VM before VMRUN.
> >
> > Presumably it's covered by the !PROTECTED_MODE clause.
> >
> >    IF ((MSR_EFER.SVME == 0) || (!PROTECTED_MODE))  // This instruction can only be executed in protected
> >      EXCEPTION [#UD]                               // mode with SVM enabled
> >
> > Section "1.3.4 Legacy Modes" describes "Protected Mode" and "Virtual-8086 Mode"
> > as separate submodes.  And the tables for most instructions differentiate between
> > Real, Virtual 8086, and Protected modes when enumerating exceptions.
>
> Right.
>
> As to CR0.PG it does seem incorrect to check it entirely, however note
> that there is this too (15.25.3 Enabling Nested Paging):
>
>         If VMRUN is executed with hCR0.PG cleared to zero and
>         NP_ENABLE set to 1, VMRUN terminates with
>         #VMEXIT(VMEXIT_INVALID)
>
> which would have to be checked in nested_vmcb_check_controls().

Yeah.

>
> >> If the goal here is to keep the checks here consistent with
> >> nested_svm_check_permissions(), aside from the new EFER check, then
> >> maybe we should also check CPL here?
> Perhaps, but nested_svm_check_permissions() is not reached with CPL=0
> because the #GP overrides the interception (table 15-7, instruction
> intercepts).  The same should be true about EFLAGS.VM=1.

It is reachable in the odd cases where KVM intercepts #GP.

But either way, I think having a CPL check in KVM_SET_NESTED_STATE is
probably the right thing to do.

>
> Paolo
>
>

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

[Paolo Bonzini]

On Fri, Aug 28, 2026 at 12:01 AM Yosry Ahmed <yosry@kernel.org> wrote:
>
> On Thu, Aug 27, 2026 at 2:42 PM Paolo Bonzini <pbonzini@redhat.com> wrote:> >> If the goal here is to keep the checks here consistent with
> > >> nested_svm_check_permissions(), aside from the new EFER check, then
> > >> maybe we should also check CPL here?
> > Perhaps, but nested_svm_check_permissions() is not reached with CPL=0
> > because the #GP overrides the interception (table 15-7, instruction
> > intercepts).  The same should be true about EFLAGS.VM=1.
>
> It is reachable in the odd cases where KVM intercepts #GP.

Oh right, it does not use the emulator (which would also perform the check).

> But either way, I think having a CPL check in KVM_SET_NESTED_STATE is
> probably the right thing to do.

Yes, that makes sense.

Paolo