Re: [PATCH v8 04/13] KVM: s390: Fix overclearing ESCA in case of error

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [Critical] For KVM_VM_S390_UCONTROL VMs, userspace can trigger an out-of-bounds read and write in `sca_clear_ext_call` and `sca_inject_ext_call` by creating a vCPU with an ID >= 248.
- [High] Memory leak of `vcpu->arch.mc` and its topped-up pages when `kvm_arch_vcpu_create()` fails.
--

--- Patch [4]: [PATCH v8 04/13] KVM: s390: Fix overclearing ESCA in case of error ---
commit 554d58551a2c2c8a2475d02299b28fea5b490098
Author: Claudio Imbrenda <imbrenda@linux.ibm.com>

KVM: s390: Fix overclearing ESCA in case of error

This commit fixes an issue where destroying a duplicated vCPU clears the ESCA
entry of the original vCPU. It skips clearing the ESCA entry if the vCPU
creation was unsuccessful.

> 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;

[Severity: Critical]
This is a pre-existing issue, but can this out-of-bounds access occur for
ucontrol VMs?

For KVM_VM_S390_UCONTROL VMs, kvm_arch_vcpu_precreate() skips the
sca_can_add_vcpu() check. This allows a user with CAP_SYS_ADMIN to create
vCPUs with IDs up to 254.

However, the sca->cpu array in struct esca_block only has
KVM_S390_ESCA_CPU_SLOTS (248) elements. Because sca_clear_ext_call() is
called unconditionally when destroying the vCPU, it indexes sca->cpu with this
unvalidated vcpu_id before the initialization check.

Would this result in an out-of-bounds read and write to adjacent kernel
memory here, and similarly in sca_inject_ext_call()?

> -	if (!kvm_s390_use_sca_entries())
> +	if (!kvm_s390_use_sca_entries() || !vcpu->arch.initialized)
>  		return;
>  	kvm_s390_clear_cpuflags(vcpu, CPUSTAT_ECALL_PEND);
>  

[ ... ]

[Severity: High]
This is a pre-existing issue, but does kvm_arch_vcpu_create() leak the MMU
cache on error paths?

If kvm_s390_new_mmu_cache() succeeds, but a later step fails (such as
gmap_new_child() for a ucontrol VM, or ultravisor PV setup failing), the error
path jumps to out_free_sie_block:

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

It appears kvm_s390_free_mmu_cache(vcpu->arch.mc) is never called here. Since
the caller kvm_vm_ioctl_create_vcpu() handles the error by freeing the base
vCPU struct without calling kvm_arch_vcpu_destroy(), does this permanently
leak vcpu->arch.mc and its internally topped-up pages?

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