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

[Sean Christopherson <seanjc@google.com>]

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 {