Re: [PATCH 18/21] KVM: s390: vsie: Add VSIE max shadow configuration

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] Using the `MIN()` and `MAX()` macros on concurrently mutable module parameters causes a TOCTOU vulnerability leading to out-of-bounds array writes.
- [Medium] Setting `vsie_shadow_sca_max` to 0 fails to disable SCA shadowing, breaking the documented administrative feature.
--

--- Patch [18]: [PATCH 18/21] KVM: s390: vsie: Add VSIE max shadow configuration ---
commit 20f6b4acf8a001837c97129c0036793d506ee32c
Author: Christoph Schlameuss <schlameuss@linux.ibm.com>

KVM: s390: vsie: Add VSIE max shadow configuration

This commit introduces vsie_shadow_scb_max and vsie_shadow_sca_max
module parameters to control the maximum number of shadow structures
kept by KVM.

> diff --git a/arch/s390/kvm/vsie.c b/arch/s390/kvm/vsie.c
> index de8931e714ca2..c392f7afe130c 100644
> --- a/arch/s390/kvm/vsie.c
> +++ b/arch/s390/kvm/vsie.c

[ ... ]

> @@ -1038,7 +1048,8 @@ static struct vsie_sca *get_vsie_sca(struct kvm_vcpu *vcpu, struct kvm_s390_sie_
>  	 * We want at least #online_vcpus shadows, so every VCPU can execute the
>  	 * VSIE in parallel. (Worst case all single core VMs.)
>  	 */
> -	max_vsie_sca = MIN(atomic_read(&kvm->online_vcpus), KVM_S390_MAX_VSIE_VCPUS);
> +	max_vsie_sca = MIN(MAX(atomic_read(&kvm->online_vcpus), vsie_shadow_sca_max),
> +			   KVM_S390_MAX_VSIE_VCPUS);

[Severity: Medium]
Does setting vsie_shadow_sca_max to 0 actually disable SCA shadowing
as documented in the commit message?

Since online_vcpus is at least 1 for an active VM, MAX(online_vcpus, 0)
will always evaluate to online_vcpus. It appears max_vsie_sca silently
defaults to the number of active VCPUs instead of 0, which would prevent
the feature from being disabled.

[Severity: High]
Could the use of MIN() and MAX() macros on a mutable module parameter lead
to a time-of-check to time-of-use out-of-bounds write?

The MIN() and MAX() macros evaluate their arguments multiple times. If
vsie_shadow_sca_max is concurrently modified via sysfs from a value < 256
to a value > 256 during evaluation, could this macro return the new
out-of-bounds value?

If max_vsie_sca exceeds 256, it could allow kvm->arch.vsie.sca_count to
exceed array capacities, leading to an out-of-bounds write here:

kvm->arch.vsie.scas[kvm->arch.vsie.sca_count] = vsie_sca;

[ ... ]

> @@ -1902,7 +1913,8 @@ static struct vsie_page *get_vsie_page(struct kvm_vcpu *vcpu, unsigned long addr
>  		put_vsie_page(vsie_page);
>  	}
>  
> -	max_vsie_page = MIN(atomic_read(&kvm->online_vcpus), KVM_S390_MAX_VSIE_VCPUS);
> +	max_vsie_page = MIN(MAX(atomic_read(&kvm->online_vcpus), vsie_shadow_scb_max),
> +			    KVM_S390_MAX_VSIE_VCPUS);

[Severity: High]
Does this have the same double-evaluation vulnerability as max_vsie_sca?

If vsie_shadow_scb_max is modified during macro evaluation, max_vsie_page
could bypass the KVM_S390_MAX_VSIE_VCPUS limit, leading to out-of-bounds
writes on the kvm->arch.vsie.pages array.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260709-vsie-sigpi-v1-0-ea9f12066408@linux.ibm.com?part=18