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

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

[Kyle Zeng]

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.  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.  In particular, legacy
SEV guests can have encrypted page tables that KVM cannot walk, so forcing
those accesses 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>
---
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         | 14 +++++++++++---
 arch/x86/kvm/x86.h         |  3 +++
 4 files changed, 19 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..aa5deb7c73aa 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 write access;
+	 * otherwise, recheck permissions for the decoded access.
 	 */
-	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 +5943,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 +6459,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;
+			/* A write fault can require a read for RMW emulation. */
+			ctxt->gpa_access = ACC_READ_MASK;
+			if (emulation_type & EMULTYPE_PF_WRITE)
+				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

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

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Read-modify-write (RMW) instructions on unmapped MMIO pages will crash SEV guests because the emulated write phase lacks authorization, forcing an impossible software page table walk.
--

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

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

This patch modifies KVM's instruction emulator to carry hardware write-fault
information and restricts the reuse of a saved GPA for writes only when the
hardware reported a write access. It introduces an access mask to authorize
writes based on the initial hardware fault or fall back to a full
translation if unauthorized.

> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> index 79468ddfe4736..aa5deb7c73aa2 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 write access;
> +	 * otherwise, recheck permissions for the decoded access.
>  	 */
> -	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) &&

[Severity: High]
Will this break the write phase of read-modify-write (RMW) instructions for
SEV guests?

If a SEV guest executes an RMW instruction (like OR [mem], reg) on an
unmapped MMIO page, the hardware triggers a Nested Page Fault (NPF) on the
initial read access. KVM will set gpa_access to ACC_READ_MASK.

When the emulator attempts the write phase, it appears this check will fail
because ACC_WRITE_MASK is not set. This causes emulator_read_write_onepage()
to fall back to vcpu_mmio_gva_to_gpa().

Does this fallback attempt a software page table walk that will fail on
encrypted guest page tables, resulting in a spurious #PF and crashing the
SEV guest?

>  	    (addr & ~PAGE_MASK) == (ctxt->gpa_val & ~PAGE_MASK)) {
>  		gpa = ctxt->gpa_val;
>  		ret = vcpu_is_mmio_gpa(vcpu, addr, gpa, write);

[ ... ]

> @@ -6454,7 +6459,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;
> +			/* A write fault can require a read for RMW emulation. */
> +			ctxt->gpa_access = ACC_READ_MASK;
> +			if (emulation_type & EMULTYPE_PF_WRITE)
> +				ctxt->gpa_access |= ACC_WRITE_MASK;

[Severity: High]
Does this limit gpa_access too strictly for read faults on RMW instructions?

As mentioned above, if an instruction naturally faults on its first (read)
access, it will only get ACC_READ_MASK here. This seems to be the root cause
that later prevents the write phase from reusing the GPA during MMIO
emulation.

>  			ctxt->gpa_val = cr2_or_gpa;
>  		}
>  	} else {

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