Re: [PATCH v7 09/14] 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] Memory leak of the VCPU MMU cache (`vcpu->arch.mc`) on `kvm_arch_vcpu_create()` error paths.
--

--- Patch [9]: [PATCH v7 09/14] KVM: s390: ucontrol: Add missing locking around gmap_remove_child() ---
commit f9a2b14fcbcb96b8df54e09c4777e2c6d45f939a
Author: Claudio Imbrenda <[email protected]>

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

This commit adds missing locking around gmap_remove_child() 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 ba811f0673d10..2741ca323edea 100644
> --- a/arch/s390/kvm/kvm-s390.c
> +++ b/arch/s390/kvm/kvm-s390.c
> @@ -3875,7 +3875,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 is a pre-existing issue, but does this error path leak the VCPU MMU
cache?

Looking at kvm_arch_vcpu_create() in arch/s390/kvm/kvm-s390.c, if a failure
happens and the code jumps to out_ucontrol_uninit or out_free_sie_block,
it appears to return the error code without calling kvm_s390_free_mmu_cache()
on vcpu->arch.mc.

Since the core KVM function kvm_vm_ioctl_create_vcpu() skips calling
kvm_arch_vcpu_destroy() when kvm_arch_vcpu_create() returns an error,
does this permanently leak the dynamically allocated cache object and its
elements?

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