Re: [GIT PULL v1 14/23] KVM: s390: Fix overclearing ESCA in case of error

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] Out-of-bounds memory accesses to the ESCA block for KVM_VM_S390_UCONTROL VMs with vcpu_id >= KVM_S390_ESCA_CPU_SLOTS (248).
- [High] Memory leak of `vcpu->arch.mc` in `kvm_arch_vcpu_create` error paths.
- [High] Memory leak of `stor_base` and `sidad` pages in `kvm_s390_pv_create_cpu` when Ultravisor call fails.
--

--- Patch [14]: [GIT PULL v1 14/23] KVM: s390: Fix overclearing ESCA in case of error ---
commit d5a31a585e995dfd927fed9a3fff444b9b4d0db1
Author: Claudio Imbrenda <imbrenda@linux.ibm.com>

KVM: s390: Fix overclearing ESCA in case of error

If an attempt is made to create a vCPU with an already existing ID,
the duplicated vCPU will be destroyed, clearing its ESCA entry. This
commit skips clearing the ESCA entry if the vCPU creation was not
successful.

Message-ID: <20260803124040.126471-5-imbrenda@linux.ibm.com>

> diff --git a/arch/s390/kvm/interrupt.c b/arch/s390/kvm/interrupt.c
> index 9e3e6b0d72ad6..2acdff130fa6c 100644
> --- a/arch/s390/kvm/interrupt.c
> +++ b/arch/s390/kvm/interrupt.c
> @@ -86,7 +86,7 @@ static void sca_clear_ext_call(struct kvm_vcpu *vcpu)
>  	struct esca_block *sca = vcpu->kvm->arch.sca;
>  	union esca_sigp_ctrl *sigp_ctrl = &sca->cpu[vcpu->vcpu_id].sigp_ctrl;
>  
> -	if (!kvm_s390_use_sca_entries())
> +	if (!kvm_s390_use_sca_entries() || !vcpu->arch.initialized)
>  		return;

[Severity: High]
This is a pre-existing issue, but does this function have an out-of-bounds
write if the VM is KVM_VM_S390_UCONTROL and the vcpu_id is >= 248?

If a user with CAP_SYS_ADMIN creates a ucontrol VM and initializes a vCPU
with a vcpu_id between 248 and 254, indexing into sca->cpu[vcpu->vcpu_id]
would be out of bounds since KVM_S390_ESCA_CPU_SLOTS is 248.

Functions like sca_clear_ext_call(), sca_inject_ext_call(), and
sca_ext_call_pending() appear to access this without bounds checking. Could
this corrupt adjacent memory pages?

[ ... ]

[Severity: High]
This is a pre-existing issue, but does kvm_arch_vcpu_create() leak
vcpu->arch.mc in the error path?

Looking at the error handling in kvm_arch_vcpu_create():

out_ucontrol_uninit:
	if (kvm_is_ucontrol(vcpu->kvm)) {
		gmap_remove_child(vcpu->arch.gmap);
		vcpu->arch.gmap = gmap_put(vcpu->arch.gmap);
	}
out_free_sie_block:
	free_page((unsigned long)(vcpu->arch.sie_block));
	return rc;

If an error occurs during kvm_s390_vcpu_setup() and it jumps to
out_ucontrol_uninit, it appears kvm_s390_free_mmu_cache() is never called
before returning. Can userspace systematically leak the KVM MMU cache by
inducing vCPU setup failures?

[Severity: High]
This is a pre-existing issue, but does kvm_s390_pv_create_cpu() leak
stor_base and sidad pages when the Ultravisor call fails?

In kvm_s390_pv_create_cpu(), if the uv_call fails, it tries to clean up:

	if (cc) {
		u16 dummy;

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

However, kvm_s390_pv_destroy_cpu() checks the handle:

	if (!kvm_s390_pv_cpu_get_handle(vcpu))
		return 0;

Since vcpu->arch.pv.handle is populated only after a successful UV call,
this will return early. Does this bypass the free_pages() and free_page()
calls for stor_base and the SIDA, permanently leaking them?

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