Re: [PATCH v4 13/13] KVM: s390: Fix sca_clear_ext_call() for UCONTROL

[sashiko-bot@kernel.org]

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

New issues:
- [High] Asymmetric resource teardown of CPUSTAT_ECALL_PEND for UCONTROL VMs leading to a permanent stuck state.
- [High] Double-decrement of `cmma_dirty_pages` counter due to a retry loop.

Pre-existing issues:
- [Critical] Out-of-bounds memory access in `sca_inject_ext_call()` and `sca_ext_call_pending()` for UCONTROL VMs.
--

--- Patch [13]: [PATCH v4 13/13] KVM: s390: Fix sca_clear_ext_call() for UCONTROL ---
commit d27d3f9f6aa096af6e829ecfb494bcc68d320453
Author: Claudio Imbrenda <imbrenda@linux.ibm.com>
Date:   Tue Jul 28 17:07:08 2026 +0000

    KVM: s390: Fix sca_clear_ext_call() for UCONTROL

This commit fixes an out-of-bounds access in sca_clear_ext_call() for UCONTROL
VMs by returning early before touching the ESCA block. UCONTROL VMs bypass ESCA
but can have vCPU IDs larger than the ESCA block capacity.

> diff --git a/arch/s390/kvm/interrupt.c b/arch/s390/kvm/interrupt.c
> index 2acdff130fa6c..8d146a0681dce 100644
> --- a/arch/s390/kvm/interrupt.c
> +++ b/arch/s390/kvm/interrupt.c
> @@ -84,10 +84,13 @@ static int sca_inject_ext_call(struct kvm_vcpu *vcpu, int src_id)
>  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;
> +	union esca_sigp_ctrl *sigp_ctrl;
>  
> -	if (!kvm_s390_use_sca_entries() || !vcpu->arch.initialized)
> +	if (!kvm_s390_use_sca_entries() || !vcpu->arch.initialized || kvm_is_ucontrol(vcpu->kvm))
>  		return;

[Severity: High]
Could this early return for UCONTROL VMs skip clearing the CPUSTAT_ECALL_PEND
flag?

Prior to this patch, if an external call was injected into a UCONTROL VM,
sca_inject_ext_call() would set the CPUSTAT_ECALL_PEND cpuflag, and
sca_clear_ext_call() would later clear it. By adding an early return here
without a corresponding check in sca_inject_ext_call(), the flag is set but
never cleared. Would this leave the VM in a permanently corrupted state across
resets?

[Severity: Critical]
This is a pre-existing issue, but UCONTROL VMs bypass the sca_can_add_vcpu()
constraints, allowing vCPU IDs up to KVM_MAX_VCPUS - 1 (254). The ESCA block
only holds KVM_S390_ESCA_CPU_SLOTS (248) entries.

While this patch prevents out-of-bounds access in sca_clear_ext_call(), it
fails to patch identical accesses in sca_inject_ext_call():

arch/s390/kvm/interrupt.c:sca_inject_ext_call() {
    ...
    union esca_sigp_ctrl *sigp_ctrl = &sca->cpu[vcpu->vcpu_id].sigp_ctrl;
    ...
}

and sca_ext_call_pending():

arch/s390/kvm/interrupt.c:sca_ext_call_pending() {
    ...
    union esca_sigp_ctrl sigp_ctrl = sca->cpu[vcpu->vcpu_id].sigp_ctrl;
    ...
}

The entry point for userspace interrupt injection, __inject_extcall(), calls
sca_inject_ext_call() based on host hardware capabilities without verifying
if the target VM is UCONTROL.

Can a malicious userspace process trigger these out-of-bounds accesses by
injecting external calls or polling for pending interrupts on a vCPU with
an ID >= 248?

> +
> +	/* Initialize after the above check, to prevent going out of bounds */
> +	sigp_ctrl = &sca->cpu[vcpu->vcpu_id].sigp_ctrl;
>  	kvm_s390_clear_cpuflags(vcpu, CPUSTAT_ECALL_PEND);
>  
>  	WRITE_ONCE(sigp_ctrl->value, 0);

[Severity: High]
I also noticed a separate issue introduced in commit 'KVM: s390: Fix
kvm_arch_commit_memory_region() when low on memory'.

Is there a risk that the retry loop in kvm_arch_commit_memory_region() causes
a double-decrement of the cmma_dirty_pages counter?

arch/s390/kvm/kvm-s390.c:kvm_arch_commit_memory_region() {
    ...
    retry:
        ...
        _dat_walk_gfn_range(old->base_gfn, old->base_gfn + old->npages,
                            kvm->arch.gmap->asce, &ops, DAT_WALK_IGN_HOLES,
                            &kvm->arch.cmma_dirty_pages);
    ...
}

Inside this loop, _dat_walk_gfn_range() uses cmma_d_count_pte() to decrement
kvm->arch.cmma_dirty_pages for every PTE where cmma_d is set:

arch/s390/kvm/kvm-s390.c:cmma_d_count_pte() {
    if (pgste_of(ptep)->cmma_d)
        atomic64_dec(walk->priv);
    ...
}

Because cmma_d_count_pte() does not clear the cmma_d bit, if
dat_delete_slot() or dat_create_slot() fails with -ENOMEM, the code jumps
to retry and executes the pass again over the exact same range.

Could this decrement the counter a second time for unchanged PTEs, leading to
an underflow and breaking CMMA dirty page tracking during VM migration?

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