Re: [PATCH v6 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:

Pre-existing issues:
- [Critical] Missing return value assignment in get_vsie_sca() leads to dereferencing ERR_PTR and a host kernel panic.
- [High] Reference leak of vsie_sca upon xa_insert failure allows a guest to permanently exhaust the SCA pool.
--

commit b0058b2ab33487566dfa6d36cbbfbcc7b06a1519
Author: Christoph Schlameuss <schlameuss@linux.ibm.com>

KVM: s390: vsie: Add VSIE max shadow configuration

This commit introduces two new module parameters, vsie_shadow_scb_max
and vsie_shadow_sca_max, to configure the maximum number of VSIE shadow
control blocks and system control areas kept by guest-1. It modifies
get_vsie_sca() and get_vsie_page() to respect these new limits.

> diff --git a/arch/s390/kvm/s390/vsie.c b/arch/s390/kvm/s390/vsie.c
> index bfd5df1f8b9ca..3a839154fdfed 100644
> --- a/arch/s390/kvm/s390/vsie.c
> +++ b/arch/s390/kvm/s390/vsie.c
[ ... ]
> @@ -1086,7 +1096,8 @@ static int get_vsie_sca(struct kvm_vcpu *vcpu, struct kvm_s390_sie_block *scb_o,
>  	 * 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();
[ ... ]
> 	} else {
> 		/* reuse previously created vsie_sca allocation for different osca */
> 		vsie_sca = get_vsie_sca_unused(kvm);
> 		/* with nr_vcpus scas one must be reusable */
> 		if (IS_ERR(vsie_sca))
> 			goto out;

[Severity: Critical]
This is a pre-existing issue, but missing a return value assignment here
can lead to a host kernel panic.

If get_vsie_sca_unused() exhausts the pool and returns ERR_PTR(-EAGAIN),
the IS_ERR check triggers a goto to the out label without setting rc.
As a result, rc remains 0 (success) while the error pointer is assigned
to *vsie_sca_out:

> out:
> 	up_write(&kvm->arch.vsie.vsie_sca_lock);
> 	if (vsie_sca_new)
> 		free_vsie_sca(kvm, vsie_sca_new);
> 	if (vsie_sca)
> 		*vsie_sca_out = vsie_sca;
> 	return rc;

Because the caller kvm_s390_handle_vsie() receives a success code, it
proceeds to call get_vsie_page_cpu_nr(), which dereferences the error
pointer.

Can a malicious guest deterministically trigger this crash by forcing memory
allocation failures on the xa_insert path to exhaust the pool?

[ ... ]
> 	rc = xa_insert(&kvm->arch.vsie.osca_to_sca, vsie_sca->sca_gpa >> SCA_ALIGNMENT_SHIFT,
> 		       vsie_sca, GFP_KERNEL_ACCOUNT);
> 	if (rc == -EBUSY)
> 		rc = 1;
> 
> out:

[Severity: High]
This isn't a bug introduced by this patch, but does this error path
permanently leak a vsie_sca reference?

If xa_insert() fails (e.g., returning -ENOMEM), it sets rc and jumps to out.
However, it skips calling put_vsie_sca(vsie_sca) to clean up the reference
count initialized to 1.

Because the caller receives an error code and also skips cleanup, the
ref_count remains stuck at 1 and the vsie_sca can never be reclaimed by
get_vsie_sca_unused().

Could a guest apply memory pressure to repeatedly trigger this, exhaust the
SCA pool, and pave the way for the ERR_PTR dereference bug above?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260827-vsie-sigpi-v6-0-8020bb53be52@linux.ibm.com?part=18