Kernel KVM virtualization development
 help / color / mirror / Atom feed
* [PATCH v3] KVM: x86: Restrict saved GPA writes to hardware write faults
@ 2026-09-15 22:26 Kyle Zeng
  2026-09-15 22:41 ` sashiko-bot
  2026-09-16 19:16 ` Sean Christopherson
  0 siblings, 2 replies; 6+ messages in thread
From: Kyle Zeng @ 2026-09-15 22:26 UTC (permalink / raw)
  To: kvm
  Cc: Sean Christopherson, Paolo Bonzini, Thomas Gleixner, Ingo Molnar,
	Borislav Petkov, Dave Hansen, Kyle Zeng, Oleg Boiko

A GPA supplied by a hardware page fault describes the access that
faulted, not an arbitrary instruction decoded afterwards.  A guest can
change an MMIO read into a store before KVM fetches the instruction.  The
saved-GPA shortcut then skips the write-aware guest page-table walk and
can issue a write through a guest-read-only mapping.

Carry hardware write-fault information into the emulator and retain it
alongside the saved GPA.  For non-SEV guests, only reuse that GPA for a
write when hardware reported a write access.  Reads and faults with
unknown access direction do not authorize an emulated write; fall back
to the existing permission-aware translation in those cases.

Keep the authorization across MMIO completion without decoding a new
instruction, and reset it when initializing a fresh emulator context.
This also covers writes reached through the cmpxchg fallback.

Preserve the shortcut for hardware-reported writes and for the read side
of read-modify-write emulation after such a fault.  For SEV guests,
preserve the existing behavior and always allow writes through the saved
GPA.  An RMW instruction can fault on its initial read, and KVM cannot
walk the encrypted guest page tables to check the subsequent write.

Fixes: 0f89b207b04a ("kvm: svm: Use the hardware provided GPA instead of page walk")
Reported-by: Oleg Boiko <oboiko@openai.com>
Assisted-by: Codex:gpt-6-astra
Signed-off-by: Kyle Zeng <kylebot@openai.com>
---
Changes in v3:
- Preserve saved-GPA writes for SEV guests, including RMW read faults.
- Drop the instruction-mutation and RMW read-access comments.
- Guard the SVM-private SEV helper with the host SVM capability check.

Changes in v2:
- Track saved-GPA access with an explicit access mask.
- Allow saved-GPA writes for any direct hardware write fault.
- Preserve read access on write faults for RMW emulation.

 arch/x86/kvm/kvm_emulate.h |  4 ++--
 arch/x86/kvm/mmu/mmu.c     |  3 +++
 arch/x86/kvm/x86.c         | 19 ++++++++++++++++---
 arch/x86/kvm/x86.h         |  3 +++
 4 files changed, 24 insertions(+), 5 deletions(-)

diff --git a/arch/x86/kvm/kvm_emulate.h b/arch/x86/kvm/kvm_emulate.h
index 3e375af15c03..10886bea82c9 100644
--- a/arch/x86/kvm/kvm_emulate.h
+++ b/arch/x86/kvm/kvm_emulate.h
@@ -354,8 +354,8 @@ struct x86_emulate_ctxt {
 	bool have_exception;
 	struct x86_exception exception;
 
-	/* GPA available */
-	bool gpa_available;
+	/* Saved GPA and permitted emulated accesses. */
+	u64 gpa_access;
 	gpa_t gpa_val;
 
 	/*
diff --git a/arch/x86/kvm/mmu/mmu.c b/arch/x86/kvm/mmu/mmu.c
index 064ecc33b926..95b9eac9962a 100644
--- a/arch/x86/kvm/mmu/mmu.c
+++ b/arch/x86/kvm/mmu/mmu.c
@@ -6632,6 +6632,9 @@ int noinline kvm_mmu_page_fault(struct kvm_vcpu *vcpu, gpa_t cr2_or_gpa, u64 err
 		return r;
 
 emulate:
+	if (direct && (error_code & PFERR_WRITE_MASK))
+		emulation_type |= EMULTYPE_PF_WRITE;
+
 	return x86_emulate_instruction(vcpu, cr2_or_gpa, emulation_type, insn,
 				       insn_len);
 }
diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index 79468ddfe473..de7fc8efeadc 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -33,6 +33,7 @@
 #include "lapic.h"
 #include "xen.h"
 #include "smm.h"
+#include "svm/svm.h"
 
 #include <linux/clocksource.h>
 #include <linux/interrupt.h>
@@ -5089,7 +5090,8 @@ static int emulator_read_write_onepage(unsigned long addr, void *val,
 	 * operation using rep will only have the initial GPA from the NPF
 	 * occurred.
 	 */
-	if (ctxt->gpa_available && emulator_can_use_gpa(ctxt) &&
+	if ((ctxt->gpa_access & (write ? ACC_WRITE_MASK : ACC_READ_MASK)) &&
+	    emulator_can_use_gpa(ctxt) &&
 	    (addr & ~PAGE_MASK) == (ctxt->gpa_val & ~PAGE_MASK)) {
 		gpa = ctxt->gpa_val;
 		ret = vcpu_is_mmio_gpa(vcpu, addr, gpa, write);
@@ -5938,7 +5940,7 @@ static void init_emulate_ctxt(struct kvm_vcpu *vcpu)
 
 	kvm_x86_call(get_cs_db_l_bits)(vcpu, &cs_db, &cs_l);
 
-	ctxt->gpa_available = false;
+	ctxt->gpa_access = 0;
 	ctxt->eflags = kvm_get_rflags(vcpu);
 	ctxt->tf = (ctxt->eflags & X86_EFLAGS_TF) != 0;
 
@@ -6454,7 +6456,18 @@ int x86_emulate_instruction(struct kvm_vcpu *vcpu, gpa_t cr2_or_gpa,
 
 		/* With shadow page tables, cr2 contains a GVA or nGPA. */
 		if (vcpu->arch.mmu->root_role.direct) {
-			ctxt->gpa_available = true;
+			ctxt->gpa_access = ACC_READ_MASK;
+			/*
+			 * Always allow writes for SEV guests, as the guest's
+			 * page tables are encrypted, i.e. KVM can't walk the
+			 * guest's page tables and so must always use the GPA
+			 * from the initial fault.  Restricting use of the GPA
+			 * to the access type that faulted would prevent KVM
+			 * from emulating RMW operations for SEV guests.
+			 */
+			if ((emulation_type & EMULTYPE_PF_WRITE) ||
+			    (cpu_feature_enabled(X86_FEATURE_SVM) && is_sev_guest(vcpu)))
+				ctxt->gpa_access |= ACC_WRITE_MASK;
 			ctxt->gpa_val = cr2_or_gpa;
 		}
 	} else {
diff --git a/arch/x86/kvm/x86.h b/arch/x86/kvm/x86.h
index 0f5919b092e4..667465680ccd 100644
--- a/arch/x86/kvm/x86.h
+++ b/arch/x86/kvm/x86.h
@@ -407,6 +407,8 @@ int x86_emulate_instruction(struct kvm_vcpu *vcpu, gpa_t cr2_or_gpa,
  * EMULTYPE_PF - Set when an intercepted #PF triggers the emulation, in which case
  *		 the CR2/GPA value pass on the stack is valid.
  *
+ * EMULTYPE_PF_WRITE - Set with EMULTYPE_PF when hardware reports a write access.
+ *
  * EMULTYPE_COMPLETE_USER_EXIT - Set when the emulator should update interruptibility
  *				 state and inject single-step #DBs after skipping
  *				 an instruction (after completing userspace I/O).
@@ -445,6 +447,7 @@ int x86_emulate_instruction(struct kvm_vcpu *vcpu, gpa_t cr2_or_gpa,
 #define EMULTYPE_COMPLETE_USER_EXIT (1 << 7)
 #define EMULTYPE_WRITE_PF_TO_SP	    (1 << 8)
 #define EMULTYPE_SKIP_SOFT_INT	    (1 << 9)
+#define EMULTYPE_PF_WRITE	    (1 << 10)
 
 #define EMULTYPE_SET_SOFT_INT_VECTOR(v)	((u32)((v) & 0xff) << 16)
 #define EMULTYPE_GET_SOFT_INT_VECTOR(e)	(((e) >> 16) & 0xff)

base-commit: 73e3f0710014fe6d4ed98cfc02292f6121db7558

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH v3] KVM: x86: Restrict saved GPA writes to hardware write faults
  2026-09-15 22:26 [PATCH v3] KVM: x86: Restrict saved GPA writes to hardware write faults Kyle Zeng
@ 2026-09-15 22:41 ` sashiko-bot
  2026-09-15 22:56   ` Kyle Zeng
  2026-09-16 19:16 ` Sean Christopherson
  1 sibling, 1 reply; 6+ messages in thread
From: sashiko-bot @ 2026-09-15 22:41 UTC (permalink / raw)
  To: Kyle Zeng; +Cc: kvm

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] kvm: x86: OOB read and type confusion via static SVM feature check
--

commit ed7805af977b5a574d9ccec8367687b0fbc85fd3
Author: Kyle Zeng <kylebot@openai.com>

KVM: x86: Restrict saved GPA writes to hardware write faults

This commit restricts emulated writes through a saved GPA to cases where
the hardware page fault actually reported a write access. It includes an
exception for SEV guests to preserve existing behavior since their guest
page tables are encrypted.

> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> index 79468ddfe4736..de7fc8efeadce 100644
> --- a/arch/x86/kvm/x86.c
> +++ b/arch/x86/kvm/x86.c
> @@ -33,6 +33,7 @@
>  #include "lapic.h"
>  #include "xen.h"
>  #include "smm.h"
> +#include "svm/svm.h"

[Severity: High]
Does including a vendor-specific internal header like "svm/svm.h" inside
generic architecture code violate KVM layering rules?

[ ... ]

> @@ -6454,7 +6456,18 @@ int x86_emulate_instruction(struct kvm_vcpu *vcpu, gpa_t cr2_or_gpa,
>  
>  		/* With shadow page tables, cr2 contains a GVA or nGPA. */
>  		if (vcpu->arch.mmu->root_role.direct) {
> -			ctxt->gpa_available = true;
> +			ctxt->gpa_access = ACC_READ_MASK;
> +			/*
> +			 * Always allow writes for SEV guests, as the guest's
> +			 * page tables are encrypted, i.e. KVM can't walk the
> +			 * guest's page tables and so must always use the GPA
> +			 * from the initial fault.  Restricting use of the GPA
> +			 * to the access type that faulted would prevent KVM
> +			 * from emulating RMW operations for SEV guests.
> +			 */
> +			if ((emulation_type & EMULTYPE_PF_WRITE) ||
> +			    (cpu_feature_enabled(X86_FEATURE_SVM) && is_sev_guest(vcpu)))

[Severity: High]
Could this lead to an out-of-bounds read and type confusion?

The cpu_feature_enabled(X86_FEATURE_SVM) check relies on a static host CPU
capability rather than checking the active KVM hypervisor module. In a nested
virtualization scenario (like QEMU with -cpu max), the host can expose both
VMX and SVM capabilities, but the kvm-intel module might be the one managing
the VM.

If a guest triggers an emulation fault on a kvm_vmx object, this path could
call is_sev_guest(vcpu). Since is_sev_guest() expects a kvm_svm vCPU, it
will blindly cast the kvm_vmx object to kvm_svm and read memory past the end
of the kvm_vmx allocation. 

If that out-of-bounds memory happens to be non-zero, it would incorrectly
grant ACC_WRITE_MASK authorization for emulated writes, leading to a
potential security bypass or host kernel panic.

> +				ctxt->gpa_access |= ACC_WRITE_MASK;
>  			ctxt->gpa_val = cr2_or_gpa;
>  		}
>  	} else {

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260915222625.99965-1-kylebot@openai.com?part=1

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v3] KVM: x86: Restrict saved GPA writes to hardware write faults
  2026-09-15 22:41 ` sashiko-bot
@ 2026-09-15 22:56   ` Kyle Zeng
  2026-09-16 19:13     ` Sean Christopherson
  0 siblings, 1 reply; 6+ messages in thread
From: Kyle Zeng @ 2026-09-15 22:56 UTC (permalink / raw)
  To: sashiko-reviews; +Cc: kvm

On Tue, Sep 15, 2026 at 10:41:03PM +0000, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [High] kvm: x86: OOB read and type confusion via static SVM feature check
> --
> 
> commit ed7805af977b5a574d9ccec8367687b0fbc85fd3
> Author: Kyle Zeng <kylebot@openai.com>
> 
> KVM: x86: Restrict saved GPA writes to hardware write faults
> 
> This commit restricts emulated writes through a saved GPA to cases where
> the hardware page fault actually reported a write access. It includes an
> exception for SEV guests to preserve existing behavior since their guest
> page tables are encrypted.
> 
> > diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> > index 79468ddfe4736..de7fc8efeadce 100644
> > --- a/arch/x86/kvm/x86.c
> > +++ b/arch/x86/kvm/x86.c
> > @@ -33,6 +33,7 @@
> >  #include "lapic.h"
> >  #include "xen.h"
> >  #include "smm.h"
> > +#include "svm/svm.h"
> 
> [Severity: High]
> Does including a vendor-specific internal header like "svm/svm.h" inside
> generic architecture code violate KVM layering rules?
> 
> [ ... ]
> 
> > @@ -6454,7 +6456,18 @@ int x86_emulate_instruction(struct kvm_vcpu *vcpu, gpa_t cr2_or_gpa,
> >  
> >  		/* With shadow page tables, cr2 contains a GVA or nGPA. */
> >  		if (vcpu->arch.mmu->root_role.direct) {
> > -			ctxt->gpa_available = true;
> > +			ctxt->gpa_access = ACC_READ_MASK;
> > +			/*
> > +			 * Always allow writes for SEV guests, as the guest's
> > +			 * page tables are encrypted, i.e. KVM can't walk the
> > +			 * guest's page tables and so must always use the GPA
> > +			 * from the initial fault.  Restricting use of the GPA
> > +			 * to the access type that faulted would prevent KVM
> > +			 * from emulating RMW operations for SEV guests.
> > +			 */
> > +			if ((emulation_type & EMULTYPE_PF_WRITE) ||
> > +			    (cpu_feature_enabled(X86_FEATURE_SVM) && is_sev_guest(vcpu)))
> 
> [Severity: High]
> Could this lead to an out-of-bounds read and type confusion?
> 
> The cpu_feature_enabled(X86_FEATURE_SVM) check relies on a static host CPU
> capability rather than checking the active KVM hypervisor module. In a nested
> virtualization scenario (like QEMU with -cpu max), the host can expose both
> VMX and SVM capabilities, but the kvm-intel module might be the one managing
> the VM.
> 
> If a guest triggers an emulation fault on a kvm_vmx object, this path could
> call is_sev_guest(vcpu). Since is_sev_guest() expects a kvm_svm vCPU, it
> will blindly cast the kvm_vmx object to kvm_svm and read memory past the end
> of the kvm_vmx allocation. 
> 
> If that out-of-bounds memory happens to be non-zero, it would incorrectly
> grant ACC_WRITE_MASK authorization for emulated writes, leading to a
> potential security bypass or host kernel panic.
> 
> > +				ctxt->gpa_access |= ACC_WRITE_MASK;
> >  			ctxt->gpa_val = cr2_or_gpa;
> >  		}
> >  	} else {
> 
> -- 
> Sashiko AI review · https://sashiko.dev/#/patchset/20260915222625.99965-1-kylebot@openai.com?part=1

On this baseline, x86_virt_init() explicitly refuses to enable
virtualization when both VMX and SVM initialize successfully. VM creation
then fails, so exposing both CPUID bits doesn't appear sufficient to
reach the path described here.

Also, is_sev_guest() casts vcpu->kvm, not the vCPU. In the configuration
I built, the access would fall within zeroed padding in struct kvm_vmx,
not beyond the allocation. That's not something to rely on for
correctness, but it doesn't support the claimed OOB either.

Is there a configuration that gets past the initialization checks and
demonstrates an incorrect access? I'd like to establish that before
treating this as a functional issue.

Thanks,
Kyle

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v3] KVM: x86: Restrict saved GPA writes to hardware write faults
  2026-09-15 22:56   ` Kyle Zeng
@ 2026-09-16 19:13     ` Sean Christopherson
  2026-09-18  3:33       ` Kyle Zeng
  0 siblings, 1 reply; 6+ messages in thread
From: Sean Christopherson @ 2026-09-16 19:13 UTC (permalink / raw)
  To: Kyle Zeng; +Cc: sashiko-reviews, kvm

On Tue, Sep 15, 2026, Kyle Zeng wrote:
> On Tue, Sep 15, 2026 at 10:41:03PM +0000, sashiko-bot@kernel.org wrote:
> > > @@ -6454,7 +6456,18 @@ int x86_emulate_instruction(struct kvm_vcpu *vcpu, gpa_t cr2_or_gpa,
> > >  
> > >  		/* With shadow page tables, cr2 contains a GVA or nGPA. */
> > >  		if (vcpu->arch.mmu->root_role.direct) {
> > > -			ctxt->gpa_available = true;
> > > +			ctxt->gpa_access = ACC_READ_MASK;
> > > +			/*
> > > +			 * Always allow writes for SEV guests, as the guest's
> > > +			 * page tables are encrypted, i.e. KVM can't walk the
> > > +			 * guest's page tables and so must always use the GPA
> > > +			 * from the initial fault.  Restricting use of the GPA
> > > +			 * to the access type that faulted would prevent KVM
> > > +			 * from emulating RMW operations for SEV guests.
> > > +			 */
> > > +			if ((emulation_type & EMULTYPE_PF_WRITE) ||
> > > +			    (cpu_feature_enabled(X86_FEATURE_SVM) && is_sev_guest(vcpu)))
> > 
> > [Severity: High]
> > Could this lead to an out-of-bounds read and type confusion?
 
...

> Is there a configuration that gets past the initialization checks and
> demonstrates an incorrect access? I'd like to establish that before
> treating this as a functional issue.

Eh, doesn't matter, my suggestion was terrible.  I forgot that is_sev_guest()
needs to check embedded data; I was thinking of TDX and SNP VMs, which have
dedicated VM types.

I think the right way to handle this is to track if a VM has protected page
tables.  That'd also help communicate/document why KVM has this weird behavior.

Untested...

diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
index 30ffaa65f589..90c950c12900 100644
--- a/arch/x86/include/asm/kvm_host.h
+++ b/arch/x86/include/asm/kvm_host.h
@@ -1166,6 +1166,7 @@ struct kvm_arch {
 	u8 mmu_valid_gen;
 	u8 vm_type;
 	bool has_private_mem;
+	bool has_protected_page_tables;
 	bool has_protected_state;
 	bool has_protected_eoi;
 	bool has_protected_pmu;
diff --git a/arch/x86/kvm/svm/sev.c b/arch/x86/kvm/svm/sev.c
index b56c39c53c98..7f544658b1cc 100644
--- a/arch/x86/kvm/svm/sev.c
+++ b/arch/x86/kvm/svm/sev.c
@@ -2955,6 +2955,7 @@ void sev_vm_init(struct kvm *kvm)
 		kvm->arch.has_protected_state = true;
 		fallthrough;
 	case KVM_X86_SEV_VM:
+		kvm->arch.has_protected_page_tables = true;
 		kvm->arch.pre_fault_allowed = !kvm->arch.has_private_mem;
 		to_kvm_sev_info(kvm)->need_init = true;
 		break;
diff --git a/arch/x86/kvm/vmx/tdx.c b/arch/x86/kvm/vmx/tdx.c
index 014545c1839b..35022f308e06 100644
--- a/arch/x86/kvm/vmx/tdx.c
+++ b/arch/x86/kvm/vmx/tdx.c
@@ -616,6 +616,7 @@ int tdx_vm_init(struct kvm *kvm)
 {
 	struct kvm_tdx *kvm_tdx = to_kvm_tdx(kvm);
 
+	kvm->arch.has_protected_page_tables = true;
 	kvm->arch.has_protected_state = true;
 	/*
 	 * TDX Module doesn't allow the hypervisor to modify the EOI-bitmap,
diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index f36b952624c8..7e5e2e587086 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -33,7 +33,6 @@
 #include "lapic.h"
 #include "xen.h"
 #include "smm.h"
-#include "svm/svm.h"
 
 #include <linux/clocksource.h>
 #include <linux/timekeeping.h>
@@ -6412,15 +6411,14 @@ int x86_emulate_instruction(struct kvm_vcpu *vcpu, gpa_t cr2_or_gpa,
 		if (vcpu->arch.mmu->root_role.direct) {
 			ctxt->gpa_access = ACC_READ_MASK;
 			/*
-			 * Always allow writes for SEV guests, as the guest's
-			 * page tables are encrypted, i.e. KVM can't walk the
-			 * guest's page tables and so must always use the GPA
-			 * from the initial fault.  Restricting use of the GPA
-			 * to the access type that faulted would prevent KVM
-			 * from emulating RMW operations for SEV guests.
+			 * Always allow writes for guests with protected page
+			 * tables, as KVM can't walk the guest's page tables,
+			 * i.e. KVM can't get the RMW protections for a given
+			 * GVA to see if the write side of a RMW operation
+			 * should be allowed.
 			 */
 			if ((emulation_type & EMULTYPE_PF_WRITE) ||
-			    (cpu_feature_enabled(X86_FEATURE_SVM) && is_sev_guest(vcpu)))
+			    vcpu->kvm->arch.has_protected_page_tables)
 				ctxt->gpa_access |= ACC_WRITE_MASK;
 			ctxt->gpa_val = cr2_or_gpa;
 		}


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH v3] KVM: x86: Restrict saved GPA writes to hardware write faults
  2026-09-15 22:26 [PATCH v3] KVM: x86: Restrict saved GPA writes to hardware write faults Kyle Zeng
  2026-09-15 22:41 ` sashiko-bot
@ 2026-09-16 19:16 ` Sean Christopherson
  1 sibling, 0 replies; 6+ messages in thread
From: Sean Christopherson @ 2026-09-16 19:16 UTC (permalink / raw)
  To: Kyle Zeng
  Cc: kvm, Paolo Bonzini, Thomas Gleixner, Ingo Molnar, Borislav Petkov,
	Dave Hansen, Oleg Boiko

On Tue, Sep 15, 2026, Kyle Zeng wrote:
> diff --git a/arch/x86/kvm/mmu/mmu.c b/arch/x86/kvm/mmu/mmu.c
> index 064ecc33b926..95b9eac9962a 100644
> --- a/arch/x86/kvm/mmu/mmu.c
> +++ b/arch/x86/kvm/mmu/mmu.c
> @@ -6632,6 +6632,9 @@ int noinline kvm_mmu_page_fault(struct kvm_vcpu *vcpu, gpa_t cr2_or_gpa, u64 err
>  		return r;
>  
>  emulate:
> +	if (direct && (error_code & PFERR_WRITE_MASK))
> +		emulation_type |= EMULTYPE_PF_WRITE;

This shouldn't be conditioned on "direct".  Yes, EMULTYPE_PF_WRITE is *currently*
only used for direct MMUs, but it's a generic flag and so should be accurate at
all times.

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v3] KVM: x86: Restrict saved GPA writes to hardware write faults
  2026-09-16 19:13     ` Sean Christopherson
@ 2026-09-18  3:33       ` Kyle Zeng
  0 siblings, 0 replies; 6+ messages in thread
From: Kyle Zeng @ 2026-09-18  3:33 UTC (permalink / raw)
  To: Sean Christopherson; +Cc: sashiko-reviews, kvm

On Wed, Sep 16, 2026 at 12:13:51PM -0700, Sean Christopherson wrote:
> On Tue, Sep 15, 2026, Kyle Zeng wrote:
> > On Tue, Sep 15, 2026 at 10:41:03PM +0000, sashiko-bot@kernel.org wrote:
> > > > @@ -6454,7 +6456,18 @@ int x86_emulate_instruction(struct kvm_vcpu *vcpu, gpa_t cr2_or_gpa,
> > > >  
> > > >  		/* With shadow page tables, cr2 contains a GVA or nGPA. */
> > > >  		if (vcpu->arch.mmu->root_role.direct) {
> > > > -			ctxt->gpa_available = true;
> > > > +			ctxt->gpa_access = ACC_READ_MASK;
> > > > +			/*
> > > > +			 * Always allow writes for SEV guests, as the guest's
> > > > +			 * page tables are encrypted, i.e. KVM can't walk the
> > > > +			 * guest's page tables and so must always use the GPA
> > > > +			 * from the initial fault.  Restricting use of the GPA
> > > > +			 * to the access type that faulted would prevent KVM
> > > > +			 * from emulating RMW operations for SEV guests.
> > > > +			 */
> > > > +			if ((emulation_type & EMULTYPE_PF_WRITE) ||
> > > > +			    (cpu_feature_enabled(X86_FEATURE_SVM) && is_sev_guest(vcpu)))
> > > 
> > > [Severity: High]
> > > Could this lead to an out-of-bounds read and type confusion?
>  
> ...
> 
> > Is there a configuration that gets past the initialization checks and
> > demonstrates an incorrect access? I'd like to establish that before
> > treating this as a functional issue.
> 
> Eh, doesn't matter, my suggestion was terrible.  I forgot that is_sev_guest()
> needs to check embedded data; I was thinking of TDX and SNP VMs, which have
> dedicated VM types.
> 
> I think the right way to handle this is to track if a VM has protected page
> tables.  That'd also help communicate/document why KVM has this weird behavior.
> 
> Untested...
> 
> diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
> index 30ffaa65f589..90c950c12900 100644
> --- a/arch/x86/include/asm/kvm_host.h
> +++ b/arch/x86/include/asm/kvm_host.h
> @@ -1166,6 +1166,7 @@ struct kvm_arch {
>  	u8 mmu_valid_gen;
>  	u8 vm_type;
>  	bool has_private_mem;
> +	bool has_protected_page_tables;
>  	bool has_protected_state;
>  	bool has_protected_eoi;
>  	bool has_protected_pmu;
> diff --git a/arch/x86/kvm/svm/sev.c b/arch/x86/kvm/svm/sev.c
> index b56c39c53c98..7f544658b1cc 100644
> --- a/arch/x86/kvm/svm/sev.c
> +++ b/arch/x86/kvm/svm/sev.c
> @@ -2955,6 +2955,7 @@ void sev_vm_init(struct kvm *kvm)
>  		kvm->arch.has_protected_state = true;
>  		fallthrough;
>  	case KVM_X86_SEV_VM:
> +		kvm->arch.has_protected_page_tables = true;
>  		kvm->arch.pre_fault_allowed = !kvm->arch.has_private_mem;
>  		to_kvm_sev_info(kvm)->need_init = true;
>  		break;
> diff --git a/arch/x86/kvm/vmx/tdx.c b/arch/x86/kvm/vmx/tdx.c
> index 014545c1839b..35022f308e06 100644
> --- a/arch/x86/kvm/vmx/tdx.c
> +++ b/arch/x86/kvm/vmx/tdx.c
> @@ -616,6 +616,7 @@ int tdx_vm_init(struct kvm *kvm)
>  {
>  	struct kvm_tdx *kvm_tdx = to_kvm_tdx(kvm);
>  
> +	kvm->arch.has_protected_page_tables = true;
>  	kvm->arch.has_protected_state = true;
>  	/*
>  	 * TDX Module doesn't allow the hypervisor to modify the EOI-bitmap,
> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> index f36b952624c8..7e5e2e587086 100644
> --- a/arch/x86/kvm/x86.c
> +++ b/arch/x86/kvm/x86.c
> @@ -33,7 +33,6 @@
>  #include "lapic.h"
>  #include "xen.h"
>  #include "smm.h"
> -#include "svm/svm.h"
>  
>  #include <linux/clocksource.h>
>  #include <linux/timekeeping.h>
> @@ -6412,15 +6411,14 @@ int x86_emulate_instruction(struct kvm_vcpu *vcpu, gpa_t cr2_or_gpa,
>  		if (vcpu->arch.mmu->root_role.direct) {
>  			ctxt->gpa_access = ACC_READ_MASK;
>  			/*
> -			 * Always allow writes for SEV guests, as the guest's
> -			 * page tables are encrypted, i.e. KVM can't walk the
> -			 * guest's page tables and so must always use the GPA
> -			 * from the initial fault.  Restricting use of the GPA
> -			 * to the access type that faulted would prevent KVM
> -			 * from emulating RMW operations for SEV guests.
> +			 * Always allow writes for guests with protected page
> +			 * tables, as KVM can't walk the guest's page tables,
> +			 * i.e. KVM can't get the RMW protections for a given
> +			 * GVA to see if the write side of a RMW operation
> +			 * should be allowed.
>  			 */
>  			if ((emulation_type & EMULTYPE_PF_WRITE) ||
> -			    (cpu_feature_enabled(X86_FEATURE_SVM) && is_sev_guest(vcpu)))
> +			    vcpu->kvm->arch.has_protected_page_tables)
>  				ctxt->gpa_access |= ACC_WRITE_MASK;
>  			ctxt->gpa_val = cr2_or_gpa;
>  		}
> 

Hi Sean,

I think the proposed patch will cause regression in legacy-SEV.

The new flag records whether KVM can read a guest´s page tables. The problem is that the patch sets it only when the VM is created, but older SEV APIs enable encryption later.

A modern SEV VM follows this sequence:
  Create VM with type KVM_X86_SEV_VM
  -> sev_vm_init() sets has_protected_page_tables = true
  -> initialize SEV

A legacy SEV VM follows a different sequence:
  Create VM with type KVM_X86_DEFAULT_VM
  -> flag stays false
  -> KVM_SEV_INIT enables SEV
  -> page tables are encrypted, but the flag is still false

The `is_sev_guest()` check recognizes the second case because it reads SEV´s actual active state. The proposed replacement would miss it.
That matters for an MMIO read-modify-write instruction. If hardware faults on the initial read, KVM grants the saved GPA only read permission. When emulation reaches the write, the incorrect flag prevents the SEV exception from applying. KVM then tries to check write permission by walking the guest´s encrypted page tables, which can fail and break the guest.
An ordinary hardware write fault still works because EMULTYPE_PF_WRITE grants write access independently.
The fix is to set the new flag when SEV initialization succeeds. Copying or migrating an encryption context must also set it on the destination VM. Setting it for every DEFAULT_VM would be wrong, because ordinary guests would then regain the vulnerable write shortcut.

V4 will be sent separately to address this issue.

Best,
Kyle

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2026-09-18  3:33 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-09-15 22:26 [PATCH v3] KVM: x86: Restrict saved GPA writes to hardware write faults Kyle Zeng
2026-09-15 22:41 ` sashiko-bot
2026-09-15 22:56   ` Kyle Zeng
2026-09-16 19:13     ` Sean Christopherson
2026-09-18  3:33       ` Kyle Zeng
2026-09-16 19:16 ` Sean Christopherson

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox