Re: [PATCH v2 6/8] KVM: s390: Fix fault-in code

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] Memory leak of the pinned host page reference when `kvm_s390_new_mmu_cache()` allocation fails.
--

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

KVM: s390: Fix fault-in code

This commit fixes the fault-in code so that it does not return success if
a concurrent unmap event invalidated the fault-in process. It ensures the
code retries properly instead of returning success without actually
faulting in the requested page.

> diff --git a/arch/s390/kvm/faultin.c b/arch/s390/kvm/faultin.c
> index ddf0ca71f3741..cf542b0a7e8e3 100644
> --- a/arch/s390/kvm/faultin.c
> +++ b/arch/s390/kvm/faultin.c

[ ... ]

> @@ -110,20 +111,19 @@ int kvm_s390_faultin_gfn(struct kvm_vcpu *vcpu, struct kvm *kvm, struct guest_fa
>  			if (!mmu_invalidate_retry_gfn(kvm, inv_seq, f->gfn)) {
>  				f->valid = true;
>  				rc = gmap_link(mc, kvm->arch.gmap, f, slot);
> -				kvm_release_faultin_page(kvm, f->page, !!rc, f->write_attempt);
> -				f->page = NULL;
>  			}
> +			kvm_release_faultin_page(kvm, f->page, !!rc, f->write_attempt);
>  		}

[Severity: High]
This isn't a bug introduced by this patch, but I have a question about how
f->page is handled when kvm_s390_new_mmu_cache() fails earlier in this loop.

Earlier in kvm_s390_faultin_gfn() in arch/s390/kvm/faultin.c,
__kvm_faultin_pfn() resolves the PFN and pins the host page, storing it
in f->page.

Then, if mc is NULL, the code attempts to allocate a new cache:

    if (!mc) {
        local_mc = kvm_s390_new_mmu_cache();
        if (!local_mc)
            return -ENOMEM;
        mc = local_mc;
    }

When kvm_s390_new_mmu_cache() fails and returns -ENOMEM, it looks like the
function returns directly without calling kvm_release_faultin_page() to drop
the previously acquired reference in f->page. 

Callers like kvm_s390_faultin_gfn_simple() and access_guest_page_with_key_gpa()
declare struct guest_fault f locally on the stack and propagate the -ENOMEM
error without inspecting or cleaning up f.page.

Can this permanently leak the pinned host page? Under memory pressure, could
an unprivileged guest repeatedly trigger page faults to accumulate leaked page
references and exhaust host memory?

> -		kvm_release_faultin_page(kvm, f->page, true, false);
>  
>  		if (rc == -ENOMEM) {
>  			rc = kvm_s390_mmu_cache_topup(mc);
>  			if (rc)
>  				return rc;
> -		} else if (rc != -EAGAIN) {
> -			return rc;
> +			rc = -EAGAIN;
>  		}
>  	}
> +
> +	return rc;
>  }

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