Re: [PATCH v4 2/7] KVM: s390: Fix dat_peek_cmma() overflow

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [Critical] Uninitialized kernel memory leak in kvm_s390_get_cmma_bits() via vmalloc array.
- [High] kvm_s390_set_cmma_bits() silently ignores memory allocation failures.
--

commit 198324f8297a62f137ba34c4a9aa74e25bb30e54
Author: Claudio Imbrenda <imbrenda@linux.ibm.com>

KVM: s390: Fix dat_peek_cmma() overflow

This commit fixes an underflow in dat_peek_cmma() when userspace passes an
out-of-bounds start address. By enforcing that the count is set to zero if
the end address is smaller than the start address, it correctly propagates
the -EFAULT error instead of erroneously reporting success.

> diff --git a/arch/s390/kvm/dat.c b/arch/s390/kvm/dat.c
> index 4a41c0247ffa2..cffac7782c4bf 100644
> --- a/arch/s390/kvm/dat.c
> +++ b/arch/s390/kvm/dat.c
> @@ -1209,7 +1209,7 @@ int dat_peek_cmma(gfn_t start, union asce asce, unsigned int *count, u8 *values)
>  	int rc;
>  
>  	rc = _dat_walk_gfn_range(start, start + *count, asce, &ops, DAT_WALK_DEFAULT, &state);
> -	*count = state.end - start;
> +	*count = state.end >= start ? state.end - start : 0;
>  	/* Return success if at least one value was saved, otherwise an error. */

[Severity: Critical]
This isn't a bug introduced by this patch, but does the CMMA retrieval path
leak uninitialized kernel memory to userspace?

In kvm_s390_get_cmma_bits(), the values array is allocated with vmalloc():

arch/s390/kvm/kvm-s390.c:kvm_s390_get_cmma_bits() {
    ...
    values = vmalloc(args->count);
    if (!values)
    ...
}

During the page table walk, if __dat_peek_cmma_crste() encounters an invalid
CRSTE, it advances the boundary but leaves the corresponding state.values
unpopulated:

arch/s390/kvm/dat.c:__dat_peek_cmma_crste() {
    ...
    if (crstep->h.i)
        state->end = min(walk->end, next);
    ...
}

Similarly, __dat_get_cmma_pte() skips clean pages:

arch/s390/kvm/dat.c:__dat_get_cmma_pte() {
    ...
    if (!READ_ONCE(*pgste_of(ptep)).cmma_d)
        return 0;
    ...
}

Because vmalloc() does not zero-initialize memory, could these gaps in the
array leak kernel heap memory when the array is unconditionally copied back
to userspace using the advanced state.end count?

[Severity: High]
This is a pre-existing issue, but does kvm_s390_set_cmma_bits() silently
ignore memory allocation failures?

If vmalloc() fails to allocate the bits array, the code jumps to the out
label:

arch/s390/kvm/kvm-s390.c:kvm_s390_set_cmma_bits() {
    ...
    bits = vmalloc(array_size(sizeof(*bits), args->count));
    if (!bits)
        goto out;
    ...
}

Since the return variable is initialized to 0 and not updated to a negative
error code like -ENOMEM before this jump, will this return 0 to userspace
and silently drop the CMMA bits, potentially leading to guest memory
attribute corruption during migration?

>  	return (rc == -EFAULT && *count > 0) ? 0 : rc;
>  }

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