Re: [PATCH v4 15/20] KVM: s390: vsie: Guard against invalid CPU address

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Critical] The newly added bounds check for `scb_o->icpua` is completely ineffective due to a signed-integer mismatch and a Time-of-Check to Time-of-Use (TOCTOU) race condition. A malicious guest can bypass this check, leading to a massive out-of-bounds array access and severe hypervisor memory corruption.
--

--- Patch [15]: [PATCH v4 15/20] KVM: s390: vsie: Guard against invalid CPU address ---
commit b018701df60566888acc3dc5acc8b060071a7256
Author: Christoph Schlameuss <[email protected]>

KVM: s390: vsie: Guard against invalid CPU address
    
Inject a specification interrupt if the CPU address is above 255.
This adds a check to ensure compliance with architectural limits and
guard against out-of-bounds accesses.

> diff --git a/arch/s390/kvm/vsie.c b/arch/s390/kvm/vsie.c
> --- a/arch/s390/kvm/vsie.c
> +++ b/arch/s390/kvm/vsie.c
> @@ -2169,6 +2169,11 @@ int kvm_s390_handle_vsie(struct kvm_vcpu *vcpu)
>  		return kvm_s390_inject_program_int(vcpu, PGM_ADDRESSING);
>  	scb_o = (struct kvm_s390_sie_block *)phys_to_virt(scb_hpa);
>  
> +	if (scb_o->icpua >= KVM_S390_MAX_VSIE_VCPUS) {

[Severity: Critical]
Is it possible for icpua to be a negative value? If icpua is a signed 16-bit
integer, a negative value would bypass this check.

Additionally, since scb_o resides in guest memory, is this check subject to a
Time-of-Check to Time-of-Use (TOCTOU) race? A guest could potentially mutate
icpua after this check passes but before it is fetched again.

For example, could the guest change the value in memory before the secondary
fetch occurs later in kvm_s390_handle_vsie():

vsie_page = get_vsie_page_cpu_nr(vcpu, vsie_sca, scb_addr, scb_o->icpua);

If a bypassed or mutated value is implicitly cast to a large unsigned index
in get_vsie_page_cpu_nr(), could this result in an out-of-bounds read and
write on vsie_sca->pages[cpu_nr]?

> +		rc = kvm_s390_inject_program_int(vcpu, PGM_SPECIFICATION);
> +		goto out_unpin;
> +	}
> +
>  	if (!use_ssca(vcpu->kvm, scb_o)) {
>  		/* get the vsie_page with pinned scb_o */
>  		vsie_page = get_vsie_page(vcpu, scb_addr);

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