Re: [PATCH] mm/page_vma_mapped: revalidate and do proper check before return device-private pmd

[Lorenzo Stoakes]

+cc Lance, linux-kernel

Your subject line is 83 characters long and is way too detailed how about 'fix
device-private PMD handling'?

You forgot to include linux-kernel@xxxxxxxxxxxxxxx on the mail, lore seems to be
a bit broken atm but in general it's helpful to include that.

Also is useful to make this [PATCH mm-hotfixes] to make it really clear it's
intended as a hotfix.

Some commit msg language nits:

On Mon, Jun 22, 2026 at 01:06:51PM +0000, Wei Yang wrote:
> For pmd_trans_huge() and pmd_is_migration_entry(), we does following
> before return the pmd entry:

Sounds better as:

	For PMD entries that satisfy pmd_trans_huge() or pmd_is_migration_entry(), we
	perform the following actions:

>
>   * re-validate pmd entry after PTL
>   * check PVMW_MIGRATION
>   * check_pmd()
>   * handle on pte level if split under us
>
> But for device-private pmd, we just return after pmd_lock().

->

	However, for device-private PMD entries, we simply acquire the PMD lock
	and return.

Also can you please give some justification here as to why all this also applies
to device-private PMD? Right now it sounds hand wavey.

>
> If a softleaf entry is present, e.g. device-private pmd, the existing
> code simply acquires the PMD lock and returns success even if
> PVMW_MIGRATION is set (indicating a migration entry is sought), meaning
> that the caller can incorrectly interpret the entry as something it is
> not, causing data corruption.

This is repetitive, you already mentioned device-private PMD, you already
mentioned that it simply acquires the PMD lock.

You should talk about what issue it caused and why:

	This is particularly problematic when PVMW_MIGRATION is set (meaning a
	migration entry is sought), as it causes a device-private PMD entry to
	be returned with a different data layout, causing memory corruption.

>
> This patch fixes commit 65edfda6f3f2 ("mm/rmap: extend rmap and migration
> support device-private entries") by following the same pattern as
> pmd_trans_huge() and pmd_is_migration_entry() for device private entry.

This is pretty useless. We see what patch it fixes in the Fixes tag, and you're
just repeating things you said above, I'd drop it.

>
> Fixes: 65edfda6f3f2 ("mm/rmap: extend rmap and migration support device-private entries")
> Cc: <stable@xxxxxxxxxxxxxxx>
> Signed-off-by: Wei Yang <richard.weiyang@xxxxxxxxx>
> Suggested-by: David Hildenbrand <david@xxxxxxxxxx>
> Cc: David Hildenbrand <david@xxxxxxxxxx>
> Cc: Balbir Singh <balbirs@xxxxxxxxxx>
> Cc: SeongJae Park <sj@xxxxxxxxxx>
> Cc: Zi Yan <ziy@xxxxxxxxxx>
> Cc: Lorenzo Stoakes <ljs@xxxxxxxxxx>
>
> ---
> v3:
>   * remove cleanup part, only fix the issue for device-private entry
>   * refine user effect description based on Lorenzo's suggestion
> v2: https://lore.kernel.org/all/20260616063436.20455-1-richard.weiyang@xxxxxxxxx/T/#u
>   * specify the possible error case of current code and user visible effect
>   * besides fix, cleanup the pmd entry handling based on David's suggestion
>
> v1: https://lore.kernel.org/linux-mm/20260508013728.21285-1-richard.weiyang@xxxxxxxxx/
> ---
>  mm/page_vma_mapped.c | 32 ++++++++++++++++++++++----------
>  1 file changed, 22 insertions(+), 10 deletions(-)
>
> diff --git a/mm/page_vma_mapped.c b/mm/page_vma_mapped.c
> index 2ccbabfb2cc1..8de3c6b82df6 100644
> --- a/mm/page_vma_mapped.c
> +++ b/mm/page_vma_mapped.c
> @@ -270,21 +270,33 @@ bool page_vma_mapped_walk(struct page_vma_mapped_walk *pvmw)
>  			spin_unlock(pvmw->ptl);
>  			pvmw->ptl = NULL;
>  		} else if (!pmd_present(pmde)) {
> -			const softleaf_t entry = softleaf_from_pmd(pmde);
> +			softleaf_t entry = softleaf_from_pmd(pmde);
>
>  			if (softleaf_is_device_private(entry)) {
>  				pvmw->ptl = pmd_lock(mm, pvmw->pmd);
> -				return true;
> -			}
>
> -			if ((pvmw->flags & PVMW_SYNC) &&
> -			    thp_vma_suitable_order(vma, pvmw->address,
> -						   PMD_ORDER) &&
> -			    (pvmw->nr_pages >= HPAGE_PMD_NR))
> -				sync_with_folio_pmd_zap(mm, pvmw->pmd);
> +				entry = softleaf_from_pmd(*pvmw->pmd);
>
> -			step_forward(pvmw, PMD_SIZE);
> -			continue;
> +				if (softleaf_is_device_private(entry)) {

This is all very horrible. You have an example of how pmde is re-got in the
pmd_trans_huge() branch and pmd_is_device_private_entry() exists...

We can just make this another branch and do the re-check more neatly.

I enclose a patch that does that (untested, please check).


> +					if (pvmw->flags & PVMW_MIGRATION)
> +						return not_found(pvmw);
> +					if (!check_pmd(softleaf_to_pfn(entry), pvmw))
> +						return not_found(pvmw);
> +					return true;
> +				}
> +				/* device-private pmd was split under us: handle on pte level */
> +				spin_unlock(pvmw->ptl);
> +				pvmw->ptl = NULL;
> +			} else {
> +				if ((pvmw->flags & PVMW_SYNC) &&
> +				    thp_vma_suitable_order(vma, pvmw->address,
> +							   PMD_ORDER) &&
> +				    (pvmw->nr_pages >= HPAGE_PMD_NR))
> +					sync_with_folio_pmd_zap(mm, pvmw->pmd);
> +
> +				step_forward(pvmw, PMD_SIZE);
> +				continue;
> +			}
>  		}
>  		if (!map_pte(pvmw, &pmde, &ptl)) {
>  			if (!pvmw->pte)
> --
> 2.34.1
>

Thanks, Lorenzo

----8<----