Re: [GIT PULL v1 10/17] KVM: s390: Initialize KVM_S390_GET_CMMA_BITS memory

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [Critical] In `dat_peek_cmma()`, if the provided `start` guest frame number (GFN) is out of bounds, `_dat_walk_gfn_range()` immediately returns `-EFAULT` without executing any callbacks. Consequently, `state.end` remains `0`. The function then computes `*count = state.end - start`, which underflows into a large unsigned integer. This underflow bypasses the error check and causes `kvm_s390_get_cmma_bits()` to perform a massive out-of-bounds read and copy kernel memory to userspace.
--

commit d8e920ce2bbd8785a01a3239ce6dc856e44c18b4
Author: Christian Borntraeger <borntraeger@linux.ibm.com>
KVM: s390: Initialize KVM_S390_GET_CMMA_BITS memory

This commit changes the output buffer allocation in kvm_s390_get_cmma_bits()
from vmalloc to vzalloc. This prevents unmapped or clean pages from exposing
uninitialized kernel memory to userspace during migration.

Message-ID: <20260611105036.11491-1-borntraeger@linux.ibm.com>

> diff --git a/arch/s390/kvm/kvm-s390.c b/arch/s390/kvm/kvm-s390.c
> index 35e5971a474f6..f643cf512f127 100644
> --- a/arch/s390/kvm/kvm-s390.c
> +++ b/arch/s390/kvm/kvm-s390.c
> @@ -2284,7 +2284,7 @@ static int kvm_s390_get_cmma_bits(struct kvm *kvm,
>  		return 0;
>  	}
>  
> -	values = vmalloc(args->count);
> +	values = vzalloc(args->count);

[Severity: Critical]
This is a pre-existing issue, but does the error handling in the peek path
allow an underflow of args->count that leads to an out-of-bounds read later?

If an out-of-bounds start_gfn is passed, dat_peek_cmma() calls
_dat_walk_gfn_range() which returns -EFAULT immediately, leaving state.end
at 0:

arch/s390/kvm/dat.c:dat_peek_cmma() {
    ...
	rc = _dat_walk_gfn_range(start, start + *count, asce, &ops, DAT_WALK_DEFAULT, &state);
	*count = state.end - start;
	/* Return success if at least one value was saved, otherwise an error. */
	return (rc == -EFAULT && *count > 0) ? 0 : rc;
}

If start is greater than 0 and state.end is 0, *count will underflow to a
large unsigned integer. Because this underflowed *count is greater than 0,
dat_peek_cmma() masks the -EFAULT and incorrectly returns 0 for success.

When execution continues in kvm_s390_get_cmma_bits(), this underflowed
args->count is passed directly to copy_to_user():

	if (copy_to_user((void __user *)args->values, values, args->count))
		ret = -EFAULT;

Can this allow a user to craft a start_gfn that triggers this underflow and
copies memory beyond the vzalloc buffer to userspace?

>  	if (!values)
>  		return -ENOMEM;
>

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260615124243.187614-1-imbrenda@linux.ibm.com?part=10