[PATCH v3 0/2] KVM: x86: Fix missing EFER validity checks

[PATCH v3 0/2] KVM: x86: Fix missing EFER validity checks

[Yosry Ahmed]

v3 of the EFER missing validity checks on KVM_SET_SREGS* fix, split out
a larger series of EFER fixes.

v2 -> v3:
- Split out KVM_SET_SREG* fix and selftest.

v2: https://lore.kernel.org/kvm/20260706195413.1966458-1-yosry@kernel.org/

Yosry Ahmed (2):
  KVM: x86: Check EFER validity on KVM_SET_SREGS*
  KVM: selftests: Extend set_sregs test to cover EFER

 arch/x86/kvm/regs.c                           |  3 +-
 .../selftests/kvm/include/x86/processor.h     |  2 +
 .../selftests/kvm/x86/set_sregs_test.c        | 83 ++++++++++++++-----
 3 files changed, 66 insertions(+), 22 deletions(-)


base-commit: 3aec122bdcaf6f8c4ecfc7f6adf6773015ced676
-- 
2.55.0.141.g00534a21ce-goog

[PATCH v3 1/2] KVM: x86: Check EFER validity on KVM_SET_SREGS*

[Yosry Ahmed]

When handling userspace SREGS writes, check the validity of EFER (i.e.
allowed bits) before writing the new value of EFER through the
per-vendor set_efer callbacks. This prevents userspace from writing
bogus values (e.g. EFER.SVME=1 with nested=0).

Note: on KVM_SET_MSRS, KVM only checks EFER validity in terms of KVM
caps, not guest caps, so it is possible to set EFER bits that are
supported by KVM but not by the guest CPUID. Potentially allowing
userspace to set msrs before CPUID.

However, for KVM_SET_SREGS*, check the validity of the set bits against
both KVM and guest caps. This is consistent with other validity checks
(e.g. for CR4) that check validity against guest caps, which already
imposes the need to set CPUID before SREGS.

Cc: stable@vger.kernel.org
Change-Id: I45701ec440e4fdd8f086eb70db0c0845fb0ed509
Signed-off-by: Yosry Ahmed <yosry@kernel.org>
---
 arch/x86/kvm/regs.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/arch/x86/kvm/regs.c b/arch/x86/kvm/regs.c
index bd8147798cc3e..8f66438989e47 100644
--- a/arch/x86/kvm/regs.c
+++ b/arch/x86/kvm/regs.c
@@ -564,7 +564,8 @@ static bool kvm_is_valid_sregs(struct kvm_vcpu *vcpu, struct kvm_sregs *sregs)
 	}
 
 	return kvm_is_valid_cr4(vcpu, sregs->cr4) &&
-	       kvm_is_valid_cr0(vcpu, sregs->cr0);
+	       kvm_is_valid_cr0(vcpu, sregs->cr0) &&
+	       kvm_valid_efer(vcpu, sregs->efer);
 }
 
 static int __set_sregs_common(struct kvm_vcpu *vcpu, struct kvm_sregs *sregs,
-- 
2.55.0.141.g00534a21ce-goog

[PATCH v3 2/2] KVM: selftests: Extend set_sregs test to cover EFER

[Yosry Ahmed]

Extend the set_sregs test to cover various bits in EFER. Update
TEST_INVALID_CR_BIT() to operate on EFER as well as CRx (and rename it
accordingly). Add test cases to check that EFER bits are disallowed
without the relevant CPUID enablement.

Assisted-by: Gemini:unknown-version
Change-Id: I588e7259c6257f40640127a52ce808a0f5afa016
Signed-off-by: Yosry Ahmed <yosry@kernel.org>
---
 .../selftests/kvm/include/x86/processor.h     |  2 +
 .../selftests/kvm/x86/set_sregs_test.c        | 83 ++++++++++++++-----
 2 files changed, 64 insertions(+), 21 deletions(-)

diff --git a/tools/testing/selftests/kvm/include/x86/processor.h b/tools/testing/selftests/kvm/include/x86/processor.h
index 7d3a27bc0d842..b161174ece453 100644
--- a/tools/testing/selftests/kvm/include/x86/processor.h
+++ b/tools/testing/selftests/kvm/include/x86/processor.h
@@ -208,6 +208,7 @@ struct kvm_x86_cpu_feature {
 #define	X86_FEATURE_PERFCTR_NB		KVM_X86_CPU_FEATURE(0x80000001, 0, ECX, 24)
 #define	X86_FEATURE_PERFCTR_LLC		KVM_X86_CPU_FEATURE(0x80000001, 0, ECX, 28)
 #define	X86_FEATURE_NX			KVM_X86_CPU_FEATURE(0x80000001, 0, EDX, 20)
+#define	X86_FEATURE_FXSR_OPT		KVM_X86_CPU_FEATURE(0x80000001, 0, EDX, 25)
 #define	X86_FEATURE_GBPAGES		KVM_X86_CPU_FEATURE(0x80000001, 0, EDX, 26)
 #define	X86_FEATURE_RDTSCP		KVM_X86_CPU_FEATURE(0x80000001, 0, EDX, 27)
 #define	X86_FEATURE_LM			KVM_X86_CPU_FEATURE(0x80000001, 0, EDX, 29)
@@ -226,6 +227,7 @@ struct kvm_x86_cpu_feature {
 #define X86_FEATURE_SEV			KVM_X86_CPU_FEATURE(0x8000001F, 0, EAX, 1)
 #define X86_FEATURE_SEV_ES		KVM_X86_CPU_FEATURE(0x8000001F, 0, EAX, 3)
 #define X86_FEATURE_SEV_SNP		KVM_X86_CPU_FEATURE(0x8000001F, 0, EAX, 4)
+#define	X86_FEATURE_AUTOIBRS		KVM_X86_CPU_FEATURE(0x80000021, 0, EAX, 8)
 #define	X86_FEATURE_GP_ON_USER_CPUID	KVM_X86_CPU_FEATURE(0x80000021, 0, EAX, 17)
 #define	X86_FEATURE_PERFMON_V2		KVM_X86_CPU_FEATURE(0x80000022, 0, EAX, 0)
 #define	X86_FEATURE_LBR_PMC_FREEZE	KVM_X86_CPU_FEATURE(0x80000022, 0, EAX, 2)
diff --git a/tools/testing/selftests/kvm/x86/set_sregs_test.c b/tools/testing/selftests/kvm/x86/set_sregs_test.c
index 8e654cc9ab168..562afab378d11 100644
--- a/tools/testing/selftests/kvm/x86/set_sregs_test.c
+++ b/tools/testing/selftests/kvm/x86/set_sregs_test.c
@@ -21,20 +21,20 @@
 #include "kvm_util.h"
 #include "processor.h"
 
-#define TEST_INVALID_CR_BIT(vcpu, cr, orig, bit)				\
+#define TEST_INVALID_SREG_BIT(vcpu, reg, orig, bit)				\
 do {										\
 	struct kvm_sregs new;							\
 	int rc;									\
 										\
 	/* Skip the sub-test, the feature/bit is supported. */			\
-	if (orig.cr & bit)							\
+	if (orig.reg & bit)							\
 		break;								\
 										\
-	memcpy(&new, &orig, sizeof(sregs));					\
-	new.cr |= bit;								\
+	memcpy(&new, &orig, sizeof(new));					\
+	new.reg |= bit;								\
 										\
 	rc = _vcpu_sregs_set(vcpu, &new);					\
-	TEST_ASSERT(rc, "KVM allowed invalid " #cr " bit (0x%lx)", bit);	\
+	TEST_ASSERT(rc, "KVM allowed invalid " #reg " bit (0x%llx)", (unsigned long long)bit); \
 										\
 	/* Sanity check that KVM didn't change anything. */			\
 	vcpu_sregs_get(vcpu, &new);						\
@@ -46,6 +46,8 @@ do {										\
 				X86_CR4_MCE | X86_CR4_PGE | X86_CR4_PCE |	\
 				X86_CR4_OSFXSR | X86_CR4_OSXMMEXCPT)
 
+#define KVM_ALWAYS_ALLOWED_EFER EFER_SCE
+
 static u64 calc_supported_cr4_feature_bits(void)
 {
 	u64 cr4 = KVM_ALWAYS_ALLOWED_CR4;
@@ -74,6 +76,24 @@ static u64 calc_supported_cr4_feature_bits(void)
 	return cr4;
 }
 
+static u64 calc_supported_efer_feature_bits(void)
+{
+	u64 efer = KVM_ALWAYS_ALLOWED_EFER;
+
+	if (kvm_cpu_has(X86_FEATURE_LM))
+		efer |= (EFER_LME | EFER_LMA);
+	if (kvm_cpu_has(X86_FEATURE_NX))
+		efer |= EFER_NX;
+	if (kvm_cpu_has(X86_FEATURE_SVM))
+		efer |= EFER_SVME;
+	if (kvm_cpu_has(X86_FEATURE_FXSR_OPT))
+		efer |= EFER_FFXSR;
+	if (kvm_cpu_has(X86_FEATURE_AUTOIBRS))
+		efer |= EFER_AUTOIBRS;
+
+	return efer;
+}
+
 static void test_cr_bits(struct kvm_vcpu *vcpu, u64 cr4)
 {
 	struct kvm_sregs sregs;
@@ -96,26 +116,45 @@ static void test_cr_bits(struct kvm_vcpu *vcpu, u64 cr4)
 		    (sregs.cr4 & X86_CR4_PKE) ? "set" : "clear");
 
 	vcpu_sregs_get(vcpu, &sregs);
-	TEST_ASSERT(sregs.cr4 == cr4, "sregs.CR4 (0x%llx) != CR4 (0x%lx)",
-		    sregs.cr4, cr4);
-
-	TEST_INVALID_CR_BIT(vcpu, cr4, sregs, X86_CR4_UMIP);
-	TEST_INVALID_CR_BIT(vcpu, cr4, sregs, X86_CR4_LA57);
-	TEST_INVALID_CR_BIT(vcpu, cr4, sregs, X86_CR4_VMXE);
-	TEST_INVALID_CR_BIT(vcpu, cr4, sregs, X86_CR4_SMXE);
-	TEST_INVALID_CR_BIT(vcpu, cr4, sregs, X86_CR4_FSGSBASE);
-	TEST_INVALID_CR_BIT(vcpu, cr4, sregs, X86_CR4_PCIDE);
-	TEST_INVALID_CR_BIT(vcpu, cr4, sregs, X86_CR4_OSXSAVE);
-	TEST_INVALID_CR_BIT(vcpu, cr4, sregs, X86_CR4_SMEP);
-	TEST_INVALID_CR_BIT(vcpu, cr4, sregs, X86_CR4_SMAP);
-	TEST_INVALID_CR_BIT(vcpu, cr4, sregs, X86_CR4_PKE);
+	TEST_ASSERT_EQ(sregs.cr4, cr4);
+
+	TEST_INVALID_SREG_BIT(vcpu, cr4, sregs, X86_CR4_UMIP);
+	TEST_INVALID_SREG_BIT(vcpu, cr4, sregs, X86_CR4_LA57);
+	TEST_INVALID_SREG_BIT(vcpu, cr4, sregs, X86_CR4_VMXE);
+	TEST_INVALID_SREG_BIT(vcpu, cr4, sregs, X86_CR4_SMXE);
+	TEST_INVALID_SREG_BIT(vcpu, cr4, sregs, X86_CR4_FSGSBASE);
+	TEST_INVALID_SREG_BIT(vcpu, cr4, sregs, X86_CR4_PCIDE);
+	TEST_INVALID_SREG_BIT(vcpu, cr4, sregs, X86_CR4_OSXSAVE);
+	TEST_INVALID_SREG_BIT(vcpu, cr4, sregs, X86_CR4_SMEP);
+	TEST_INVALID_SREG_BIT(vcpu, cr4, sregs, X86_CR4_SMAP);
+	TEST_INVALID_SREG_BIT(vcpu, cr4, sregs, X86_CR4_PKE);
 
 	for (i = 32; i < 64; i++)
-		TEST_INVALID_CR_BIT(vcpu, cr0, sregs, BIT(i));
+		TEST_INVALID_SREG_BIT(vcpu, cr0, sregs, BIT(i));
 
 	/* NW without CD is illegal, as is PG without PE. */
-	TEST_INVALID_CR_BIT(vcpu, cr0, sregs, X86_CR0_NW);
-	TEST_INVALID_CR_BIT(vcpu, cr0, sregs, X86_CR0_PG);
+	TEST_INVALID_SREG_BIT(vcpu, cr0, sregs, X86_CR0_NW);
+	TEST_INVALID_SREG_BIT(vcpu, cr0, sregs, X86_CR0_PG);
+}
+
+static void test_efer_bits(struct kvm_vcpu *vcpu, u64 efer)
+{
+	struct kvm_sregs sregs;
+	int rc;
+
+	vcpu_sregs_get(vcpu, &sregs);
+	sregs.efer |= efer;
+	rc = _vcpu_sregs_set(vcpu, &sregs);
+	TEST_ASSERT(!rc, "Failed to set supported EFER bits (0x%llx)", sregs.efer);
+
+	vcpu_sregs_get(vcpu, &sregs);
+	TEST_ASSERT_EQ(sregs.efer, efer);
+
+	TEST_INVALID_SREG_BIT(vcpu, efer, sregs, EFER_LME);
+	TEST_INVALID_SREG_BIT(vcpu, efer, sregs, EFER_NX);
+	TEST_INVALID_SREG_BIT(vcpu, efer, sregs, EFER_SVME);
+	TEST_INVALID_SREG_BIT(vcpu, efer, sregs, EFER_FFXSR);
+	TEST_INVALID_SREG_BIT(vcpu, efer, sregs, EFER_AUTOIBRS);
 }
 
 int main(int argc, char *argv[])
@@ -132,6 +171,7 @@ int main(int argc, char *argv[])
 	 */
 	vm = vm_create_barebones();
 	vcpu = __vm_vcpu_add(vm, 0);
+	test_efer_bits(vcpu, KVM_ALWAYS_ALLOWED_EFER);
 	test_cr_bits(vcpu, KVM_ALWAYS_ALLOWED_CR4);
 	kvm_vm_free(vm);
 
@@ -151,6 +191,7 @@ int main(int argc, char *argv[])
 		    sregs.apic_base);
 
 	test_cr_bits(vcpu, calc_supported_cr4_feature_bits());
+	test_efer_bits(vcpu, calc_supported_efer_feature_bits());
 
 	kvm_vm_free(vm);
 
-- 
2.55.0.141.g00534a21ce-goog

Re: [PATCH v3 1/2] KVM: x86: Check EFER validity on KVM_SET_SREGS*

[Yosry Ahmed]

On Mon, Jul 13, 2026 at 11:02 AM Yosry Ahmed <yosry@kernel.org> wrote:
>
> When handling userspace SREGS writes, check the validity of EFER (i.e.
> allowed bits) before writing the new value of EFER through the
> per-vendor set_efer callbacks. This prevents userspace from writing
> bogus values (e.g. EFER.SVME=1 with nested=0).
>
> Note: on KVM_SET_MSRS, KVM only checks EFER validity in terms of KVM
> caps, not guest caps, so it is possible to set EFER bits that are
> supported by KVM but not by the guest CPUID. Potentially allowing
> userspace to set msrs before CPUID.
>
> However, for KVM_SET_SREGS*, check the validity of the set bits against
> both KVM and guest caps. This is consistent with other validity checks
> (e.g. for CR4) that check validity against guest caps, which already
> imposes the need to set CPUID before SREGS.
>
> Cc: stable@vger.kernel.org
> Change-Id: I45701ec440e4fdd8f086eb70db0c0845fb0ed509

Forgot to drop these, sorry :/

Re: [PATCH v3 1/2] KVM: x86: Check EFER validity on KVM_SET_SREGS*

[sashiko-bot]

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

Pre-existing issues:
- [High] This problem wasn't introduced by this patch, but the return value of `kvm_x86_call(set_efer)` is ignored in `__set_sregs_common()`, leading to a silent failure when nested state allocation fails.
--

--- Patch [1]: [PATCH v3 1/2] KVM: x86: Check EFER validity on KVM_SET_SREGS* ---
commit 6edd9e84461af3c58ac965b84f89b7c4985ddeb3
Author: Yosry Ahmed <yosry@kernel.org>

KVM: x86: Check EFER validity on KVM_SET_SREGS*

This commit adds checks to validate EFER against both KVM and guest caps
during KVM_SET_SREGS* writes, preventing userspace from writing bogus values
through the per-vendor set_efer callbacks.

> diff --git a/arch/x86/kvm/regs.c b/arch/x86/kvm/regs.c
> index bd8147798cc3e..8f66438989e47 100644
> --- a/arch/x86/kvm/regs.c
> +++ b/arch/x86/kvm/regs.c

[ ... ]

> @@ -564,7 +564,8 @@ static bool kvm_is_valid_sregs(struct kvm_vcpu *vcpu, struct kvm_sregs *sregs)
>  	}
>  
>  	return kvm_is_valid_cr4(vcpu, sregs->cr4) &&
> -	       kvm_is_valid_cr0(vcpu, sregs->cr0);
> +	       kvm_is_valid_cr0(vcpu, sregs->cr0) &&
> +	       kvm_valid_efer(vcpu, sregs->efer);
>  }
>  
>  static int __set_sregs_common(struct kvm_vcpu *vcpu, struct kvm_sregs *sregs,
>  			      int *mmu_reset_needed, bool update_pdptrs)
>  {

[ ... ]

>  	vcpu->arch.cr3 = sregs->cr3;
>  	kvm_register_mark_dirty(vcpu, VCPU_REG_CR3);
>  	kvm_x86_call(post_set_cr3)(vcpu, sregs->cr3);
>  
>  	*mmu_reset_needed |= vcpu->arch.efer != sregs->efer;
>  	kvm_x86_call(set_efer)(vcpu, sregs->efer);

[Severity: High]
This isn't a bug introduced by this patch, but should the return value
of kvm_x86_call() be checked here?

If a userspace process calls the KVM_SET_SREGS ioctl with EFER.SVME set,
kvm_x86_call(set_efer) could call into svm_set_efer(), which attempts to
allocate nested state via svm_allocate_nested().

If the system is under memory pressure, this allocation can fail and return
-ENOMEM. Because the return value is ignored in __set_sregs_common(),
doesn't this lead to a silent failure where KVM fails to set the guest's
EFER register but still returns success to userspace?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260713180153.2728382-1-yosry@kernel.org?part=1

Re: [PATCH v3 1/2] KVM: x86: Check EFER validity on KVM_SET_SREGS*

[Yosry Ahmed]

On Mon, Jul 13, 2026 at 11:14 AM <sashiko-bot@kernel.org> wrote:
>
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
>
> Pre-existing issues:
> - [High] This problem wasn't introduced by this patch, but the return value of `kvm_x86_call(set_efer)` is ignored in `__set_sregs_common()`, leading to a silent failure when nested state allocation fails.
> --
>
> --- Patch [1]: [PATCH v3 1/2] KVM: x86: Check EFER validity on KVM_SET_SREGS* ---
> commit 6edd9e84461af3c58ac965b84f89b7c4985ddeb3
> Author: Yosry Ahmed <yosry@kernel.org>
>
> KVM: x86: Check EFER validity on KVM_SET_SREGS*
>
> This commit adds checks to validate EFER against both KVM and guest caps
> during KVM_SET_SREGS* writes, preventing userspace from writing bogus values
> through the per-vendor set_efer callbacks.
>
> > diff --git a/arch/x86/kvm/regs.c b/arch/x86/kvm/regs.c
> > index bd8147798cc3e..8f66438989e47 100644
> > --- a/arch/x86/kvm/regs.c
> > +++ b/arch/x86/kvm/regs.c
>
> [ ... ]
>
> > @@ -564,7 +564,8 @@ static bool kvm_is_valid_sregs(struct kvm_vcpu *vcpu, struct kvm_sregs *sregs)
> >       }
> >
> >       return kvm_is_valid_cr4(vcpu, sregs->cr4) &&
> > -            kvm_is_valid_cr0(vcpu, sregs->cr0);
> > +            kvm_is_valid_cr0(vcpu, sregs->cr0) &&
> > +            kvm_valid_efer(vcpu, sregs->efer);
> >  }
> >
> >  static int __set_sregs_common(struct kvm_vcpu *vcpu, struct kvm_sregs *sregs,
> >                             int *mmu_reset_needed, bool update_pdptrs)
> >  {
>
> [ ... ]
>
> >       vcpu->arch.cr3 = sregs->cr3;
> >       kvm_register_mark_dirty(vcpu, VCPU_REG_CR3);
> >       kvm_x86_call(post_set_cr3)(vcpu, sregs->cr3);
> >
> >       *mmu_reset_needed |= vcpu->arch.efer != sregs->efer;
> >       kvm_x86_call(set_efer)(vcpu, sregs->efer);
>
> [Severity: High]
> This isn't a bug introduced by this patch, but should the return value
> of kvm_x86_call() be checked here?
>
> If a userspace process calls the KVM_SET_SREGS ioctl with EFER.SVME set,
> kvm_x86_call(set_efer) could call into svm_set_efer(), which attempts to
> allocate nested state via svm_allocate_nested().
>
> If the system is under memory pressure, this allocation can fail and return
> -ENOMEM. Because the return value is ignored in __set_sregs_common(),
> doesn't this lead to a silent failure where KVM fails to set the guest's
> EFER register but still returns success to userspace?

This seems like a real issue, and I think it's not very easy to fix
because it seems like KVM_SET_SREGS is currently atomic, we either do
all the updates or we fail. So we can't just fail in the middle of
__set_sregs_common() if set_efer() fails, we have to either undo the
previous changes or re-order the writes to do EFER first. Re-ordering
seems like it should be fine, I don't immediately see svm_set_efer()
or vmx_set_efer() depending on any of the previous updates, but I
didn't look too closely.

Anyway, this is completely orthogonal to this patch AFAICT.