Re: [PATCH] mm: migrate_device: fix pte_pfn/pte_dirty called on non-present PTE

[Kefeng Wang <wangkefeng.wang@huawei.com>]

On 7/7/2026 9:45 AM, Zi Yan wrote:
> On Mon Jul 6, 2026 at 2:01 PM EDT, Andrew Morton wrote:
>> On Mon, 6 Jul 2026 19:19:58 +0800 Kefeng Wang <wangkefeng.wang@huawei.com> wrote:
>>
>>> pte_pfn() and pte_dirty() have undefined behaviour when called on a
>>> non-present PTE. In migrate_vma_collect_pmd(), these functions may be
>>> invoked on non-present entries (e.g., swap PTEs), leading to potential
>>> crashes from pte_pfn() or incorrect dirty folio accounting from
>>> pte_dirty(). Fix both by guarding with pte_present() checks.
>>>
>>> Fixes: fd35ca3d12cc ("mm/migrate_device.c: copy pte dirty bit to page")
>>> Fixes: a3589e1d5fe3 ("mm/migrate_device.c: add missing flush_cache_page()")
>>
>> Should we cc:stable?

Yes.


>>
>> Sashiko might have found a pre-existing issue:
>> 	https://sashiko.dev/#/patchset/20260706111958.3649651-1-wangkefeng.wang@huawei.com
> 
> I looked into the issue and came up with a fix below. It looks ugly, so
> feel free to make a better one out of it.
> 
>  From 7fa06512638a0febb79a00dba63f12818a85788b Mon Sep 17 00:00:00 2001
> From: Zi Yan <ziy@nvidia.com>
> Date: Mon, 6 Jul 2026 21:25:29 -0400
> Subject: [PATCH] mm/migrate_device: avoid scanning/collecting PFNs within a
>   PMD
> 
> migrate_vma_collect_pmd() can drop pte lock to split a large folio and
> restart. But the code does not handle restart properly and can cause
> collecting inconsistent PFNs and overflowing migrate->dst and migrate->src
> arrays. Fix it by:
> 1. avoiding migrate_vma_collect_huge_pmd() if collection is in progress,
> 2. skipping the rest of the range if pmd no longer points to a pte page.
> 
> Fixes: a30b48bf1b244 ("mm/migrate_device: implement THP migration of zone device pages")
> Reported-by: Sashiko <sashiko-bot@kernel.org>
> Closes: https://sashiko.dev/#/patchset/20260706111958.3649651-1-wangkefeng.wang@huawei.com
> Assisted-by: Claude:claude-opus-4-8
> Signed-off-by: Zi Yan <ziy@nvidia.com>


Thank you for your fix, it looks good, there is some repetition (3
times) regarding flush+skip in migrate_vma_collect_pmd(), but for bug
fixes, keep it minimal is better. I don't have new ideas to improve it.

> ---
>   mm/migrate_device.c | 18 ++++++++++++++++--
>   1 file changed, 16 insertions(+), 2 deletions(-)
> 
> diff --git a/mm/migrate_device.c b/mm/migrate_device.c
> index 052167f9ad547..d86b3989d0c59 100644
> --- a/mm/migrate_device.c
> +++ b/mm/migrate_device.c
> @@ -257,7 +257,11 @@ static int migrate_vma_collect_pmd(pmd_t *pmdp,
>   	pte_t *ptep;
>   
>   again:
> -	if (pmd_trans_huge(*pmdp) || !pmd_present(*pmdp)) {
> +	/*
> +	 * Only check pmd when addr is at start, namely no pte is scanned or
> +	 * collected, to avoid collecting inconsistent PFNs.
> +	 */
> +	if (addr == start && (pmd_trans_huge(*pmdp) || !pmd_present(*pmdp))) {
>   		int ret = migrate_vma_collect_huge_pmd(pmdp, start, end, walk, fault_folio);
>   
>   		if (ret == -EAGAIN)
> @@ -267,8 +271,18 @@ static int migrate_vma_collect_pmd(pmd_t *pmdp,
>   	}
>   
>   	ptep = pte_offset_map_lock(mm, pmdp, start, &ptl);
> -	if (!ptep)
> +	if (!ptep) {
> +		/*
> +		 * Skip the rest if pmd becomes huge or cleared. Flush if any
> +		 * pte is modified
> +		 */
> +		if (addr != start) {
> +			if (unmapped)
> +				flush_tlb_range(walk->vma, start, end);
> +			return migrate_vma_collect_skip(addr, end, walk);
> +		}
>   		goto again;
> +	}
>   	lazy_mmu_mode_enable();
>   	ptep += (addr - start) / PAGE_SIZE;
>