Re: [PATCH v4 17/20] KVM: s390: vsie: Add VSIE max shadow configuration

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Critical] A `vsie_page` actively in use by a non-SCA VSIE execution can be corrupted (zeroed out) if its previously associated `vsie_sca` is concurrently selected for reuse, leading to host memory corruption.
--

--- Patch [17]: [PATCH v4 17/20] KVM: s390: vsie: Add VSIE max shadow configuration ---
commit a1cb743a25bd4d366ca8bd61632e7dad29b9214d
Author: Christoph Schlameuss <[email protected]>

KVM: s390: vsie: Add VSIE max shadow configuration

This commit introduces two new module parameters, vsie_shadow_scb_max and
vsie_shadow_sca_max. These parameters allow overriding the maximum number of
shadow control blocks and system control areas kept by KVM to minimize
repeated shadowing efforts.

> diff --git a/arch/s390/kvm/vsie.c b/arch/s390/kvm/vsie.c
> index 6088a9c31564f..5f6dd4b9e9fd7 100644
> --- a/arch/s390/kvm/vsie.c
> +++ b/arch/s390/kvm/vsie.c
[ ... ]
> @@ -1063,7 +1073,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_t(unsigned int, max_t(unsigned int, atomic_read(&kvm->online_vcpus),
> +				vsie_shadow_sca_max), KVM_S390_MAX_VSIE_VCPUS);
>  
>  	if (kvm->arch.vsie.sca_count < max_vsie_sca) {
>  		vsie_sca_new = alloc_vsie_sca();

[Severity: Critical]
Can a vsie_page actively in use by a non-SCA VSIE execution be corrupted
(zeroed out) if its previously associated vsie_sca is concurrently selected
for reuse?

When a guest VCPU executes a VSIE instruction with an SCA, KVM saves a pointer
to the allocated vsie_page in vsie_sca->pages[cpu_nr] during
get_vsie_page_cpu_nr().

If another VCPU later executes a non-SCA VSIE using the same SCB address,
it fetches this same vsie_page, sets vsie_page->vsie_sca = NULL in
kvm_s390_handle_vsie(), and enters guest execution. The page pointer remains in
vsie_sca->pages.

Concurrently, if the guest exhausts the max_vsie_sca limit (which can now be
influenced by vsie_shadow_sca_max), KVM will recycle the original vsie_sca.
In get_vsie_sca(), this reuse path iterates over vsie_sca->pages:

arch/s390/kvm/vsie.c:get_vsie_sca() {
    ...
    vsie_sca = get_reuseable_vsie_sca(kvm);
    ...
    for (cpu_nr = 0; cpu_nr < KVM_S390_MAX_VSIE_VCPUS; cpu_nr++) {
        vsie_page_n = vsie_sca->pages[cpu_nr];
        if (!vsie_page_n)
            continue;

        unpin_scb(kvm, vsie_page_n);
        release_gmap_shadow_safe(kvm, vsie_page_n);
        memset(vsie_page_n, 0, sizeof(struct vsie_page));
        ...
    }
    ...
}

Does this unconditional memset() corrupt the vsie_page while it is actively
being used by the non-SCA VSIE execution, without checking if it is currently
in use (VSIE_PAGE_IN_USE)?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=17