Re: [PATCH v5 08/13] KVM: s390: ucontrol: Add missing locking around gmap_remove_child()

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] The struct kvm_s390_mmu_cache (vcpu->arch.mc) is permanently leaked on error paths in kvm_arch_vcpu_create().
--

--- Patch [8]: [PATCH v5 08/13] KVM: s390: ucontrol: Add missing locking around gmap_remove_child() ---
commit 1255c78b1a63e5de1172d37ceb311f1fd31eb54f
Author: Claudio Imbrenda <[email protected]>

KVM: s390: ucontrol: Add missing locking around gmap_remove_child()

gmap_remove_child() needs to be called while holding the children_lock
of the parent gmap. This was not the case in the error handling path of
kvm_arch_vcpu_create() for UCONTROL guests.

> diff --git a/arch/s390/kvm/kvm-s390.c b/arch/s390/kvm/kvm-s390.c
> index 2ae431030621e..c04b7e84c4346 100644
> --- a/arch/s390/kvm/kvm-s390.c
> +++ b/arch/s390/kvm/kvm-s390.c
> @@ -3869,7 +3869,9 @@ int kvm_arch_vcpu_create(struct kvm_vcpu *vcpu)
>  
>  out_ucontrol_uninit:
>  	if (kvm_is_ucontrol(vcpu->kvm)) {
> +		spin_lock(&vcpu->kvm->arch.gmap->children_lock);
>  		gmap_remove_child(vcpu->arch.gmap);
> +		spin_unlock(&vcpu->kvm->arch.gmap->children_lock);
>  		vcpu->arch.gmap = gmap_put(vcpu->arch.gmap);
>  	}
>  out_free_sie_block:

[Severity: High]
This isn't a bug introduced by this patch, but does this error path
permanently leak the struct kvm_s390_mmu_cache (vcpu->arch.mc)?

Looking at kvm_arch_vcpu_create() in arch/s390/kvm/kvm-s390.c, vcpu->arch.mc
is dynamically allocated earlier in the function via kvm_s390_new_mmu_cache().

If a subsequent setup step fails (such as gmap_new_child or
kvm_s390_vcpu_setup) and execution jumps to out_ucontrol_uninit or
out_free_sie_block, the function returns without calling
kvm_s390_free_mmu_cache(vcpu->arch.mc).

Since the KVM core bypasses kvm_arch_vcpu_destroy() when vCPU creation fails,
could this lead to a permanent memory leak if a user repeatedly creates VMs
and artificially triggers failures?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=8