Re: [PATCH v6 02/10] KVM: s390: Fix dat_peek_cmma() overflow

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] The KVM_S390_GET_CMMA_BITS ioctl allocates the `values` array using `vmalloc()` without zeroing the memory. During the page table walk, `dat_peek_cmma()` and `dat_get_cmma()` can skip unmapped segments or clean pages, leaving uninitialized gaps in the array. When copied back to userspace, this leaks up to 1MB of uninitialized kernel heap memory.
--

commit 561661723442584bc83ff1dfa1a8372280efbf28
Author: Claudio Imbrenda <imbrenda@linux.ibm.com>

KVM: s390: Fix dat_peek_cmma() overflow

If userspace passes a start address that is out of bounds,
_dat_walk_gfn_range() will fail with -EFAULT, but state.end will not be
touched and will stay 0. This will cause *count to underflow and report
a very high number, and the function will end up 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;

[Severity: High]
This is a pre-existing issue, but does this code leak uninitialized kernel
memory to userspace?

The KVM_S390_GET_CMMA_BITS ioctl allocates the values array using vmalloc()
without zeroing it:

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

During the page table walk here, unmapped segments or clean pages can be
skipped without writing to the corresponding index in the values array,
such as in __dat_get_cmma_pte():

arch/s390/kvm/dat.c:__dat_get_cmma_pte() {
    ...
    state->values[gfn - state->start] = pgste.usage | pgste.nodat << 6;
    ...
}

Because state.end can advance without filling all gaps in the array, the count
calculated here will include uninitialized elements. Since the ioctl copies
this entire buffer back to userspace based on the returned count, could this
leak previous kernel heap content?

Would it be safer to use vzalloc() in kvm_s390_get_cmma_bits() instead?

>  	/* Return success if at least one value was saved, otherwise an error. */
>  	return (rc == -EFAULT && *count > 0) ? 0 : rc;
>  }

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