All of lore.kernel.org
 help / color / mirror / Atom feed
From: Sean Christopherson <seanjc@google.com>
To: Amirmohammad Eftekhar <amirmohammad.eftekhar@cispa.de>
Cc: w@1wt.eu, security@kernel.org, dwmw2@infradead.org, paul@xen.org,
	 pbonzini@redhat.com, stable@vger.kernel.org
Subject: Re: [BUG] KVM: x86/xen: gfn_to_pfn_cache left active after mmu_notifier release leads to a host use-after-free write
Date: Thu, 22 Oct 2026 15:03:07 +0000	[thread overview]
Message-ID: <atolq8545YGo4iKx@google.com> (raw)
In-Reply-To: <20261017193911.40182-1-amirmohammad.eftekhar@cispa.de>

On Sat, Oct 17, 2026, Amirmohammad Eftekhar wrote:
> ---
>  virt/kvm/kvm_main.c | 12 ++++++++++++
>  1 file changed, 12 insertions(+)
> 
> diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
> index 479b7f5bf..6ac6a71fe 100644
> --- a/virt/kvm/kvm_main.c
> +++ b/virt/kvm/kvm_main.c
> @@ -891,6 +891,18 @@ static void kvm_mmu_notifier_release(struct mmu_notifier *mn,
> 
>  	idx = srcu_read_lock(&kvm->srcu);
>  	kvm_flush_shadow_all(kvm);
> +
> +	/*
> +	 * Invalidate all gfn_to_pfn_caches before KVM is unhooked from the
> +	 * mmu_notifier.  Release is the last callback KVM will receive for this
> +	 * mm, so the invalidate_range events that exit_mmap() raises when it
> +	 * frees the pages can no longer reach the caches; without this the
> +	 * caches are left valid, pointing at pages that are about to be freed,
> +	 * and an async writer (e.g. the Xen event-channel timer) can perform a
> +	 * use-after-free write through the stale kernel mapping.
> +	 */

Eh, I would omit the comment, IMO this falls into the "obviously necessary"
category once the code exists.

> +	gfn_to_pfn_cache_invalidate_start(kvm, 0, ULONG_MAX);

I'm pretty sure this should go inside kvm_flush_shadow_all(), both because this
isn't unique to mmu_notifiers, even though that's the only remaining user of
kvm_flush_shadow_all().  E.g. in a hypothetical world where kvm_flush_shadow_all()
was used to ensure there were no outstanding mappings anywhere in KVM, hooking
only kvm_mmu_notifier_release() would do the wrong thing.

The other wrinkle is that super strictly speaking the invalidation needs to happen
before kvm_arch_guest_memory_reclaimed().  In practice, there are no interactions
between SEV's cache maintenance and GPCs, but conceptually the GPC invalidation
should occur first.  That would also match the ordering in 
kvm_mmu_notifier_invalidate_range_start().

Another stupid nit: ULONG_MAX is somewhat misleading because
gfn_to_pfn_cache_invalidate_start() takes an exclusive @end.  It's not a bug
because GPCs only ever deal with user addresses, i.e. it should be impossible to
have a cache at ULONG_MAX-PAGE_SIZE.  Ugh, except for maybe s390, which selects
ALTERNATE_USER_ADDRESS_SPACE?  CONFIG_HAVE_KVM_PFNCACHE is only selected by x86,
but it still seems like we should do this "right" the first time.

Rather than pass literal numbers, what if (over a few patches) we massage
gfn_to_pfn_cache_invalidate_start() to take a range pointer, and then add an
"invalidate all" wrapper that passes a NULL range?

Anyone object to taking this on-list?  Unless I'm missing something, this only
affects the Xen timer, and there are preciously few end users of KVM Xen emulation.

E.g. (completely untested)

diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
index cf4cb586f140..c2e16370f5b8 100644
--- a/virt/kvm/kvm_main.c
+++ b/virt/kvm/kvm_main.c
@@ -350,6 +350,7 @@ void kvm_flush_remote_tlbs_memslot(struct kvm *kvm,
 static void kvm_flush_shadow_all(struct kvm *kvm)
 {
 	kvm_arch_flush_shadow_all(kvm);
+	gfn_to_pfn_cache_invalidate_all(kvm);
 	kvm_arch_guest_memory_reclaimed(kvm);
 }
 
@@ -764,7 +765,7 @@ static int kvm_mmu_notifier_invalidate_range_start(struct mmu_notifier *mn,
 	 * mn_active_invalidate_count (see above) instead of
 	 * mmu_invalidate_in_progress.
 	 */
-	gfn_to_pfn_cache_invalidate_start(kvm, range->start, range->end);
+	gfn_to_pfn_cache_invalidate_start(kvm, range);
 
 	/*
 	 * If one or more memslots were found and thus zapped, notify arch code
diff --git a/virt/kvm/kvm_mm.h b/virt/kvm/kvm_mm.h
index c65cfdb14f1c..f46d999d056a 100644
--- a/virt/kvm/kvm_mm.h
+++ b/virt/kvm/kvm_mm.h
@@ -6,6 +6,8 @@
 #include <linux/kvm.h>
 #include <linux/kvm_types.h>
 
+struct mmu_notifier_range;
+
 /*
  * Architectures can choose whether to use an rwlock or spinlock
  * for the mmu_lock.  These macros, for use in common code
@@ -60,14 +62,19 @@ kvm_pfn_t hva_to_pfn(struct kvm_follow_pfn *kfp);
 
 #ifdef CONFIG_HAVE_KVM_PFNCACHE
 void gfn_to_pfn_cache_invalidate_start(struct kvm *kvm,
-				       unsigned long start,
-				       unsigned long end);
+				       const struct mmu_notifier_range *range);
+
+static inline void gfn_to_pfn_cache_invalidate_all(struct kvm *kvm)
+{
+	gfn_to_pfn_cache_invalidate_start(kvm, NULL);
+}
+
 #else
 static inline void gfn_to_pfn_cache_invalidate_start(struct kvm *kvm,
-						     unsigned long start,
-						     unsigned long end)
+						     const struct mmu_notifier_range *range)
 {
 }
+static inline void gfn_to_pfn_cache_invalidate_all(struct kvm *kvm) { }
 #endif /* HAVE_KVM_PFNCACHE */
 
 #endif /* __KVM_MM_H__ */
diff --git a/virt/kvm/pfncache.c b/virt/kvm/pfncache.c
index 3659686b97c2..3f0edb46ec57 100644
--- a/virt/kvm/pfncache.c
+++ b/virt/kvm/pfncache.c
@@ -19,11 +19,18 @@
 
 #include "kvm_mm.h"
 
+static bool gfn_to_pfn_cache_needs_invalidation(struct gfn_to_pfn_cache *gpc,
+						const struct mmu_notifier_range *range)
+{
+	return gpc->valid && !is_error_noslot_pfn(gpc->pfn) &&
+	       (!range || (gpc->uhva >= range->start && gpc->uhva < range->end));
+}
+
 /*
  * MMU notifier 'invalidate_range_start' hook.
  */
-void gfn_to_pfn_cache_invalidate_start(struct kvm *kvm, unsigned long start,
-				       unsigned long end)
+void gfn_to_pfn_cache_invalidate_start(struct kvm *kvm,
+				       const struct mmu_notifier_range *range)
 {
 	struct gfn_to_pfn_cache *gpc;
 
@@ -32,8 +39,7 @@ void gfn_to_pfn_cache_invalidate_start(struct kvm *kvm, unsigned long start,
 		read_lock_irq(&gpc->lock);
 
 		/* Only a single page so no need to care about length */
-		if (gpc->valid && !is_error_noslot_pfn(gpc->pfn) &&
-		    gpc->uhva >= start && gpc->uhva < end) {
+		if (gfn_to_pfn_cache_needs_invalidation(gpc, range)) {
 			read_unlock_irq(&gpc->lock);
 
 			/*
@@ -45,8 +51,8 @@ void gfn_to_pfn_cache_invalidate_start(struct kvm *kvm, unsigned long start,
 			 */
 
 			write_lock_irq(&gpc->lock);
-			if (gpc->valid && !is_error_noslot_pfn(gpc->pfn) &&
-			    gpc->uhva >= start && gpc->uhva < end)
+			
+			if (gfn_to_pfn_cache_needs_invalidation(gpc, range))
 				gpc->valid = false;
 			write_unlock_irq(&gpc->lock);
 			continue;



  reply	other threads:[~2026-10-22 15:03 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-10-17 19:39 [BUG] KVM: x86/xen: gfn_to_pfn_cache left active after mmu_notifier release leads to a host use-after-free write Amirmohammad Eftekhar
2026-10-22 15:03 ` Sean Christopherson [this message]
2026-10-22 15:28   ` Amirmohammad Eftekhar
2026-10-23  9:29   ` Amirmohammad Eftekhar
2026-10-31  9:05   ` Amirmohammad Eftekhar

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=atolq8545YGo4iKx@google.com \
    --to=seanjc@google.com \
    --cc=amirmohammad.eftekhar@cispa.de \
    --cc=dwmw2@infradead.org \
    --cc=paul@xen.org \
    --cc=pbonzini@redhat.com \
    --cc=security@kernel.org \
    --cc=stable@vger.kernel.org \
    --cc=w@1wt.eu \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.