[PATCH v2 0/4] Stop returning struct page from guest_memfd PFN lookup

[PATCH v2 0/4] Stop returning struct page from guest_memfd PFN lookup

[Ackerley Tng]

KVM currently expects kvm_gmem_get_pfn() to return a refcounted struct
page. Callers (such as x86 TDP MMU, arm64 Stage-2 fault handler, and SEV-SNP
VMSA / RMP handlers) hold this refcount across page fault handling.

Holding a page refcount across fault handling is problematic for guest_memfd.
In-place memory conversions between confidential computing shared and private
states inspect folio refcounts to ensure exclusive ownership by guest_memfd.  A
concurrent guest page fault taking a reference on the folio causes conversions
to fail due to an elevated refcount.

guest_memfd already notifies KVM of page invalidations, so users of guest_memfd
within KVM only need to respect the MMU invalidation protocol to safely rely on
guest_memfd to ensure page presence.

This series first prepares the SEV-SNP handlers by treating unassigned RMP
entries as benign races on PSMASH failure (which can occur on concurrent
truncation) and dropping page references early in the RMP fault and VMSA reload
paths. It then updates kvm_gmem_get_pfn() to drop the folio reference internally
and stop returning a struct page pointer across x86 and arm64.

Removing struct page from kvm_gmem_get_pfn() also moves KVM closer toward
supporting memory backends that are not backed by struct page.

I really want in-place conversions to merge in time for 7.4 and so I went
ahead to try this, building off Sean's sample code [1].

[1] https://lore.kernel.org/all/an5RJYTwlYeym--O@google.com/

I also split the patch up so it's easier to review :)

Changes from v1:

+ sev_handle_rmp_fault() does need to adopt the MMU invalidation protocol,
  please see reason in patch description.

v1: https://patch.msgid.link/20260818-gmem-no-return-page-v1-0-4f8d939efdbc@google.com

Signed-off-by: Ackerley Tng <ackerleytng@google.com>
---
Ackerley Tng (2):
      KVM: SEV: Treat unassigned RMP entry as benign race on PSMASH failure
      KVM: SEV: Drop page refcount early in VMSA reload

Sean Christopherson (2):
      KVM: SEV: Drop page refcount early during RMP fault handling
      KVM: guest_memfd: Stop returning struct page from PFN lookup

 arch/arm64/kvm/mmu.c     |  4 ++--
 arch/arm64/kvm/nested.c  |  4 ++--
 arch/x86/kvm/mmu/mmu.c   |  2 +-
 arch/x86/kvm/svm/sev.c   | 45 +++++++++++++++++++++++++--------------------
 include/linux/kvm_host.h |  6 ++----
 virt/kvm/guest_memfd.c   |  9 ++-------
 6 files changed, 34 insertions(+), 36 deletions(-)
---
base-commit: 1b731e5ded480bd1e5546aed35584238661ce72e
change-id: 20260818-gmem-no-return-page-614927a29f97

Best regards,
--  
Ackerley Tng <ackerleytng@google.com>

[PATCH v2 1/4] KVM: SEV: Treat unassigned RMP entry as benign race on PSMASH failure

[Ackerley Tng]

When handling an RMP fault, KVM attempts to split a 2MB page via PSMASH.
If PSMASH fails, the only expected return value is FAIL_BADADDR, which does
not distinguish the reason for the bad address. Hence, another RMP entry
lookup is required to determine whether the failure was benign.

Specifically, KVM re-checks the RMP entry to determine if another CPU raced
and already smashed the entry into 4KB pages.

A concurrent operation (such as guest_memfd truncation or hole punching)
can also race and transition the page to shared, removing the page from the
RMP table and causing PSMASH to fail. This can happen even if the page is
still referenced by KVM, because guest_memfd reclaim transitions the RMP
entry to shared when the folio is removed from the page cache.

Treat an unassigned RMP entry as an expected race when re-checking after a
failed PSMASH, and skip logging an error warning.

Fixes: c63cf135cc99 ("KVM: SEV: Add support to handle RMP nested page faults")
Signed-off-by: Ackerley Tng <ackerleytng@google.com>
---
 arch/x86/kvm/svm/sev.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/arch/x86/kvm/svm/sev.c b/arch/x86/kvm/svm/sev.c
index fcb41dfde4c02..b2738362a928b 100644
--- a/arch/x86/kvm/svm/sev.c
+++ b/arch/x86/kvm/svm/sev.c
@@ -5074,10 +5074,11 @@ void sev_handle_rmp_fault(struct kvm_vcpu *vcpu, gpa_t gpa, u64 error_code)
 		/*
 		 * Look it up again. If it's 4K now then the PSMASH may have
 		 * raced with another process and the issue has already resolved
-		 * itself.
+		 * itself. If it's not assigned, then this must have raced with
+		 * another process that made this page shared.
 		 */
 		if (!snp_lookup_rmpentry(pfn, &assigned, &rmp_level) &&
-		    assigned && rmp_level == PG_LEVEL_4K)
+		    ((assigned && rmp_level == PG_LEVEL_4K) || !assigned))
 			goto out;
 
 		pr_warn_ratelimited("SEV: Unable to split RMP entry for GPA 0x%llx PFN 0x%llx ret %d\n",

-- 
2.55.0.699.gb54405d56f-goog

[PATCH v2 2/4] KVM: SEV: Drop page refcount early during RMP fault handling

[Ackerley Tng]

From: Sean Christopherson <seanjc@google.com>

When handling an RMP fault, KVM retrieves the PFN for a private GPA from
guest_memfd.

Drop the page reference immediately after retrieving the PFN instead of
holding it across the entire handler so that the later patch can follow up
with completely not returning refcounted pages from kvm_gmem_get_pfn().

On a first look, existing RMP table handling (psmash and checking for
errors) might seem like it works fine, since truncation of the page from
guest_memfd would have called rmp_make_shared() and removed the PFN from
the RMP table. However, that is insufficient since a freed page may already
be used in a different SNP VM.

Hence, adopt the MMU invalidation protocol to guard committing anything
based on the PFN.

Signed-off-by: Sean Christopherson <seanjc@google.com>
Signed-off-by: Ackerley Tng <ackerleytng@google.com>
---
 arch/x86/kvm/svm/sev.c | 39 ++++++++++++++++++++++++---------------
 1 file changed, 24 insertions(+), 15 deletions(-)

diff --git a/arch/x86/kvm/svm/sev.c b/arch/x86/kvm/svm/sev.c
index b2738362a928b..b34b11d7f8fad 100644
--- a/arch/x86/kvm/svm/sev.c
+++ b/arch/x86/kvm/svm/sev.c
@@ -5003,6 +5003,7 @@ void sev_handle_rmp_fault(struct kvm_vcpu *vcpu, gpa_t gpa, u64 error_code)
 	struct kvm_memory_slot *slot;
 	struct kvm *kvm = vcpu->kvm;
 	int order, rmp_level, ret;
+	unsigned long mmu_seq;
 	struct page *page;
 	bool assigned;
 	kvm_pfn_t pfn;
@@ -5030,18 +5031,22 @@ void sev_handle_rmp_fault(struct kvm_vcpu *vcpu, gpa_t gpa, u64 error_code)
 		return;
 	}
 
+	mmu_seq = kvm->mmu_invalidate_seq;
+	smp_rmb();
+
 	ret = kvm_gmem_get_pfn(kvm, slot, gfn, &pfn, &page, &order);
 	if (ret) {
 		pr_warn_ratelimited("SEV: Unexpected RMP fault, no backing page for private GPA 0x%llx\n",
 				    gpa);
 		return;
 	}
+	kvm_release_page_unused(page);
 
 	ret = snp_lookup_rmpentry(pfn, &assigned, &rmp_level);
 	if (ret || !assigned) {
 		pr_warn_ratelimited("SEV: Unexpected RMP fault, no assigned RMP entry found for GPA 0x%llx PFN 0x%llx error %d\n",
 				    gpa, pfn, ret);
-		goto out_no_trace;
+		return;
 	}
 
 	/*
@@ -5069,27 +5074,31 @@ void sev_handle_rmp_fault(struct kvm_vcpu *vcpu, gpa_t gpa, u64 error_code)
 	if (rmp_level == PG_LEVEL_4K)
 		goto out;
 
-	ret = snp_rmptable_psmash(pfn);
-	if (ret) {
-		/*
-		 * Look it up again. If it's 4K now then the PSMASH may have
-		 * raced with another process and the issue has already resolved
-		 * itself. If it's not assigned, then this must have raced with
-		 * another process that made this page shared.
-		 */
-		if (!snp_lookup_rmpentry(pfn, &assigned, &rmp_level) &&
-		    ((assigned && rmp_level == PG_LEVEL_4K) || !assigned))
+	scoped_guard(read_lock, &kvm->mmu_lock) {
+		if (mmu_invalidate_retry_gfn(kvm, mmu_seq, gfn))
 			goto out;
 
-		pr_warn_ratelimited("SEV: Unable to split RMP entry for GPA 0x%llx PFN 0x%llx ret %d\n",
-				    gpa, pfn, ret);
+		ret = snp_rmptable_psmash(pfn);
+		if (ret) {
+			/*
+			 * Look it up again. If it's 4K now then the PSMASH may
+			 * have raced with another process and the issue has
+			 * already resolved itself. If it's not assigned, then
+			 * this must have raced with another process that made
+			 * this page shared.
+			 */
+			if (!snp_lookup_rmpentry(pfn, &assigned, &rmp_level) &&
+			    ((assigned && rmp_level == PG_LEVEL_4K) || !assigned))
+				goto out;
+
+			pr_warn_ratelimited("SEV: Unable to split RMP entry for GPA 0x%llx PFN 0x%llx ret %d\n",
+					    gpa, pfn, ret);
+		}
 	}
 
 	kvm_zap_gfn_range(kvm, gfn, gfn + PTRS_PER_PMD);
 out:
 	trace_kvm_rmp_fault(vcpu, gpa, pfn, error_code, rmp_level, ret);
-out_no_trace:
-	kvm_release_page_unused(page);
 }
 
 static bool is_pfn_range_shared(kvm_pfn_t start, kvm_pfn_t end)

-- 
2.55.0.699.gb54405d56f-goog

[PATCH v2 3/4] KVM: SEV: Drop page refcount early in VMSA reload

[Ackerley Tng]

When reloading the guest VMSA for an SEV-SNP vCPU, KVM retrieves the PFN
from guest_memfd.

Drop the page reference immediately after retrieving the PFN instead of
holding it across MMU lock acquisition in preparation for a follow-up patch
to stop returning page pointers from guest_memfd PFN lookups.

This is safe because the page's validity and presence are governed by KVM's
MMU invalidation protocol rather than the page reference.

No functional change intended.

Signed-off-by: Ackerley Tng <ackerleytng@google.com>
---
 arch/x86/kvm/svm/sev.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/arch/x86/kvm/svm/sev.c b/arch/x86/kvm/svm/sev.c
index b34b11d7f8fad..d3d620bc04dce 100644
--- a/arch/x86/kvm/svm/sev.c
+++ b/arch/x86/kvm/svm/sev.c
@@ -4062,6 +4062,7 @@ static void __sev_snp_reload_vmsa(struct kvm_vcpu *vcpu, gpa_t gpa)
 	 */
 	if (kvm_gmem_get_pfn(vcpu->kvm, slot, gfn, &pfn, &page, NULL))
 		return;
+	kvm_release_page_clean(page);
 
 	read_lock(&kvm->mmu_lock);
 	/*
@@ -4076,8 +4077,6 @@ static void __sev_snp_reload_vmsa(struct kvm_vcpu *vcpu, gpa_t gpa)
 	else
 		svm->vmcb->control.vmsa_pa = pfn_to_hpa(pfn);
 	read_unlock(&kvm->mmu_lock);
-
-	kvm_release_page_clean(page);
 }
 
 /*

-- 
2.55.0.699.gb54405d56f-goog

[PATCH v2 4/4] KVM: guest_memfd: Stop returning struct page from PFN lookup

[Ackerley Tng]

From: Sean Christopherson <seanjc@google.com>

KVM currently expects guest_memfd PFN lookups to return a refcounted
struct page, which callers hold across fault handling.

Holding a page reference across fault handling is problematic for
guest_memfd. In-place memory conversions between confidential
computing shared and private states inspect folio refcounts to ensure
exclusive ownership by guest_memfd. A concurrent guest page fault
taking a reference on the folio causes conversions to fail due to an
elevated refcount.

guest_memfd already notifies KVM of page invalidations, so callers
within KVM only need to respect the MMU invalidation protocol to safely
rely on guest_memfd for page presence.

Furthermore, removing struct page from the guest_memfd PFN lookup moves
KVM closer toward supporting memory backends that are not backed by
struct page.

Drop the folio reference immediately before returning from the
guest_memfd PFN lookup, and stop returning the struct page pointer.

For ARM, initialize the local page pointer to NULL so that the shared
cleanup path that releases fault-in pages safely no-ops for guest_memfd.

For x86, no additional changes are required in the MMU fault path
because the page fault tracking structure is zero-initialized at the
start of page fault handling, ensuring the refcounted page pointer is
already NULL.

Reported-by: Yan Zhao <yan.y.zhao@intel.com>
Closes: https://lore.kernel.org/all/anZ4W9o5pTWIEgMY@yzhao56-desk.sh.intel.com/
Signed-off-by: Sean Christopherson <seanjc@google.com>
Co-developed-by: Yan Zhao <yan.y.zhao@intel.com>
Signed-off-by: Yan Zhao <yan.y.zhao@intel.com>
Co-developed-by: Ackerley Tng <ackerleytng@google.com>
Signed-off-by: Ackerley Tng <ackerleytng@google.com>
---
 arch/arm64/kvm/mmu.c     | 4 ++--
 arch/arm64/kvm/nested.c  | 4 ++--
 arch/x86/kvm/mmu/mmu.c   | 2 +-
 arch/x86/kvm/svm/sev.c   | 8 ++------
 include/linux/kvm_host.h | 6 ++----
 virt/kvm/guest_memfd.c   | 9 ++-------
 6 files changed, 11 insertions(+), 22 deletions(-)

diff --git a/arch/arm64/kvm/mmu.c b/arch/arm64/kvm/mmu.c
index 6c941aaa10c63..e5d637a5ec558 100644
--- a/arch/arm64/kvm/mmu.c
+++ b/arch/arm64/kvm/mmu.c
@@ -1613,7 +1613,7 @@ static int gmem_abort(const struct kvm_s2_fault_desc *s2fd)
 	enum kvm_pgtable_prot prot = KVM_PGTABLE_PROT_R;
 	struct kvm_pgtable *pgt = s2fd->vcpu->arch.hw_mmu->pgt;
 	unsigned long mmu_seq;
-	struct page *page;
+	struct page *page = NULL;
 	struct kvm *kvm = s2fd->vcpu->kvm;
 	void *memcache = NULL;
 	kvm_pfn_t pfn;
@@ -1641,7 +1641,7 @@ static int gmem_abort(const struct kvm_s2_fault_desc *s2fd)
 	/* Pairs with the smp_wmb() in kvm_mmu_invalidate_end(). */
 	smp_rmb();
 
-	ret = kvm_gmem_get_pfn(kvm, s2fd->memslot, gfn, &pfn, &page, NULL);
+	ret = kvm_gmem_get_pfn(kvm, s2fd->memslot, gfn, &pfn, NULL);
 	if (ret) {
 		kvm_prepare_memory_fault_exit(s2fd->vcpu, s2fd->fault_ipa, PAGE_SIZE,
 					      write_fault, exec_fault, false);
diff --git a/arch/arm64/kvm/nested.c b/arch/arm64/kvm/nested.c
index fb54f6dad995c..43523bb17621a 100644
--- a/arch/arm64/kvm/nested.c
+++ b/arch/arm64/kvm/nested.c
@@ -1360,7 +1360,7 @@ static int kvm_translate_vncr(struct kvm_vcpu *vcpu, bool *is_gmem)
 	bool write_fault, writable;
 	unsigned long mmu_seq;
 	struct vncr_tlb *vt;
-	struct page *page;
+	struct page *page = NULL;
 	u64 va, pfn, gfn;
 	int ret;
 
@@ -1411,7 +1411,7 @@ static int kvm_translate_vncr(struct kvm_vcpu *vcpu, bool *is_gmem)
 		if (is_error_noslot_pfn(pfn) || (write_fault && !writable))
 			return -EFAULT;
 	} else {
-		ret = kvm_gmem_get_pfn(vcpu->kvm, memslot, gfn, &pfn, &page, NULL);
+		ret = kvm_gmem_get_pfn(vcpu->kvm, memslot, gfn, &pfn, NULL);
 		if (ret) {
 			kvm_prepare_memory_fault_exit(vcpu, vt->wr.pa, PAGE_SIZE,
 					      write_fault, false, false);
diff --git a/arch/x86/kvm/mmu/mmu.c b/arch/x86/kvm/mmu/mmu.c
index c519e8e8d646f..129d403308051 100644
--- a/arch/x86/kvm/mmu/mmu.c
+++ b/arch/x86/kvm/mmu/mmu.c
@@ -4604,7 +4604,7 @@ static int kvm_mmu_faultin_pfn_gmem(struct kvm_vcpu *vcpu,
 	}
 
 	r = kvm_gmem_get_pfn(vcpu->kvm, fault->slot, fault->gfn, &fault->pfn,
-			     &fault->refcounted_page, &max_order);
+			     &max_order);
 	if (r) {
 		kvm_mmu_prepare_memory_fault_exit(vcpu, fault);
 		return r;
diff --git a/arch/x86/kvm/svm/sev.c b/arch/x86/kvm/svm/sev.c
index d3d620bc04dce..32db979daaaf0 100644
--- a/arch/x86/kvm/svm/sev.c
+++ b/arch/x86/kvm/svm/sev.c
@@ -4016,7 +4016,6 @@ static void __sev_snp_reload_vmsa(struct kvm_vcpu *vcpu, gpa_t gpa)
 	struct kvm *kvm = vcpu->kvm;
 	gfn_t gfn = gpa_to_gfn(gpa);
 	unsigned long mmu_seq;
-	struct page *page;
 	kvm_pfn_t pfn;
 
 	lockdep_assert_held(&svm->sev_es.snp_vmsa_mutex);
@@ -4060,9 +4059,8 @@ static void __sev_snp_reload_vmsa(struct kvm_vcpu *vcpu, gpa_t gpa)
 	 * The new VMSA will be private memory guest memory, so retrieve the
 	 * PFN from the gmem backend.
 	 */
-	if (kvm_gmem_get_pfn(vcpu->kvm, slot, gfn, &pfn, &page, NULL))
+	if (kvm_gmem_get_pfn(vcpu->kvm, slot, gfn, &pfn, NULL))
 		return;
-	kvm_release_page_clean(page);
 
 	read_lock(&kvm->mmu_lock);
 	/*
@@ -5003,7 +5001,6 @@ void sev_handle_rmp_fault(struct kvm_vcpu *vcpu, gpa_t gpa, u64 error_code)
 	struct kvm *kvm = vcpu->kvm;
 	int order, rmp_level, ret;
 	unsigned long mmu_seq;
-	struct page *page;
 	bool assigned;
 	kvm_pfn_t pfn;
 	gfn_t gfn;
@@ -5033,13 +5030,12 @@ void sev_handle_rmp_fault(struct kvm_vcpu *vcpu, gpa_t gpa, u64 error_code)
 	mmu_seq = kvm->mmu_invalidate_seq;
 	smp_rmb();
 
-	ret = kvm_gmem_get_pfn(kvm, slot, gfn, &pfn, &page, &order);
+	ret = kvm_gmem_get_pfn(kvm, slot, gfn, &pfn, &order);
 	if (ret) {
 		pr_warn_ratelimited("SEV: Unexpected RMP fault, no backing page for private GPA 0x%llx\n",
 				    gpa);
 		return;
 	}
-	kvm_release_page_unused(page);
 
 	ret = snp_lookup_rmpentry(pfn, &assigned, &rmp_level);
 	if (ret || !assigned) {
diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h
index 03bfc92864b6e..502465119ca0c 100644
--- a/include/linux/kvm_host.h
+++ b/include/linux/kvm_host.h
@@ -2586,13 +2586,11 @@ static inline bool kvm_mem_is_private(struct kvm *kvm, gfn_t gfn)
 
 #ifdef CONFIG_KVM_GUEST_MEMFD
 int kvm_gmem_get_pfn(struct kvm *kvm, struct kvm_memory_slot *slot,
-		     gfn_t gfn, kvm_pfn_t *pfn, struct page **page,
-		     int *max_order);
+		     gfn_t gfn, kvm_pfn_t *pfn, int *max_order);
 #else
 static inline int kvm_gmem_get_pfn(struct kvm *kvm,
 				   struct kvm_memory_slot *slot, gfn_t gfn,
-				   kvm_pfn_t *pfn, struct page **page,
-				   int *max_order)
+				   kvm_pfn_t *pfn, int *max_order)
 {
 	KVM_BUG_ON(1, kvm);
 	return -EIO;
diff --git a/virt/kvm/guest_memfd.c b/virt/kvm/guest_memfd.c
index b596486d184ca..589762140c3ef 100644
--- a/virt/kvm/guest_memfd.c
+++ b/virt/kvm/guest_memfd.c
@@ -751,8 +751,7 @@ static struct folio *__kvm_gmem_get_pfn(struct file *file,
 }
 
 int kvm_gmem_get_pfn(struct kvm *kvm, struct kvm_memory_slot *slot,
-		     gfn_t gfn, kvm_pfn_t *pfn, struct page **page,
-		     int *max_order)
+		     gfn_t gfn, kvm_pfn_t *pfn, int *max_order)
 {
 	pgoff_t index = kvm_gmem_get_index(slot, gfn);
 	struct folio *folio;
@@ -780,11 +779,7 @@ int kvm_gmem_get_pfn(struct kvm *kvm, struct kvm_memory_slot *slot,
 #endif
 
 	folio_unlock(folio);
-
-	if (!r)
-		*page = folio_file_page(folio, index);
-	else
-		folio_put(folio);
+	folio_put(folio);
 
 	return r;
 }

-- 
2.55.0.699.gb54405d56f-goog

Re: [PATCH v2 3/4] KVM: SEV: Drop page refcount early in VMSA reload

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [Critical] A preexisting vulnerability in `sev_gmem_make_shared` allows a malicious SEV-SNP guest to cause a host kernel panic by transitioning a `guest_memfd` page to VMSA state. When the page is subsequently freed, KVM fails to reclaim it and returns the still-firmware-owned page to the host page allocator.
--

--- Patch [3]: [PATCH v2 3/4] KVM: SEV: Drop page refcount early in VMSA reload ---
commit 597ff1c52e82911967f2122ff1d77aa79a4a2d9b
Author: Ackerley Tng <ackerleytng@google.com>

KVM: SEV: Drop page refcount early in VMSA reload

This commit modifies the guest VMSA reload path for SEV-SNP vCPUs to drop
the page reference immediately after retrieving the PFN from guest_memfd.
This avoids holding the reference across MMU lock acquisition and prepares
for future changes. The page's presence remains governed by KVM's MMU
invalidation protocol.

> diff --git a/arch/x86/kvm/svm/sev.c b/arch/x86/kvm/svm/sev.c
> index b34b11d7f8fad..d3d620bc04dce 100644
> --- a/arch/x86/kvm/svm/sev.c
> +++ b/arch/x86/kvm/svm/sev.c
> @@ -4062,6 +4062,7 @@ static void __sev_snp_reload_vmsa(struct kvm_vcpu *vcpu, gpa_t gpa)
>  	 */
>  	if (kvm_gmem_get_pfn(vcpu->kvm, slot, gfn, &pfn, &page, NULL))
>  		return;
> +	kvm_release_page_clean(page);
>  
>  	read_lock(&kvm->mmu_lock);
>  	/*
> @@ -4076,8 +4077,6 @@ static void __sev_snp_reload_vmsa(struct kvm_vcpu *vcpu, gpa_t gpa)
>  	else
>  		svm->vmcb->control.vmsa_pa = pfn_to_hpa(pfn);
>  	read_unlock(&kvm->mmu_lock);
> -
> -	kvm_release_page_clean(page);
>  }

[Severity: Critical]
This is a pre-existing issue, but can this allow a malicious guest to
cause a host kernel panic?

Looking at sev_gmem_make_shared() in arch/x86/kvm/svm/sev.c:

arch/x86/kvm/svm/sev.c:sev_gmem_make_shared() {
    ...
		rc = rmp_make_shared(pfn, use_2m_update ? PG_LEVEL_2M : PG_LEVEL_4K);
		if (WARN_ONCE(rc, "SEV: Failed to update RMP entry for PFN 0x%llx error %d\n",
			      pfn, rc))
			goto next_pfn;
    ...
}

If an untrusted SEV-SNP guest modifies a guest_memfd page state to VMSA
using RMPADJUST, and the page is subsequently freed, rmp_make_shared()
will architecturally fail on the VMSA page.

Because the error is bypassed without calling snp_leak_pages(), the
firmware-owned page is returned to the host page allocator.

Could this lead to an RMP violation and a host kernel panic when the host
later attempts to access the freed page?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260818-gmem-no-return-page-v2-0-5298f42d49bb@google.com?part=3

Re: [PATCH v2 2/4] KVM: SEV: Drop page refcount early during RMP fault handling

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] Dropping the page refcount early introduces a race condition where a concurrent `guest_memfd` invalidation can trigger a false-positive 'Unexpected RMP fault' kernel warning.
--

--- Patch [2]: [PATCH v2 2/4] KVM: SEV: Drop page refcount early during RMP fault handling ---
commit 7d0f356d8592ce1804052b0fdb61fccf3a11ecaa
Author: Sean Christopherson <seanjc@google.com>

KVM: SEV: Drop page refcount early during RMP fault handling

When handling an RMP fault, KVM retrieves the PFN for a private GPA from
guest_memfd. This patch drops the page reference immediately after retrieving
the PFN instead of holding it across the entire handler, and adopts the MMU
invalidation protocol to guard committing anything based on the PFN.

> diff --git a/arch/x86/kvm/svm/sev.c b/arch/x86/kvm/svm/sev.c
> index b2738362a928b..b34b11d7f8fad 100644
> --- a/arch/x86/kvm/svm/sev.c
> +++ b/arch/x86/kvm/svm/sev.c

[ ... ]

> @@ -5030,18 +5031,22 @@ void sev_handle_rmp_fault(struct kvm_vcpu *vcpu, gpa_t gpa, u64 error_code)
>  		return;
>  	}
>  
> +	mmu_seq = kvm->mmu_invalidate_seq;
> +	smp_rmb();
> +
>  	ret = kvm_gmem_get_pfn(kvm, slot, gfn, &pfn, &page, &order);
>  	if (ret) {
>  		pr_warn_ratelimited("SEV: Unexpected RMP fault, no backing page for private GPA 0x%llx\n",
>  				    gpa);
>  		return;
>  	}
> +	kvm_release_page_unused(page);
>  
>  	ret = snp_lookup_rmpentry(pfn, &assigned, &rmp_level);
>  	if (ret || !assigned) {
>  		pr_warn_ratelimited("SEV: Unexpected RMP fault, no assigned RMP entry found for GPA 0x%llx PFN 0x%llx error %d\n",
>  				    gpa, pfn, ret);
> -		goto out_no_trace;
> +		return;
>  	}

[Severity: Medium]
Since the page reference is dropped before this lookup in
sev_handle_rmp_fault(), could a concurrent guest_memfd hole-punch drop
the file's reference to the page and free it?

If the page is freed, the RMP entry would transition to shared/unassigned.
Could this then cause snp_lookup_rmpentry() to see the page as unassigned
and incorrectly emit this warning before the mmu_invalidate_retry_gfn()
check happens?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260818-gmem-no-return-page-v2-0-5298f42d49bb@google.com?part=2

Re: [PATCH v2 4/4] KVM: guest_memfd: Stop returning struct page from PFN lookup

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [Critical] Concurrent execution of KVM_SEV_SNP_LAUNCH_UPDATE and a vCPU page fault triggers a race condition leading to a host kernel panic.
--

--- Patch [4]: [PATCH v2 4/4] KVM: guest_memfd: Stop returning struct page from PFN lookup ---
commit 766989bc1910148a83bffa481ae22b1d1eb71f01
Author: Sean Christopherson <seanjc@google.com>

KVM: guest_memfd: Stop returning struct page from PFN lookup

This commit stops returning a refcounted struct page from guest_memfd PFN
lookups, dropping the folio reference immediately instead. This prevents
concurrent guest page faults from holding elevated refcounts that would
interfere with in-place memory conversions between shared and private states.

Link: https://lore.kernel.org/all/anZ4W9o5pTWIEgMY@yzhao56-desk.sh.intel.com/

> diff --git a/virt/kvm/guest_memfd.c b/virt/kvm/guest_memfd.c
> index b596486d184ca..589762140c3ef 100644
> --- a/virt/kvm/guest_memfd.c
> +++ b/virt/kvm/guest_memfd.c

[ ... ]

> @@ -780,11 +779,7 @@ int kvm_gmem_get_pfn(struct kvm *kvm, struct kvm_memory_slot *slot,
>  #endif
>  
>  	folio_unlock(folio);
> -
> -	if (!r)
> -		*page = folio_file_page(folio, index);
> -	else
> -		folio_put(folio);
> +	folio_put(folio);
>  
>  	return r;
>  }

[Severity: Critical]
This is a pre-existing issue, but is it possible for a concurrent page
fault to trigger a host kernel panic via a hardware RMP fault here?

Looking at kvm_gmem_get_pfn() in virt/kvm/guest_memfd.c, if the folio is
not up to date, it zeroes the page and marks it up to date:

    if (!folio_test_uptodate(folio)) {
        clear_highpage(folio_page(folio, 0));
        folio_mark_uptodate(folio);
    }

However, in the KVM_SEV_SNP_LAUNCH_UPDATE path, __kvm_gmem_populate()
fetches a folio, unlocks it, and calls post_populate() (which maps to
sev_gmem_post_populate() and calls rmp_make_private()) before finally
marking the folio up to date.

If a malicious userspace process races a vCPU page fault against
KVM_SEV_SNP_LAUNCH_UPDATE, could the vCPU thread running kvm_gmem_get_pfn()
acquire the lock on the not-yet-uptodate folio and call clear_highpage()
after rmp_make_private() has already executed?

If clear_highpage() writes to the page after it has been made RMP-private,
this would trigger a fatal hardware RMP fault on the host CPU. If it writes
just before, it appears it could silently corrupt the encrypted firmware
payload.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260818-gmem-no-return-page-v2-0-5298f42d49bb@google.com?part=4

Re: [PATCH v2 4/4] KVM: guest_memfd: Stop returning struct page from PFN lookup

[Suzuki K Poulose]

On 18/08/2026 10:15, Ackerley Tng wrote:
> From: Sean Christopherson <seanjc@google.com>
> 
> KVM currently expects guest_memfd PFN lookups to return a refcounted
> struct page, which callers hold across fault handling.
> 
> Holding a page reference across fault handling is problematic for
> guest_memfd. In-place memory conversions between confidential
> computing shared and private states inspect folio refcounts to ensure
> exclusive ownership by guest_memfd. A concurrent guest page fault
> taking a reference on the folio causes conversions to fail due to an
> elevated refcount.
> 
> guest_memfd already notifies KVM of page invalidations, so callers
> within KVM only need to respect the MMU invalidation protocol to safely
> rely on guest_memfd for page presence.
> 
> Furthermore, removing struct page from the guest_memfd PFN lookup moves
> KVM closer toward supporting memory backends that are not backed by
> struct page.
> 
> Drop the folio reference immediately before returning from the
> guest_memfd PFN lookup, and stop returning the struct page pointer.
> 
> For ARM, initialize the local page pointer to NULL so that the shared
> cleanup path that releases fault-in pages safely no-ops for guest_memfd.
> 
> For x86, no additional changes are required in the MMU fault path
> because the page fault tracking structure is zero-initialized at the
> start of page fault handling, ensuring the refcounted page pointer is
> already NULL.
> 
> Reported-by: Yan Zhao <yan.y.zhao@intel.com>
> Closes: https://lore.kernel.org/all/anZ4W9o5pTWIEgMY@yzhao56-desk.sh.intel.com/
> Signed-off-by: Sean Christopherson <seanjc@google.com>
> Co-developed-by: Yan Zhao <yan.y.zhao@intel.com>
> Signed-off-by: Yan Zhao <yan.y.zhao@intel.com>
> Co-developed-by: Ackerley Tng <ackerleytng@google.com>
> Signed-off-by: Ackerley Tng <ackerleytng@google.com>
> ---
>   arch/arm64/kvm/mmu.c     | 4 ++--
>   arch/arm64/kvm/nested.c  | 4 ++--
>   arch/x86/kvm/mmu/mmu.c   | 2 +-
>   arch/x86/kvm/svm/sev.c   | 8 ++------
>   include/linux/kvm_host.h | 6 ++----
>   virt/kvm/guest_memfd.c   | 9 ++-------
>   6 files changed, 11 insertions(+), 22 deletions(-)
> 
> diff --git a/arch/arm64/kvm/mmu.c b/arch/arm64/kvm/mmu.c
> index 6c941aaa10c63..e5d637a5ec558 100644
> --- a/arch/arm64/kvm/mmu.c
> +++ b/arch/arm64/kvm/mmu.c
> @@ -1613,7 +1613,7 @@ static int gmem_abort(const struct kvm_s2_fault_desc *s2fd)
>   	enum kvm_pgtable_prot prot = KVM_PGTABLE_PROT_R;
>   	struct kvm_pgtable *pgt = s2fd->vcpu->arch.hw_mmu->pgt;
>   	unsigned long mmu_seq;
> -	struct page *page;
> +	struct page *page = NULL;
>   	struct kvm *kvm = s2fd->vcpu->kvm;
>   	void *memcache = NULL;
>   	kvm_pfn_t pfn;
> @@ -1641,7 +1641,7 @@ static int gmem_abort(const struct kvm_s2_fault_desc *s2fd)
>   	/* Pairs with the smp_wmb() in kvm_mmu_invalidate_end(). */
>   	smp_rmb();
>   
> -	ret = kvm_gmem_get_pfn(kvm, s2fd->memslot, gfn, &pfn, &page, NULL);
> +	ret = kvm_gmem_get_pfn(kvm, s2fd->memslot, gfn, &pfn, NULL);
>   	if (ret) {
>   		kvm_prepare_memory_fault_exit(s2fd->vcpu, s2fd->fault_ipa, PAGE_SIZE,
>   					      write_fault, exec_fault, false);

Since this function only deals with the gmem backed aborts, you could 
remove the variable and the call to kvm_release_faultin_page() below.


> diff --git a/arch/arm64/kvm/nested.c b/arch/arm64/kvm/nested.c
> index fb54f6dad995c..43523bb17621a 100644
> --- a/arch/arm64/kvm/nested.c
> +++ b/arch/arm64/kvm/nested.c
> @@ -1360,7 +1360,7 @@ static int kvm_translate_vncr(struct kvm_vcpu *vcpu, bool *is_gmem)
>   	bool write_fault, writable;
>   	unsigned long mmu_seq;
>   	struct vncr_tlb *vt;
> -	struct page *page;
> +	struct page *page = NULL;
>   	u64 va, pfn, gfn;
>   	int ret;
>   
> @@ -1411,7 +1411,7 @@ static int kvm_translate_vncr(struct kvm_vcpu *vcpu, bool *is_gmem)
>   		if (is_error_noslot_pfn(pfn) || (write_fault && !writable))
>   			return -EFAULT;
>   	} else {
> -		ret = kvm_gmem_get_pfn(vcpu->kvm, memslot, gfn, &pfn, &page, NULL);
> +		ret = kvm_gmem_get_pfn(vcpu->kvm, memslot, gfn, &pfn, NULL);
>   		if (ret) {
>   			kvm_prepare_memory_fault_exit(vcpu, vt->wr.pa, PAGE_SIZE,
>   					      write_fault, false, false);

This is safe too, as we only use the page for 
kvm_release_faultin_page(), and it can tolerate a NULL page. So, this
looks fine to me.

With the cleanup above,

Reviewed-by: Suzuki K Poulose <suzuki.poulose@arm.com>

Re: [PATCH v2 0/4] Stop returning struct page from guest_memfd PFN lookup

[David Hildenbrand (Arm)]

On 8/18/26 11:15, Ackerley Tng wrote:
> KVM currently expects kvm_gmem_get_pfn() to return a refcounted struct
> page. Callers (such as x86 TDP MMU, arm64 Stage-2 fault handler, and SEV-SNP
> VMSA / RMP handlers) hold this refcount across page fault handling.
> 
> Holding a page refcount across fault handling is problematic for guest_memfd.
> In-place memory conversions between confidential computing shared and private
> states inspect folio refcounts to ensure exclusive ownership by guest_memfd.  A
> concurrent guest page fault taking a reference on the folio causes conversions
> to fail due to an elevated refcount.

Right. Won't we still, at least temporarily, grab a reference while looking up
the folio in the page cache, or will we be preventing that concurrent race with
locking?

> 
> guest_memfd already notifies KVM of page invalidations, so users of guest_memfd
> within KVM only need to respect the MMU invalidation protocol to safely rely on
> guest_memfd to ensure page presence.

Yes, the invalidation protocol is the crucial part. If we get that wrong, we're
in holy CVE land.

For GUP-fast, there was a similar discussion with MMU notifiers, but to this
day, KVM actually grabs+drops references.

[...]

> Removing struct page from kvm_gmem_get_pfn() also moves KVM closer toward
> supporting memory backends that are not backed by struct page.

Agreed, they should not be messing with the struct page at all.

-- 
Cheers,

David

Re: [PATCH v2 0/4] Stop returning struct page from guest_memfd PFN lookup

[Sean Christopherson]

On Tue, Aug 18, 2026, David Hildenbrand (Arm) wrote:
> On 8/18/26 11:15, Ackerley Tng wrote:
> > KVM currently expects kvm_gmem_get_pfn() to return a refcounted struct
> > page. Callers (such as x86 TDP MMU, arm64 Stage-2 fault handler, and SEV-SNP
> > VMSA / RMP handlers) hold this refcount across page fault handling.
> > 
> > Holding a page refcount across fault handling is problematic for guest_memfd.
> > In-place memory conversions between confidential computing shared and private
> > states inspect folio refcounts to ensure exclusive ownership by guest_memfd.  A
> > concurrent guest page fault taking a reference on the folio causes conversions
> > to fail due to an elevated refcount.
> 
> Right. Won't we still, at least temporarily, grab a reference while looking up
> the folio in the page cache, or will we be preventing that concurrent race with
> locking?

The latter.  What I want to aim for is that if the relevant guest_memfd range
has never been mmap()'d and there are no memory failures, then conversion is
guaranteed to not fail due to elevated refcounts.

Or to put it a different way, I want KVM's ABI to be that pausing vCPU is *NOT*
required to perform an in-place conversion.

Re: [PATCH v2 1/4] KVM: SEV: Treat unassigned RMP entry as benign race on PSMASH failure

[Michael Roth]

On Tue, Aug 18, 2026 at 09:15:52AM +0000, Ackerley Tng wrote:
> When handling an RMP fault, KVM attempts to split a 2MB page via PSMASH.
> If PSMASH fails, the only expected return value is FAIL_BADADDR, which does
> not distinguish the reason for the bad address. Hence, another RMP entry
> lookup is required to determine whether the failure was benign.
> 
> Specifically, KVM re-checks the RMP entry to determine if another CPU raced
> and already smashed the entry into 4KB pages.
> 
> A concurrent operation (such as guest_memfd truncation or hole punching)
> can also race and transition the page to shared, removing the page from the
> RMP table and causing PSMASH to fail. This can happen even if the page is
> still referenced by KVM, because guest_memfd reclaim transitions the RMP
> entry to shared when the folio is removed from the page cache.
> 
> Treat an unassigned RMP entry as an expected race when re-checking after a
> failed PSMASH, and skip logging an error warning.
> 
> Fixes: c63cf135cc99 ("KVM: SEV: Add support to handle RMP nested page faults")
> Signed-off-by: Ackerley Tng <ackerleytng@google.com>

Reviewed-by: Michael Roth <michael.roth@amd.com>

> ---
>  arch/x86/kvm/svm/sev.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/x86/kvm/svm/sev.c b/arch/x86/kvm/svm/sev.c
> index fcb41dfde4c02..b2738362a928b 100644
> --- a/arch/x86/kvm/svm/sev.c
> +++ b/arch/x86/kvm/svm/sev.c
> @@ -5074,10 +5074,11 @@ void sev_handle_rmp_fault(struct kvm_vcpu *vcpu, gpa_t gpa, u64 error_code)
>  		/*
>  		 * Look it up again. If it's 4K now then the PSMASH may have
>  		 * raced with another process and the issue has already resolved
> -		 * itself.
> +		 * itself. If it's not assigned, then this must have raced with
> +		 * another process that made this page shared.
>  		 */
>  		if (!snp_lookup_rmpentry(pfn, &assigned, &rmp_level) &&
> -		    assigned && rmp_level == PG_LEVEL_4K)
> +		    ((assigned && rmp_level == PG_LEVEL_4K) || !assigned))
>  			goto out;
>  
>  		pr_warn_ratelimited("SEV: Unable to split RMP entry for GPA 0x%llx PFN 0x%llx ret %d\n",
> 
> -- 
> 2.55.0.699.gb54405d56f-goog
> 

Re: [PATCH v2 2/4] KVM: SEV: Drop page refcount early during RMP fault handling

[Michael Roth]

On Tue, Aug 18, 2026 at 09:15:53AM +0000, Ackerley Tng wrote:
> From: Sean Christopherson <seanjc@google.com>
> 
> When handling an RMP fault, KVM retrieves the PFN for a private GPA from
> guest_memfd.
> 
> Drop the page reference immediately after retrieving the PFN instead of
> holding it across the entire handler so that the later patch can follow up
> with completely not returning refcounted pages from kvm_gmem_get_pfn().

Regarding this point:

> 
> On a first look, existing RMP table handling (psmash and checking for
> errors) might seem like it works fine, since truncation of the page from
> guest_memfd would have called rmp_make_shared() and removed the PFN from
> the RMP table. However, that is insufficient since a freed page may already
> be used in a different SNP VM.

In the code this patch is applied on top of, I think the kvm_gmem_get_pfn()
ref is enough to avoid the reused-by-another-SNP-VM scenario until after
caller releases the ref, so I think the above explanation should be adjusted
to also be preparatory for "the later patch".

Other than that:

Reviewed-by: Michael Roth <michael.roth@amd.com>

> 
> Hence, adopt the MMU invalidation protocol to guard committing anything
> based on the PFN.
> 
> Signed-off-by: Sean Christopherson <seanjc@google.com>
> Signed-off-by: Ackerley Tng <ackerleytng@google.com>
> ---
>  arch/x86/kvm/svm/sev.c | 39 ++++++++++++++++++++++++---------------
>  1 file changed, 24 insertions(+), 15 deletions(-)
> 
> diff --git a/arch/x86/kvm/svm/sev.c b/arch/x86/kvm/svm/sev.c
> index b2738362a928b..b34b11d7f8fad 100644
> --- a/arch/x86/kvm/svm/sev.c
> +++ b/arch/x86/kvm/svm/sev.c
> @@ -5003,6 +5003,7 @@ void sev_handle_rmp_fault(struct kvm_vcpu *vcpu, gpa_t gpa, u64 error_code)
>  	struct kvm_memory_slot *slot;
>  	struct kvm *kvm = vcpu->kvm;
>  	int order, rmp_level, ret;
> +	unsigned long mmu_seq;
>  	struct page *page;
>  	bool assigned;
>  	kvm_pfn_t pfn;
> @@ -5030,18 +5031,22 @@ void sev_handle_rmp_fault(struct kvm_vcpu *vcpu, gpa_t gpa, u64 error_code)
>  		return;
>  	}
>  
> +	mmu_seq = kvm->mmu_invalidate_seq;
> +	smp_rmb();
> +
>  	ret = kvm_gmem_get_pfn(kvm, slot, gfn, &pfn, &page, &order);
>  	if (ret) {
>  		pr_warn_ratelimited("SEV: Unexpected RMP fault, no backing page for private GPA 0x%llx\n",
>  				    gpa);
>  		return;
>  	}
> +	kvm_release_page_unused(page);
>  
>  	ret = snp_lookup_rmpentry(pfn, &assigned, &rmp_level);
>  	if (ret || !assigned) {
>  		pr_warn_ratelimited("SEV: Unexpected RMP fault, no assigned RMP entry found for GPA 0x%llx PFN 0x%llx error %d\n",
>  				    gpa, pfn, ret);
> -		goto out_no_trace;
> +		return;
>  	}
>  
>  	/*
> @@ -5069,27 +5074,31 @@ void sev_handle_rmp_fault(struct kvm_vcpu *vcpu, gpa_t gpa, u64 error_code)
>  	if (rmp_level == PG_LEVEL_4K)
>  		goto out;
>  
> -	ret = snp_rmptable_psmash(pfn);
> -	if (ret) {
> -		/*
> -		 * Look it up again. If it's 4K now then the PSMASH may have
> -		 * raced with another process and the issue has already resolved
> -		 * itself. If it's not assigned, then this must have raced with
> -		 * another process that made this page shared.
> -		 */
> -		if (!snp_lookup_rmpentry(pfn, &assigned, &rmp_level) &&
> -		    ((assigned && rmp_level == PG_LEVEL_4K) || !assigned))
> +	scoped_guard(read_lock, &kvm->mmu_lock) {
> +		if (mmu_invalidate_retry_gfn(kvm, mmu_seq, gfn))
>  			goto out;
>  
> -		pr_warn_ratelimited("SEV: Unable to split RMP entry for GPA 0x%llx PFN 0x%llx ret %d\n",
> -				    gpa, pfn, ret);
> +		ret = snp_rmptable_psmash(pfn);
> +		if (ret) {
> +			/*
> +			 * Look it up again. If it's 4K now then the PSMASH may
> +			 * have raced with another process and the issue has
> +			 * already resolved itself. If it's not assigned, then
> +			 * this must have raced with another process that made
> +			 * this page shared.
> +			 */
> +			if (!snp_lookup_rmpentry(pfn, &assigned, &rmp_level) &&
> +			    ((assigned && rmp_level == PG_LEVEL_4K) || !assigned))
> +				goto out;
> +
> +			pr_warn_ratelimited("SEV: Unable to split RMP entry for GPA 0x%llx PFN 0x%llx ret %d\n",
> +					    gpa, pfn, ret);
> +		}
>  	}
>  
>  	kvm_zap_gfn_range(kvm, gfn, gfn + PTRS_PER_PMD);
>  out:
>  	trace_kvm_rmp_fault(vcpu, gpa, pfn, error_code, rmp_level, ret);
> -out_no_trace:
> -	kvm_release_page_unused(page);
>  }
>  
>  static bool is_pfn_range_shared(kvm_pfn_t start, kvm_pfn_t end)
> 
> -- 
> 2.55.0.699.gb54405d56f-goog
> 

Re: [PATCH v2 3/4] KVM: SEV: Drop page refcount early in VMSA reload

[Michael Roth]

On Tue, Aug 18, 2026 at 09:15:54AM +0000, Ackerley Tng wrote:
> When reloading the guest VMSA for an SEV-SNP vCPU, KVM retrieves the PFN
> from guest_memfd.
> 
> Drop the page reference immediately after retrieving the PFN instead of
> holding it across MMU lock acquisition in preparation for a follow-up patch
> to stop returning page pointers from guest_memfd PFN lookups.
> 
> This is safe because the page's validity and presence are governed by KVM's
> MMU invalidation protocol rather than the page reference.
> 
> No functional change intended.
> 
> Signed-off-by: Ackerley Tng <ackerleytng@google.com>

Reviewed-by: Michael Roth <michael.roth@amd.com>

> ---
>  arch/x86/kvm/svm/sev.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/arch/x86/kvm/svm/sev.c b/arch/x86/kvm/svm/sev.c
> index b34b11d7f8fad..d3d620bc04dce 100644
> --- a/arch/x86/kvm/svm/sev.c
> +++ b/arch/x86/kvm/svm/sev.c
> @@ -4062,6 +4062,7 @@ static void __sev_snp_reload_vmsa(struct kvm_vcpu *vcpu, gpa_t gpa)
>  	 */
>  	if (kvm_gmem_get_pfn(vcpu->kvm, slot, gfn, &pfn, &page, NULL))
>  		return;
> +	kvm_release_page_clean(page);
>  
>  	read_lock(&kvm->mmu_lock);
>  	/*
> @@ -4076,8 +4077,6 @@ static void __sev_snp_reload_vmsa(struct kvm_vcpu *vcpu, gpa_t gpa)
>  	else
>  		svm->vmcb->control.vmsa_pa = pfn_to_hpa(pfn);
>  	read_unlock(&kvm->mmu_lock);
> -
> -	kvm_release_page_clean(page);
>  }
>  
>  /*
> 
> -- 
> 2.55.0.699.gb54405d56f-goog
> 

Re: [PATCH v2 4/4] KVM: guest_memfd: Stop returning struct page from PFN lookup

[Michael Roth]

On Tue, Aug 18, 2026 at 09:15:55AM +0000, Ackerley Tng wrote:
> From: Sean Christopherson <seanjc@google.com>
> 
> KVM currently expects guest_memfd PFN lookups to return a refcounted
> struct page, which callers hold across fault handling.
> 
> Holding a page reference across fault handling is problematic for
> guest_memfd. In-place memory conversions between confidential
> computing shared and private states inspect folio refcounts to ensure
> exclusive ownership by guest_memfd. A concurrent guest page fault
> taking a reference on the folio causes conversions to fail due to an
> elevated refcount.
> 
> guest_memfd already notifies KVM of page invalidations, so callers
> within KVM only need to respect the MMU invalidation protocol to safely
> rely on guest_memfd for page presence.
> 
> Furthermore, removing struct page from the guest_memfd PFN lookup moves
> KVM closer toward supporting memory backends that are not backed by
> struct page.
> 
> Drop the folio reference immediately before returning from the
> guest_memfd PFN lookup, and stop returning the struct page pointer.
> 
> For ARM, initialize the local page pointer to NULL so that the shared
> cleanup path that releases fault-in pages safely no-ops for guest_memfd.
> 
> For x86, no additional changes are required in the MMU fault path
> because the page fault tracking structure is zero-initialized at the
> start of page fault handling, ensuring the refcounted page pointer is
> already NULL.
> 
> Reported-by: Yan Zhao <yan.y.zhao@intel.com>
> Closes: https://lore.kernel.org/all/anZ4W9o5pTWIEgMY@yzhao56-desk.sh.intel.com/
> Signed-off-by: Sean Christopherson <seanjc@google.com>
> Co-developed-by: Yan Zhao <yan.y.zhao@intel.com>
> Signed-off-by: Yan Zhao <yan.y.zhao@intel.com>
> Co-developed-by: Ackerley Tng <ackerleytng@google.com>
> Signed-off-by: Ackerley Tng <ackerleytng@google.com>

Other than what Suzuki pointed out:

Reviewed-by: Michael Roth <michael.roth@amd.com>
Tested-by: Michael Roth <michael.roth@amd.com>

> ---
>  arch/arm64/kvm/mmu.c     | 4 ++--
>  arch/arm64/kvm/nested.c  | 4 ++--
>  arch/x86/kvm/mmu/mmu.c   | 2 +-
>  arch/x86/kvm/svm/sev.c   | 8 ++------
>  include/linux/kvm_host.h | 6 ++----
>  virt/kvm/guest_memfd.c   | 9 ++-------
>  6 files changed, 11 insertions(+), 22 deletions(-)
> 
> diff --git a/arch/arm64/kvm/mmu.c b/arch/arm64/kvm/mmu.c
> index 6c941aaa10c63..e5d637a5ec558 100644
> --- a/arch/arm64/kvm/mmu.c
> +++ b/arch/arm64/kvm/mmu.c
> @@ -1613,7 +1613,7 @@ static int gmem_abort(const struct kvm_s2_fault_desc *s2fd)
>  	enum kvm_pgtable_prot prot = KVM_PGTABLE_PROT_R;
>  	struct kvm_pgtable *pgt = s2fd->vcpu->arch.hw_mmu->pgt;
>  	unsigned long mmu_seq;
> -	struct page *page;
> +	struct page *page = NULL;
>  	struct kvm *kvm = s2fd->vcpu->kvm;
>  	void *memcache = NULL;
>  	kvm_pfn_t pfn;
> @@ -1641,7 +1641,7 @@ static int gmem_abort(const struct kvm_s2_fault_desc *s2fd)
>  	/* Pairs with the smp_wmb() in kvm_mmu_invalidate_end(). */
>  	smp_rmb();
>  
> -	ret = kvm_gmem_get_pfn(kvm, s2fd->memslot, gfn, &pfn, &page, NULL);
> +	ret = kvm_gmem_get_pfn(kvm, s2fd->memslot, gfn, &pfn, NULL);
>  	if (ret) {
>  		kvm_prepare_memory_fault_exit(s2fd->vcpu, s2fd->fault_ipa, PAGE_SIZE,
>  					      write_fault, exec_fault, false);
> diff --git a/arch/arm64/kvm/nested.c b/arch/arm64/kvm/nested.c
> index fb54f6dad995c..43523bb17621a 100644
> --- a/arch/arm64/kvm/nested.c
> +++ b/arch/arm64/kvm/nested.c
> @@ -1360,7 +1360,7 @@ static int kvm_translate_vncr(struct kvm_vcpu *vcpu, bool *is_gmem)
>  	bool write_fault, writable;
>  	unsigned long mmu_seq;
>  	struct vncr_tlb *vt;
> -	struct page *page;
> +	struct page *page = NULL;
>  	u64 va, pfn, gfn;
>  	int ret;
>  
> @@ -1411,7 +1411,7 @@ static int kvm_translate_vncr(struct kvm_vcpu *vcpu, bool *is_gmem)
>  		if (is_error_noslot_pfn(pfn) || (write_fault && !writable))
>  			return -EFAULT;
>  	} else {
> -		ret = kvm_gmem_get_pfn(vcpu->kvm, memslot, gfn, &pfn, &page, NULL);
> +		ret = kvm_gmem_get_pfn(vcpu->kvm, memslot, gfn, &pfn, NULL);
>  		if (ret) {
>  			kvm_prepare_memory_fault_exit(vcpu, vt->wr.pa, PAGE_SIZE,
>  					      write_fault, false, false);
> diff --git a/arch/x86/kvm/mmu/mmu.c b/arch/x86/kvm/mmu/mmu.c
> index c519e8e8d646f..129d403308051 100644
> --- a/arch/x86/kvm/mmu/mmu.c
> +++ b/arch/x86/kvm/mmu/mmu.c
> @@ -4604,7 +4604,7 @@ static int kvm_mmu_faultin_pfn_gmem(struct kvm_vcpu *vcpu,
>  	}
>  
>  	r = kvm_gmem_get_pfn(vcpu->kvm, fault->slot, fault->gfn, &fault->pfn,
> -			     &fault->refcounted_page, &max_order);
> +			     &max_order);
>  	if (r) {
>  		kvm_mmu_prepare_memory_fault_exit(vcpu, fault);
>  		return r;
> diff --git a/arch/x86/kvm/svm/sev.c b/arch/x86/kvm/svm/sev.c
> index d3d620bc04dce..32db979daaaf0 100644
> --- a/arch/x86/kvm/svm/sev.c
> +++ b/arch/x86/kvm/svm/sev.c
> @@ -4016,7 +4016,6 @@ static void __sev_snp_reload_vmsa(struct kvm_vcpu *vcpu, gpa_t gpa)
>  	struct kvm *kvm = vcpu->kvm;
>  	gfn_t gfn = gpa_to_gfn(gpa);
>  	unsigned long mmu_seq;
> -	struct page *page;
>  	kvm_pfn_t pfn;
>  
>  	lockdep_assert_held(&svm->sev_es.snp_vmsa_mutex);
> @@ -4060,9 +4059,8 @@ static void __sev_snp_reload_vmsa(struct kvm_vcpu *vcpu, gpa_t gpa)
>  	 * The new VMSA will be private memory guest memory, so retrieve the
>  	 * PFN from the gmem backend.
>  	 */
> -	if (kvm_gmem_get_pfn(vcpu->kvm, slot, gfn, &pfn, &page, NULL))
> +	if (kvm_gmem_get_pfn(vcpu->kvm, slot, gfn, &pfn, NULL))
>  		return;
> -	kvm_release_page_clean(page);
>  
>  	read_lock(&kvm->mmu_lock);
>  	/*
> @@ -5003,7 +5001,6 @@ void sev_handle_rmp_fault(struct kvm_vcpu *vcpu, gpa_t gpa, u64 error_code)
>  	struct kvm *kvm = vcpu->kvm;
>  	int order, rmp_level, ret;
>  	unsigned long mmu_seq;
> -	struct page *page;
>  	bool assigned;
>  	kvm_pfn_t pfn;
>  	gfn_t gfn;
> @@ -5033,13 +5030,12 @@ void sev_handle_rmp_fault(struct kvm_vcpu *vcpu, gpa_t gpa, u64 error_code)
>  	mmu_seq = kvm->mmu_invalidate_seq;
>  	smp_rmb();
>  
> -	ret = kvm_gmem_get_pfn(kvm, slot, gfn, &pfn, &page, &order);
> +	ret = kvm_gmem_get_pfn(kvm, slot, gfn, &pfn, &order);
>  	if (ret) {
>  		pr_warn_ratelimited("SEV: Unexpected RMP fault, no backing page for private GPA 0x%llx\n",
>  				    gpa);
>  		return;
>  	}
> -	kvm_release_page_unused(page);
>  
>  	ret = snp_lookup_rmpentry(pfn, &assigned, &rmp_level);
>  	if (ret || !assigned) {
> diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h
> index 03bfc92864b6e..502465119ca0c 100644
> --- a/include/linux/kvm_host.h
> +++ b/include/linux/kvm_host.h
> @@ -2586,13 +2586,11 @@ static inline bool kvm_mem_is_private(struct kvm *kvm, gfn_t gfn)
>  
>  #ifdef CONFIG_KVM_GUEST_MEMFD
>  int kvm_gmem_get_pfn(struct kvm *kvm, struct kvm_memory_slot *slot,
> -		     gfn_t gfn, kvm_pfn_t *pfn, struct page **page,
> -		     int *max_order);
> +		     gfn_t gfn, kvm_pfn_t *pfn, int *max_order);
>  #else
>  static inline int kvm_gmem_get_pfn(struct kvm *kvm,
>  				   struct kvm_memory_slot *slot, gfn_t gfn,
> -				   kvm_pfn_t *pfn, struct page **page,
> -				   int *max_order)
> +				   kvm_pfn_t *pfn, int *max_order)
>  {
>  	KVM_BUG_ON(1, kvm);
>  	return -EIO;
> diff --git a/virt/kvm/guest_memfd.c b/virt/kvm/guest_memfd.c
> index b596486d184ca..589762140c3ef 100644
> --- a/virt/kvm/guest_memfd.c
> +++ b/virt/kvm/guest_memfd.c
> @@ -751,8 +751,7 @@ static struct folio *__kvm_gmem_get_pfn(struct file *file,
>  }
>  
>  int kvm_gmem_get_pfn(struct kvm *kvm, struct kvm_memory_slot *slot,
> -		     gfn_t gfn, kvm_pfn_t *pfn, struct page **page,
> -		     int *max_order)
> +		     gfn_t gfn, kvm_pfn_t *pfn, int *max_order)
>  {
>  	pgoff_t index = kvm_gmem_get_index(slot, gfn);
>  	struct folio *folio;
> @@ -780,11 +779,7 @@ int kvm_gmem_get_pfn(struct kvm *kvm, struct kvm_memory_slot *slot,
>  #endif
>  
>  	folio_unlock(folio);
> -
> -	if (!r)
> -		*page = folio_file_page(folio, index);
> -	else
> -		folio_put(folio);
> +	folio_put(folio);
>  
>  	return r;
>  }
> 
> -- 
> 2.55.0.699.gb54405d56f-goog
> 

Re: [PATCH v2 0/4] Stop returning struct page from guest_memfd PFN lookup

[David Hildenbrand (Arm)]

On 8/18/26 21:55, Sean Christopherson wrote:
> On Tue, Aug 18, 2026, David Hildenbrand (Arm) wrote:
>> On 8/18/26 11:15, Ackerley Tng wrote:
>>> KVM currently expects kvm_gmem_get_pfn() to return a refcounted struct
>>> page. Callers (such as x86 TDP MMU, arm64 Stage-2 fault handler, and SEV-SNP
>>> VMSA / RMP handlers) hold this refcount across page fault handling.
>>>
>>> Holding a page refcount across fault handling is problematic for guest_memfd.
>>> In-place memory conversions between confidential computing shared and private
>>> states inspect folio refcounts to ensure exclusive ownership by guest_memfd.  A
>>> concurrent guest page fault taking a reference on the folio causes conversions
>>> to fail due to an elevated refcount.
>>
>> Right. Won't we still, at least temporarily, grab a reference while looking up
>> the folio in the page cache, or will we be preventing that concurrent race with
>> locking?
> 
> The latter.  What I want to aim for is that if the relevant guest_memfd range
> has never been mmap()'d and there are no memory failures, then conversion is
> guaranteed to not fail due to elevated refcounts.
> 
> Or to put it a different way, I want KVM's ABI to be that pausing vCPU is *NOT*
> required to perform an in-place conversion.

Having the VM access a page that is currently under conversion (triggered by the
VM) should not be the common case, no? Except, prefaulting, of course.

-- 
Cheers,

David

Re: [PATCH v2 4/4] KVM: guest_memfd: Stop returning struct page from PFN lookup

[Yan Zhao]

On Tue, Aug 18, 2026 at 09:15:55AM +0000, Ackerley Tng wrote:
> From: Sean Christopherson <seanjc@google.com>
> 
> KVM currently expects guest_memfd PFN lookups to return a refcounted
> struct page, which callers hold across fault handling.
> 
> Holding a page reference across fault handling is problematic for
> guest_memfd. In-place memory conversions between confidential
> computing shared and private states inspect folio refcounts to ensure
> exclusive ownership by guest_memfd. A concurrent guest page fault
> taking a reference on the folio causes conversions to fail due to an
> elevated refcount.
Nit:
As this series is based on kvm-x86/next, where there's no in-place memory
conversion yet, kvm_gmem_get_pfn() does not hold shared filemap invalidate lock.

However, the benefit of dropping the folio reference immediately before
returning from the guest_memfd PFN lookup -- preventing conversion failures due
to an elevated refcount -- should be effective only if the reference is dropped
before releasing the shared filemap invalidate lock.

Do we need to make this info clear, since I think it's important? :)

> guest_memfd already notifies KVM of page invalidations, so callers
> within KVM only need to respect the MMU invalidation protocol to safely
> rely on guest_memfd for page presence.

Could we also explain why the lack of SetPageDirty() (and mark_page_accessed())
for a gmem page, due to NULL being passed to kvm_release_faultin_page(), is
harmless?

> Furthermore, removing struct page from the guest_memfd PFN lookup moves
> KVM closer toward supporting memory backends that are not backed by
> struct page.
> 
> Drop the folio reference immediately before returning from the
> guest_memfd PFN lookup, and stop returning the struct page pointer.
> 
> For ARM, initialize the local page pointer to NULL so that the shared
> cleanup path that releases fault-in pages safely no-ops for guest_memfd.
> 
> For x86, no additional changes are required in the MMU fault path
> because the page fault tracking structure is zero-initialized at the
> start of page fault handling, ensuring the refcounted page pointer is
> already NULL.
>
Otherwise, except for the missing hunk on arm, LGTM.

Tested-by: Yan Zhao <yan.y.zhao@intel.com> 

Re: [PATCH v2 0/4] Stop returning struct page from guest_memfd PFN lookup

[Sean Christopherson]

On Wed, Aug 19, 2026, David Hildenbrand (Arm) wrote:
> On 8/18/26 21:55, Sean Christopherson wrote:
> > On Tue, Aug 18, 2026, David Hildenbrand (Arm) wrote:
> >> On 8/18/26 11:15, Ackerley Tng wrote:
> >>> KVM currently expects kvm_gmem_get_pfn() to return a refcounted struct
> >>> page. Callers (such as x86 TDP MMU, arm64 Stage-2 fault handler, and SEV-SNP
> >>> VMSA / RMP handlers) hold this refcount across page fault handling.
> >>>
> >>> Holding a page refcount across fault handling is problematic for guest_memfd.
> >>> In-place memory conversions between confidential computing shared and private
> >>> states inspect folio refcounts to ensure exclusive ownership by guest_memfd.  A
> >>> concurrent guest page fault taking a reference on the folio causes conversions
> >>> to fail due to an elevated refcount.
> >>
> >> Right. Won't we still, at least temporarily, grab a reference while looking up
> >> the folio in the page cache, or will we be preventing that concurrent race with
> >> locking?
> > 
> > The latter.  What I want to aim for is that if the relevant guest_memfd range
> > has never been mmap()'d and there are no memory failures, then conversion is
> > guaranteed to not fail due to elevated refcounts.
> > 
> > Or to put it a different way, I want KVM's ABI to be that pausing vCPU is *NOT*
> > required to perform an in-place conversion.
> 
> Having the VM access a page that is currently under conversion (triggered by the
> VM) should not be the common case, no? Except, prefaulting, of course.

"not be the common case" is likely an understatement.  In practice, I don't it
will happen outside of guest bugs and KVM testcases.

It's the testcases that I want to "unblock" though.  If we commit to never having
to pause vCPUs, even if the guest is misbehaving, then that gives us deterministic
behavior we can validate, i.e. a way to detect similar regressions in the future.

I don't expect any regressions would be super problematic, but being able to treat
any failed conversion as a KVM bug (for the curated setup) mitigates the risk of
death by a thousand cuts, i.e. reduces the risk of gradually degrading conversion
performance because more and more transient references being taken by KVM.

Re: [PATCH v2 2/4] KVM: SEV: Drop page refcount early during RMP fault handling

[Ackerley Tng]

sashiko-bot@kernel.org writes:

> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [Medium] Dropping the page refcount early introduces a race condition where a concurrent `guest_memfd` invalidation can trigger a false-positive 'Unexpected RMP fault' kernel warning.
> --
>
> --- Patch [2]: [PATCH v2 2/4] KVM: SEV: Drop page refcount early during RMP fault handling ---
> commit 7d0f356d8592ce1804052b0fdb61fccf3a11ecaa
> Author: Sean Christopherson <seanjc@google.com>
>
> KVM: SEV: Drop page refcount early during RMP fault handling
>
> When handling an RMP fault, KVM retrieves the PFN for a private GPA from
> guest_memfd. This patch drops the page reference immediately after retrieving
> the PFN instead of holding it across the entire handler, and adopts the MMU
> invalidation protocol to guard committing anything based on the PFN.
>
>> diff --git a/arch/x86/kvm/svm/sev.c b/arch/x86/kvm/svm/sev.c
>> index b2738362a928b..b34b11d7f8fad 100644
>> --- a/arch/x86/kvm/svm/sev.c
>> +++ b/arch/x86/kvm/svm/sev.c
>
> [ ... ]
>
>> @@ -5030,18 +5031,22 @@ void sev_handle_rmp_fault(struct kvm_vcpu *vcpu, gpa_t gpa, u64 error_code)
>>  		return;
>>  	}
>>
>> +	mmu_seq = kvm->mmu_invalidate_seq;
>> +	smp_rmb();
>> +
>>  	ret = kvm_gmem_get_pfn(kvm, slot, gfn, &pfn, &page, &order);
>>  	if (ret) {
>>  		pr_warn_ratelimited("SEV: Unexpected RMP fault, no backing page for private GPA 0x%llx\n",
>>  				    gpa);
>>  		return;
>>  	}
>> +	kvm_release_page_unused(page);
>>
>>  	ret = snp_lookup_rmpentry(pfn, &assigned, &rmp_level);
>>  	if (ret || !assigned) {
>>  		pr_warn_ratelimited("SEV: Unexpected RMP fault, no assigned RMP entry found for GPA 0x%llx PFN 0x%llx error %d\n",
>>  				    gpa, pfn, ret);
>> -		goto out_no_trace;
>> +		return;
>>  	}
>
> [Severity: Medium]
> Since the page reference is dropped before this lookup in
> sev_handle_rmp_fault(), could a concurrent guest_memfd hole-punch drop
> the file's reference to the page and free it?
>
> If the page is freed, the RMP entry would transition to shared/unassigned.
> Could this then cause snp_lookup_rmpentry() to see the page as unassigned
> and incorrectly emit this warning before the mmu_invalidate_retry_gfn()
> check happens?
>

Yes, this can happen. I'll also check for invalidations before
warning in the next revision.

> --
> Sashiko AI review · https://sashiko.dev/#/patchset/20260818-gmem-no-return-page-v2-0-5298f42d49bb@google.com?part=2

Re: [PATCH v2 4/4] KVM: guest_memfd: Stop returning struct page from PFN lookup

[Ackerley Tng]

sashiko-bot@kernel.org writes:

> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
>
> Pre-existing issues:
> - [Critical] Concurrent execution of KVM_SEV_SNP_LAUNCH_UPDATE and a vCPU page fault triggers a race condition leading to a host kernel panic.
> --
>
> --- Patch [4]: [PATCH v2 4/4] KVM: guest_memfd: Stop returning struct page from PFN lookup ---
> commit 766989bc1910148a83bffa481ae22b1d1eb71f01
> Author: Sean Christopherson <seanjc@google.com>
>
> KVM: guest_memfd: Stop returning struct page from PFN lookup
>
> This commit stops returning a refcounted struct page from guest_memfd PFN
> lookups, dropping the folio reference immediately instead. This prevents
> concurrent guest page faults from holding elevated refcounts that would
> interfere with in-place memory conversions between shared and private states.
>
> Link: https://lore.kernel.org/all/anZ4W9o5pTWIEgMY@yzhao56-desk.sh.intel.com/
>
>> diff --git a/virt/kvm/guest_memfd.c b/virt/kvm/guest_memfd.c
>> index b596486d184ca..589762140c3ef 100644
>> --- a/virt/kvm/guest_memfd.c
>> +++ b/virt/kvm/guest_memfd.c
>
> [ ... ]
>
>> @@ -780,11 +779,7 @@ int kvm_gmem_get_pfn(struct kvm *kvm, struct kvm_memory_slot *slot,
>>  #endif
>>
>>  	folio_unlock(folio);
>> -
>> -	if (!r)
>> -		*page = folio_file_page(folio, index);
>> -	else
>> -		folio_put(folio);
>> +	folio_put(folio);
>>
>>  	return r;
>>  }
>
> [Severity: Critical]
> This is a pre-existing issue, but is it possible for a concurrent page
> fault to trigger a host kernel panic via a hardware RMP fault here?
>

I thought guest vCPU faults and populate are mutually exclusive, but I
couldn't actually find anything that blocks populate from running after
a guest is finalized.

So suppose the vCPU does fault in a page, the zeroing and make_private
happens under folio lock. After the page is obtained on the populate
side, there's no more zeroing, and sev_gmem_post_populate() would fail
since the page is already assigned.

Suppose populate goes first, the folio_lock keeps the vCPU fault
out. The folio is zeroed and made private. This time,
sev_gmem_post_populate() would fail because the guest has been
finalized. Still no zeroing after making private.

If populate finishes successfully, then the folio is marked
uptodate. This is done outside of the folio_lock(), but at this time the
vCPU can't run so by the time any vCPU fault can happen, the folio would
be uptodate, and there would be no more zeroing.

Seems like all is good.

After conversions, filemap_invalidate_lock() would make getting a pfn
and populate exclusive, so that's even better.

Let me know if I missed anything.

> Looking at kvm_gmem_get_pfn() in virt/kvm/guest_memfd.c, if the folio is
> not up to date, it zeroes the page and marks it up to date:
>
>     if (!folio_test_uptodate(folio)) {
>         clear_highpage(folio_page(folio, 0));
>         folio_mark_uptodate(folio);
>     }
>
> However, in the KVM_SEV_SNP_LAUNCH_UPDATE path, __kvm_gmem_populate()
> fetches a folio, unlocks it, and calls post_populate() (which maps to
> sev_gmem_post_populate() and calls rmp_make_private()) before finally
> marking the folio up to date.
>
> If a malicious userspace process races a vCPU page fault against
> KVM_SEV_SNP_LAUNCH_UPDATE, could the vCPU thread running kvm_gmem_get_pfn()
> acquire the lock on the not-yet-uptodate folio and call clear_highpage()
> after rmp_make_private() has already executed?
>
> If clear_highpage() writes to the page after it has been made RMP-private,
> this would trigger a fatal hardware RMP fault on the host CPU. If it writes
> just before, it appears it could silently corrupt the encrypted firmware
> payload.
>
> --
> Sashiko AI review · https://sashiko.dev/#/patchset/20260818-gmem-no-return-page-v2-0-5298f42d49bb@google.com?part=4

Re: [PATCH v2 3/4] KVM: SEV: Drop page refcount early in VMSA reload

[Ackerley Tng]

sashiko-bot@kernel.org writes:

> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
>
> Pre-existing issues:
> - [Critical] A preexisting vulnerability in `sev_gmem_make_shared` allows a malicious SEV-SNP guest to cause a host kernel panic by transitioning a `guest_memfd` page to VMSA state. When the page is subsequently freed, KVM fails to reclaim it and returns the still-firmware-owned page to the host page allocator.
> --
>
> --- Patch [3]: [PATCH v2 3/4] KVM: SEV: Drop page refcount early in VMSA reload ---
> commit 597ff1c52e82911967f2122ff1d77aa79a4a2d9b
> Author: Ackerley Tng <ackerleytng@google.com>
>
> KVM: SEV: Drop page refcount early in VMSA reload
>
> This commit modifies the guest VMSA reload path for SEV-SNP vCPUs to drop
> the page reference immediately after retrieving the PFN from guest_memfd.
> This avoids holding the reference across MMU lock acquisition and prepares
> for future changes. The page's presence remains governed by KVM's MMU
> invalidation protocol.
>
>> diff --git a/arch/x86/kvm/svm/sev.c b/arch/x86/kvm/svm/sev.c
>> index b34b11d7f8fad..d3d620bc04dce 100644
>> --- a/arch/x86/kvm/svm/sev.c
>> +++ b/arch/x86/kvm/svm/sev.c
>> @@ -4062,6 +4062,7 @@ static void __sev_snp_reload_vmsa(struct kvm_vcpu *vcpu, gpa_t gpa)
>>  	 */
>>  	if (kvm_gmem_get_pfn(vcpu->kvm, slot, gfn, &pfn, &page, NULL))
>>  		return;
>> +	kvm_release_page_clean(page);
>>
>>  	read_lock(&kvm->mmu_lock);
>>  	/*
>> @@ -4076,8 +4077,6 @@ static void __sev_snp_reload_vmsa(struct kvm_vcpu *vcpu, gpa_t gpa)
>>  	else
>>  		svm->vmcb->control.vmsa_pa = pfn_to_hpa(pfn);
>>  	read_unlock(&kvm->mmu_lock);
>> -
>> -	kvm_release_page_clean(page);
>>  }
>
> [Severity: Critical]
> This is a pre-existing issue, but can this allow a malicious guest to
> cause a host kernel panic?
>
> Looking at sev_gmem_make_shared() in arch/x86/kvm/svm/sev.c:
>
> arch/x86/kvm/svm/sev.c:sev_gmem_make_shared() {
>     ...
> 		rc = rmp_make_shared(pfn, use_2m_update ? PG_LEVEL_2M : PG_LEVEL_4K);
> 		if (WARN_ONCE(rc, "SEV: Failed to update RMP entry for PFN 0x%llx error %d\n",
> 			      pfn, rc))
> 			goto next_pfn;
>     ...
> }
>
> If an untrusted SEV-SNP guest modifies a guest_memfd page state to VMSA
> using RMPADJUST, and the page is subsequently freed, rmp_make_shared()
> will architecturally fail on the VMSA page.
>

I need some help with this one.

For the initial VMSA page, IIUC snp_safe_alloc_page() gets a
non-guest_memfd page. This won't ever go through sev_gmem_make_shared(),
so we're good. snp_leak_pages() doesn't actually leak pages, it just
tracks leaked pages, but __free_page() is skipped so the page is really
leaked.

> Because the error is bypassed without calling snp_leak_pages(), the
> firmware-owned page is returned to the host page allocator.
>

For guest-provided VMSA pages, those can be guest_memfd pages. The
refcount taken is the one guest_memfd holds. Without considering
conversions, just considering truncation, sev_gmem_make_shared() is
called from kvm_gmem_free_folio(). Sashiko is saying that
rmp_make_shared() will fail, and the folio is freed.

Sounds like this would be a problem if the folio is used elsewhere...?

> Could this lead to an RMP violation and a host kernel panic when the host
> later attempts to access the freed page?
>
> --
> Sashiko AI review · https://sashiko.dev/#/patchset/20260818-gmem-no-return-page-v2-0-5298f42d49bb@google.com?part=3

Re: [PATCH v2 4/4] KVM: guest_memfd: Stop returning struct page from PFN lookup

[Ackerley Tng]

Yan Zhao <yan.y.zhao@intel.com> writes:

> On Tue, Aug 18, 2026 at 09:15:55AM +0000, Ackerley Tng wrote:
>> From: Sean Christopherson <seanjc@google.com>
>>
>> KVM currently expects guest_memfd PFN lookups to return a refcounted
>> struct page, which callers hold across fault handling.
>>
>> Holding a page reference across fault handling is problematic for
>> guest_memfd. In-place memory conversions between confidential
>> computing shared and private states inspect folio refcounts to ensure
>> exclusive ownership by guest_memfd. A concurrent guest page fault
>> taking a reference on the folio causes conversions to fail due to an
>> elevated refcount.
> Nit:
> As this series is based on kvm-x86/next, where there's no in-place memory
> conversion yet, kvm_gmem_get_pfn() does not hold shared filemap invalidate lock.
>
> However, the benefit of dropping the folio reference immediately before
> returning from the guest_memfd PFN lookup -- preventing conversion failures due
> to an elevated refcount -- should be effective only if the reference is dropped
> before releasing the shared filemap invalidate lock.
>
> Do we need to make this info clear, since I think it's important? :)
>

Is this what you meant?

  In-place conversions uses the filemap_invalidate_lock() for
  synchronization of shared/private state. In kvm_gmem_get_pfn(), the
  PFN needs to be prepared according to its shared/private state. Hence,
  the filemap_invalidate_lock() is held while guest_memfd gets a folio
  and decides to make private before returning a PFN.

  In kvm_gmem_get_pfn(), the folio refcount is dropped before releasing
  filemap_invalidate_lock(). This ensures that a competing conversion
  grabbing the filemap_invalidate_lock() will never see an elevated
  refcount due to guest_memfd's folio-getting process.

It seems a bit weird to fit this into the commit message for this
patch. I think I could put the above two paragraphs into the patch that
introduces the filemap_invalidate_lock in kvm_gmem_get_pfn()?

>> guest_memfd already notifies KVM of page invalidations, so callers
>> within KVM only need to respect the MMU invalidation protocol to safely
>> rely on guest_memfd for page presence.
>>
>> Furthermore, removing struct page from the guest_memfd PFN lookup moves
>> KVM closer toward supporting memory backends that are not backed by
>> struct page.
>
> Could we also explain why the lack of SetPageDirty() (and mark_page_accessed())
> for a gmem page, due to NULL being passed to kvm_release_faultin_page(), is
> harmless?
>

Sounds good. What do you think of this, continuing from the paragraph
beginning "Furthermore":

  Drop the folio reference immediately before returning from the
  guest_memfd PFN lookup, and stop returning the struct page pointer.

  ARM's gmem_abort() is guest_memfd specific. Since guest_memfd no longer
  returns a page pointer, there's also no need to do any
  freeing. kvm_release_faultin_page() originally also serves to set the page
  dirty and accessed under some conditions. The dirty and accessed flags
  don't matter for guest_memfd anyway, so it is safe to just drop the call to
  kvm_release_faultin_page().

  For ARM's kvm_translate_vncr(), initialize the local page pointer to NULL
  so that the shared cleanup path that releases fault-in pages safely no-ops
  for guest_memfd.

  For x86, no additional changes are required in the MMU fault path
  because the page fault tracking structure is zero-initialized at the
  start of page fault handling, ensuring the refcounted page pointer is
  already NULL.

>>
>> [...snip...]
>>

Re: [PATCH v2 2/4] KVM: SEV: Drop page refcount early during RMP fault handling

[Ackerley Tng]

Michael Roth <michael.roth@amd.com> writes:

> On Tue, Aug 18, 2026 at 09:15:53AM +0000, Ackerley Tng wrote:
>> From: Sean Christopherson <seanjc@google.com>
>>
>> When handling an RMP fault, KVM retrieves the PFN for a private GPA from
>> guest_memfd.
>>
>> Drop the page reference immediately after retrieving the PFN instead of
>> holding it across the entire handler so that the later patch can follow up
>> with completely not returning refcounted pages from kvm_gmem_get_pfn().
>
> Regarding this point:
>
>>
>> On a first look, existing RMP table handling (psmash and checking for
>> errors) might seem like it works fine, since truncation of the page from
>> guest_memfd would have called rmp_make_shared() and removed the PFN from
>> the RMP table. However, that is insufficient since a freed page may already
>> be used in a different SNP VM.
>
> In the code this patch is applied on top of, I think the kvm_gmem_get_pfn()
> ref is enough to avoid the reused-by-another-SNP-VM scenario until after
> caller releases the ref, so I think the above explanation should be adjusted
> to also be preparatory for "the later patch".
>

I think this sequence of events is possible:

CPU 0: sev_handle_rmp_fault()
CPU 0:   kvm_gmem_get_pfn()
CPU 0:     filemap_invalidate_lock()
CPU 0:     refcount++
CPU 0:     filemap_invalidate_unlock()
CPU 0:     refcount-- <<== because kvm_release_page_unused(page);

CPU 1: truncate()
CPU 1:   filemap_invalidate_lock()
CPU 1:   refcount--
CPU 1:     kvm_gmem_free_folio()
CPU 1:       sev_gmem_make_shared()
CPU 1:     folio is freed
CPU 1:   filemap_invalidate_unlock()

CPU 2: in some other SNP VM,
CPU 2: kvm_gmem_get_pfn() gets the freed folio
CPU 2:   sev_gmem_make_private()

CPU 0:   snp_lookup_rmpentry(pfn, &assigned, &rmp_level);
           <<== it's assigned but to some other SNP VM
CPU 0:   snp_rmptable_psmash(pfn);
           <<== this psmash would be smashing in some other SNP VM

And hence I think we do need to check for invalidations using the MMU
invalidation protocol.

> Other than that:
>
> Reviewed-by: Michael Roth <michael.roth@amd.com>
>
>>
>> Hence, adopt the MMU invalidation protocol to guard committing anything
>> based on the PFN.
>>
>> Signed-off-by: Sean Christopherson <seanjc@google.com>
>> Signed-off-by: Ackerley Tng <ackerleytng@google.com>
>> ---
>>  arch/x86/kvm/svm/sev.c | 39 ++++++++++++++++++++++++---------------
>>  1 file changed, 24 insertions(+), 15 deletions(-)
>>
>> diff --git a/arch/x86/kvm/svm/sev.c b/arch/x86/kvm/svm/sev.c
>> index b2738362a928b..b34b11d7f8fad 100644
>> --- a/arch/x86/kvm/svm/sev.c
>> +++ b/arch/x86/kvm/svm/sev.c
>> @@ -5003,6 +5003,7 @@ void sev_handle_rmp_fault(struct kvm_vcpu *vcpu, gpa_t gpa, u64 error_code)
>>  	struct kvm_memory_slot *slot;
>>  	struct kvm *kvm = vcpu->kvm;
>>  	int order, rmp_level, ret;
>> +	unsigned long mmu_seq;
>>  	struct page *page;
>>  	bool assigned;
>>  	kvm_pfn_t pfn;
>> @@ -5030,18 +5031,22 @@ void sev_handle_rmp_fault(struct kvm_vcpu *vcpu, gpa_t gpa, u64 error_code)
>>  		return;
>>  	}
>>
>> +	mmu_seq = kvm->mmu_invalidate_seq;
>> +	smp_rmb();
>> +
>>  	ret = kvm_gmem_get_pfn(kvm, slot, gfn, &pfn, &page, &order);
>>  	if (ret) {
>>  		pr_warn_ratelimited("SEV: Unexpected RMP fault, no backing page for private GPA 0x%llx\n",
>>  				    gpa);
>>  		return;
>>  	}
>> +	kvm_release_page_unused(page);
>>
>>  	ret = snp_lookup_rmpentry(pfn, &assigned, &rmp_level);
>>  	if (ret || !assigned) {
>>  		pr_warn_ratelimited("SEV: Unexpected RMP fault, no assigned RMP entry found for GPA 0x%llx PFN 0x%llx error %d\n",
>>  				    gpa, pfn, ret);
>> -		goto out_no_trace;
>> +		return;
>>  	}
>>
>>  	/*
>> @@ -5069,27 +5074,31 @@ void sev_handle_rmp_fault(struct kvm_vcpu *vcpu, gpa_t gpa, u64 error_code)
>>  	if (rmp_level == PG_LEVEL_4K)
>>  		goto out;
>>
>> -	ret = snp_rmptable_psmash(pfn);
>> -	if (ret) {
>> -		/*
>> -		 * Look it up again. If it's 4K now then the PSMASH may have
>> -		 * raced with another process and the issue has already resolved
>> -		 * itself. If it's not assigned, then this must have raced with
>> -		 * another process that made this page shared.
>> -		 */
>> -		if (!snp_lookup_rmpentry(pfn, &assigned, &rmp_level) &&
>> -		    ((assigned && rmp_level == PG_LEVEL_4K) || !assigned))
>> +	scoped_guard(read_lock, &kvm->mmu_lock) {
>> +		if (mmu_invalidate_retry_gfn(kvm, mmu_seq, gfn))
>>  			goto out;
>>
>> -		pr_warn_ratelimited("SEV: Unable to split RMP entry for GPA 0x%llx PFN 0x%llx ret %d\n",
>> -				    gpa, pfn, ret);
>> +		ret = snp_rmptable_psmash(pfn);
>> +		if (ret) {
>> +			/*
>> +			 * Look it up again. If it's 4K now then the PSMASH may
>> +			 * have raced with another process and the issue has
>> +			 * already resolved itself. If it's not assigned, then
>> +			 * this must have raced with another process that made
>> +			 * this page shared.
>> +			 */
>> +			if (!snp_lookup_rmpentry(pfn, &assigned, &rmp_level) &&
>> +			    ((assigned && rmp_level == PG_LEVEL_4K) || !assigned))
>> +				goto out;
>> +
>> +			pr_warn_ratelimited("SEV: Unable to split RMP entry for GPA 0x%llx PFN 0x%llx ret %d\n",
>> +					    gpa, pfn, ret);
>> +		}
>>  	}
>>
>>  	kvm_zap_gfn_range(kvm, gfn, gfn + PTRS_PER_PMD);
>>  out:
>>  	trace_kvm_rmp_fault(vcpu, gpa, pfn, error_code, rmp_level, ret);
>> -out_no_trace:
>> -	kvm_release_page_unused(page);
>>  }
>>
>>  static bool is_pfn_range_shared(kvm_pfn_t start, kvm_pfn_t end)
>>
>> --
>> 2.55.0.699.gb54405d56f-goog
>>

Re: [PATCH v2 2/4] KVM: SEV: Drop page refcount early during RMP fault handling

[Michael Roth]

On Thu, Aug 20, 2026 at 07:58:19AM -0700, Ackerley Tng wrote:
> Michael Roth <michael.roth@amd.com> writes:
> 
> > On Tue, Aug 18, 2026 at 09:15:53AM +0000, Ackerley Tng wrote:
> >> From: Sean Christopherson <seanjc@google.com>
> >>
> >> When handling an RMP fault, KVM retrieves the PFN for a private GPA from
> >> guest_memfd.
> >>
> >> Drop the page reference immediately after retrieving the PFN instead of
> >> holding it across the entire handler so that the later patch can follow up
> >> with completely not returning refcounted pages from kvm_gmem_get_pfn().
> >
> > Regarding this point:
> >
> >>
> >> On a first look, existing RMP table handling (psmash and checking for
> >> errors) might seem like it works fine, since truncation of the page from
> >> guest_memfd would have called rmp_make_shared() and removed the PFN from
> >> the RMP table. However, that is insufficient since a freed page may already
> >> be used in a different SNP VM.
> >
> > In the code this patch is applied on top of, I think the kvm_gmem_get_pfn()
> > ref is enough to avoid the reused-by-another-SNP-VM scenario until after
> > caller releases the ref, so I think the above explanation should be adjusted
> > to also be preparatory for "the later patch".
> >
> 
> I think this sequence of events is possible:
> 
> CPU 0: sev_handle_rmp_fault()
> CPU 0:   kvm_gmem_get_pfn()
> CPU 0:     filemap_invalidate_lock()
> CPU 0:     refcount++
> CPU 0:     filemap_invalidate_unlock()
> CPU 0:     refcount-- <<== because kvm_release_page_unused(page);

Prior to this patch, the kvm_release_page_unused() isn't done until
after the snp_lookup_rmpentry()/snp_rmptable_psmash().

It's possible CPU 1 truncate sequence below can run concurrently
after filemap_invalidate_unlock() above, but even though it calls
free_folio(), which might zap the RMP entry, the filemap code will
only put the refs that it has on the folio so it wouldn't actually
get freed back to the buggy allocator, so it doesn't seem like the
psmash-another-guest scenario is reachable. I could certainly be
misreading things though.

The psmash-a-now-shared-entry scenario because of a race with the
VMM/truncate path seems possible prior to this patch, but that's
pretty similar to the psmash-a-now-shared-entry because of a race
with the guest scenario and which would generate spurious warning
messages to console, and that would be similarly addressed via
patch 1 I think. I'm not sure 

-Mike

> 
> CPU 1: truncate()
> CPU 1:   filemap_invalidate_lock()
> CPU 1:   refcount--
> CPU 1:     kvm_gmem_free_folio()
> CPU 1:       sev_gmem_make_shared()
> CPU 1:     folio is freed
> CPU 1:   filemap_invalidate_unlock()
> 
> CPU 2: in some other SNP VM,
> CPU 2: kvm_gmem_get_pfn() gets the freed folio
> CPU 2:   sev_gmem_make_private()
> 
> CPU 0:   snp_lookup_rmpentry(pfn, &assigned, &rmp_level);
>            <<== it's assigned but to some other SNP VM
> CPU 0:   snp_rmptable_psmash(pfn);
>            <<== this psmash would be smashing in some other SNP VM
> 
> And hence I think we do need to check for invalidations using the MMU
> invalidation protocol.
> 
> > Other than that:
> >
> > Reviewed-by: Michael Roth <michael.roth@amd.com>
> >
> >>
> >> Hence, adopt the MMU invalidation protocol to guard committing anything
> >> based on the PFN.
> >>
> >> Signed-off-by: Sean Christopherson <seanjc@google.com>
> >> Signed-off-by: Ackerley Tng <ackerleytng@google.com>
> >> ---
> >>  arch/x86/kvm/svm/sev.c | 39 ++++++++++++++++++++++++---------------
> >>  1 file changed, 24 insertions(+), 15 deletions(-)
> >>
> >> diff --git a/arch/x86/kvm/svm/sev.c b/arch/x86/kvm/svm/sev.c
> >> index b2738362a928b..b34b11d7f8fad 100644
> >> --- a/arch/x86/kvm/svm/sev.c
> >> +++ b/arch/x86/kvm/svm/sev.c
> >> @@ -5003,6 +5003,7 @@ void sev_handle_rmp_fault(struct kvm_vcpu *vcpu, gpa_t gpa, u64 error_code)
> >>  	struct kvm_memory_slot *slot;
> >>  	struct kvm *kvm = vcpu->kvm;
> >>  	int order, rmp_level, ret;
> >> +	unsigned long mmu_seq;
> >>  	struct page *page;
> >>  	bool assigned;
> >>  	kvm_pfn_t pfn;
> >> @@ -5030,18 +5031,22 @@ void sev_handle_rmp_fault(struct kvm_vcpu *vcpu, gpa_t gpa, u64 error_code)
> >>  		return;
> >>  	}
> >>
> >> +	mmu_seq = kvm->mmu_invalidate_seq;
> >> +	smp_rmb();
> >> +
> >>  	ret = kvm_gmem_get_pfn(kvm, slot, gfn, &pfn, &page, &order);
> >>  	if (ret) {
> >>  		pr_warn_ratelimited("SEV: Unexpected RMP fault, no backing page for private GPA 0x%llx\n",
> >>  				    gpa);
> >>  		return;
> >>  	}
> >> +	kvm_release_page_unused(page);
> >>
> >>  	ret = snp_lookup_rmpentry(pfn, &assigned, &rmp_level);
> >>  	if (ret || !assigned) {
> >>  		pr_warn_ratelimited("SEV: Unexpected RMP fault, no assigned RMP entry found for GPA 0x%llx PFN 0x%llx error %d\n",
> >>  				    gpa, pfn, ret);
> >> -		goto out_no_trace;
> >> +		return;
> >>  	}
> >>
> >>  	/*
> >> @@ -5069,27 +5074,31 @@ void sev_handle_rmp_fault(struct kvm_vcpu *vcpu, gpa_t gpa, u64 error_code)
> >>  	if (rmp_level == PG_LEVEL_4K)
> >>  		goto out;
> >>
> >> -	ret = snp_rmptable_psmash(pfn);
> >> -	if (ret) {
> >> -		/*
> >> -		 * Look it up again. If it's 4K now then the PSMASH may have
> >> -		 * raced with another process and the issue has already resolved
> >> -		 * itself. If it's not assigned, then this must have raced with
> >> -		 * another process that made this page shared.
> >> -		 */
> >> -		if (!snp_lookup_rmpentry(pfn, &assigned, &rmp_level) &&
> >> -		    ((assigned && rmp_level == PG_LEVEL_4K) || !assigned))
> >> +	scoped_guard(read_lock, &kvm->mmu_lock) {
> >> +		if (mmu_invalidate_retry_gfn(kvm, mmu_seq, gfn))
> >>  			goto out;
> >>
> >> -		pr_warn_ratelimited("SEV: Unable to split RMP entry for GPA 0x%llx PFN 0x%llx ret %d\n",
> >> -				    gpa, pfn, ret);
> >> +		ret = snp_rmptable_psmash(pfn);
> >> +		if (ret) {
> >> +			/*
> >> +			 * Look it up again. If it's 4K now then the PSMASH may
> >> +			 * have raced with another process and the issue has
> >> +			 * already resolved itself. If it's not assigned, then
> >> +			 * this must have raced with another process that made
> >> +			 * this page shared.
> >> +			 */
> >> +			if (!snp_lookup_rmpentry(pfn, &assigned, &rmp_level) &&
> >> +			    ((assigned && rmp_level == PG_LEVEL_4K) || !assigned))
> >> +				goto out;
> >> +
> >> +			pr_warn_ratelimited("SEV: Unable to split RMP entry for GPA 0x%llx PFN 0x%llx ret %d\n",
> >> +					    gpa, pfn, ret);
> >> +		}
> >>  	}
> >>
> >>  	kvm_zap_gfn_range(kvm, gfn, gfn + PTRS_PER_PMD);
> >>  out:
> >>  	trace_kvm_rmp_fault(vcpu, gpa, pfn, error_code, rmp_level, ret);
> >> -out_no_trace:
> >> -	kvm_release_page_unused(page);
> >>  }
> >>
> >>  static bool is_pfn_range_shared(kvm_pfn_t start, kvm_pfn_t end)
> >>
> >> --
> >> 2.55.0.699.gb54405d56f-goog
> >>

Re: [PATCH v2 2/4] KVM: SEV: Drop page refcount early during RMP fault handling

[Ackerley Tng]

Michael Roth <michael.roth@amd.com> writes:

> On Thu, Aug 20, 2026 at 07:58:19AM -0700, Ackerley Tng wrote:
>> Michael Roth <michael.roth@amd.com> writes:
>>
>> > On Tue, Aug 18, 2026 at 09:15:53AM +0000, Ackerley Tng wrote:
>> >> From: Sean Christopherson <seanjc@google.com>
>> >>
>> >> When handling an RMP fault, KVM retrieves the PFN for a private GPA from
>> >> guest_memfd.
>> >>
>> >> Drop the page reference immediately after retrieving the PFN instead of
>> >> holding it across the entire handler so that the later patch can follow up
>> >> with completely not returning refcounted pages from kvm_gmem_get_pfn().
>> >
>> > Regarding this point:
>> >
>> >>
>> >> On a first look, existing RMP table handling (psmash and checking for
>> >> errors) might seem like it works fine, since truncation of the page from
>> >> guest_memfd would have called rmp_make_shared() and removed the PFN from
>> >> the RMP table. However, that is insufficient since a freed page may already
>> >> be used in a different SNP VM.
>> >
>> > In the code this patch is applied on top of, I think the kvm_gmem_get_pfn()
>> > ref is enough to avoid the reused-by-another-SNP-VM scenario until after
>> > caller releases the ref, so I think the above explanation should be adjusted
>> > to also be preparatory for "the later patch".
>> >
>>
>> I think this sequence of events is possible:
>>
>> CPU 0: sev_handle_rmp_fault()
>> CPU 0:   kvm_gmem_get_pfn()
>> CPU 0:     filemap_invalidate_lock()
>> CPU 0:     refcount++
>> CPU 0:     filemap_invalidate_unlock()
>> CPU 0:     refcount-- <<== because kvm_release_page_unused(page);
>
> Prior to this patch, the kvm_release_page_unused() isn't done until
> after the snp_lookup_rmpentry()/snp_rmptable_psmash().
>
> It's possible CPU 1 truncate sequence below can run concurrently
> after filemap_invalidate_unlock() above, but even though it calls
> free_folio(), which might zap the RMP entry, the filemap code will
> only put the refs that it has on the folio so it wouldn't actually
> get freed back to the buggy allocator, so it doesn't seem like the
> psmash-another-guest scenario is reachable. I could certainly be
> misreading things though.
>

Ah I see what you mean. I think we mean the same thing, let me add to
the commit message that I meant after dropping the refcount early. Does
this help?

  The filemap_invalidate_lock() is already dropped in kvm_gmem_get_pfn()
  before returning to sev_handle_rmp_fault(). After dropping the
  refcount earlier with kvm_release_page_unused(), these scenarios are
  possible:

  1. Since the filemap_invalidate_lock() is dropped, the page can be
     truncated (or in future, converted), and the RMP entry is now
     shared.

     In this case, existing RMP table handling (psmash and checking for
     errors) would be sufficient. On finding a shared entry, psmashing
     would fail gracefully and no warning would be emitted.

  2. The page is truncated and freed, and then re-allocated to another
     SNP VM. The RMP entry is now assigned, but to another SNP VM.

     To address this, adopt the MMU invalidation protocol to guard
     psmashing.

> The psmash-a-now-shared-entry scenario because of a race with the
> VMM/truncate path seems possible prior to this patch, but that's
> pretty similar to the psmash-a-now-shared-entry because of a race
> with the guest scenario and which would generate spurious warning
> messages to console, and that would be similarly addressed via
> patch 1 I think. I'm not sure
>

Yup. I like your psmash-another-guest vs psmash-a-now-shared-entry
classification.

> -Mike
>
>>
>> CPU 1: truncate()
>> CPU 1:   filemap_invalidate_lock()
>> CPU 1:   refcount--
>> CPU 1:     kvm_gmem_free_folio()
>> CPU 1:       sev_gmem_make_shared()
>> CPU 1:     folio is freed
>> CPU 1:   filemap_invalidate_unlock()
>>
>> CPU 2: in some other SNP VM,
>> CPU 2: kvm_gmem_get_pfn() gets the freed folio
>> CPU 2:   sev_gmem_make_private()
>>
>> CPU 0:   snp_lookup_rmpentry(pfn, &assigned, &rmp_level);
>>            <<== it's assigned but to some other SNP VM
>> CPU 0:   snp_rmptable_psmash(pfn);
>>            <<== this psmash would be smashing in some other SNP VM
>>
>> And hence I think we do need to check for invalidations using the MMU
>> invalidation protocol.
>>
>> > Other than that:
>> >
>> > Reviewed-by: Michael Roth <michael.roth@amd.com>
>> >
>> >>
>> >> Hence, adopt the MMU invalidation protocol to guard committing anything
>> >> based on the PFN.
>> >>
>> >> Signed-off-by: Sean Christopherson <seanjc@google.com>
>> >> Signed-off-by: Ackerley Tng <ackerleytng@google.com>
>> >> ---
>> >>  arch/x86/kvm/svm/sev.c | 39 ++++++++++++++++++++++++---------------
>> >>  1 file changed, 24 insertions(+), 15 deletions(-)
>> >>
>> >> diff --git a/arch/x86/kvm/svm/sev.c b/arch/x86/kvm/svm/sev.c
>> >> index b2738362a928b..b34b11d7f8fad 100644
>> >> --- a/arch/x86/kvm/svm/sev.c
>> >> +++ b/arch/x86/kvm/svm/sev.c
>> >> @@ -5003,6 +5003,7 @@ void sev_handle_rmp_fault(struct kvm_vcpu *vcpu, gpa_t gpa, u64 error_code)
>> >>  	struct kvm_memory_slot *slot;
>> >>  	struct kvm *kvm = vcpu->kvm;
>> >>  	int order, rmp_level, ret;
>> >> +	unsigned long mmu_seq;
>> >>  	struct page *page;
>> >>  	bool assigned;
>> >>  	kvm_pfn_t pfn;
>> >> @@ -5030,18 +5031,22 @@ void sev_handle_rmp_fault(struct kvm_vcpu *vcpu, gpa_t gpa, u64 error_code)
>> >>  		return;
>> >>  	}
>> >>
>> >> +	mmu_seq = kvm->mmu_invalidate_seq;
>> >> +	smp_rmb();
>> >> +
>> >>  	ret = kvm_gmem_get_pfn(kvm, slot, gfn, &pfn, &page, &order);
>> >>  	if (ret) {
>> >>  		pr_warn_ratelimited("SEV: Unexpected RMP fault, no backing page for private GPA 0x%llx\n",
>> >>  				    gpa);
>> >>  		return;
>> >>  	}
>> >> +	kvm_release_page_unused(page);
>> >>
>> >>  	ret = snp_lookup_rmpentry(pfn, &assigned, &rmp_level);
>> >>  	if (ret || !assigned) {
>> >>  		pr_warn_ratelimited("SEV: Unexpected RMP fault, no assigned RMP entry found for GPA 0x%llx PFN 0x%llx error %d\n",
>> >>  				    gpa, pfn, ret);
>> >> -		goto out_no_trace;
>> >> +		return;
>> >>  	}
>> >>
>> >>  	/*
>> >> @@ -5069,27 +5074,31 @@ void sev_handle_rmp_fault(struct kvm_vcpu *vcpu, gpa_t gpa, u64 error_code)
>> >>  	if (rmp_level == PG_LEVEL_4K)
>> >>  		goto out;
>> >>
>> >> -	ret = snp_rmptable_psmash(pfn);
>> >> -	if (ret) {
>> >> -		/*
>> >> -		 * Look it up again. If it's 4K now then the PSMASH may have
>> >> -		 * raced with another process and the issue has already resolved
>> >> -		 * itself. If it's not assigned, then this must have raced with
>> >> -		 * another process that made this page shared.
>> >> -		 */
>> >> -		if (!snp_lookup_rmpentry(pfn, &assigned, &rmp_level) &&
>> >> -		    ((assigned && rmp_level == PG_LEVEL_4K) || !assigned))
>> >> +	scoped_guard(read_lock, &kvm->mmu_lock) {
>> >> +		if (mmu_invalidate_retry_gfn(kvm, mmu_seq, gfn))
>> >>  			goto out;
>> >>
>> >> -		pr_warn_ratelimited("SEV: Unable to split RMP entry for GPA 0x%llx PFN 0x%llx ret %d\n",
>> >> -				    gpa, pfn, ret);
>> >> +		ret = snp_rmptable_psmash(pfn);
>> >> +		if (ret) {
>> >> +			/*
>> >> +			 * Look it up again. If it's 4K now then the PSMASH may
>> >> +			 * have raced with another process and the issue has
>> >> +			 * already resolved itself. If it's not assigned, then
>> >> +			 * this must have raced with another process that made
>> >> +			 * this page shared.
>> >> +			 */
>> >> +			if (!snp_lookup_rmpentry(pfn, &assigned, &rmp_level) &&
>> >> +			    ((assigned && rmp_level == PG_LEVEL_4K) || !assigned))
>> >> +				goto out;
>> >> +
>> >> +			pr_warn_ratelimited("SEV: Unable to split RMP entry for GPA 0x%llx PFN 0x%llx ret %d\n",
>> >> +					    gpa, pfn, ret);
>> >> +		}
>> >>  	}
>> >>
>> >>  	kvm_zap_gfn_range(kvm, gfn, gfn + PTRS_PER_PMD);
>> >>  out:
>> >>  	trace_kvm_rmp_fault(vcpu, gpa, pfn, error_code, rmp_level, ret);
>> >> -out_no_trace:
>> >> -	kvm_release_page_unused(page);
>> >>  }
>> >>
>> >>  static bool is_pfn_range_shared(kvm_pfn_t start, kvm_pfn_t end)
>> >>
>> >> --
>> >> 2.55.0.699.gb54405d56f-goog
>> >>

Re: [PATCH v2 2/4] KVM: SEV: Drop page refcount early during RMP fault handling

[Michael Roth]

On Thu, Aug 20, 2026 at 03:35:31PM -0700, Ackerley Tng wrote:
> Michael Roth <michael.roth@amd.com> writes:
> 
> > On Thu, Aug 20, 2026 at 07:58:19AM -0700, Ackerley Tng wrote:
> >> Michael Roth <michael.roth@amd.com> writes:
> >>
> >> > On Tue, Aug 18, 2026 at 09:15:53AM +0000, Ackerley Tng wrote:
> >> >> From: Sean Christopherson <seanjc@google.com>
> >> >>
> >> >> When handling an RMP fault, KVM retrieves the PFN for a private GPA from
> >> >> guest_memfd.
> >> >>
> >> >> Drop the page reference immediately after retrieving the PFN instead of
> >> >> holding it across the entire handler so that the later patch can follow up
> >> >> with completely not returning refcounted pages from kvm_gmem_get_pfn().
> >> >
> >> > Regarding this point:
> >> >
> >> >>
> >> >> On a first look, existing RMP table handling (psmash and checking for
> >> >> errors) might seem like it works fine, since truncation of the page from
> >> >> guest_memfd would have called rmp_make_shared() and removed the PFN from
> >> >> the RMP table. However, that is insufficient since a freed page may already
> >> >> be used in a different SNP VM.
> >> >
> >> > In the code this patch is applied on top of, I think the kvm_gmem_get_pfn()
> >> > ref is enough to avoid the reused-by-another-SNP-VM scenario until after
> >> > caller releases the ref, so I think the above explanation should be adjusted
> >> > to also be preparatory for "the later patch".
> >> >
> >>
> >> I think this sequence of events is possible:
> >>
> >> CPU 0: sev_handle_rmp_fault()
> >> CPU 0:   kvm_gmem_get_pfn()
> >> CPU 0:     filemap_invalidate_lock()
> >> CPU 0:     refcount++
> >> CPU 0:     filemap_invalidate_unlock()
> >> CPU 0:     refcount-- <<== because kvm_release_page_unused(page);
> >
> > Prior to this patch, the kvm_release_page_unused() isn't done until
> > after the snp_lookup_rmpentry()/snp_rmptable_psmash().
> >
> > It's possible CPU 1 truncate sequence below can run concurrently
> > after filemap_invalidate_unlock() above, but even though it calls
> > free_folio(), which might zap the RMP entry, the filemap code will
> > only put the refs that it has on the folio so it wouldn't actually
> > get freed back to the buggy allocator, so it doesn't seem like the
> > psmash-another-guest scenario is reachable. I could certainly be
> > misreading things though.
> >
> 
> Ah I see what you mean. I think we mean the same thing, let me add to
> the commit message that I meant after dropping the refcount early. Does
> this help?
> 
>   The filemap_invalidate_lock() is already dropped in kvm_gmem_get_pfn()
>   before returning to sev_handle_rmp_fault(). After dropping the
>   refcount earlier with kvm_release_page_unused(), these scenarios are
>   possible:
> 
>   1. Since the filemap_invalidate_lock() is dropped, the page can be
>      truncated (or in future, converted), and the RMP entry is now
>      shared.
> 
>      In this case, existing RMP table handling (psmash and checking for
>      errors) would be sufficient. On finding a shared entry, psmashing
>      would fail gracefully and no warning would be emitted.
> 
>   2. The page is truncated and freed, and then re-allocated to another
>      SNP VM. The RMP entry is now assigned, but to another SNP VM.
> 
>      To address this, adopt the MMU invalidation protocol to guard
>      psmashing.

This reads kinda weird to me, as if with #2 we're documenting a "bug" that
this patch fixes, but the bug would only exist if we partially applied the
bits of this patch the drops the ref counts earlier and left out the
bits of the patch that introduce the mmu notifier logic that replaces it.

I think with patch 1 applied (which covers the
psmash-a-now-shared-entry case while retaining the original refcount
logic), the only thing this patch is doing is replacing the elevated
refcount logic with the MMU invalidation logic as prep for dropping
reliance of refcounts entirely.

I think if the wording was simplified to just say something to that effect
it would make it clearer that this and patch #3 are prep for #4, and that
patch #1 is the only patch here that would make potentially make sense for
a downstream to backport without the other bits.

-Mike

> 
> > The psmash-a-now-shared-entry scenario because of a race with the
> > VMM/truncate path seems possible prior to this patch, but that's
> > pretty similar to the psmash-a-now-shared-entry because of a race
> > with the guest scenario and which would generate spurious warning
> > messages to console, and that would be similarly addressed via
> > patch 1 I think. I'm not sure
> >
> 
> Yup. I like your psmash-another-guest vs psmash-a-now-shared-entry
> classification.
> 
> > -Mike
> >
> >>
> >> CPU 1: truncate()
> >> CPU 1:   filemap_invalidate_lock()
> >> CPU 1:   refcount--
> >> CPU 1:     kvm_gmem_free_folio()
> >> CPU 1:       sev_gmem_make_shared()
> >> CPU 1:     folio is freed
> >> CPU 1:   filemap_invalidate_unlock()
> >>
> >> CPU 2: in some other SNP VM,
> >> CPU 2: kvm_gmem_get_pfn() gets the freed folio
> >> CPU 2:   sev_gmem_make_private()
> >>
> >> CPU 0:   snp_lookup_rmpentry(pfn, &assigned, &rmp_level);
> >>            <<== it's assigned but to some other SNP VM
> >> CPU 0:   snp_rmptable_psmash(pfn);
> >>            <<== this psmash would be smashing in some other SNP VM
> >>
> >> And hence I think we do need to check for invalidations using the MMU
> >> invalidation protocol.
> >>
> >> > Other than that:
> >> >
> >> > Reviewed-by: Michael Roth <michael.roth@amd.com>
> >> >
> >> >>
> >> >> Hence, adopt the MMU invalidation protocol to guard committing anything
> >> >> based on the PFN.
> >> >>
> >> >> Signed-off-by: Sean Christopherson <seanjc@google.com>
> >> >> Signed-off-by: Ackerley Tng <ackerleytng@google.com>
> >> >> ---
> >> >>  arch/x86/kvm/svm/sev.c | 39 ++++++++++++++++++++++++---------------
> >> >>  1 file changed, 24 insertions(+), 15 deletions(-)
> >> >>
> >> >> diff --git a/arch/x86/kvm/svm/sev.c b/arch/x86/kvm/svm/sev.c
> >> >> index b2738362a928b..b34b11d7f8fad 100644
> >> >> --- a/arch/x86/kvm/svm/sev.c
> >> >> +++ b/arch/x86/kvm/svm/sev.c
> >> >> @@ -5003,6 +5003,7 @@ void sev_handle_rmp_fault(struct kvm_vcpu *vcpu, gpa_t gpa, u64 error_code)
> >> >>  	struct kvm_memory_slot *slot;
> >> >>  	struct kvm *kvm = vcpu->kvm;
> >> >>  	int order, rmp_level, ret;
> >> >> +	unsigned long mmu_seq;
> >> >>  	struct page *page;
> >> >>  	bool assigned;
> >> >>  	kvm_pfn_t pfn;
> >> >> @@ -5030,18 +5031,22 @@ void sev_handle_rmp_fault(struct kvm_vcpu *vcpu, gpa_t gpa, u64 error_code)
> >> >>  		return;
> >> >>  	}
> >> >>
> >> >> +	mmu_seq = kvm->mmu_invalidate_seq;
> >> >> +	smp_rmb();
> >> >> +
> >> >>  	ret = kvm_gmem_get_pfn(kvm, slot, gfn, &pfn, &page, &order);
> >> >>  	if (ret) {
> >> >>  		pr_warn_ratelimited("SEV: Unexpected RMP fault, no backing page for private GPA 0x%llx\n",
> >> >>  				    gpa);
> >> >>  		return;
> >> >>  	}
> >> >> +	kvm_release_page_unused(page);
> >> >>
> >> >>  	ret = snp_lookup_rmpentry(pfn, &assigned, &rmp_level);
> >> >>  	if (ret || !assigned) {
> >> >>  		pr_warn_ratelimited("SEV: Unexpected RMP fault, no assigned RMP entry found for GPA 0x%llx PFN 0x%llx error %d\n",
> >> >>  				    gpa, pfn, ret);
> >> >> -		goto out_no_trace;
> >> >> +		return;
> >> >>  	}
> >> >>
> >> >>  	/*
> >> >> @@ -5069,27 +5074,31 @@ void sev_handle_rmp_fault(struct kvm_vcpu *vcpu, gpa_t gpa, u64 error_code)
> >> >>  	if (rmp_level == PG_LEVEL_4K)
> >> >>  		goto out;
> >> >>
> >> >> -	ret = snp_rmptable_psmash(pfn);
> >> >> -	if (ret) {
> >> >> -		/*
> >> >> -		 * Look it up again. If it's 4K now then the PSMASH may have
> >> >> -		 * raced with another process and the issue has already resolved
> >> >> -		 * itself. If it's not assigned, then this must have raced with
> >> >> -		 * another process that made this page shared.
> >> >> -		 */
> >> >> -		if (!snp_lookup_rmpentry(pfn, &assigned, &rmp_level) &&
> >> >> -		    ((assigned && rmp_level == PG_LEVEL_4K) || !assigned))
> >> >> +	scoped_guard(read_lock, &kvm->mmu_lock) {
> >> >> +		if (mmu_invalidate_retry_gfn(kvm, mmu_seq, gfn))
> >> >>  			goto out;
> >> >>
> >> >> -		pr_warn_ratelimited("SEV: Unable to split RMP entry for GPA 0x%llx PFN 0x%llx ret %d\n",
> >> >> -				    gpa, pfn, ret);
> >> >> +		ret = snp_rmptable_psmash(pfn);
> >> >> +		if (ret) {
> >> >> +			/*
> >> >> +			 * Look it up again. If it's 4K now then the PSMASH may
> >> >> +			 * have raced with another process and the issue has
> >> >> +			 * already resolved itself. If it's not assigned, then
> >> >> +			 * this must have raced with another process that made
> >> >> +			 * this page shared.
> >> >> +			 */
> >> >> +			if (!snp_lookup_rmpentry(pfn, &assigned, &rmp_level) &&
> >> >> +			    ((assigned && rmp_level == PG_LEVEL_4K) || !assigned))
> >> >> +				goto out;
> >> >> +
> >> >> +			pr_warn_ratelimited("SEV: Unable to split RMP entry for GPA 0x%llx PFN 0x%llx ret %d\n",
> >> >> +					    gpa, pfn, ret);
> >> >> +		}
> >> >>  	}
> >> >>
> >> >>  	kvm_zap_gfn_range(kvm, gfn, gfn + PTRS_PER_PMD);
> >> >>  out:
> >> >>  	trace_kvm_rmp_fault(vcpu, gpa, pfn, error_code, rmp_level, ret);
> >> >> -out_no_trace:
> >> >> -	kvm_release_page_unused(page);
> >> >>  }
> >> >>
> >> >>  static bool is_pfn_range_shared(kvm_pfn_t start, kvm_pfn_t end)
> >> >>
> >> >> --
> >> >> 2.55.0.699.gb54405d56f-goog
> >> >>

Re: [PATCH v2 2/4] KVM: SEV: Drop page refcount early during RMP fault handling

[Ackerley Tng]

Michael Roth <michael.roth@amd.com> writes:

> On Thu, Aug 20, 2026 at 03:35:31PM -0700, Ackerley Tng wrote:
>> Michael Roth <michael.roth@amd.com> writes:
>>
>> > On Thu, Aug 20, 2026 at 07:58:19AM -0700, Ackerley Tng wrote:
>> >> Michael Roth <michael.roth@amd.com> writes:
>> >>
>> >> > On Tue, Aug 18, 2026 at 09:15:53AM +0000, Ackerley Tng wrote:
>> >> >> From: Sean Christopherson <seanjc@google.com>
>> >> >>
>> >> >> When handling an RMP fault, KVM retrieves the PFN for a private GPA from
>> >> >> guest_memfd.
>> >> >>
>> >> >> Drop the page reference immediately after retrieving the PFN instead of
>> >> >> holding it across the entire handler so that the later patch can follow up
>> >> >> with completely not returning refcounted pages from kvm_gmem_get_pfn().
>> >> >
>> >> > Regarding this point:
>> >> >
>> >> >>
>> >> >> On a first look, existing RMP table handling (psmash and checking for
>> >> >> errors) might seem like it works fine, since truncation of the page from
>> >> >> guest_memfd would have called rmp_make_shared() and removed the PFN from
>> >> >> the RMP table. However, that is insufficient since a freed page may already
>> >> >> be used in a different SNP VM.
>> >> >
>> >> > In the code this patch is applied on top of, I think the kvm_gmem_get_pfn()
>> >> > ref is enough to avoid the reused-by-another-SNP-VM scenario until after
>> >> > caller releases the ref, so I think the above explanation should be adjusted
>> >> > to also be preparatory for "the later patch".
>> >> >
>> >>
>> >> I think this sequence of events is possible:
>> >>
>> >> CPU 0: sev_handle_rmp_fault()
>> >> CPU 0:   kvm_gmem_get_pfn()
>> >> CPU 0:     filemap_invalidate_lock()
>> >> CPU 0:     refcount++
>> >> CPU 0:     filemap_invalidate_unlock()
>> >> CPU 0:     refcount-- <<== because kvm_release_page_unused(page);
>> >
>> > Prior to this patch, the kvm_release_page_unused() isn't done until
>> > after the snp_lookup_rmpentry()/snp_rmptable_psmash().
>> >
>> > It's possible CPU 1 truncate sequence below can run concurrently
>> > after filemap_invalidate_unlock() above, but even though it calls
>> > free_folio(), which might zap the RMP entry, the filemap code will
>> > only put the refs that it has on the folio so it wouldn't actually
>> > get freed back to the buggy allocator, so it doesn't seem like the
>> > psmash-another-guest scenario is reachable. I could certainly be
>> > misreading things though.
>> >
>>
>> Ah I see what you mean. I think we mean the same thing, let me add to
>> the commit message that I meant after dropping the refcount early. Does
>> this help?
>>
>>   The filemap_invalidate_lock() is already dropped in kvm_gmem_get_pfn()
>>   before returning to sev_handle_rmp_fault(). After dropping the
>>   refcount earlier with kvm_release_page_unused(), these scenarios are
>>   possible:
>>
>>   1. Since the filemap_invalidate_lock() is dropped, the page can be
>>      truncated (or in future, converted), and the RMP entry is now
>>      shared.
>>
>>      In this case, existing RMP table handling (psmash and checking for
>>      errors) would be sufficient. On finding a shared entry, psmashing
>>      would fail gracefully and no warning would be emitted.
>>
>>   2. The page is truncated and freed, and then re-allocated to another
>>      SNP VM. The RMP entry is now assigned, but to another SNP VM.
>>
>>      To address this, adopt the MMU invalidation protocol to guard
>>      psmashing.
>
> This reads kinda weird to me, as if with #2 we're documenting a "bug" that
> this patch fixes, but the bug would only exist if we partially applied the
> bits of this patch the drops the ref counts earlier and left out the
> bits of the patch that introduce the mmu notifier logic that replaces it.
>

Thanks, I adjusted the commit message to focus on drop refcount + adopt
KVM MMU invalidation protocol in v3, please see v3! Thanks!

> I think with patch 1 applied (which covers the
> psmash-a-now-shared-entry case while retaining the original refcount
> logic), the only thing this patch is doing is replacing the elevated
> refcount logic with the MMU invalidation logic as prep for dropping
> reliance of refcounts entirely.
>
> I think if the wording was simplified to just say something to that effect
> it would make it clearer that this and patch #3 are prep for #4, and that
> patch #1 is the only patch here that would make potentially make sense for
> a downstream to backport without the other bits.
>
> -Mike
>
>>
>> > The psmash-a-now-shared-entry scenario because of a race with the
>> > VMM/truncate path seems possible prior to this patch, but that's
>> > pretty similar to the psmash-a-now-shared-entry because of a race
>> > with the guest scenario and which would generate spurious warning
>> > messages to console, and that would be similarly addressed via
>> > patch 1 I think. I'm not sure
>> >
>>
>> Yup. I like your psmash-another-guest vs psmash-a-now-shared-entry
>> classification.
>>
>> > -Mike
>> >
>> >>
>> >> CPU 1: truncate()
>> >> CPU 1:   filemap_invalidate_lock()
>> >> CPU 1:   refcount--
>> >> CPU 1:     kvm_gmem_free_folio()
>> >> CPU 1:       sev_gmem_make_shared()
>> >> CPU 1:     folio is freed
>> >> CPU 1:   filemap_invalidate_unlock()
>> >>
>> >> CPU 2: in some other SNP VM,
>> >> CPU 2: kvm_gmem_get_pfn() gets the freed folio
>> >> CPU 2:   sev_gmem_make_private()
>> >>
>> >> CPU 0:   snp_lookup_rmpentry(pfn, &assigned, &rmp_level);
>> >>            <<== it's assigned but to some other SNP VM
>> >> CPU 0:   snp_rmptable_psmash(pfn);
>> >>            <<== this psmash would be smashing in some other SNP VM
>> >>
>> >> And hence I think we do need to check for invalidations using the MMU
>> >> invalidation protocol.
>> >>
>> >> > Other than that:
>> >> >
>> >> > Reviewed-by: Michael Roth <michael.roth@amd.com>
>> >> >
>> >> >>
>> >> >> Hence, adopt the MMU invalidation protocol to guard committing anything
>> >> >> based on the PFN.
>> >> >>
>> >> >> Signed-off-by: Sean Christopherson <seanjc@google.com>
>> >> >> Signed-off-by: Ackerley Tng <ackerleytng@google.com>
>> >> >> ---
>> >> >>  arch/x86/kvm/svm/sev.c | 39 ++++++++++++++++++++++++---------------
>> >> >>  1 file changed, 24 insertions(+), 15 deletions(-)
>> >> >>
>> >> >> diff --git a/arch/x86/kvm/svm/sev.c b/arch/x86/kvm/svm/sev.c
>> >> >> index b2738362a928b..b34b11d7f8fad 100644
>> >> >> --- a/arch/x86/kvm/svm/sev.c
>> >> >> +++ b/arch/x86/kvm/svm/sev.c
>> >> >> @@ -5003,6 +5003,7 @@ void sev_handle_rmp_fault(struct kvm_vcpu *vcpu, gpa_t gpa, u64 error_code)
>> >> >>  	struct kvm_memory_slot *slot;
>> >> >>  	struct kvm *kvm = vcpu->kvm;
>> >> >>  	int order, rmp_level, ret;
>> >> >> +	unsigned long mmu_seq;
>> >> >>  	struct page *page;
>> >> >>  	bool assigned;
>> >> >>  	kvm_pfn_t pfn;
>> >> >> @@ -5030,18 +5031,22 @@ void sev_handle_rmp_fault(struct kvm_vcpu *vcpu, gpa_t gpa, u64 error_code)
>> >> >>  		return;
>> >> >>  	}
>> >> >>
>> >> >> +	mmu_seq = kvm->mmu_invalidate_seq;
>> >> >> +	smp_rmb();
>> >> >> +
>> >> >>  	ret = kvm_gmem_get_pfn(kvm, slot, gfn, &pfn, &page, &order);
>> >> >>  	if (ret) {
>> >> >>  		pr_warn_ratelimited("SEV: Unexpected RMP fault, no backing page for private GPA 0x%llx\n",
>> >> >>  				    gpa);
>> >> >>  		return;
>> >> >>  	}
>> >> >> +	kvm_release_page_unused(page);
>> >> >>
>> >> >>  	ret = snp_lookup_rmpentry(pfn, &assigned, &rmp_level);
>> >> >>  	if (ret || !assigned) {
>> >> >>  		pr_warn_ratelimited("SEV: Unexpected RMP fault, no assigned RMP entry found for GPA 0x%llx PFN 0x%llx error %d\n",
>> >> >>  				    gpa, pfn, ret);
>> >> >> -		goto out_no_trace;
>> >> >> +		return;
>> >> >>  	}
>> >> >>
>> >> >>  	/*
>> >> >> @@ -5069,27 +5074,31 @@ void sev_handle_rmp_fault(struct kvm_vcpu *vcpu, gpa_t gpa, u64 error_code)
>> >> >>  	if (rmp_level == PG_LEVEL_4K)
>> >> >>  		goto out;
>> >> >>
>> >> >> -	ret = snp_rmptable_psmash(pfn);
>> >> >> -	if (ret) {
>> >> >> -		/*
>> >> >> -		 * Look it up again. If it's 4K now then the PSMASH may have
>> >> >> -		 * raced with another process and the issue has already resolved
>> >> >> -		 * itself. If it's not assigned, then this must have raced with
>> >> >> -		 * another process that made this page shared.
>> >> >> -		 */
>> >> >> -		if (!snp_lookup_rmpentry(pfn, &assigned, &rmp_level) &&
>> >> >> -		    ((assigned && rmp_level == PG_LEVEL_4K) || !assigned))
>> >> >> +	scoped_guard(read_lock, &kvm->mmu_lock) {
>> >> >> +		if (mmu_invalidate_retry_gfn(kvm, mmu_seq, gfn))
>> >> >>  			goto out;
>> >> >>
>> >> >> -		pr_warn_ratelimited("SEV: Unable to split RMP entry for GPA 0x%llx PFN 0x%llx ret %d\n",
>> >> >> -				    gpa, pfn, ret);
>> >> >> +		ret = snp_rmptable_psmash(pfn);
>> >> >> +		if (ret) {
>> >> >> +			/*
>> >> >> +			 * Look it up again. If it's 4K now then the PSMASH may
>> >> >> +			 * have raced with another process and the issue has
>> >> >> +			 * already resolved itself. If it's not assigned, then
>> >> >> +			 * this must have raced with another process that made
>> >> >> +			 * this page shared.
>> >> >> +			 */
>> >> >> +			if (!snp_lookup_rmpentry(pfn, &assigned, &rmp_level) &&
>> >> >> +			    ((assigned && rmp_level == PG_LEVEL_4K) || !assigned))
>> >> >> +				goto out;
>> >> >> +
>> >> >> +			pr_warn_ratelimited("SEV: Unable to split RMP entry for GPA 0x%llx PFN 0x%llx ret %d\n",
>> >> >> +					    gpa, pfn, ret);
>> >> >> +		}
>> >> >>  	}
>> >> >>
>> >> >>  	kvm_zap_gfn_range(kvm, gfn, gfn + PTRS_PER_PMD);
>> >> >>  out:
>> >> >>  	trace_kvm_rmp_fault(vcpu, gpa, pfn, error_code, rmp_level, ret);
>> >> >> -out_no_trace:
>> >> >> -	kvm_release_page_unused(page);
>> >> >>  }
>> >> >>
>> >> >>  static bool is_pfn_range_shared(kvm_pfn_t start, kvm_pfn_t end)
>> >> >>
>> >> >> --
>> >> >> 2.55.0.699.gb54405d56f-goog
>> >> >>

Re: [PATCH v2 2/4] KVM: SEV: Drop page refcount early during RMP fault handling

[Sean Christopherson]

On Thu, Aug 20, 2026, Michael Roth wrote:
> On Thu, Aug 20, 2026 at 03:35:31PM -0700, Ackerley Tng wrote:
> > Michael Roth <michael.roth@amd.com> writes:
> > Ah I see what you mean. I think we mean the same thing, let me add to
> > the commit message that I meant after dropping the refcount early. Does
> > this help?
> > 
> >   The filemap_invalidate_lock() is already dropped in kvm_gmem_get_pfn()
> >   before returning to sev_handle_rmp_fault(). After dropping the
> >   refcount earlier with kvm_release_page_unused(), these scenarios are
> >   possible:
> > 
> >   1. Since the filemap_invalidate_lock() is dropped, the page can be
> >      truncated (or in future, converted), and the RMP entry is now
> >      shared.
> > 
> >      In this case, existing RMP table handling (psmash and checking for
> >      errors) would be sufficient. On finding a shared entry, psmashing
> >      would fail gracefully and no warning would be emitted.
> > 
> >   2. The page is truncated and freed, and then re-allocated to another
> >      SNP VM. The RMP entry is now assigned, but to another SNP VM.
> > 
> >      To address this, adopt the MMU invalidation protocol to guard
> >      psmashing.
> 
> This reads kinda weird to me, as if with #2 we're documenting a "bug" that
> this patch fixes, but the bug would only exist if we partially applied the
> bits of this patch the drops the ref counts earlier and left out the
> bits of the patch that introduce the mmu notifier logic that replaces it.
> 
> I think with patch 1 applied (which covers the
> psmash-a-now-shared-entry case while retaining the original refcount
> logic), the only thing this patch is doing is replacing the elevated
> refcount logic with the MMU invalidation logic as prep for dropping
> reliance of refcounts entirely.

(I had already typed this up before I saw Ackerley's response, so dagnabbit I'm
hitting send).

Agreed.  Less is more in this case, unless you want to explain all of the gory
details of how KVM handles MMU invalidations.

  Rework KVM's handling of RMP faults to rely on MMU invalidation logic for
  safety, instead of the current approach of holding onto a folio reference
  until the RMP operations are complete.  I.e. drop the reference gifted by
  guest_memfd immediately after getting the PFN, and instead do RMP updates
  under mmu_lock, after checking for relevant MMU invalidations.

  This will allow dropping guest_memfd's reference gifting entirely, which is
  ideally how KVM would operate for all "follow PFN" operations (GUP has many
  more complications, which is why KVM holds a reference across page faults
  *on top* of the standard MMU invalidation logic).

Re: [PATCH v2 4/4] KVM: guest_memfd: Stop returning struct page from PFN lookup

[Yan Zhao]

On Thu, Aug 20, 2026 at 07:47:51AM -0700, Ackerley Tng wrote:
> Yan Zhao <yan.y.zhao@intel.com> writes:
> 
> > On Tue, Aug 18, 2026 at 09:15:55AM +0000, Ackerley Tng wrote:
> >> From: Sean Christopherson <seanjc@google.com>
> >>
> >> KVM currently expects guest_memfd PFN lookups to return a refcounted
> >> struct page, which callers hold across fault handling.
> >>
> >> Holding a page reference across fault handling is problematic for
> >> guest_memfd. In-place memory conversions between confidential
> >> computing shared and private states inspect folio refcounts to ensure
> >> exclusive ownership by guest_memfd. A concurrent guest page fault
> >> taking a reference on the folio causes conversions to fail due to an
> >> elevated refcount.
> > Nit:
> > As this series is based on kvm-x86/next, where there's no in-place memory
> > conversion yet, kvm_gmem_get_pfn() does not hold shared filemap invalidate lock.
> >
> > However, the benefit of dropping the folio reference immediately before
> > returning from the guest_memfd PFN lookup -- preventing conversion failures due
> > to an elevated refcount -- should be effective only if the reference is dropped
> > before releasing the shared filemap invalidate lock.
> >
> > Do we need to make this info clear, since I think it's important? :)
> >
> 
> Is this what you meant?
> 
>   In-place conversions uses the filemap_invalidate_lock() for
>   synchronization of shared/private state. In kvm_gmem_get_pfn(), the
>   PFN needs to be prepared according to its shared/private state. Hence,
>   the filemap_invalidate_lock() is held while guest_memfd gets a folio
>   and decides to make private before returning a PFN.
> 
>   In kvm_gmem_get_pfn(), the folio refcount is dropped before releasing
>   filemap_invalidate_lock(). This ensures that a competing conversion
>   grabbing the filemap_invalidate_lock() will never see an elevated
>   refcount due to guest_memfd's folio-getting process.
> 
> It seems a bit weird to fit this into the commit message for this
> patch. I think I could put the above two paragraphs into the patch that
> introduces the filemap_invalidate_lock in kvm_gmem_get_pfn()?
Ok. Makes sense.

> >> guest_memfd already notifies KVM of page invalidations, so callers
> >> within KVM only need to respect the MMU invalidation protocol to safely
> >> rely on guest_memfd for page presence.
> >>
> >> Furthermore, removing struct page from the guest_memfd PFN lookup moves
> >> KVM closer toward supporting memory backends that are not backed by
> >> struct page.
> >
> > Could we also explain why the lack of SetPageDirty() (and mark_page_accessed())
> > for a gmem page, due to NULL being passed to kvm_release_faultin_page(), is
> > harmless?
> >
> 
> Sounds good. What do you think of this, continuing from the paragraph
> beginning "Furthermore":
> 
>   Drop the folio reference immediately before returning from the
>   guest_memfd PFN lookup, and stop returning the struct page pointer.
> 
>   ARM's gmem_abort() is guest_memfd specific. Since guest_memfd no longer
>   returns a page pointer, there's also no need to do any
>   freeing. kvm_release_faultin_page() originally also serves to set the page
>   dirty and accessed under some conditions. The dirty and accessed flags
>   don't matter for guest_memfd anyway, so it is safe to just drop the call to
>   kvm_release_faultin_page().
> 
>   For ARM's kvm_translate_vncr(), initialize the local page pointer to NULL
>   so that the shared cleanup path that releases fault-in pages safely no-ops
>   for guest_memfd.
> 
>   For x86, no additional changes are required in the MMU fault path
>   because the page fault tracking structure is zero-initialized at the
>   start of page fault handling, ensuring the refcounted page pointer is
>   already NULL.
LGTM. Thank you!