Re: [PATCHv2 6/7] KVM: PPC: Book3S HV: Fix error handling during vcore create

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] KVM: PPC: Book3S HV: inverted vcore check in kvmppc_core_vcpu_create_hv()
--

commit 03a4203b0b5c05e6f77faafc6f01a945c2457b16
Author: Ritesh Harjani (IBM) <ritesh.list@gmail.com>

KVM: PPC: Book3S HV: Fix error handling during vcore create

This commit adds a NULL check on the vcore returned by kvmppc_vcore_create()
in kvmppc_core_vcpu_create_hv() before updating kvm->arch.vcores[core] and
incrementing kvm->arch.online_vcores.

> diff --git a/arch/powerpc/kvm/book3s_hv.c b/arch/powerpc/kvm/book3s_hv.c
> index 22283d46e8fe6..7975e5de89d0e 100644
> --- a/arch/powerpc/kvm/book3s_hv.c
> +++ b/arch/powerpc/kvm/book3s_hv.c
> @@ -3090,10 +3090,12 @@ static int kvmppc_core_vcpu_create_hv(struct kvm_vcpu *vcpu)
>  			err = -ENOMEM;
>  			vcore = kvmppc_vcore_create(kvm,
>  					id & ~(kvm->arch.smt_mode - 1));
> -			mutex_lock(&kvm->arch.mmu_setup_lock);
> -			kvm->arch.vcores[core] = vcore;
> -			kvm->arch.online_vcores++;
> -			mutex_unlock(&kvm->arch.mmu_setup_lock);
> +			if (!vcore) {
> +				mutex_lock(&kvm->arch.mmu_setup_lock);
> +				kvm->arch.vcores[core] = vcore;
> +				kvm->arch.online_vcores++;
> +				mutex_unlock(&kvm->arch.mmu_setup_lock);
> +			}

[Severity: High]
Is the condition in if (!vcore) inverted here in
kvmppc_core_vcpu_create_hv()?

kvmppc_vcore_create() returns a non-NULL struct kvmppc_vcore pointer on
success and NULL on allocation failure, and kvmppc_core_vcpu_create_hv()
checks for failure immediately below:

	if (!vcore)
		return err;

With if (!vcore) guarding the mmu_setup_lock block, a successful
kvmppc_vcore_create() skips storing vcore in kvm->arch.vcores[core] and
incrementing kvm->arch.online_vcores, while still attaching the vcore to
vcpu->arch.vcore. Because kvmppc_free_vcores() frees vcores by iterating over
kvm->arch.vcores[], any allocated vcore will be leaked on VM teardown, and
callers relying on kvm->arch.vcores[] or kvm->arch.online_vcores (such as
sibling thread attachment, duplicate vCPU ID detection, kvmppc_update_lpcr(),
and kvmhv_set_smt_mode()) will see inconsistent state. Conversely, if
kvmppc_vcore_create() fails and returns NULL, kvm->arch.online_vcores is
incremented before returning -ENOMEM.

Should this check be if (vcore) instead?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/ca419693358e3af49b1dfa7887b34b8ef91a5daf.1793543627.git.ritesh.list@gmail.com?part=1