[PATCH v1 0/2] KVM: x86: cr8 reserved bit check

[PATCH v1 0/2] KVM: x86: cr8 reserved bit check

[Tharit Tangkijwanichakul]

kvm_is_valid_sregs() validates the incoming CR0, CR4, and efer values on
KVM_SET_SREGS but never checks CR8. When userspace passes a CR8 value with
any reserved bit [63:4] set, __set_sregs_common() forwards it to
kvm_set_cr8(), which rejects the reserved bits and returns early. That
return value is not checked, so the ioctl reports success while the
requested value is silently dropped, and a subsequent KVM_GET_SREGS then
returns a CR8 different from the one userspace believed it had written.

This was found by code inspection of kvm_is_valid_sregs() and confirmed
with the selftest added in patch 2.

Patch 1 factors the reserved-bit check into kvm_is_valid_cr8() and uses it
in both kvm_set_cr8() and kvm_is_valid_sregs() so that KVM_SET_SREGS
rejects reserved CR8 bits up front.

Patch 2 extends set_sregs_test to cover CR8: it verifies that bits [3:0]
can be set and read back, and that every reserved bit [63:4] is rejected
by KVM_SET_SREGS.

Testing
=======
Tested on an Intel host:
                              Unpatched kernel    Patched kernel
set_sregs_test CR8 case       FAIL                PASS
KVM x86 selftests             baseline            no regressions
kvm-unit-tests                baseline            no regressions

Tharit Tangkijwanichakul (2):
  KVM: x86: Reject reserved CR8 bits in KVM_SET_SREGS
  KVM: selftests: Add CR8 reserved-bit checks to set_sregs_test

 arch/x86/kvm/regs.c                              |  8 +++++++-
 tools/testing/selftests/kvm/x86/set_sregs_test.c | 12 ++++++++++++
 2 files changed, 19 insertions(+), 1 deletion(-)

-- 
2.53.0

[PATCH v1 1/2] KVM: x86: Reject reserved CR8 bits in KVM_SET_SREGS

[Tharit Tangkijwanichakul]

kvm_is_valid_sregs() validates the incoming CR0, CR4, and efer values but
never checks CR8.

When userspace passes a CR8 value with any of the
reserved bits [63:4] set, __set_sregs_common() forwards it to
kvm_set_cr8(), which rejects the reserved bits and returns early. That
return value is not checked, so the ioctl reports success while the
requested value is silently dropped. A subsequent KVM_GET_SREGS then
returns a CR8 different from the one userspace believed it had written.

Factor the reserved-bit check out into kvm_is_valid_cr8() and use it both
in kvm_set_cr8() and in kvm_is_valid_sregs().

Fixes: 2f5bb3fe5835 ("KVM: x86: Move the bulk of register specific code from x86.c to regs.c")
Signed-off-by: Tharit Tangkijwanichakul <tharitt97@gmail.com>
---
 arch/x86/kvm/regs.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/arch/x86/kvm/regs.c b/arch/x86/kvm/regs.c
index 8f66438989e4..fad31b59c622 100644
--- a/arch/x86/kvm/regs.c
+++ b/arch/x86/kvm/regs.c
@@ -440,9 +440,14 @@ int kvm_set_cr4(struct kvm_vcpu *vcpu, unsigned long cr4)
 }
 EXPORT_SYMBOL_FOR_KVM_INTERNAL(kvm_set_cr4);
 
+static bool kvm_is_valid_cr8(unsigned long cr8)
+{
+	return !(cr8 & CR8_RESERVED_BITS);
+}
+
 int kvm_set_cr8(struct kvm_vcpu *vcpu, unsigned long cr8)
 {
-	if (cr8 & CR8_RESERVED_BITS)
+	if (!kvm_is_valid_cr8(cr8))
 		return 1;
 	if (lapic_in_kernel(vcpu))
 		kvm_lapic_set_tpr(vcpu, cr8);
@@ -565,6 +570,7 @@ 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_cr8(sregs->cr8) &&
 	       kvm_valid_efer(vcpu, sregs->efer);
 }
 
-- 
2.53.0

Re: [PATCH v1 1/2] KVM: x86: Reject reserved CR8 bits in KVM_SET_SREGS

[Sean Christopherson]

On Tue, Sep 08, 2026, Tharit Tangkijwanichakul wrote:
> kvm_is_valid_sregs() validates the incoming CR0, CR4, and efer values but
> never checks CR8.
> 
> When userspace passes a CR8 value with any of the
> reserved bits [63:4] set, __set_sregs_common() forwards it to
> kvm_set_cr8(), which rejects the reserved bits and returns early. That
> return value is not checked, so the ioctl reports success while the
> requested value is silently dropped. A subsequent KVM_GET_SREGS then
> returns a CR8 different from the one userspace believed it had written.
> 
> Factor the reserved-bit check out into kvm_is_valid_cr8() and use it both
> in kvm_set_cr8() and in kvm_is_valid_sregs().
> 
> Fixes: 2f5bb3fe5835 ("KVM: x86: Move the bulk of register specific code from x86.c to regs.c")

Heh, this goes back much further than just moving code around, all the way to:

  6aa8b732ca01 ("[PATCH] kvm: userspace interface")

which did this in kvm_vcpu_ioctl_set_sregs():

  6aa8b732ca01 (Avi Kivity         2006-12-10 02:21:36 -0800 2109)        vcpu->cr8 = sregs->cr8;

I suppose one could argue that:

  Fixes: 7017fc3d1a12 ("KVM: Define and use cr8 access functions")

is more appropriate, since this specific behavior was introduced then.  I'll
probably just shove both in there and massage the changelog to explain the history.

No need for a v2, I'll fixup when applying.  Thanks!

[PATCH v1 2/2] KVM: selftests: Add CR8 reserved-bit checks to set_sregs_test

[Tharit Tangkijwanichakul]

Extend set_sregs_test to cover CR8. Verify that the architecturally
allowed bits [3:0] can be set and read back, that every reserved bit
[63:4] is rejected by KVM_SET_SREGS, and that a successful set is
observable via KVM_GET_SREGS.

Without the accompanying fix to kvm_is_valid_sregs(), setting a reserved
CR8 bit is silently accepted and the read-back value diverges from the
value written, so this test fails on an unpatched kernel and passes once
the fix is applied.

Signed-off-by: Tharit Tangkijwanichakul <tharitt97@gmail.com>
---
 tools/testing/selftests/kvm/x86/set_sregs_test.c | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/tools/testing/selftests/kvm/x86/set_sregs_test.c b/tools/testing/selftests/kvm/x86/set_sregs_test.c
index 603226ffe437..4e7c35179438 100644
--- a/tools/testing/selftests/kvm/x86/set_sregs_test.c
+++ b/tools/testing/selftests/kvm/x86/set_sregs_test.c
@@ -135,6 +135,18 @@ static void test_cr_bits(struct kvm_vcpu *vcpu, u64 cr4)
 	/* NW without CD is illegal, as is PG without PE. */
 	TEST_INVALID_SREG_BIT(vcpu, cr0, sregs, X86_CR0_NW);
 	TEST_INVALID_SREG_BIT(vcpu, cr0, sregs, X86_CR0_PG);
+
+	/* CR8 bits 3:0 are writable; bits 63:4 are reserved. */
+	vcpu_sregs_get(vcpu, &sregs);
+	sregs.cr8 = 0xf;
+	rc = _vcpu_sregs_set(vcpu, &sregs);
+	TEST_ASSERT(!rc, "Failed to set valid CR8 value 0xf");
+
+	vcpu_sregs_get(vcpu, &sregs);
+	TEST_ASSERT_EQ(sregs.cr8, 0xf);
+
+	for (i = 4; i < 64; i++)
+		TEST_INVALID_SREG_BIT(vcpu, cr8, sregs, BIT_ULL(i));
 }
 
 static void test_efer_bits(struct kvm_vcpu *vcpu, u64 efer)
-- 
2.53.0