[PATCH v4 0/7] KVM: s390: A bunch of gmap-related fixes

[PATCH v4 0/7] KVM: s390: A bunch of gmap-related fixes

[Claudio Imbrenda]

Fix handling of _PAGE_UNUSED pte bit. The bit was lingering around
longer than it should have, causing issues. This is yet a different
approach from the previous version.

Fix some cmma dirty tracking functions.

Also fix a few other small issues in gmap.

v3->v4:
* Improve/fix descriptions of patches 1 and 3
* Fix handle_{sske,pfmf} under memory pressure
* Fix cmma dirty tracking and its selftest

v2->v3:
* Fix _PAGE_UNUSED in yet a different way
* Fix a potential overflow in dat_peek_cmma()
* Some minor gmap fixes that are not actual bugs yet

v1->v2:
* Completely different approach, entirely new patch

Claudio Imbrenda (7):
  s390/mm: Fix handling of _PAGE_UNUSED pte bit
  KVM: s390: Fix dat_peek_cmma() overflow
  KVM: s390: Do not set special large pages dirty
  KVM: s390: Fix code typo in gmap_protect_asce_top_level()
  KVM: s390: Fix handle_{sske,pfmf} under memory pressure
  KVM: s390: cmma: Fix cmma dirty tracking
  KVM: s390: selftests: Fix cmma selftest

 arch/s390/include/asm/pgtable.h              |  4 ++--
 arch/s390/kvm/dat.c                          |  5 ++++-
 arch/s390/kvm/gmap.c                         | 18 ++++++++++++++----
 arch/s390/kvm/kvm-s390.c                     |  5 +----
 arch/s390/kvm/priv.c                         |  8 ++++++--
 arch/s390/mm/gmap_helpers.c                  |  3 ++-
 tools/testing/selftests/kvm/s390/cmma_test.c |  3 +++
 7 files changed, 32 insertions(+), 14 deletions(-)

-- 
2.54.0

[PATCH v4 1/7] s390/mm: Fix handling of _PAGE_UNUSED pte bit

[Claudio Imbrenda]

The _PAGE_UNUSED softbit should not really be lying around. Its sole
purpose is to signal to try_to_unmap_one() and try_to_migrate_one()
that the page can be discarded instead of being moved / swapped.

KVM has no way to know why a page is being unmapped, so it sets the bit
on userspace ptes corresponding to unused guest pages every time they
get unmapped. KVM has no reasonable way to clear the bit once the page
is in use again.

While set_ptes() checks and clears the bit, other paths that set new
ptes did not. This led to used pages being thrown out as if they were
unused, causing guest corruption.

Fix the issue by clearing the _PAGE_UNUSED bit for present ptes in
set_pte(), i.e. whenever a present pte is getting set. The check in
set_ptes() is then redundant and can be removed.

Also fix gmap_helper_try_set_pte_unused() to only set the bit if the
pte is present; the _PAGE_UNUSED bit is only defined for present ptes
and thus should not be set for non-present ptes.

Signed-off-by: Claudio Imbrenda <imbrenda@linux.ibm.com>
Fixes: c98175b7917f ("KVM: s390: Add gmap_helper_set_unused()")
Acked-by: Heiko Carstens <hca@linux.ibm.com>
---
 arch/s390/include/asm/pgtable.h | 4 ++--
 arch/s390/mm/gmap_helpers.c     | 3 ++-
 2 files changed, 4 insertions(+), 3 deletions(-)

diff --git a/arch/s390/include/asm/pgtable.h b/arch/s390/include/asm/pgtable.h
index ca376a9b8e41..d03663483f76 100644
--- a/arch/s390/include/asm/pgtable.h
+++ b/arch/s390/include/asm/pgtable.h
@@ -980,6 +980,8 @@ static inline void set_pmd(pmd_t *pmdp, pmd_t pmd)
 
 static inline void set_pte(pte_t *ptep, pte_t pte)
 {
+	if (pte_present(pte))
+		pte = clear_pte_bit(pte, __pgprot(_PAGE_UNUSED));
 	WRITE_ONCE(*ptep, pte);
 }
 
@@ -1332,8 +1334,6 @@ pgprot_t pgprot_writecombine(pgprot_t prot);
 static inline void set_ptes(struct mm_struct *mm, unsigned long addr,
 			      pte_t *ptep, pte_t entry, unsigned int nr)
 {
-	if (pte_present(entry))
-		entry = clear_pte_bit(entry, __pgprot(_PAGE_UNUSED));
 	page_table_check_ptes_set(mm, addr, ptep, entry, nr);
 	for (;;) {
 		set_pte(ptep, entry);
diff --git a/arch/s390/mm/gmap_helpers.c b/arch/s390/mm/gmap_helpers.c
index 1cfe4724fbe2..60023b6fdcb1 100644
--- a/arch/s390/mm/gmap_helpers.c
+++ b/arch/s390/mm/gmap_helpers.c
@@ -181,7 +181,8 @@ void gmap_helper_try_set_pte_unused(struct mm_struct *mm, unsigned long vmaddr)
 	if (IS_ERR_OR_NULL(ptep))
 		return;
 
-	__atomic64_or(_PAGE_UNUSED, (long *)ptep);
+	if (pte_present(*ptep))
+		__atomic64_or(_PAGE_UNUSED, (long *)ptep);
 	pte_unmap_unlock(ptep, ptl);
 }
 EXPORT_SYMBOL_GPL(gmap_helper_try_set_pte_unused);
-- 
2.54.0

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

[Claudio Imbrenda]

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.

Fix by only setting *count if the end address is not smaller than the
starting address. This way invalid starting addresses will correctly
return -EFAULT and *count will correctly indicate that no values have
been returned.

Fixes: 7b368470e1a4 ("KVM: s390: KVM page table management functions: CMMA")
Signed-off-by: Claudio Imbrenda <imbrenda@linux.ibm.com>
---
 arch/s390/kvm/dat.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/s390/kvm/dat.c b/arch/s390/kvm/dat.c
index 4a41c0247ffa..cffac7782c4b 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. */
 	return (rc == -EFAULT && *count > 0) ? 0 : rc;
 }
-- 
2.54.0

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

[sashiko-bot]

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

[PATCH v4 3/7] KVM: s390: Do not set special large pages dirty

[Claudio Imbrenda]

Special pages / folios should not be set dirty. This also applies to
large pages.

Add a missing check in gmap_clear_young_crste() to prevent setting the
large page dirty if it is a special page.

Signed-off-by: Claudio Imbrenda <imbrenda@linux.ibm.com>
Fixes: a2c17f9270cc ("KVM: s390: New gmap code")
---
 arch/s390/kvm/gmap.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/s390/kvm/gmap.c b/arch/s390/kvm/gmap.c
index 52d55ddea8d4..3192f610f696 100644
--- a/arch/s390/kvm/gmap.c
+++ b/arch/s390/kvm/gmap.c
@@ -327,7 +327,7 @@ static long gmap_clear_young_crste(union crste *crstep, gfn_t gfn, gfn_t end, st
 		new.h.i = 1;
 		new.s.fc1.y = 0;
 		new.s.fc1.prefix_notif = 0;
-		if (new.s.fc1.d || !new.h.p)
+		if ((new.s.fc1.d || !new.h.p) && !new.s.fc1.s)
 			folio_set_dirty(phys_to_folio(crste_origin_large(crste)));
 		new.s.fc1.d = 0;
 		new.h.p = 1;
-- 
2.54.0

Re: [PATCH v4 3/7] KVM: s390: Do not set special large pages dirty

[sashiko-bot]

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

Pre-existing issues:
- [Critical] Missing Special Page Validations in PV / Unmap Paths
- [Critical] Unsafe SSKE Execution on Special Pages
--

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

KVM: s390: Do not set special large pages dirty

This commit prevents setting special large pages dirty. It adds a missing
check in gmap_clear_young_crste() to ensure the large page is not a
special page before setting it dirty.

> diff --git a/arch/s390/kvm/gmap.c b/arch/s390/kvm/gmap.c
> --- a/arch/s390/kvm/gmap.c
> +++ b/arch/s390/kvm/gmap.c
> @@ -327,7 +327,7 @@ static long gmap_clear_young_crste(union crste *crstep, gfn_t gfn, gfn_t end, st
>  		new.h.i = 1;
>  		new.s.fc1.y = 0;
>  		new.s.fc1.prefix_notif = 0;
> -		if (new.s.fc1.d || !new.h.p)
> +		if ((new.s.fc1.d || !new.h.p) && !new.s.fc1.s)
>  			folio_set_dirty(phys_to_folio(crste_origin_large(crste)));

[Severity: Critical]
This is a pre-existing issue, but I noticed there are other places where
special page validation seems to be missing in PV and unmap paths.

If a malicious userspace maps device memory (VM_PFNMAP) to a PV guest,
special shadow page table entries are created. When KVM unmaps these pages,
_gmap_unmap_crste() calls phys_to_folio() on the physical frame number:

arch/s390/kvm/gmap.c:_gmap_unmap_crste() {
    ...
    if (old.s.fc1.pr && test_bit(GMAP_FLAG_EXPORT_ON_UNMAP, &priv->gmap->flags))
        folio = phys_to_folio(crste_origin_large(old));
    ...
}

Could this result in a garbage pointer if the entry is a special page,
leading to a host panic when passed to uv_convert_from_secure_folio()
which immediately calls folio_get()?

A similar pattern appears in _gmap_unmap_pte() without checking ptep->s.s:

arch/s390/kvm/gmap.c:_gmap_unmap_pte() {
    ...
    if (ptep->s.pr && test_bit(GMAP_FLAG_EXPORT_ON_UNMAP, &priv->gmap->flags))
        folio = pfn_folio(ptep->h.pfra);
    ...
}

And in _destroy_pages_crste():

arch/s390/kvm/gmap.c:_destroy_pages_crste() {
    ...
    for ( ; cur < end; cur += PAGE_SIZE)
        __kvm_s390_pv_destroy_page(phys_to_page(cur));
    ...
}

As well as _destroy_pages_pte() and _kvm_s390_pv_make_secure() in
arch/s390/kvm/pv.c. Do these paths also need special bit checks to
prevent panics from malicious userspace or guests?


[Severity: Critical]
This is also a pre-existing issue, but does KVM_S390_RESET_SKEYS safely
handle special pages?

When userspace triggers the KVM_S390_RESET_SKEYS ioctl, dat_reset_skeys_pte()
and dat_reset_skeys_crste() walk the gmap and invoke page_set_storage_key()
without checking if the page is special. For example:

arch/s390/kvm/dat.c:dat_reset_skeys_pte() {
    ...
    if (ptep->s.pr)
        page_set_storage_key(pte_origin(*ptep), PAGE_DEFAULT_KEY, 1);
    ...
}

arch/s390/kvm/dat.c:dat_reset_skeys_crste() {
    ...
    for ( ; addr < end; addr += PAGE_SIZE)
        page_set_storage_key(addr, PAGE_DEFAULT_KEY, 1);
    ...
}

If page_set_storage_key() executes the hardware sske instruction on a physical
address not backed by valid system RAM, could this cause an s390 Addressing
Exception and panic the host?

>  		new.s.fc1.d = 0;
>  		new.h.p = 1;

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

[PATCH v4 4/7] KVM: s390: Fix code typo in gmap_protect_asce_top_level()

[Claudio Imbrenda]

The correct length to pass to kvm_s390_get_guest_pages() is asce.tl + 1,
not asce.dt + 1. It was a typo, which, due to fortuitous circumstances,
did not cause bugs. It should nonetheless be fixed.

Signed-off-by: Claudio Imbrenda <imbrenda@linux.ibm.com>
Fixes: e5f98a6899bd ("KVM: s390: Add some helper functions needed for vSIE")
---
 arch/s390/kvm/gmap.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/s390/kvm/gmap.c b/arch/s390/kvm/gmap.c
index 3192f610f696..e6e786811db8 100644
--- a/arch/s390/kvm/gmap.c
+++ b/arch/s390/kvm/gmap.c
@@ -1262,7 +1262,7 @@ static int gmap_protect_asce_top_level(struct kvm_s390_mmu_cache *mc, struct gma
 	/* Pairs with the smp_wmb() in kvm_mmu_invalidate_end(). */
 	smp_rmb();
 
-	rc = kvm_s390_get_guest_pages(sg->kvm, context.f, asce.rsto, asce.dt + 1, false);
+	rc = kvm_s390_get_guest_pages(sg->kvm, context.f, asce.rsto, asce.tl + 1, false);
 	if (rc > 0)
 		rc = -EFAULT;
 	if (!rc)
-- 
2.54.0

[PATCH v4 5/7] KVM: s390: Fix handle_{sske,pfmf} under memory pressure

[Claudio Imbrenda]

Under heavy memory pressure, handle_sske() and handle_pfmf() might
cause an endless loop if the mmu cache runs empty, the atomic
allocations fail, and the top-up function also fails. While quite
unlikely, that scenario is not impossible.

Fix the issue by not ignoring the return value of
kvm_s390_mmu_cache_topup(), and appropriately returning an error code
in case of failure.

Fixes: e38c884df921 ("KVM: s390: Switch to new gmap")
Signed-off-by: Claudio Imbrenda <imbrenda@linux.ibm.com>
---
 arch/s390/kvm/priv.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/arch/s390/kvm/priv.c b/arch/s390/kvm/priv.c
index 447ec7ed423d..9bc6fd02ff77 100644
--- a/arch/s390/kvm/priv.c
+++ b/arch/s390/kvm/priv.c
@@ -366,7 +366,9 @@ static int handle_sske(struct kvm_vcpu *vcpu)
 		if (rc > 1)
 			return kvm_s390_inject_program_int(vcpu, PGM_ADDRESSING);
 		if (rc == -ENOMEM) {
-			kvm_s390_mmu_cache_topup(vcpu->arch.mc);
+			rc = kvm_s390_mmu_cache_topup(vcpu->arch.mc);
+			if (rc)
+				return rc;
 			continue;
 		}
 		if (rc < 0)
@@ -1122,7 +1124,9 @@ static int handle_pfmf(struct kvm_vcpu *vcpu)
 			if (rc > 1)
 				return kvm_s390_inject_program_int(vcpu, rc);
 			if (rc == -ENOMEM) {
-				kvm_s390_mmu_cache_topup(vcpu->arch.mc);
+				rc = kvm_s390_mmu_cache_topup(vcpu->arch.mc);
+				if (rc)
+					return rc;
 				continue;
 			}
 			if (rc < 0)
-- 
2.54.0

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

[Claudio Imbrenda]

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.

When starting migration mode, count how many pages actually have a pte
(and PGSTE), instead of blindly counting the number of pages in all
memslots. Avoid double-counting pages whose cmma information has been
updated concurrently.

Also fix dat_get_cmma() to properly wrap around if the first attempt
reached the end of guest memory without finding cmma-dirty pages.

Fixes: e38c884df921 ("KVM: s390: Switch to new gmap")
Signed-off-by: Claudio Imbrenda <imbrenda@linux.ibm.com>
---
 arch/s390/kvm/dat.c      |  3 +++
 arch/s390/kvm/gmap.c     | 14 ++++++++++++--
 arch/s390/kvm/kvm-s390.c |  5 +----
 3 files changed, 16 insertions(+), 6 deletions(-)

diff --git a/arch/s390/kvm/dat.c b/arch/s390/kvm/dat.c
index cffac7782c4b..0ad4ebc80eba 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);
+	/* 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/gmap.c b/arch/s390/kvm/gmap.c
index e6e786811db8..e3d620af8a85 100644
--- a/arch/s390/kvm/gmap.c
+++ b/arch/s390/kvm/gmap.c
@@ -1075,7 +1075,16 @@ int gmap_protect_rmap(struct kvm_s390_mmu_cache *mc, struct gmap *sg, gfn_t p_gf
 
 static long __set_cmma_dirty_pte(union pte *ptep, gfn_t gfn, gfn_t next, struct dat_walk *walk)
 {
-	__atomic64_or(PGSTE_CMMA_D_BIT, &pgste_of(ptep)->val);
+	union pgste pgste;
+
+	pgste = pgste_get_lock(ptep);
+	/* Avoid double-counting when concurrent updates happen */
+	if (!pgste.cmma_d) {
+		pgste.cmma_d = 1;
+		atomic64_inc(walk->priv);
+	}
+	pgste_set_unlock(ptep, pgste);
+
 	if (need_resched())
 		return next;
 	return 0;
@@ -1089,7 +1098,8 @@ void gmap_set_cmma_all_dirty(struct gmap *gmap)
 	do {
 		scoped_guard(read_lock, &gmap->kvm->mmu_lock)
 			gfn = _dat_walk_gfn_range(gfn, asce_end(gmap->asce), gmap->asce, &ops,
-						  DAT_WALK_IGN_HOLES, NULL);
+						  DAT_WALK_IGN_HOLES,
+						  &gmap->kvm->arch.cmma_dirty_pages);
 		cond_resched();
 	} while (gfn);
 }
diff --git a/arch/s390/kvm/kvm-s390.c b/arch/s390/kvm/kvm-s390.c
index ffb20a64d328..907d2ca9951d 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);
-	kvm->arch.migration_mode = 1;
 	kvm_s390_sync_request_broadcast(kvm, KVM_REQ_START_MIGRATION);
 	return 0;
 }
-- 
2.54.0

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

[sashiko-bot]

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

[PATCH v4 7/7] KVM: s390: selftests: Fix cmma selftest

[Claudio Imbrenda]

The existing cmma selftest depended on the host allocating page tables
for all present memslots. Since the gmap rewrite, memory that is not
accessed by the guest might not have page tables allocated yet.

This caused the test to fail due to a mismatch in the assertion.

Fix by having the guest access also the second half of the test
memslot, thus guaranteeing that its page tables are present.

Signed-off-by: Claudio Imbrenda <imbrenda@linux.ibm.com>
Fixes: e38c884df921 ("KVM: s390: Switch to new gmap")
---
 tools/testing/selftests/kvm/s390/cmma_test.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/tools/testing/selftests/kvm/s390/cmma_test.c b/tools/testing/selftests/kvm/s390/cmma_test.c
index e39a724fe860..04f34b7ec127 100644
--- a/tools/testing/selftests/kvm/s390/cmma_test.c
+++ b/tools/testing/selftests/kvm/s390/cmma_test.c
@@ -39,11 +39,14 @@ static void guest_do_one_essa(void)
 {
 	asm volatile(
 		/* load TEST_DATA_START_GFN into r1 */
+		"	xgr 1,1\n"
 		"	llilf 1,%[start_gfn]\n"
 		/* calculate the address from the gfn */
 		"	sllg 1,1,12(0)\n"
 		/* set the first page in TEST_DATA memslot to STABLE */
 		"	.insn rrf,0xb9ab0000,2,1,1,0\n"
+		"	agfi 1,0x100000\n"
+		"	.insn rrf,0xb9ab0000,2,1,1,0\n"
 		/* hypercall */
 		"	diag 0,0,0x501\n"
 		"0:	j 0b"
-- 
2.54.0

Re: [PATCH v4 7/7] KVM: s390: selftests: Fix cmma selftest

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] Outdated comments and function naming due to inline assembly changes.
--

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

KVM: s390: selftests: Fix cmma selftest

The existing cmma selftest depended on the host allocating page tables
for all present memslots. Since the gmap rewrite, memory that is not
accessed by the guest might not have page tables allocated yet.

> diff --git a/tools/testing/selftests/kvm/s390/cmma_test.c b/tools/testing/selftests/kvm/s390/cmma_test.c
> index e39a724fe860e..04f34b7ec1279 100644
> --- a/tools/testing/selftests/kvm/s390/cmma_test.c
> +++ b/tools/testing/selftests/kvm/s390/cmma_test.c
> @@ -39,11 +39,14 @@ static void guest_do_one_essa(void)
>  {
>  	asm volatile(
>  		/* load TEST_DATA_START_GFN into r1 */
> +		"	xgr 1,1\n"
>  		"	llilf 1,%[start_gfn]\n"
>  		/* calculate the address from the gfn */
>  		"	sllg 1,1,12(0)\n"
>  		/* set the first page in TEST_DATA memslot to STABLE */
>  		"	.insn rrf,0xb9ab0000,2,1,1,0\n"
> +		"	agfi 1,0x100000\n"
> +		"	.insn rrf,0xb9ab0000,2,1,1,0\n"

[Severity: Low]
This isn't a bug, but with the addition of the second .insn instruction, the
code now accesses a second page. Should the inline comment and the function
name guest_do_one_essa (along with its docstring claiming it dirties exactly
one page) be updated to reflect that multiple pages are now being touched?

>  		/* hypercall */
>  		"	diag 0,0,0x501\n"
>  		"0:	j 0b"

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