Re: [PATCH 1/2] KVM: SEV: Free have_run_cpus during VM destruction even if VM is no longer SEV

[Sean Christopherson <seanjc@google.com>]

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);