Re: [PATCH v5 02/13] KVM: s390: Fix kvm_s390_vcpu_unsetup_cmma()

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 allocated in kvm_arch_vcpu_create() is leaked on error paths.
--

--- Patch [2]: [PATCH v5 02/13] KVM: s390: Fix kvm_s390_vcpu_unsetup_cmma() ---
commit 0d3b94e982f008ae0ede697fd75113aea3b668c3
Author: Claudio Imbrenda <[email protected]>

KVM: s390: Fix kvm_s390_vcpu_unsetup_cmma()

In some cases kvm_s390_vcpu_unsetup_cmma() can be called with a 0 cbrlo.
This commit fixes an issue where running with V != R causes free_page() to
attempt freeing physical page 0 by ensuring cbrlo is only freed if it is
non-zero.

> diff --git a/arch/s390/kvm/kvm-s390.c b/arch/s390/kvm/kvm-s390.c
> index 2fe2e46b7ac39..096b426c1281b 100644
> --- a/arch/s390/kvm/kvm-s390.c
> +++ b/arch/s390/kvm/kvm-s390.c
> @@ -3675,7 +3675,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;
>  }

This is a pre-existing issue, but while reviewing this file I noticed a
memory leak in kvm_arch_vcpu_create().

When a subsequent initialization step inside kvm_arch_vcpu_create() fails,
the function jumps to the out_free_sie_block label:

arch/s390/kvm/kvm-s390.c:kvm_arch_vcpu_create() {
    ...
	vcpu->arch.mc = kvm_s390_new_mmu_cache();
	if (!vcpu->arch.mc)
		return -ENOMEM;
    ...
out_free_sie_block:
	free_page((unsigned long)(vcpu->arch.sie_block));
	return rc;
}

[Severity: High]
Does this error path permanently leak the struct kvm_s390_mmu_cache
allocated earlier in the function, since vcpu->arch.mc is not freed here?

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