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

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] The dynamically allocated MMU cache (`vcpu->arch.mc`) and its associated cached pages are permanently leaked in the error paths of `kvm_arch_vcpu_create()`.
--

--- Patch [9]: [PATCH v8 09/13] KVM: s390: ucontrol: Add missing locking around gmap_remove_child() ---
commit 468ac730893edf1b533b2a8b139847cb8a9ef03c
Author: Claudio Imbrenda <imbrenda@linux.ibm.com>

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 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 dynamically
allocated MMU cache (vcpu->arch.mc) and its internal cached pages?

Earlier in kvm_arch_vcpu_create(), the cache is allocated:

	vcpu->arch.mc = kvm_s390_new_mmu_cache();
	if (!vcpu->arch.mc)
		return -ENOMEM;

If the function encounters an error and jumps to out_free_sie_block, it cleans
up the sie_page but returns without calling kvm_s390_free_mmu_cache().

The core KVM code in kvm_vm_ioctl_create_vcpu() skips calling
kvm_arch_vcpu_destroy() when the architecture-specific create function fails,
which might permanently leak the mc memory. This could allow unprivileged
users to trigger a denial of service (OOM) through memory exhaustion by
creating PV vCPUs in a misconfigured environment or exhausting limits.

This leak remains unfixed at the end of the patch series.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260803124040.126471-1-imbrenda@linux.ibm.com?part=9