[PATCH] KVM: s390: gmap: protect entire guest top-level table when shadowing

[PATCH] KVM: s390: gmap: protect entire guest top-level table when shadowing

[Naveed Khan]

gmap_protect_asce_top_level() write-protects the guest top-level region-
or segment-table so that later modifications by the guest are noticed and
the shadow is unshadowed.  It faults in and protects asce.dt + 1 pages,
but the number of pages occupied by the top-level table is given by the
table-length field (tl), not by the designation-type field (dt).  dt and
tl are independent fields of the ASCE: tl + 1 is the size of the table in
4K pages, while dt only encodes the table type.

The rest of the code already uses the table length.  The guest table
walker in walk_guest_tables() bounds the top-level index with asce.tl,
and _gmap_handle_vsie_unshadow_event() computes the table extent as
[rsto, rsto + guest_asce.tl + 1).  Only the protection step uses dt.

Whenever tl > dt - for instance a guest using a full four-page segment
table (dt == 0, tl == 3) - only dt + 1 pages of the table get write-
protected and armed with the vsie notification bit.  The remaining pages
stay writable, so the guest can modify top-level table entries located in
them without triggering gmap_unshadow().  The shadow then keeps stale
translations, allowing the lower guest to keep accessing host pages that
the upper guest's tables no longer map, breaking isolation between the
nested guests.

Protect asce.tl + 1 pages, matching the table walker and the unshadow
event handler.  context.f has room for the maximum of CRST_TABLE_PAGES
(4) pages, so the upper bound is unchanged.

Fixes: abeb7eb57f16 ("KVM: s390: vsie: Use mmu cache to allocate rmap")
Signed-off-by: Naveed Khan <naveed@digiscrypt.com>
---
diff --git a/arch/s390/kvm/gmap.c b/arch/s390/kvm/gmap.c
index 77829f787d..e1e734cbc0 100644
--- a/arch/s390/kvm/gmap.c
+++ b/arch/s390/kvm/gmap.c
@@ -1287,7 +1287,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.52.0

Re: [PATCH] KVM: s390: gmap: protect entire guest top-level table when shadowing

[sashiko-bot]

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

Pre-existing issues:
- [Critical] Pre-existing AB-BA deadlock between the gmap `children_lock` and the `kvm->mmu_lock`.
--

commit adfa2afc574e2132990a75c0f57245c13f93a6d8
Author: Naveed Khan <naveed@digiscrypt.com>

KVM: s390: gmap: protect entire guest top-level table when shadowing

This commit updates the top-level guest table protection to use the table
length rather than the designation type. This ensures that the entire table
is write-protected, preventing nested guests from modifying writable
trailing pages and retaining stale translations.

> 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
> @@ -1263,7 +1263,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)
>  		rc = _gmap_protect_asce_top_level(mc, sg, &context);

[Severity: Critical]
This isn't a bug introduced by this patch, but is there a pre-existing AB-BA
deadlock between the gmap children_lock and kvm->mmu_lock here?

When unshadowing an existing real-space mapping, gmap_create_shadow() takes
the locks in this order:

arch/s390/kvm/gmap.c:gmap_create_shadow() {
	scoped_guard(spinlock, &parent->children_lock) {
		...
		scoped_guard(write_lock, &parent->kvm->mmu_lock) {
		...
}

Concurrently, when creating a virtual-space shadow, gmap_create_shadow()
releases children_lock and calls gmap_protect_asce_top_level(), which
eventually calls __gmap_protect_asce_top_level() where the locks are
taken in the reverse order:

arch/s390/kvm/gmap.c:__gmap_protect_asce_top_level() {
	guard(write_lock)(&sg->kvm->mmu_lock);
	...
	scoped_guard(spinlock, &parent->children_lock) {
	...
}

Since sg->kvm is identically parent->kvm, could this cause a deadlock
if one VCPU creates a real-space shadow while another concurrently creates a
virtual-space shadow for the same parent gmap?

>  	if (rc)
>  		kvm_s390_release_faultin_array(sg->kvm, context.f, true);
>  	return rc;
>  }

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/178223359030.58041.119573473883113694@digiscrypt.com?part=1

Re: [PATCH] KVM: s390: gmap: protect entire guest top-level table when shadowing

[Christian Borntraeger]

Am 23.06.26 um 18:53 schrieb Naveed Khan:
[..]

> Fixes: abeb7eb57f16 ("KVM: s390: vsie: Use mmu cache to allocate rmap")
> Signed-off-by: Naveed Khan <naveed@digiscrypt.com>
> ---
> diff --git a/arch/s390/kvm/gmap.c b/arch/s390/kvm/gmap.c
> index 77829f787d..e1e734cbc0 100644
> --- a/arch/s390/kvm/gmap.c
> +++ b/arch/s390/kvm/gmap.c
> @@ -1287,7 +1287,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).]
there is a patch already in the making for quite some time
https://lore.kernel.org/kvm/20260623153331.233784-5-imbrenda@linux.ibm.com/
https://lore.kernel.org/kvm/20260616165110.360921-5-imbrenda@linux.ibm.com/

This code area is currently under heavy work and we do read sashiko reviews so
there is probably no point in doing LLM review in this code area at the moment.