Kernel KVM virtualization development
 help / color / mirror / Atom feed
* [PATCH 0/2] KVM: SEV: Fix intra-host migration cache bugs
@ 2026-09-23 16:37 Sean Christopherson
  2026-09-23 16:37 ` [PATCH 1/2] KVM: SEV: Free have_run_cpus during VM destruction even if VM is no longer SEV Sean Christopherson
                   ` (2 more replies)
  0 siblings, 3 replies; 8+ messages in thread
From: Sean Christopherson @ 2026-09-23 16:37 UTC (permalink / raw)
  To: Sean Christopherson, Paolo Bonzini; +Cc: kvm, linux-kernel, Stefan Teodorescu

Fix a memory leak and a cache maintenance issue related to doing intra-host
migration on an SEV guest.  TL;DR: the source VM becomes a non-SEV guest after
intra-host migration, and so KVM fails to do the right thing because its cache
maintenance stuff keys off sev_guest().

Sean Christopherson (2):
  KVM: SEV: Free have_run_cpus during VM destruction even if VM is no
    longer SEV
  KVM: SEV: Do cache maintenance on the source VM during intra-host
    migration

 arch/x86/kvm/svm/sev.c | 18 ++++++++++++++++--
 1 file changed, 16 insertions(+), 2 deletions(-)


base-commit: 30b5175943e709911702d8a9364145e911f57e3f
-- 
2.55.0.1082.g2b9226bbc0-goog


^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH 1/2] KVM: SEV: Free have_run_cpus during VM destruction even if VM is no longer SEV
  2026-09-23 16:37 [PATCH 0/2] KVM: SEV: Fix intra-host migration cache bugs Sean Christopherson
@ 2026-09-23 16:37 ` Sean Christopherson
  2026-09-23 16:53   ` sashiko-bot
  2026-09-23 16:37 ` [PATCH 2/2] KVM: SEV: Do cache maintenance on the source VM during intra-host migration Sean Christopherson
  2026-09-26  4:37 ` [PATCH 0/2] KVM: SEV: Fix intra-host migration cache bugs Paolo Bonzini
  2 siblings, 1 reply; 8+ messages in thread
From: Sean Christopherson @ 2026-09-23 16:37 UTC (permalink / raw)
  To: Sean Christopherson, Paolo Bonzini; +Cc: kvm, linux-kernel, Stefan Teodorescu

Unconditionally free SEV's "have run CPUs" cpumask in the VM destroy path,
i.e. even for what appear to be non-SEV VMs, as an SEV VM becomes a non-SEV
VM if its state is intra-host migrated.  Alternatively, the mask could be
freed in sev_migrate_from() when "converting" the source VM, but that gets
annoying because ideally KVM would nullify the mask to guard against UAF,
and nullifying the mask would need be conditioned on CPUMASK_OFFSTACK=y.

Freeing the mask during sev_migrate_from() is also not robust against other
KVM bugs, though that's kind of a moot point since any such bugs would show
up even if sev->active is never set.  I.e. KVM must get that side of things
correct.  But, that's not a great reason to add more code just to make
things marginally less robust.

Fixes: 6f38f8c57464 ("KVM: SVM: Flush cache only on CPUs running SEV guest")
Cc: stable@vger.kernel.org
Reported-by: Stefan Teodorescu <fane@google.com>
Signed-off-by: Sean Christopherson <seanjc@google.com>
---
 arch/x86/kvm/svm/sev.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/arch/x86/kvm/svm/sev.c b/arch/x86/kvm/svm/sev.c
index 63e8cfa9bf55..c9242c936a40 100644
--- a/arch/x86/kvm/svm/sev.c
+++ b/arch/x86/kvm/svm/sev.c
@@ -2981,13 +2981,17 @@ void sev_vm_destroy(struct kvm *kvm)
 	struct list_head *head = &sev->regions_list;
 	struct list_head *pos, *q;
 
+	/*
+	 * Free the mask even if the VM is not *currently* an SEV VM, as it may
+	 * have been an SEV VM prior to intra-host migration.
+	 */
+	free_cpumask_var(sev->have_run_cpus);
+
 	if (!sev_guest(kvm))
 		return;
 
 	WARN_ON(!list_empty(&sev->mirror_vms));
 
-	free_cpumask_var(sev->have_run_cpus);
-
 	/*
 	 * If this is a mirror VM, remove it from the owner's list of a mirrors
 	 * and skip ASID cleanup (the ASID is tied to the lifetime of the owner).
-- 
2.55.0.1082.g2b9226bbc0-goog


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH 2/2] KVM: SEV: Do cache maintenance on the source VM during intra-host migration
  2026-09-23 16:37 [PATCH 0/2] KVM: SEV: Fix intra-host migration cache bugs Sean Christopherson
  2026-09-23 16:37 ` [PATCH 1/2] KVM: SEV: Free have_run_cpus during VM destruction even if VM is no longer SEV Sean Christopherson
@ 2026-09-23 16:37 ` Sean Christopherson
  2026-09-23 16:53   ` sashiko-bot
  2026-09-26  4:37 ` [PATCH 0/2] KVM: SEV: Fix intra-host migration cache bugs Paolo Bonzini
  2 siblings, 1 reply; 8+ messages in thread
From: Sean Christopherson @ 2026-09-23 16:37 UTC (permalink / raw)
  To: Sean Christopherson, Paolo Bonzini; +Cc: kvm, linux-kernel, Stefan Teodorescu

Manually perform cache maintenance on the source VM during intra-host
migration to ensure no stale data is left in CPU caches after the VM is
destroyed.  Because the source VM is "converted" to a non-SEV VM, KVM's
memory reclaim flows won't trigger cache maintenance, e.g. when all guest
memory is reclaimed in response to detaching from the mmu_notifier.

Note, relying on the destination VM to do cache maintenance isn't an option
as KVM doesn't require identical guest memory configurations, i.e. the
source VM may have access to memory that the destination VM does not.
Enforcing equivalent memory configurations is infeasible, as it would
require a *deep* comparison of memslots, e.g. to verify that not only are
the memslot identical, but what the memslots point at is also identical.

Fixes: b56639318bb2 ("KVM: SEV: Add support for SEV intra host migration")
Cc: stable@vger.kernel.org
Reported-by: Stefan Teodorescu <fane@google.com>
Signed-off-by: Sean Christopherson <seanjc@google.com>
---
 arch/x86/kvm/svm/sev.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/arch/x86/kvm/svm/sev.c b/arch/x86/kvm/svm/sev.c
index c9242c936a40..71923cb72d1d 100644
--- a/arch/x86/kvm/svm/sev.c
+++ b/arch/x86/kvm/svm/sev.c
@@ -2048,6 +2048,12 @@ static void sev_migrate_from(struct kvm *dst_kvm, struct kvm *src_kvm)
 	src->pages_locked = 0;
 	src->es_active = false;
 
+	/*
+	 * Do cache maintenance on the source VM as it is no longer an SEV VM,
+	 * i.e. memory reclaim flows won't trigger cache maintenance on the VM.
+	 */
+	sev_writeback_caches(src_kvm);
+
 	list_cut_before(&dst->regions_list, &src->regions_list, &src->regions_list);
 
 	mutex_lock(&sev_mirror_lock);
@@ -2187,6 +2193,10 @@ int sev_vm_move_enc_context_from(struct kvm *kvm, unsigned int source_fd)
 	 * the set of CPUs from the source.  If a CPU was used to run a vCPU in
 	 * the source VM but is never used for the destination VM, then the CPU
 	 * can only have cached memory that was accessible to the source VM.
+	 * Furthermore, KVM *must* perform cache maintenance on the source VM,
+	 * as the source VM may have access to memory that the destination VM
+	 * does not, i.e. KVM could skip flushes if memory is reclaimed from
+	 * the old VM but not the new VM.
 	 */
 	if (!zalloc_cpumask_var(&dst_sev->have_run_cpus, GFP_KERNEL_ACCOUNT)) {
 		ret = -ENOMEM;
-- 
2.55.0.1082.g2b9226bbc0-goog


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [PATCH 2/2] KVM: SEV: Do cache maintenance on the source VM during intra-host migration
  2026-09-23 16:37 ` [PATCH 2/2] KVM: SEV: Do cache maintenance on the source VM during intra-host migration Sean Christopherson
@ 2026-09-23 16:53   ` sashiko-bot
  2026-09-23 17:16     ` Sean Christopherson
  0 siblings, 1 reply; 8+ messages in thread
From: sashiko-bot @ 2026-09-23 16:53 UTC (permalink / raw)
  To: Sean Christopherson; +Cc: kvm

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Critical] kvm: sev: TOCTOU race in sev_migrate_from causes host memory corruption
--

commit b97586a7411bd5d8a55967b35859f6e485f10868
Author: Sean Christopherson <seanjc@google.com>

KVM: SEV: Do cache maintenance on the source VM during intra-host migration

This commit manually performs cache maintenance on the source VM during
intra-host migration. Since the source VM is converted to a non-SEV VM,
automatic memory reclaim flows will not trigger the required cache
maintenance, potentially leaving stale data in the CPU caches.

> diff --git a/arch/x86/kvm/svm/sev.c b/arch/x86/kvm/svm/sev.c
> index c9242c936a401..71923cb72d1d1 100644
> --- a/arch/x86/kvm/svm/sev.c
> +++ b/arch/x86/kvm/svm/sev.c
> @@ -2048,6 +2048,12 @@ static void sev_migrate_from(struct kvm *dst_kvm, struct kvm *src_kvm)
>  	src->pages_locked = 0;
>  	src->es_active = false;
>  
> +	/*
> +	 * Do cache maintenance on the source VM as it is no longer an SEV VM,
> +	 * i.e. memory reclaim flows won't trigger cache maintenance on the VM.
> +	 */
> +	sev_writeback_caches(src_kvm);

[Severity: Critical]
Is there a race condition here if memory is reclaimed concurrently?

If a userspace process or kernel reclaim triggers a memory unmap (for
example, via MADV_DONTNEED) concurrently with the KVM ioctl, it could happen
in the window between setting src->active to false and calling
sev_writeback_caches():

CPU1 in sev_migrate_from():
    src->active = false;

CPU2 running concurrent unmap:
    Since active is false, the automatic cache flush in
    sev_guest_memory_reclaimed is skipped.
    The host frees and reallocates the page.

CPU1 in sev_migrate_from():
    sev_writeback_caches(src_kvm);
    Executes a hardware cache flush (wbnoinvd), which writes the guest's old
    dirty ciphertext over the new page owner's data.

Because sev_migrate_from runs without mmap_lock or mmu_lock, MMU notifiers
can run concurrently. Can this cause host memory corruption by overwriting
newly allocated host pages with stale ciphertext?

> +
>  	list_cut_before(&dst->regions_list, &src->regions_list, &src->regions_list);
>  
>  	mutex_lock(&sev_mirror_lock);

[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260923163721.1584779-1-seanjc@google.com?part=2

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH 1/2] KVM: SEV: Free have_run_cpus during VM destruction even if VM is no longer SEV
  2026-09-23 16:37 ` [PATCH 1/2] KVM: SEV: Free have_run_cpus during VM destruction even if VM is no longer SEV Sean Christopherson
@ 2026-09-23 16:53   ` sashiko-bot
  2026-09-23 17:27     ` Sean Christopherson
  0 siblings, 1 reply; 8+ messages in thread
From: sashiko-bot @ 2026-09-23 16:53 UTC (permalink / raw)
  To: Sean Christopherson; +Cc: kvm

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Critical] kvm: double free of sev->have_run_cpus on initialization failure
--

commit 35fcc7370ece167eff16a7738a6c47e4815c1ae4
Author: Sean Christopherson <seanjc@google.com>

KVM: SEV: Free have_run_cpus during VM destruction even if VM is no longer SEV

Unconditionally free SEV's "have run CPUs" cpumask in the VM destroy path,
i.e. even for what appear to be non-SEV VMs, as an SEV VM becomes a non-SEV
VM if its state is intra-host migrated.

> diff --git a/arch/x86/kvm/svm/sev.c b/arch/x86/kvm/svm/sev.c
> index 63e8cfa9bf559..c9242c936a401 100644
> --- a/arch/x86/kvm/svm/sev.c
> +++ b/arch/x86/kvm/svm/sev.c
> @@ -2981,13 +2981,17 @@ void sev_vm_destroy(struct kvm *kvm)
>  	struct list_head *head = &sev->regions_list;
>  	struct list_head *pos, *q;
>  
> +	/*
> +	 * Free the mask even if the VM is not *currently* an SEV VM, as it may
> +	 * have been an SEV VM prior to intra-host migration.
> +	 */
> +	free_cpumask_var(sev->have_run_cpus);

[Severity: Critical]
Can this cause a double free of sev->have_run_cpus?

If a user issues a KVM_SEV_INIT ioctl and __sev_guest_init() fails after
allocating the cpumask, the error path in __sev_guest_init() frees the
mask but leaves the pointer intact:

arch/x86/kvm/svm/sev.c:__sev_guest_init() {
    ...
e_free:
	free_cpumask_var(sev->have_run_cpus);
    ...
}

Because free_cpumask_var() receives the mask by value, the pointer remains
dangling. When the user later closes the VM file descriptor,
kvm_destroy_vm() calls sev_vm_destroy().

Prior to this patch, the early return on !sev_guest(kvm) prevented the second
free. By moving free_cpumask_var() before the !sev_guest(kvm) check, won't
we unconditionally double free the dangling pointer?

> +
>  	if (!sev_guest(kvm))
>  		return;
>  
>  	WARN_ON(!list_empty(&sev->mirror_vms));
>  
> -	free_cpumask_var(sev->have_run_cpus);
> -
>  	/*
>  	 * If this is a mirror VM, remove it from the owner's list of a mirrors
>  	 * and skip ASID cleanup (the ASID is tied to the lifetime of the owner).

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260923163721.1584779-1-seanjc@google.com?part=1

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH 2/2] KVM: SEV: Do cache maintenance on the source VM during intra-host migration
  2026-09-23 16:53   ` sashiko-bot
@ 2026-09-23 17:16     ` Sean Christopherson
  0 siblings, 0 replies; 8+ messages in thread
From: Sean Christopherson @ 2026-09-23 17:16 UTC (permalink / raw)
  To: sashiko-reviews; +Cc: kvm

On Wed, Sep 23, 2026, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [Critical] kvm: sev: TOCTOU race in sev_migrate_from causes host memory corruption
> --
> 
> commit b97586a7411bd5d8a55967b35859f6e485f10868
> Author: Sean Christopherson <seanjc@google.com>
> 
> KVM: SEV: Do cache maintenance on the source VM during intra-host migration
> 
> This commit manually performs cache maintenance on the source VM during
> intra-host migration. Since the source VM is converted to a non-SEV VM,
> automatic memory reclaim flows will not trigger the required cache
> maintenance, potentially leaving stale data in the CPU caches.
> 
> > diff --git a/arch/x86/kvm/svm/sev.c b/arch/x86/kvm/svm/sev.c
> > index c9242c936a401..71923cb72d1d1 100644
> > --- a/arch/x86/kvm/svm/sev.c
> > +++ b/arch/x86/kvm/svm/sev.c
> > @@ -2048,6 +2048,12 @@ static void sev_migrate_from(struct kvm *dst_kvm, struct kvm *src_kvm)
> >  	src->pages_locked = 0;
> >  	src->es_active = false;
> >  
> > +	/*
> > +	 * Do cache maintenance on the source VM as it is no longer an SEV VM,
> > +	 * i.e. memory reclaim flows won't trigger cache maintenance on the VM.
> > +	 */
> > +	sev_writeback_caches(src_kvm);
> 
> [Severity: Critical]
> Is there a race condition here if memory is reclaimed concurrently?
> 
> If a userspace process or kernel reclaim triggers a memory unmap (for
> example, via MADV_DONTNEED) concurrently with the KVM ioctl, it could happen
> in the window between setting src->active to false and calling
> sev_writeback_caches():
> 
> CPU1 in sev_migrate_from():
>     src->active = false;
> 
> CPU2 running concurrent unmap:
>     Since active is false, the automatic cache flush in
>     sev_guest_memory_reclaimed is skipped.
>     The host frees and reallocates the page.
> 
> CPU1 in sev_migrate_from():
>     sev_writeback_caches(src_kvm);
>     Executes a hardware cache flush (wbnoinvd), which writes the guest's old
>     dirty ciphertext over the new page owner's data.
> 
> Because sev_migrate_from runs without mmap_lock or mmu_lock, MMU notifiers
> can run concurrently. Can this cause host memory corruption by overwriting
> newly allocated host pages with stale ciphertext?

Oof, yeah.  It's a tiny window, but also trivial to fix and document.

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH 1/2] KVM: SEV: Free have_run_cpus during VM destruction even if VM is no longer SEV
  2026-09-23 16:53   ` sashiko-bot
@ 2026-09-23 17:27     ` Sean Christopherson
  0 siblings, 0 replies; 8+ messages in thread
From: Sean Christopherson @ 2026-09-23 17:27 UTC (permalink / raw)
  To: sashiko-reviews; +Cc: kvm

On Wed, Sep 23, 2026, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [Critical] kvm: double free of sev->have_run_cpus on initialization failure
> --
> 
> commit 35fcc7370ece167eff16a7738a6c47e4815c1ae4
> Author: Sean Christopherson <seanjc@google.com>
> 
> KVM: SEV: Free have_run_cpus during VM destruction even if VM is no longer SEV
> 
> Unconditionally free SEV's "have run CPUs" cpumask in the VM destroy path,
> i.e. even for what appear to be non-SEV VMs, as an SEV VM becomes a non-SEV
> VM if its state is intra-host migrated.
> 
> > diff --git a/arch/x86/kvm/svm/sev.c b/arch/x86/kvm/svm/sev.c
> > index 63e8cfa9bf559..c9242c936a401 100644
> > --- a/arch/x86/kvm/svm/sev.c
> > +++ b/arch/x86/kvm/svm/sev.c
> > @@ -2981,13 +2981,17 @@ void sev_vm_destroy(struct kvm *kvm)
> >  	struct list_head *head = &sev->regions_list;
> >  	struct list_head *pos, *q;
> >  
> > +	/*
> > +	 * Free the mask even if the VM is not *currently* an SEV VM, as it may
> > +	 * have been an SEV VM prior to intra-host migration.
> > +	 */
> > +	free_cpumask_var(sev->have_run_cpus);
> 
> [Severity: Critical]
> Can this cause a double free of sev->have_run_cpus?
> 
> If a user issues a KVM_SEV_INIT ioctl and __sev_guest_init() fails after
> allocating the cpumask, the error path in __sev_guest_init() frees the
> mask but leaves the pointer intact:
> 
> arch/x86/kvm/svm/sev.c:__sev_guest_init() {
>     ...
> e_free:
> 	free_cpumask_var(sev->have_run_cpus);
>     ...
> }
> 
> Because free_cpumask_var() receives the mask by value, the pointer remains
> dangling. When the user later closes the VM file descriptor,
> kvm_destroy_vm() calls sev_vm_destroy().
> 
> Prior to this patch, the early return on !sev_guest(kvm) prevented the second
> free. By moving free_cpumask_var() before the !sev_guest(kvm) check, won't
> we unconditionally double free the dangling pointer?

Ugh, what a pain.  Ideally, free_cpumask_var() would nullify the pointer.  But
for a KVM-only "fix", it's easy enough to do the mask allocation last so that
there's no need to unwind and figure out how to nullify the pointer when it's
off-"stack".

diff --git a/arch/x86/kvm/svm/sev.c b/arch/x86/kvm/svm/sev.c
index 96b4ed220eaf..4cbd38a48c5a 100644
--- a/arch/x86/kvm/svm/sev.c
+++ b/arch/x86/kvm/svm/sev.c
@@ -545,16 +545,16 @@ static int __sev_guest_init(struct kvm *kvm, struct kvm_sev_cmd *argp,
 	if (ret)
 		goto e_free_asid;
 
-	if (!zalloc_cpumask_var(&sev->have_run_cpus, GFP_KERNEL_ACCOUNT)) {
-		ret = -ENOMEM;
-		goto e_free_asid;
-	}
-
 	/* This needs to happen after SEV/SNP firmware initialization. */
 	if (snp_active) {
 		ret = snp_guest_req_init(kvm);
 		if (ret)
-			goto e_free;
+			goto e_free_asid;
+	}
+
+	if (!zalloc_cpumask_var(&sev->have_run_cpus, GFP_KERNEL_ACCOUNT)) {
+		ret = -ENOMEM;
+		goto e_free;
 	}
 
 	INIT_LIST_HEAD(&sev->regions_list);
@@ -566,7 +566,8 @@ static int __sev_guest_init(struct kvm *kvm, struct kvm_sev_cmd *argp,
 	return 0;
 
 e_free:
-	free_cpumask_var(sev->have_run_cpus);
+	if (snp_active)
+		snp_guest_req_cleanup(kvm);
 e_free_asid:
 	argp->error = init_args.error;
 	sev_asid_free(sev);

^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [PATCH 0/2] KVM: SEV: Fix intra-host migration cache bugs
  2026-09-23 16:37 [PATCH 0/2] KVM: SEV: Fix intra-host migration cache bugs Sean Christopherson
  2026-09-23 16:37 ` [PATCH 1/2] KVM: SEV: Free have_run_cpus during VM destruction even if VM is no longer SEV Sean Christopherson
  2026-09-23 16:37 ` [PATCH 2/2] KVM: SEV: Do cache maintenance on the source VM during intra-host migration Sean Christopherson
@ 2026-09-26  4:37 ` Paolo Bonzini
  2 siblings, 0 replies; 8+ messages in thread
From: Paolo Bonzini @ 2026-09-26  4:37 UTC (permalink / raw)
  To: Sean Christopherson; +Cc: kvm, linux-kernel, Stefan Teodorescu

Queued, thanks.

Paolo



^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2026-09-26  4:37 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-09-23 16:37 [PATCH 0/2] KVM: SEV: Fix intra-host migration cache bugs Sean Christopherson
2026-09-23 16:37 ` [PATCH 1/2] KVM: SEV: Free have_run_cpus during VM destruction even if VM is no longer SEV Sean Christopherson
2026-09-23 16:53   ` sashiko-bot
2026-09-23 17:27     ` Sean Christopherson
2026-09-23 16:37 ` [PATCH 2/2] KVM: SEV: Do cache maintenance on the source VM during intra-host migration Sean Christopherson
2026-09-23 16:53   ` sashiko-bot
2026-09-23 17:16     ` Sean Christopherson
2026-09-26  4:37 ` [PATCH 0/2] KVM: SEV: Fix intra-host migration cache bugs Paolo Bonzini

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox