Re: [PATCH v8 05/13] 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] Hardware-dependent ABI inconsistency for UCONTROL VMs when injecting KVM_S390_INT_EXTERNAL_CALL via KVM_S390_IRQ.
--

--- Patch [5]: [PATCH v8 05/13] KVM: s390: ucontrol: Fix sca_clear_ext_call() ---
commit 450e8d67402bbac296dca888444bff5561662e98
Author: Claudio Imbrenda <imbrenda@linux.ibm.com>

KVM: s390: ucontrol: Fix sca_clear_ext_call()

This commit fixes an out-of-bounds access in sca_clear_ext_call() for
UCONTROL VMs by explicitly not touching ESCA. It also fences the
KVM_S390_INTERRUPT ioctl for UCONTROL VMs and adds extra checks to make
sure UCONTROL VMs won't touch ESCA.

> 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
[ ... ]
> @@ -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]
Will this create a hardware-dependent ABI for UCONTROL VMs when injecting
KVM_S390_INT_EXTERNAL_CALL via KVM_S390_IRQ?

Looking at __inject_extcall(), sca_inject_ext_call() is only executed 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 (test_and_set_bit(IRQ_PEND_EXT_EXTERNAL, &li->pending_irqs))
    ...
}

If the host lacks ESCA support, the UCONTROL check here in
sca_inject_ext_call() is bypassed, and the external call is successfully
queued.

Unlike KVM_S390_INTERRUPT, the KVM_S390_IRQ ioctl is not fenced for UCONTROL
VMs, allowing userspace to trigger this inconsistent behavior. Could this
cause the same ioctl to fail on modern hosts but succeed on older ones?

Would it be better to place the UCONTROL check higher up in __inject_extcall()
to ensure consistent behavior, or to also fence KVM_S390_IRQ for UCONTROL VMs?

>  
> +	sigp_ctrl = &sca->cpu[vcpu->vcpu_id].sigp_ctrl;
>  	old_val = READ_ONCE(*sigp_ctrl);
>  	old_val.c = 0;
>

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