Re: [GIT PULL v1 15/23] KVM: s390: ucontrol: Fix sca_clear_ext_call()

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] UCONTROL VM external call injection fails or succeeds inconsistently depending on the host's hardware ESCA support.
--

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

KVM: s390: ucontrol: Fix sca_clear_ext_call()

This commit addresses an issue where cleaning up a UCONTROL VM would touch
memory outside of the allocated ESCA block. It fixes this by avoiding ESCA
access for UCONTROL VMs and fencing the KVM_S390_INTERRUPT ioctl for them.

> diff --git a/arch/s390/kvm/interrupt.c b/arch/s390/kvm/interrupt.c
> index 2acdff130fa6c..6b3f97a7513b3 100644
> --- a/arch/s390/kvm/interrupt.c
> +++ b/arch/s390/kvm/interrupt.c
> @@ -45,13 +45,16 @@ static struct kvm_s390_gib *gib;
>  static int sca_ext_call_pending(struct kvm_vcpu *vcpu, int *src_id)
>  {
>  	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_test_cpuflags(vcpu, CPUSTAT_ECALL_PEND))
>  		return 0;
> +	if (kvm_is_ucontrol(vcpu->kvm))
> +		return 0;

[Severity: Medium]
Will this cause pending software external calls to be ignored on hosts with
ESCA?

If the host supports ESCA and kvm_s390_use_sca_entries() is true, the system
will rely on sca_ext_call_pending(). By returning 0 here for UCONTROL VMs,
won't any pending software external calls be ignored?

[ ... ]

> @@ -60,13 +63,16 @@ static int sca_ext_call_pending(struct kvm_vcpu *vcpu, int *src_id)
>  
>  static int sca_inject_ext_call(struct kvm_vcpu *vcpu, int src_id)
>  {
> -	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 old_val, new_val = {.scn = src_id, .c = 1};
> +	struct esca_block *sca = vcpu->kvm->arch.sca;
> +	union esca_sigp_ctrl *sigp_ctrl;
>  	int expect, rc;
>  
>  	BUG_ON(!kvm_s390_use_sca_entries());
> +	if (kvm_is_ucontrol(vcpu->kvm))
> +		return -EINVAL;

[Severity: Medium]
Does this create an ABI inconsistency where UCONTROL VMs behave differently
based on the host's hardware capabilities?

When userspace calls the KVM_S390_IRQ ioctl on a UCONTROL VM to inject
KVM_S390_INT_EXTERNAL_CALL, __inject_extcall() checks if the host
supports ESCA:

arch/s390/kvm/interrupt.c:__inject_extcall() {
    ...
    if (kvm_s390_use_sca_entries() && !kvm_s390_pv_cpu_get_handle(vcpu))
        return sca_inject_ext_call(vcpu, src_id);
    ...
}

If the host doesn't support ESCA, it bypasses sca_inject_ext_call() and
successfully queues the interrupt via the software fallback. 

But if the host does support ESCA, it calls sca_inject_ext_call() which
will now hit this check and return -EINVAL instead of queuing it.

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