Re: [PATCH v6 10/10] KVM: s390: vsie: Avoid potential deadlock with real spaces

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Returning `-EAGAIN` without releasing the newly allocated `gmap` structure causes a severe memory leak, potentially leading to a host DoS under lock contention.
--

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

KVM: s390: vsie: Avoid potential deadlock with real spaces

This commit converts the inner locking of kvm->mmu_lock in gmap_create_shadow()
to a trylock. It returns -EAGAIN if the lock is busy to avoid a potential
deadlock with real spaces when the natural lock ordering is reversed.

> 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
[ ... ]
> @@ -1349,8 +1349,12 @@ struct gmap *gmap_create_shadow(struct kvm_s390_mmu_cache *mc, struct gmap *pare
>  			/* Only allow one real-space gmap shadow. */
>  			list_for_each_entry(sg, &parent->children, list) {
>  				if (sg->guest_asce.r) {
> -					scoped_guard(write_lock, &parent->kvm->mmu_lock)
> +					if (write_trylock(&parent->kvm->mmu_lock)) {
>  						gmap_unshadow(sg);
> +						write_unlock(&parent->kvm->mmu_lock);
> +					} else {
> +						return ERR_PTR(-EAGAIN);
> +					}

[Severity: High]
Does returning here leak the memory allocated for the new gmap structure?

Looking at the code earlier in gmap_create_shadow(), the new gmap is allocated
unconditionally before this lock attempt:

arch/s390/kvm/gmap.c:gmap_create_shadow() {
    ...
	/* Create a new shadow gmap. */
	new = gmap_new(parent->kvm, asce.r ? 1UL << (64 - PAGE_SHIFT) : asce_end(asce));
	if (!new)
		return ERR_PTR(-ENOMEM);
    ...
	scoped_guard(spinlock, &parent->children_lock) {
		...
		if (asce.r) {
			/* Only allow one real-space gmap shadow. */
			list_for_each_entry(sg, &parent->children, list) {
				if (sg->guest_asce.r) {
					if (write_trylock(&parent->kvm->mmu_lock)) {
						gmap_unshadow(sg);
						write_unlock(&parent->kvm->mmu_lock);
					} else {
						return ERR_PTR(-EAGAIN);
					}

When write_trylock() fails, should we call gmap_put(new) before returning
ERR_PTR(-EAGAIN) to avoid leaking the gmap structure and its page tables?

Since callers like vsie_run() will loop and retry on -EAGAIN, could this
lead to rapid memory exhaustion under lock contention?

>  					break;
>  				}
>  			}

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