Re: [PATCH v6 17/21] KVM: s390: vsie: Allow guest-3 cpu add and remove with ssca

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] TOCTOU race dynamically flipping the `VSIE_SCA_ESCA` flag allows a malicious guest to trigger a host WARN_ON_ONCE DoS.
- [High] `_shadow_sca` leaves stale shadow SCA entries for removed CPUs because it fails to fully clear the `ssca.cpu` array when the Maximum CPU Number (`mcn`) changes.
--

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

KVM: s390: vsie: Allow guest-3 cpu add and remove with ssca

This commit allows dynamically adding and removing processors for a running
guest-3 configuration. It accomplishes this by copying the Maximum CPU Number
(mcn) and fully re-initializing the shadowed System Control Area (ssca)
whenever a change in the mcn is detected.

> diff --git a/arch/s390/kvm/s390/vsie.c b/arch/s390/kvm/s390/vsie.c
> index 16273cf5cbff5..bfd5df1f8b9ca 100644
> --- a/arch/s390/kvm/s390/vsie.c
> +++ b/arch/s390/kvm/s390/vsie.c
[ ... ]
> @@ -2142,17 +2179,17 @@ static int _shadow_sca(struct kvm_vcpu *vcpu, struct vsie_page *vsie_page,
>  	bool is_esca = sie_uses_esca(vsie_page->scb_o);
>  	unsigned int cpu_nr, cpu_slots;
>  	hpa_t sca_o_entry_hpa;
> -	unsigned long *mcn;
>  	int rc;
>  
>  	if (is_esca)
> -		mcn = phys_to_virt(sca_o_hpa(vsie_sca)) + offsetof(struct esca_block, mcn);
> +		set_bit(VSIE_SCA_ESCA, &vsie_sca->flags);
>  	else
> -		mcn = phys_to_virt(sca_o_hpa(vsie_sca)) + offsetof(struct bsca_block, mcn);

[Severity: High]
If a malicious guest concurrently flips the ECB2_ESCA bit in its SCB after
vsie_sca is pinned, _shadow_sca() will read this bit and dynamically toggle
the VSIE_SCA_ESCA flag on the shared vsie_sca structure.

Could dynamically flipping this flag based on untrusted guest memory introduce
a time-of-check to time-of-use (TOCTOU) race?

This can cause subsequent ESCA-based offset calculations to exceed the
initially pinned BSCA pages, leading to a guest-triggerable out-of-bounds
WARN_ON assertion when the corrupted flag is later used:

get_sca_entry_addr() {
    ...
	pn = ((vsie_sca->sca_gpa & ~PAGE_MASK) + cpu_offset) >> PAGE_SHIFT;
	offset = (vsie_sca->sca_gpa + cpu_offset) & ~PAGE_MASK;
	if (WARN_ON_ONCE(pn >= vsie_sca->sca_o_nr_pages))
		return -EINVAL;
    ...
}

> +		clear_bit(VSIE_SCA_ESCA, &vsie_sca->flags);
> +	sca_mcn_copy(vsie_sca, phys_to_virt(sca_o_hpa(vsie_sca)));
>  
>  	/* pin and make shadow for ALL scb in the sca */
>  	cpu_slots = is_esca ? KVM_S390_MAX_VSIE_VCPUS : KVM_S390_BSCA_CPU_SLOTS;
> -	for_each_set_bit_inv(cpu_nr, mcn, cpu_slots) {
> +	for_each_set_bit_inv(cpu_nr, (unsigned long *)&vsie_sca->mcn, cpu_slots) {

[Severity: High]
If the guest removes a CPU, its bit in the Maximum CPU Number (mcn) mask
transitions from 1 to 0.

By using for_each_set_bit_inv(), we skip the removed CPU and fail to clear
its corresponding ssda and ossea entries in the ssca.cpu array.

Does iterating only over the currently set bits in mcn leave stale shadow
SCA entries for removed CPUs?

Could this leave hardware-visible pointers intact, potentially causing the
physical hardware to erroneously route intercepted hardware-assisted SIGP
operations to a stale shadow control block?

>  		rc = get_sca_entry_addr(vcpu->kvm, vsie_sca, cpu_nr, NULL, &sca_o_entry_hpa);
>  		if (rc)
>  			break;

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