Re: [GIT PULL v1 13/23] KVM: s390: Fix kvm_s390_vcpu_unsetup_cmma()

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] Unrecoverable memory leak of `vcpu->arch.mc` during vCPU creation failure in `kvm_arch_vcpu_create()`.
- [High] Leaked `stor_base` and `sida_addr` memory pages when Protected Virtualization (PV) vCPU creation fails.
--

--- Patch [13]: [GIT PULL v1 13/23] KVM: s390: Fix kvm_s390_vcpu_unsetup_cmma() ---
commit 4fc054529959a7d0bf80c4a0b469e82c4a224bb3
Author: Claudio Imbrenda <imbrenda@linux.ibm.com>

KVM: s390: Fix kvm_s390_vcpu_unsetup_cmma()
    
This commit fixes an issue where a zero cbrlo value could lead to an attempt
to free physical page 0 when V != R. It resolves this by checking that cbrlo
is non-zero before calling free_page().

Link: <20260803124040.126471-4-imbrenda@linux.ibm.com>

> diff --git a/arch/s390/kvm/kvm-s390.c b/arch/s390/kvm/kvm-s390.c
> index 1b3290a5ad1ae..9be27db0a21e6 100644
> --- a/arch/s390/kvm/kvm-s390.c
> +++ b/arch/s390/kvm/kvm-s390.c
[ ... ]
> @@ -3678,7 +3678,8 @@ static void kvm_s390_vcpu_crypto_setup(struct kvm_vcpu *vcpu)
>  
>  void kvm_s390_vcpu_unsetup_cmma(struct kvm_vcpu *vcpu)
>  {
> -	free_page((unsigned long)phys_to_virt(vcpu->arch.sie_block->cbrlo));
> +	if (vcpu->arch.sie_block->cbrlo)
> +		free_page((unsigned long)phys_to_virt(vcpu->arch.sie_block->cbrlo));
>  	vcpu->arch.sie_block->cbrlo = 0;
>  }

[Severity: High]
This isn't a bug introduced by this patch, but while reviewing the vCPU
creation paths in this subsystem, I noticed a potential unrecoverable memory
leak.

In arch/s390/kvm/kvm-s390.c:kvm_arch_vcpu_create(), the mmu cache is
successfully allocated:

    vcpu->arch.mc = kvm_s390_new_mmu_cache();

However, if a subsequent operation fails and the function jumps to the error
path, it appears to miss freeing the cache:

    out_free_sie_block:
        free_page((unsigned long)(vcpu->arch.sie_block));
        return rc;

Does this leak the struct kvm_s390_mmu_cache and its internal allocations
when the core KVM code skips calling kvm_arch_vcpu_destroy() on an
arch-level creation failure?

[Severity: High]
This is also a pre-existing issue, but looking at the Protected
Virtualization code in arch/s390/kvm/pv.c, there might be another memory
leak during vCPU creation failure.

In kvm_s390_pv_create_cpu(), if the UV call fails, it attempts to clean up:

    if (cc) {
        u16 dummy;

        kvm_s390_pv_destroy_cpu(vcpu, &dummy, &dummy);
        return -EIO;
    }

Because the handle is only populated upon success, it is still 0 during this
cleanup call. kvm_s390_pv_destroy_cpu() checks the handle and returns early:

    if (!kvm_s390_pv_cpu_get_handle(vcpu))
        return 0;

Could this leave the stor_base and sida_addr memory pages permanently leaked
if userspace repeatedly exhausts UV limits to fail the call?

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