Re: [PATCH v3 06/10] KVM: Initialize a vCPU's index to '-1' while it's being created

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] The vCPU index is not unwound back to -1 on the `create_vcpu_fd()` error path, defeating the patch's attempt to suppress false-positive lockdep assertions for unreachable vCPUs.
--

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

KVM: Initialize a vCPU's index to '-1' while it's being created

This commit initializes a vCPU's index to -1 during creation to prevent KVM
from incorrectly treating an unreachable vCPU as vCPU0. It also unwinds the
index back to -1 if inserting the vCPU into the array fails, suppressing
false positive lockdep assertions during teardown.

> diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
> index e44c20c049610..98da4c889ffc6 100644
> --- a/virt/kvm/kvm_main.c
> +++ b/virt/kvm/kvm_main.c
[ ... ]
> @@ -4216,11 +4218,18 @@ static int kvm_vm_ioctl_create_vcpu(struct kvm *kvm, unsigned long id)
>  		goto unlock_vcpu_destroy;
>  	}
>  
> +	/*
> +	 * Set the vCPU's index *before* the vCPU is reachable by other tasks.
> +	 * Unwind the index back to -1 on failure so that KVM can use the index
> +	 * to detect that the vCPU is unreachable, e.g. for lockdep asserts.
> +	 */
>  	vcpu->vcpu_idx = atomic_read(&kvm->online_vcpus);
>  	r = xa_insert(&kvm->vcpu_array, vcpu->vcpu_idx, vcpu, GFP_KERNEL_ACCOUNT);
>  	WARN_ON_ONCE(r == -EBUSY);
> -	if (r)
> +	if (r) {
> +		vcpu->vcpu_idx = -1;
>  		goto unlock_vcpu_destroy;
> +	}
>  
>  	/*
>  	 * Now it's all set up, let userspace reach it.  Grab the vCPU's mutex

[Severity: Medium]
Should the vCPU index also be unwound back to -1 on the create_vcpu_fd()
error path?

If a local process exhausts its file descriptor limit, create_vcpu_fd()
will fail and jump to the kvm_put_xa_erase label in kvm_vm_ioctl_create_vcpu():

kvm_put_xa_erase:
	mutex_unlock(&vcpu->mutex);
	kvm_put_kvm_no_destroy(kvm);
	xa_erase(&kvm->vcpu_array, vcpu->vcpu_idx);
unlock_vcpu_destroy:
	mutex_unlock(&kvm->lock);

Since xa_erase() is called here without resetting vcpu->vcpu_idx = -1, does this
leave the vCPU with a valid index during the subsequent kvm_arch_vcpu_destroy()
call, potentially triggering the same lockdep assertion splats this patch
is trying to prevent?

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