[PATCH] KVM: pfncache: invalidate sibling GPCs before releasing retry pages

Heitor Alves de Siqueira posted 1 patch 23 hours ago
virt/kvm/pfncache.c | 33 +++++++++++++++++++++++++++++++++
1 file changed, 33 insertions(+)
[PATCH] KVM: pfncache: invalidate sibling GPCs before releasing retry pages
Posted by Heitor Alves de Siqueira 23 hours ago
When refreshing a GPC, hva_to_pfn_retry() drops the GPC write lock and
re-acquires it on each retry iteration. During the unlocked window, it
can release the page from the previous iteration via
kvm_release_page_unused().

If a concurrent munmap removes the page-table reference during that
window, the GUP reference could be the last one left:
kvm_release_page_unused() drops it and the page is freed while sibling
GPCs could still hold mappings to the same physical page. A KVM_RUN that
hits kvm_setup_guest_pvclock() through such a sibling GPC then reads
freed memory, causing the use-after-free reported by syzbot:

  BUG: KASAN: use-after-free in kvm_setup_guest_pvclock+0x439/0x620
  Read of size 4 at addr ffff88804d02c320

The existing gfn_to_pfn_cache_invalidate_start() can't reliably prevent
this: the page is released without holding any GPC locks, so the
mmu_notifier invalidation may reach the sibling only after the page has
already been freed.

Fix by invalidating sibling GPCs that have a matching PFN before
releasing the page. This is done via the new kvm_gpc_invalidate_pfn()
helper, similar to gfn_to_pfn_cache_invalidate_start().

Fixes: 58cd407ca4c6 ("KVM: Fix multiple races in gfn=>pfn cache refresh")
Reported-by: syzbot+fb7c2dd166d3ea63df2a@syzkaller.appspotmail.com
Closes: https://syzbot.org/bug?extid=fb7c2dd166d3ea63df2a
Cc: stable@kernel.org
Signed-off-by: Heitor Alves de Siqueira <halves@igalia.com>
---
 virt/kvm/pfncache.c | 33 +++++++++++++++++++++++++++++++++
 1 file changed, 33 insertions(+)

diff --git a/virt/kvm/pfncache.c b/virt/kvm/pfncache.c
index 728d2c1b488a..9ea595e90d08 100644
--- a/virt/kvm/pfncache.c
+++ b/virt/kvm/pfncache.c
@@ -152,6 +152,37 @@ static inline bool mmu_notifier_retry_cache(struct kvm *kvm, unsigned long mmu_s
 	return kvm->mmu_invalidate_seq != mmu_seq;
 }
 
+static void kvm_gpc_invalidate_pfn(struct kvm *kvm, kvm_pfn_t pfn)
+{
+	struct gfn_to_pfn_cache *gpc;
+
+	if (is_error_noslot_pfn(pfn))
+		return;
+
+	spin_lock(&kvm->gpc_lock);
+	list_for_each_entry(gpc, &kvm->gpc_list, list) {
+		read_lock_irq(&gpc->lock);
+
+		if (gpc->valid && gpc->pfn == pfn) {
+			read_unlock_irq(&gpc->lock);
+
+			/*
+			 * Re-check if invalidation is still necessary
+			 * after acquiring the write lock.
+			 * See gfn_to_pfn_cache_invalidate_start()
+			 */
+			write_lock_irq(&gpc->lock);
+			if (gpc->valid && gpc->pfn == pfn)
+				gpc->valid = false;
+			write_unlock_irq(&gpc->lock);
+			continue;
+		}
+
+		read_unlock_irq(&gpc->lock);
+	}
+	spin_unlock(&kvm->gpc_lock);
+}
+
 static kvm_pfn_t hva_to_pfn_retry(struct gfn_to_pfn_cache *gpc)
 {
 	/* Note, the new page offset may be different than the old! */
@@ -201,6 +232,7 @@ static kvm_pfn_t hva_to_pfn_retry(struct gfn_to_pfn_cache *gpc)
 			if (new_khva != old_khva)
 				gpc_unmap(new_pfn, new_khva);
 
+			kvm_gpc_invalidate_pfn(gpc->kvm, new_pfn);
 			kvm_release_page_unused(page);
 
 			cond_resched();
@@ -221,6 +253,7 @@ static kvm_pfn_t hva_to_pfn_retry(struct gfn_to_pfn_cache *gpc)
 			new_khva = gpc_map(new_pfn);
 
 		if (!new_khva) {
+			kvm_gpc_invalidate_pfn(gpc->kvm, new_pfn);
 			kvm_release_page_unused(page);
 			goto out_error;
 		}

---
base-commit: 48a5a7ab8d6ab7090564339e039c421f315de912
change-id: 20260720-kvm_guest_pvclock-0a290302e848

Best regards,
--  
Heitor Alves de Siqueira <halves@igalia.com>