Re: [PATCH v3 07/10] KVM: s390: Lock pte when making page secure

[sashiko-bot@kernel.org]

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

New issues:
- [High] If try_get_locked_pte returns NULL for a THP, the host PTE lock is bypassed. A concurrent split of the THP allows freezing the folio without the PTE lock, reintroducing the GUP race the patch intended to fix.

Pre-existing issues:
- [High] Host kernel panic when a PV guest attempts to secure memory backed by raw PFNs (e.g., PFNMAP/VFIO without a struct page).
--

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

KVM: s390: Lock pte when making page secure

This commit attempts to fix a race with GUP when freezing folios during the
secure page import process by temporarily locking the PTE.

> diff --git a/arch/s390/kvm/pv.c b/arch/s390/kvm/pv.c
> index c2dafd812a3b2..4b865e75351c1 100644
> --- a/arch/s390/kvm/pv.c
> +++ b/arch/s390/kvm/pv.c

[ ... ]

> @@ -103,9 +105,21 @@ static void _kvm_s390_pv_make_secure(struct guest_fault *f)
>  {
>  	struct pv_make_secure *priv = f->priv;
>  	struct folio *folio;
> +	spinlock_t *ptl;	/* pte lock from try_get_locked_pte() */
> +	pte_t *ptep;
>  
>  	folio = pfn_folio(f->pfn);

[Severity: High]
This is a pre-existing issue, but can this cause a host kernel panic if the
guest attempts to secure memory backed by raw PFNs (e.g., PFNMAP/VFIO)
that lack a struct page?

If kvm_s390_faultin_gfn() resolves the GFN to a raw PFN but leaves
f->page = NULL, blindly converting it via pfn_folio() accesses memory
outside the bounds of the vmemmap array, returning a garbage pointer.

Later in __kvm_s390_pv_make_secure(), the code does:

    if (!f->page)
        folio_get(folio);

Would this try to increment the reference count of that garbage pointer?

>  	priv->rc = -EAGAIN;
> +
> +	if (!mmap_read_trylock(priv->kvm->mm))
> +		return;
> +
> +	ptep = try_get_locked_pte(priv->kvm->mm, gfn_to_hva(priv->kvm, f->gfn), &ptl);
> +	if (IS_ERR_VALUE(ptep)) {

[Severity: High]
Since try_get_locked_pte() returns NULL for large PMDs (THPs) or unmapped
entries, and IS_ERR_VALUE(NULL) evaluates to false, does this proceed
with ptep == NULL and no PTE lock?

If the memory is a THP and a concurrent operation triggers a THP split
immediately after try_get_locked_pte() returns NULL, the THP becomes
order-0 folios. The KVM thread then successfully locks the newly-split
order-0 folio and freezes it since it bypassed the PTE lock entirely.

Could a concurrent slow GUP on the same address then acquire the PTE lock,
read the PTE, and call try_grab_folio(), triggering a WARN_ON_ONCE because
the folio refcount is 0?

> +		priv->rc = PTR_ERR(ptep);
> +		goto out;
> +	}
> +
>  	if (folio_trylock(folio)) {
>  		priv->rc = __kvm_s390_pv_make_secure(f, folio);

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