Linux kernel -stable discussions
 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: 4+ 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

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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox