[PATCH] KVM: x86: Restrict saved GPA writes to hardware write faults

Kernel KVM virtualization development
 help / color / mirror / Atom feed

* [PATCH] KVM: x86: Restrict saved GPA writes to hardware write faults
@ 2026-08-28 19:30 Kyle Zeng
  2026-08-28 19:49 ` sashiko-bot
  2026-08-28 23:12 ` Sean Christopherson
  0 siblings, 2 replies; 4+ messages in thread
From: Kyle Zeng @ 2026-08-28 19:30 UTC (permalink / raw)
  To: kvm
  Cc: Sean Christopherson, Paolo Bonzini, Thomas Gleixner, Ingo Molnar,
	Borislav Petkov, Dave Hansen, x86, Chris Ayoub, 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 final-write fault information into the emulator and retain it
alongside the saved GPA.  Only reuse that GPA for a write when hardware
reported a final data write.  Reads, faults with unknown access direction,
and implicit guest page-table writes 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-authorized writes.  In particular,
legacy SEV guests can have encrypted page tables that KVM cannot walk,
so forcing every emulated write through a software walk is not suitable.

Fixes: 0f89b207b04a ("kvm: svm: Use the hardware provided GPA instead of page walk")
Reported-by: Oleg Boiko <oboiko@openai.com>
Assisted-by: Codex:gpt-5.6-sol
Signed-off-by: Kyle Zeng <kylebot@openai.com>
---
 arch/x86/kvm/kvm_emulate.h |  3 ++-
 arch/x86/kvm/mmu/mmu.c     | 10 ++++++++++
 arch/x86/kvm/x86.c         |  9 ++++++++-
 arch/x86/kvm/x86.h         |  4 ++++
 4 files changed, 24 insertions(+), 2 deletions(-)

diff --git a/arch/x86/kvm/kvm_emulate.h b/arch/x86/kvm/kvm_emulate.h
index 3e375af15c03..aceca9d3a16d 100644
--- a/arch/x86/kvm/kvm_emulate.h
+++ b/arch/x86/kvm/kvm_emulate.h
@@ -354,8 +354,9 @@ struct x86_emulate_ctxt {
 	bool have_exception;
 	struct x86_exception exception;
 
-	/* GPA available */
+	/* GPA and write access supplied by a hardware page fault. */
 	bool gpa_available;
+	bool gpa_write;
 	gpa_t gpa_val;
 
 	/*
diff --git a/arch/x86/kvm/mmu/mmu.c b/arch/x86/kvm/mmu/mmu.c
index 064ecc33b926..223698cbb9be 100644
--- a/arch/x86/kvm/mmu/mmu.c
+++ b/arch/x86/kvm/mmu/mmu.c
@@ -6632,6 +6632,16 @@ int noinline kvm_mmu_page_fault(struct kvm_vcpu *vcpu, gpa_t cr2_or_gpa, u64 err
 		return r;
 
 emulate:
+	/*
+	 * A write during a guest page walk does not authorize the instruction
+	 * itself to write to the faulting GPA.  Require a final write access
+	 * before allowing the emulator to reuse the GPA for writes.
+	 */
+	if (direct && (error_code & PFERR_WRITE_MASK) &&
+	    (error_code & PFERR_GUEST_FINAL_MASK) &&
+	    !(error_code & PFERR_GUEST_PAGE_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..8c46aa3468c2 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -5088,8 +5088,13 @@ static int emulator_read_write_onepage(unsigned long addr, void *val,
 	 * Note, this cannot be used on string operations since string
 	 * operation using rep will only have the initial GPA from the NPF
 	 * occurred.
+	 *
+	 * The guest may have changed the instruction since the fault.  Reuse
+	 * the GPA for writes only if hardware reported a final write access;
+	 * otherwise, recheck permissions for the decoded access.
 	 */
-	if (ctxt->gpa_available && emulator_can_use_gpa(ctxt) &&
+	if (ctxt->gpa_available && (!write || ctxt->gpa_write) &&
+	    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);
@@ -5939,6 +5944,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_write = false;
 	ctxt->eflags = kvm_get_rflags(vcpu);
 	ctxt->tf = (ctxt->eflags & X86_EFLAGS_TF) != 0;
 
@@ -6455,6 +6461,7 @@ 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_write = emulation_type & EMULTYPE_PF_WRITE;
 			ctxt->gpa_val = cr2_or_gpa;
 		}
 	} else {
diff --git a/arch/x86/kvm/x86.h b/arch/x86/kvm/x86.h
index 0f5919b092e4..2dd456ad3e2d 100644
--- a/arch/x86/kvm/x86.h
+++ b/arch/x86/kvm/x86.h
@@ -407,6 +407,9 @@ 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 to the
+ *		     final GPA, not an implicit guest page-table 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 +448,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	    BIT(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] 4+ messages in thread

* Re: [PATCH] KVM: x86: Restrict saved GPA writes to hardware write faults
  2026-08-28 19:30 [PATCH] KVM: x86: Restrict saved GPA writes to hardware write faults Kyle Zeng
@ 2026-08-28 19:49 ` sashiko-bot
  2026-08-28 21:02   ` Kyle Zeng
  2026-08-28 23:12 ` Sean Christopherson
  1 sibling, 1 reply; 4+ messages in thread
From: sashiko-bot @ 2026-08-28 19:49 UTC (permalink / raw)
  To: Kyle Zeng; +Cc: kvm

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] The patch breaks legacy SEV guests on older AMD CPUs (e.g., Zen 1 and Zen 2) by relying on `PFERR_GUEST_FINAL_MASK`, which is not populated by older hardware.
--

commit f344c94c8318fa7052f7268b565d10c7815dcc45
Author: Kyle Zeng <kylebot@openai.com>
KVM: x86: Restrict saved GPA writes to hardware write faults

This commit restricts reusing a hardware-supplied GPA for emulated writes 
by requiring a final hardware write fault. It retains the GPA and write 
authorization information across emulator contexts.

> diff --git a/arch/x86/kvm/mmu/mmu.c b/arch/x86/kvm/mmu/mmu.c
> index 064ecc33b9267..223698cbb9be6 100644
> --- a/arch/x86/kvm/mmu/mmu.c
> +++ b/arch/x86/kvm/mmu/mmu.c

[ ... ]

> @@ -6632,6 +6632,16 @@ int noinline kvm_mmu_page_fault(struct kvm_vcpu *vcpu, gpa_t cr2_or_gpa, u64 err
>  		return r;
>  
>  emulate:
> +	/*
> +	 * A write during a guest page walk does not authorize the instruction
> +	 * itself to write to the faulting GPA.  Require a final write access
> +	 * before allowing the emulator to reuse the GPA for writes.
> +	 */
> +	if (direct && (error_code & PFERR_WRITE_MASK) &&
> +	    (error_code & PFERR_GUEST_FINAL_MASK) &&
> +	    !(error_code & PFERR_GUEST_PAGE_MASK))
> +		emulation_type |= EMULTYPE_PF_WRITE;
> +

[Severity: High]
Does this logic break legacy SEV guests on older AMD hardware (e.g., Zen 1
and Zen 2) by requiring PFERR_GUEST_FINAL_MASK?

When an MMIO write in a legacy SEV guest triggers a nested page fault on
these older processors, the hardware does not set PFERR_GUEST_FINAL_MASK
(as this bit requires GMET or SEV-ES). Because this flag is missing,
kvm_mmu_page_fault() will not append EMULTYPE_PF_WRITE to the emulation
type.

Without EMULTYPE_PF_WRITE, ctxt->gpa_write evaluates to false, causing the
emulator to fall back to a software page table walk via
vcpu_mmio_gva_to_gpa(). Since SEV guest page tables are encrypted, the
software walk will fail, resulting in fatal emulation failures and guest
crashes.

>  	return x86_emulate_instruction(vcpu, cr2_or_gpa, emulation_type, insn,
>  				       insn_len);
>  }

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

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

* Re: [PATCH] KVM: x86: Restrict saved GPA writes to hardware write faults
  2026-08-28 19:49 ` sashiko-bot
@ 2026-08-28 21:02   ` Kyle Zeng
  0 siblings, 0 replies; 4+ messages in thread
From: Kyle Zeng @ 2026-08-28 21:02 UTC (permalink / raw)
  To: sashiko-reviews; +Cc: kvm

> the hardware does not set PFERR_GUEST_FINAL_MASK
> (as this bit requires GMET or SEV-ES).

Is that requirement documented somewhere? AFAICT, these are ordinary NPF
bits. Bit 32 identifies a fault on the final GPA, and bit 33 identifies
a fault during a guest page-table walk. Both are already described in the
Current AMD manual
(https://kib.kiev.ua/x86docs/AMD/AMD64/24593_APM_v2-r3.44.pdf)

KVM also started preserving these hardware-provided bits in
5e3525195196 ("KVM: nSVM: propagate the NPF EXITINFO to the guest")
back in 2014. See the original patch (https://lkml.iu.edu/1409.0/01003.html).

If a CPU really doesn't set FINAL for a final-GPA write fault, then yes,
falling back to a software walk would be a problem for SEV. I haven't
tested this on Naples or Rome, though. Is there a known erratum, or an
actual EXITINFO1 value showing this on either CPU?

Thanks,
Kyle

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

* Re: [PATCH] KVM: x86: Restrict saved GPA writes to hardware write faults
  2026-08-28 19:30 [PATCH] KVM: x86: Restrict saved GPA writes to hardware write faults Kyle Zeng
  2026-08-28 19:49 ` sashiko-bot
@ 2026-08-28 23:12 ` Sean Christopherson
  1 sibling, 0 replies; 4+ messages in thread
From: Sean Christopherson @ 2026-08-28 23:12 UTC (permalink / raw)
  To: Kyle Zeng
  Cc: kvm, Paolo Bonzini, Thomas Gleixner, Ingo Molnar, Borislav Petkov,
	Dave Hansen, x86, Chris Ayoub, Oleg Boiko

On Fri, Aug 28, 2026, Kyle Zeng wrote:
> 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 final-write fault information into the emulator and retain it
> alongside the saved GPA.  Only reuse that GPA for a write when hardware
> reported a final data write.  Reads, faults with unknown access direction,
> and implicit guest page-table writes do not authorize an emulated write;
> fall back to the existing permission-aware translation in those cases.

Why not?  As noted above, the GPA is the faulting GPA, not the final GPA.  If the
guest kernel is crazy/broken enough to try and put its page tables in emulated
MMIO space, then IMO it's a-ok to assume the guest has given userspace permission
to generate writes to those addresses.

Then it doesn't matter what protections the final translation has, and so the SEV
problem goes away.

> diff --git a/arch/x86/kvm/mmu/mmu.c b/arch/x86/kvm/mmu/mmu.c
> index 064ecc33b926..223698cbb9be 100644
> --- a/arch/x86/kvm/mmu/mmu.c
> +++ b/arch/x86/kvm/mmu/mmu.c
> @@ -6632,6 +6632,16 @@ int noinline kvm_mmu_page_fault(struct kvm_vcpu *vcpu, gpa_t cr2_or_gpa, u64 err
>  		return r;
>  
>  emulate:
> +	/*
> +	 * A write during a guest page walk does not authorize the instruction
> +	 * itself to write to the faulting GPA.  Require a final write access
> +	 * before allowing the emulator to reuse the GPA for writes.
> +	 */
> +	if (direct && (error_code & PFERR_WRITE_MASK) &&
> +	    (error_code & PFERR_GUEST_FINAL_MASK) &&
> +	    !(error_code & PFERR_GUEST_PAGE_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..8c46aa3468c2 100644
> --- a/arch/x86/kvm/x86.c
> +++ b/arch/x86/kvm/x86.c
> @@ -5088,8 +5088,13 @@ static int emulator_read_write_onepage(unsigned long addr, void *val,
>  	 * Note, this cannot be used on string operations since string
>  	 * operation using rep will only have the initial GPA from the NPF
>  	 * occurred.
> +	 *
> +	 * The guest may have changed the instruction since the fault.  Reuse
> +	 * the GPA for writes only if hardware reported a final write access;
> +	 * otherwise, recheck permissions for the decoded access.
>  	 */
> -	if (ctxt->gpa_available && emulator_can_use_gpa(ctxt) &&
> +	if (ctxt->gpa_available && (!write || ctxt->gpa_write) &&

LOL, because I can't help myself.  In the spirit of evil bitwise operations when
doing paging protection checks, what if we do:

diff --git a/arch/x86/kvm/kvm_emulate.h b/arch/x86/kvm/kvm_emulate.h
index 3e375af15c03..5b74fcd183f5 100644
--- a/arch/x86/kvm/kvm_emulate.h
+++ b/arch/x86/kvm/kvm_emulate.h
@@ -355,7 +355,7 @@ struct x86_emulate_ctxt {
 	struct x86_exception exception;
 
 	/* GPA available */
-	bool gpa_available;
+	u64 gpa_access;
 	gpa_t gpa_val;
 
 	/*
diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index 4b3681796c75..7470ff762975 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -5089,7 +5089,7 @@ 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 & BIT(write) && 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 +5938,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 +6454,10 @@ 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;
+			if (emulation_type & EMULTYPE_PF_WRITE)
+				ctxt->gpa_access = ACC_WRITE_MASK;
+			else
+				ctxt->gpa_access = ACC_READ_MASK;
 			ctxt->gpa_val = cr2_or_gpa;
 		}
 	} else {

Or a slightly less evil version:

diff --git a/arch/x86/kvm/kvm_emulate.h b/arch/x86/kvm/kvm_emulate.h
index 3e375af15c03..5b74fcd183f5 100644
--- a/arch/x86/kvm/kvm_emulate.h
+++ b/arch/x86/kvm/kvm_emulate.h
@@ -355,7 +355,7 @@ struct x86_emulate_ctxt {
 	struct x86_exception exception;
 
 	/* GPA available */
-	bool gpa_available;
+	u64 gpa_access;
 	gpa_t gpa_val;
 
 	/*
diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index 4b3681796c75..d6522e38008a 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -5089,7 +5089,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 +5939,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 +6455,10 @@ 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;
+			if (emulation_type & EMULTYPE_PF_WRITE)
+				ctxt->gpa_access = ACC_WRITE_MASK;
+			else
+				ctxt->gpa_access = ACC_READ_MASK;
 			ctxt->gpa_val = cr2_or_gpa;
 		}
 	} else {

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

end of thread, other threads:[~2026-08-28 23:12 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-08-28 19:30 [PATCH] KVM: x86: Restrict saved GPA writes to hardware write faults Kyle Zeng
2026-08-28 19:49 ` sashiko-bot
2026-08-28 21:02   ` Kyle Zeng
2026-08-28 23:12 ` Sean Christopherson

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