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;
next prev parent 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