Re: [PATCH v4 6/7] KVM: s390: cmma: Fix cmma dirty tracking

[sashiko-bot@kernel.org]

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

New issues:
- [High] The `cmma_dirty_pages` counter permanently leaks and becomes artificially inflated across VM CMMA resets or memslot removals, leading to an infinite polling loop in userspace during the final migration phase.

Pre-existing issues:
- [High] `kvm_s390_get_cmma_bits` leaks uninitialized kernel memory to userspace because the `values` buffer is allocated with `vmalloc` and gaps corresponding to non-dirty pages are never initialized.
--

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

KVM: s390: cmma: Fix cmma dirty tracking
    
It is possible that some guest memory areas have not been touched yet
when starting migration mode, and thus have no ptes allocated. Only
existing and allocated ptes should count toward the total of dirty cmma
entries.

> diff --git a/arch/s390/kvm/dat.c b/arch/s390/kvm/dat.c
> index cffac7782c4bf..0ad4ebc80ebaf 100644
> --- a/arch/s390/kvm/dat.c
> +++ b/arch/s390/kvm/dat.c
> @@ -1253,6 +1253,9 @@ int dat_get_cmma(union asce asce, gfn_t *start, unsigned int *count, u8 *values,
>  	};
>  
>  	_dat_walk_gfn_range(*start, asce_end(asce), asce, &ops, DAT_WALK_IGN_HOLES, &state);

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

In kvm_s390_get_cmma_bits(), the values array is allocated using vmalloc()
which does not clear the memory:

kvm_s390_get_cmma_bits() {
    ...
    values = vmalloc(args->count);
    if (!values)
        return -ENOMEM;
    ...
}

When dat_get_cmma() populates this array via __dat_get_cmma_pte(), it iterates
over the guest's page tables and only writes to indices corresponding to pages
where cmma_d == 1.

For pages where cmma_d == 0, the callback simply returns and skips them,
leaving those intermediate array indices untouched:

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

The total count returned is calculated as state.end - state.start,
encompassing the entire span from the first to the last dirty page found,
including the skipped uninitialized gaps.

This array is then copied to userspace via copy_to_user(), leaking
uninitialized kernel memory.

> +	/* If no dirty pages were found, wrap around and continue searching */
> +	if (*start && state.start == -1)
> +		_dat_walk_gfn_range(0, *start, asce, &ops, DAT_WALK_IGN_HOLES, &state);
>  
>  	if (state.start == -1) {
>  		*count = 0;

[ ... ]

> diff --git a/arch/s390/kvm/kvm-s390.c b/arch/s390/kvm/kvm-s390.c
> index ffb20a64d328f..907d2ca9951da 100644
> --- a/arch/s390/kvm/kvm-s390.c
> +++ b/arch/s390/kvm/kvm-s390.c
> @@ -1191,7 +1191,6 @@ static int kvm_s390_vm_start_migration(struct kvm *kvm)
>  {
>  	struct kvm_memory_slot *ms;
>  	struct kvm_memslots *slots;
> -	unsigned long ram_pages = 0;
>  	int bkt;
>  
>  	/* migration mode already enabled */
> @@ -1208,12 +1207,10 @@ static int kvm_s390_vm_start_migration(struct kvm *kvm)
>  	kvm_for_each_memslot(ms, bkt, slots) {
>  		if (!ms->dirty_bitmap)
>  			return -EINVAL;
> -		ram_pages += ms->npages;
>  	}
> +	kvm->arch.migration_mode = 1;
>  	/* mark all the pages as dirty */
>  	gmap_set_cmma_all_dirty(kvm->arch.gmap);
> -	atomic64_set(&kvm->arch.cmma_dirty_pages, ram_pages);

[Severity: High]
Does removing this reset of cmma_dirty_pages cause the counter to permanently
leak and become artificially inflated?

The cmma_dirty_pages counter tracks the number of pages with the cmma_d bit
set.

However, when CMMA states are cleared (e.g., via the KVM_S390_VM_MEM_CLR_CMMA
ioctl) or when memory is unmapped, the cmma_d bits are cleared but the
cmma_dirty_pages counter is never decremented:

arch/s390/kvm/dat.c:dat_reset_cmma_pte() {
    ...
    pgste.cmma_d = 0;
    ...
}

When migration is started again, __set_cmma_dirty_pte() unconditionally
increments cmma_dirty_pages for every page that has its cmma_d bit flipped
from 0 to 1, compounding the leaked counter:

arch/s390/kvm/gmap.c:__set_cmma_dirty_pte() {
    ...
    if (!pgste.cmma_d) {
        pgste.cmma_d = 1;
        atomic64_inc(walk->priv);
    }
    ...
}

During migration, KVM reports this artificially inflated counter to userspace
via args->remaining.

Once QEMU successfully retrieves all actual dirty pages, KVM will return
count == 0 but remaining > 0 (due to the leak). Could this trap QEMU in an
infinite migration polling loop waiting for pages that do not exist?

> -	kvm->arch.migration_mode = 1;
>  	kvm_s390_sync_request_broadcast(kvm, KVM_REQ_START_MIGRATION);
>  	return 0;
>  }

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