Re: [PATCH v16 05/12] mm/hmm: migrate collection in HMM pagewalk - pte level

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 6 potential issue(s) to consider:
- [Critical] mm/hmm: folio locks held across handle_mm_fault() and broken migration PTE rollback
- [High] mm/hmm: folio split failure in hmm_vma_handle_migrate_prepare() aborts PMD walk
- [High] mm/hmm: hmm_vma_handle_migrate_prepare() skips pte_protnone() and unowned PTEs
- [High] mm/hmm: blocking folio_lock() after folio_get() in migrate_vma_split_folio()
- [High] mm/hmm: UAF and lock drop on tail fault_page in migrate_vma_split_folio()
- [Medium] mm/hmm: fix zero pfn handling in hmm_vma_handle_migrate_prepare()
--

commit 16c2ec58848b6b4b76d718d33377a04eaa06ff45
Author: Mika Penttilä <mpenttil@redhat.com>

mm/hmm: migrate collection in HMM pagewalk - pte level

This commit implements migrate_vma_split_folio() and
hmm_vma_handle_migrate_prepare() in mm/hmm.c to split large folios and
install migration page table entries during pagewalk-based device
migration.

> diff --git a/mm/hmm.c b/mm/hmm.c
> index 119e70dbca56f..0ce947d0e5727 100644
> --- a/mm/hmm.c
> +++ b/mm/hmm.c
> @@ -491,6 +491,63 @@ static int hmm_vma_handle_absent_pmd(struct mm_walk *walk, unsigned long start,
>  #endif  /* CONFIG_ARCH_HAS_PMD_SOFTLEAVES */
>  
>  #ifdef CONFIG_DEVICE_MIGRATION
> +/**
> + * migrate_vma_split_folio() - Helper function to split a THP folio
> + * @folio: the folio to split
> + * @fault_page: struct page associated with the fault if any
> + * @hmm_vma_walk: walk in progress
> + * @ptep: pte_t * for unmap and unlock ptl
> + *
> + * Returns 0 on success
> + */
> +static int migrate_vma_split_folio(struct folio *folio,
> +				   struct page *fault_page,
> +				   struct hmm_vma_walk *hmm_vma_walk,
> +				   pte_t *ptep)
> +{
> +	int ret;
> +	struct folio *fault_folio = fault_page ? page_folio(fault_page) : NULL;
> +	struct folio *new_fault_folio = NULL;
> +
> +	if (folio != fault_folio)
> +		folio_get(folio);
> +
> +	pte_unmap_unlock(ptep, hmm_vma_walk->ptl);
> +	hmm_vma_walk->ptelocked = false;
> +
> +	if (folio != fault_folio)
> +		folio_lock(folio);
> +
> +	ret = split_folio(folio);

[Severity: High]
When hmm_range_fault() -> hmm_vma_walk_pmd() ->
hmm_vma_handle_migrate_prepare() calls migrate_vma_split_folio() on a large
system or device-private folio with folio != fault_folio,
migrate_vma_split_folio() takes a reference via folio_get(folio), drops the
page table lock with pte_unmap_unlock(ptep, hmm_vma_walk->ptl), and then
blocks on folio_lock(folio) before calling split_folio(folio).

Does this sequence introduce three regressions?

First, when concurrent tasks attempt to split or migrate the same large
folio, any task blocked in folio_lock(folio) holds an extra reference on
folio, causing the lock holder's split_folio() -> __folio_split() reference
count freeze check to fail with -EAGAIN (which
hmm_vma_handle_migrate_prepare() converts into -EFAULT).

Second, because the page table lock is dropped before folio_lock(folio) is
acquired, another thread can finish splitting folio and release the folio
lock before folio_lock(folio) returns. Calling split_folio(folio) without
re-checking folio_test_large(folio) under the folio lock will trigger
VM_WARN_ON_ONCE_FOLIO(!folio_test_large(folio), folio) in __folio_split()
and fail with -EINVAL.

Third, earlier iterations of the page table walk in hmm_vma_walk_pmd() lock
collected folios via folio_trylock() and replace their page table entries
with migration entries (and fault_folio is also held locked when
migrate->fault_page is set). Blocking unconditionally on folio_lock(folio)
while holding locks and active migration entries on previously collected
folios violates the trylock-only requirement documented in
hmm_vma_handle_migrate_prepare() and can deadlock against concurrent
migrations or tasks waiting in migration_entry_wait().

> +	if (ret) {
> +		if (folio != fault_folio) {
> +			folio_unlock(folio);
> +			folio_put(folio);
> +		}
> +		return ret;
> +	}
> +
> +	new_fault_folio = fault_page ? page_folio(fault_page) : NULL;
> +
> +	/*
> +	 * Ensure the lock is held on the correct
> +	 * folio after the split
> +	 */
> +	if (!new_fault_folio) {
> +		folio_unlock(folio);
> +		folio_put(folio);
> +	} else if (folio != new_fault_folio) {
> +		if (new_fault_folio != fault_folio) {
> +			folio_get(new_fault_folio);
> +			folio_lock(new_fault_folio);
> +		}
> +		folio_unlock(folio);
> +		folio_put(folio);
> +	}

[Severity: High]
Can this trigger a use-after-free when fault_page is a tail subpage of a
large device-private folio?

In mm/memory.c:do_swap_page(), when a page fault hits a device-private
entry mapping a tail subpage (vmf->page), do_swap_page() locks and takes a
single reference on folio = page_folio(vmf->page), drops vmf->ptl, and
calls pgmap->ops->migrate_to_ram(vmf) with migrate->fault_page = vmf->page:

mm/memory.c:do_swap_page() {
    ...
    folio = page_folio(vmf->page);
    if (folio_trylock(folio)) {
        struct dev_pagemap *pgmap;

        folio_get(folio);
        pte_unmap_unlock(vmf->pte, vmf->ptl);
        pgmap = page_pgmap(vmf->page);
        ret = pgmap->ops->migrate_to_ram(vmf);
        /* migrate_to_ram() might have split the folio. */
        folio = page_folio(vmf->page);
        folio_unlock(folio);
        folio_put(folio);
    ...
}

In migrate_vma_split_folio(), fault_folio = page_folio(fault_page) equals
folio on entry. Calling split_folio(folio) passes lock_at = &folio->page
instead of fault_page (unlike split_huge_page(fault_page)) to
mm/huge_memory.c:__folio_split():

mm/huge_memory.c:__folio_split() {
    ...
    for (new_folio = folio; new_folio != end_folio; new_folio = next) {
        next = folio_next(new_folio);
        if (new_folio == page_folio(lock_at))
            continue;
        ...
        if (is_swapcache && !folio_mapped(new_folio))
            folio_free_swap(new_folio);
        folio_unlock(new_folio);
        folio_put(new_folio);
    }
}

Because lock_at is &folio->page (subpage 0), __folio_split() unlocks and
calls folio_put() on every other subfolio resulting from the split,
including new_fault_folio = page_folio(fault_page). If fault_page's page
table entry was concurrently zapped (for instance via MADV_DONTNEED after
do_swap_page() dropped vmf->ptl or right after __folio_split() unlocked
new_fault_folio), new_fault_folio has no remaining references and is freed
before split_folio(folio) returns.

Won't migrate_vma_split_folio() then call folio_get(new_fault_folio) and
folio_lock(new_fault_folio) on the unpinned and freed new_fault_folio
(hitting VM_BUG_ON_FOLIO(folio_ref_zero_or_close_to_overflow(folio), folio)
or a use-after-free) while also holding two order-0 folio locks at once?

> +
> +	return 0;
> +}
> +
>  static int hmm_vma_handle_migrate_prepare_pmd(const struct mm_walk *walk,
>  					      pmd_t *pmdp,
>  					      unsigned long start,
> @@ -501,6 +558,11 @@ static int hmm_vma_handle_migrate_prepare_pmd(const struct mm_walk *walk,
>  	return 0;
>  }
>  
> +/*
> + * Install migration entries if migration requested, either from fault
> + * or migrate paths.
> + *
> + */
>  static int hmm_vma_handle_migrate_prepare(const struct mm_walk *walk,
>  					  pmd_t *pmdp,
>  					  pte_t *ptep,
> @@ -508,8 +570,207 @@ static int hmm_vma_handle_migrate_prepare(const struct mm_walk *walk,
>  					  unsigned long *hmm_pfn,
>  					  bool *unmapped)
>  {
> -	// TODO: implement migration entry insertion
> +	struct hmm_vma_walk *hmm_vma_walk = walk->private;
> +	struct hmm_range *range = hmm_vma_walk->range;
> +	struct migrate_vma *migrate = range->migrate;
> +	struct mm_struct *mm = walk->vma->vm_mm;
> +	struct folio *fault_folio = NULL;
> +	enum migrate_vma_info minfo;
> +	struct dev_pagemap *pgmap;
> +	bool anon_exclusive;
> +	struct folio *folio;
> +	unsigned long pfn;
> +	struct page *page;
> +	softleaf_t entry;
> +	pte_t pte, swp_pte;
> +	bool writable = false;
> +
> +	// Do we want to migrate at all?
> +	minfo = hmm_select_migrate(range);
> +	if (!minfo)
> +		return 0;
> +
> +	WARN_ON_ONCE(!migrate);
> +	HMM_ASSERT_PTE_LOCKED(hmm_vma_walk, true);
> +
> +	fault_folio = migrate->fault_page ?
> +		page_folio(migrate->fault_page) : NULL;
> +
> +	pte = ptep_get(ptep);
> +
> +	if (pte_none(pte)) {
> +		if (vma_is_anonymous(walk->vma)) {
> +			*hmm_pfn &= HMM_PFN_INOUT_FLAGS;
> +			*hmm_pfn |= HMM_PFN_MIGRATE;
> +			goto out;
> +		}
> +	}
> +
> +	if (!(hmm_pfn[0] & HMM_PFN_VALID))
> +		goto out;
> +
> +	if (!pte_present(pte)) {
> +		/*
> +		 * Only care about unaddressable device page special
> +		 * page table entry. Other special swap entries are not
> +		 * migratable, and we ignore regular swapped page.
> +		 */
> +		entry = softleaf_from_pte(pte);
> +		if (!softleaf_is_device_private(entry))
> +			goto out;
> +
> +		if (!(minfo & MIGRATE_VMA_SELECT_DEVICE_PRIVATE))
> +			goto out;
> +
> +		page = softleaf_to_page(entry);
> +		folio = page_folio(page);
> +		if (folio->pgmap->owner != migrate->pgmap_owner)
> +			goto out;

[Severity: High]
Does gating hmm_vma_handle_migrate_prepare() on HMM_PFN_VALID cause
hmm_range_fault() to skip pte_protnone() pages and unowned device-private
entries?

When migrate_vma_setup() is switched to hmm_range_fault() in the later
commit "mm: enable device page migration from HMM pagewalk", existing
callers (drm_pagemap_migrate_to_devmem(), svm_migrate_ram_to_vram(),
nouveau_dmem_migrate_vma(), and dmirror_migrate_to_device()) do not set
MIGRATE_VMA_FAULT, so range.default_flags is HMM_PFN_REQ_MIGRATE without
HMM_PFN_REQ_FAULT.

In hmm_vma_walk_pmd() -> hmm_vma_handle_pte(), when a present entry has
pte_protnone(pte) set (for example by task_numa_work() ->
change_prot_numa() or userfaultfd), pte_to_hmm_pfn_flags() returns 0:

mm/hmm.c:pte_to_hmm_pfn_flags() {
    if (pte_none(pte) || !pte_present(pte) || pte_protnone(pte))
        return 0;
    return pte_write(pte) ? (HMM_PFN_VALID | HMM_PFN_WRITE) : HMM_PFN_VALID;
}

Because hmm_pte_need_fault() also returns 0 when HMM_PFN_REQ_FAULT is not
set, hmm_vma_handle_pte() writes pte_pfn(pte) without HMM_PFN_VALID into
*hmm_pfn. Then hmm_vma_handle_migrate_prepare() checks
!(hmm_pfn[0] & HMM_PFN_VALID) and skips the page, whereas
migrate_vma_collect_pmd() previously collected all present normal pages
regardless of pte_protnone(pte) (and migrate_hmm_range_setup() and
hmm_pfn_collected() also require HMM_PFN_VALID to distinguish a collected
page from a zero page). This causes full-range migrations such as
drm_pagemap_migrate_to_devmem() to fail with -EBUSY and partial migrations
in amdkfd and nouveau to leave pages behind in system memory.

Similarly, for non-present device-private entries, hmm_vma_handle_pte()
only sets HMM_PFN_VALID when page_pgmap(softleaf_to_page(entry))->owner
matches range->dev_private_owner (and faults a read-only device-private
entry to system memory if HMM_PFN_REQ_FAULT | HMM_PFN_REQ_WRITE is set):

mm/hmm.c:hmm_vma_handle_pte() {
    ...
    if (softleaf_is_device_private(entry) &&
        page_pgmap(softleaf_to_page(entry))->owner ==
        range->dev_private_owner) {
        cpu_flags = HMM_PFN_VALID;
        if (softleaf_is_device_private_write(entry))
            cpu_flags |= HMM_PFN_WRITE;
        new_pfn_flags = softleaf_to_pfn(entry) | cpu_flags;
        goto out;
    }
}

Doesn't that make the !pte_present(pte) device-private migration path here
unreachable unless range->dev_private_owner is also set to
migrate->pgmap_owner?

> +
> +		if (folio_test_large(folio)) {
> +			int ret;
> +
> +			ret = migrate_vma_split_folio(folio,
> +						      migrate->fault_page,
> +						      hmm_vma_walk,
> +						      ptep);
> +			if (ret)
> +				goto out_error;
> +			return -EAGAIN;
> +		}
> +
> +		pfn = page_to_pfn(page);
> +		if (softleaf_is_device_private_write(entry))
> +			writable = true;
> +	} else {
> +		pfn = pte_pfn(pte);
> +		if (is_zero_pfn(pfn) &&
> +		    (minfo & MIGRATE_VMA_SELECT_SYSTEM)) {
> +			*hmm_pfn = HMM_PFN_MIGRATE;
> +			goto out;
> +		}

[Severity: Medium]
When is_zero_pfn(pfn) && (minfo & MIGRATE_VMA_SELECT_SYSTEM) is true,
hmm_vma_handle_pte() has already set:

    *hmm_pfn = (*hmm_pfn & HMM_PFN_INOUT_FLAGS) | pte_pfn(pte) | HMM_PFN_VALID;

Unlike the pte_none(pte) branch above (which preserves HMM_PFN_INOUT_FLAGS
and checks vma_is_anonymous(walk->vma)) and hmm_pfns_fill() (used for
pmd_none and is_huge_zero_folio() in the later commit "mm/hmm: migrate
collection in HMM pagewalk - pmd level"), assigning *hmm_pfn =
HMM_PFN_MIGRATE directly wipes out sticky HMM_PFN_INOUT_FLAGS
(HMM_PFN_DMA_MAPPED | HMM_PFN_P2PDMA | HMM_PFN_P2PDMA_BUS).

It also omits the vma_is_anonymous(walk->vma) check, even though
mm/migrate_device.c:migrate_vma_insert_page() refuses to populate
non-anonymous mappings:

mm/migrate_device.c:migrate_vma_insert_page() {
    /* Only allow populating anonymous memory */
    if (!vma_is_anonymous(vma))
        goto abort;
}

If a non-anonymous mapping maps a zero page (for example via ksm zero-page
merging in a private file mapping or dax_load_hole()), won't setting
*hmm_pfn = HMM_PFN_MIGRATE cause migrate_hmm_range_setup() to set
migrate->src[i] = MIGRATE_PFN_MIGRATE and increment migrate->cpages for a
page that migrate_vma_insert_page() will reject, while also losing the
valid zero-page pfn and HMM_PFN_VALID in range->hmm_pfns for callers like
nouveau_dmem_migrate_fault() that fall back to mapping in place?

> +		page = vm_normal_page(walk->vma, addr, pte);
> +		if (page && !is_zone_device_page(page) &&
> +		    !(minfo & MIGRATE_VMA_SELECT_SYSTEM)) {
> +			goto out;
> +		} else if (page && is_device_coherent_page(page)) {
> +			pgmap = page_pgmap(page);
> +
> +			if (!(minfo &
> +			      MIGRATE_VMA_SELECT_DEVICE_COHERENT) ||
> +			    pgmap->owner != migrate->pgmap_owner)
> +				goto out;
> +		}
> +
> +		folio = page ? page_folio(page) : NULL;
> +		if (folio && folio_test_large(folio)) {
> +			int ret;
> +
> +			ret = migrate_vma_split_folio(folio,
> +						      migrate->fault_page,
> +						      hmm_vma_walk,
> +						      ptep);
> +			if (ret)
> +				goto out_error;
> +			return -EAGAIN;
> +		}

[Severity: High]
When migrate_vma_split_folio() fails (both here and in the !pte_present(pte)
branch above), hmm_vma_handle_migrate_prepare() jumps to out_error and
returns -EFAULT.

In hmm_vma_walk_pmd(), any non-zero, non--EAGAIN return from
hmm_vma_handle_migrate_prepare() fills [addr, end) with HMM_PFN_ERROR and
aborts the loop while returning 0:

mm/hmm.c:hmm_vma_walk_pmd() {
    ...
    r = hmm_vma_handle_migrate_prepare(walk, pmdp, ptep, addr, hmm_pfns, &unmapped);
    ...
    if (r) {
        /* A non -EAGAIN error here means migrate_vma_split_folio()
         * already dropped the PTE lock and cleared ptelocked.
         */
        HMM_ASSERT_UNLOCKED(hmm_vma_walk);
        if (unmapped)
            flush_tlb_range(walk->vma, start, addr);
        hmm_pfns_fill(addr, end, hmm_vma_walk, HMM_PFN_ERROR);
        return 0;
    }
}

split_folio() can routinely fail with -EBUSY or -EAGAIN when a folio has
extra references (such as gup pins or per-cpu lru caches) or -EINVAL when a
mapping has min_order > 0. When that happens during a call to
hmm_range_fault() with HMM_PFN_REQ_MIGRATE (such as migrate_vma_setup() in
"mm: enable device page migration from HMM pagewalk",
do_fault_and_migrate() in "lib/test_hmm: add a new testcase for the migrate
on fault", or nouveau_dmem_migrate_fault() in "drm/nouveau/svm: add support
for migrate on fault"), doesn't returning -EFAULT cause two regressions?

First, at addr, hmm_vma_handle_pte() already populated *hmm_pfns with the
valid pfn and HMM_PFN_VALID, which hmm_pfns_fill() overwrites with
HMM_PFN_ERROR. For callers like nouveau_dmem_migrate_fault() that fall back
to mapping range.hmm_pfns[0] in place via nouveau_dmem_hmm_pfn_to_map()
when a page is not migrated, losing HMM_PFN_VALID causes the in-place
mapping to fail.

Second, for the remaining entries in [addr + PAGE_SIZE, end) of the pmd,
hmm_vma_walk_pmd() never runs hmm_vma_handle_pte() or
hmm_vma_handle_migrate_prepare(), so unmapped or non-present pages in
[addr + PAGE_SIZE, end) are never faulted in when HMM_PFN_REQ_FAULT (or
MIGRATE_VMA_FAULT / MIGRATE_VMA_WRITE) is set, and valid order-0 pages in
[addr + PAGE_SIZE, end) are never collected for migration, yet
hmm_range_fault() returns 0 with HMM_PFN_ERROR in range->hmm_pfns.

> +
> +		writable = pte_write(pte);
> +	}
> +
> +	if (!page || !page->mapping)
> +		goto out;
> +
> +	/*
> +	 * By getting a reference on the folio we pin it and that blocks
> +	 * any kind of migration. Side effect is that it "freezes" the
> +	 * pte.
> +	 *
> +	 * We drop this reference after isolating the folio from the lru
> +	 * for non device folio (device folio are not on the lru and thus
> +	 * can't be dropped from it).
> +	 */
> +	folio = page_folio(page);
> +	folio_get(folio);
> +
> +	/*
> +	 * We rely on folio_trylock() to avoid deadlock between
> +	 * concurrent migrations where each is waiting on the others
> +	 * folio lock. If we can't immediately lock the folio we fail this
> +	 * migration as it is only best effort anyway.
> +	 *
> +	 * If we can lock the folio it's safe to set up a migration entry
> +	 * now. In the common case where the folio is mapped once in a
> +	 * single process setting up the migration entry now is an
> +	 * optimisation to avoid walking the rmap later with
> +	 * try_to_migrate().
> +	 */
> +
> +	if (fault_folio == folio || folio_trylock(folio)) {
> +		anon_exclusive = folio_test_anon(folio) &&
> +			PageAnonExclusive(page);
> +
> +		if (pte_present(pte))
> +			flush_cache_page(walk->vma, addr, pfn);
> +
> +		if (anon_exclusive) {
> +			pte = ptep_clear_flush(walk->vma, addr, ptep);
> +
> +			if (folio_try_share_anon_rmap_pte(folio, page)) {
> +				set_pte_at(mm, addr, ptep, pte);
> +				if (fault_folio != folio)
> +					folio_unlock(folio);
> +				folio_put(folio);
> +				goto out;
> +			}
> +		} else {
> +			pte = ptep_get_and_clear(mm, addr, ptep);
> +		}
> +
> +		if (pte_present(pte) && pte_dirty(pte))
> +			folio_mark_dirty(folio);
> +
> +		/* Setup special migration page table entry */
> +		if (writable)
> +			entry = make_writable_migration_entry(pfn);
> +		else if (anon_exclusive)
> +			entry = make_readable_exclusive_migration_entry(pfn);
> +		else
> +			entry = make_readable_migration_entry(pfn);
> +
> +		if (pte_present(pte)) {
> +			if (pte_young(pte))
> +				entry = make_migration_entry_young(entry);
> +			if (pte_dirty(pte))
> +				entry = make_migration_entry_dirty(entry);
> +		}
> +
> +		swp_pte = swp_entry_to_pte(entry);
> +		if (pte_present(pte)) {
> +			if (pte_soft_dirty(pte))
> +				swp_pte = pte_swp_mksoft_dirty(swp_pte);
> +			if (pte_uffd(pte))
> +				swp_pte = pte_swp_mkuffd(swp_pte);
> +		} else {
> +			if (pte_swp_soft_dirty(pte))
> +				swp_pte = pte_swp_mksoft_dirty(swp_pte);
> +			if (pte_swp_uffd(pte))
> +				swp_pte = pte_swp_mkuffd(swp_pte);
> +		}
> +
> +		set_pte_at(mm, addr, ptep, swp_pte);
> +		folio_remove_rmap_pte(folio, page, walk->vma);
> +		folio_put(folio);
> +		*hmm_pfn |= HMM_PFN_MIGRATE;
> +		if (pte_present(pte))
> +			*unmapped = true;

[Severity: Critical]
While the later commit "mm/hmm: implement rollback for device page
migration in HMM pagewalk" adds hmm_vma_handle_migrate_prepare_rollback()
and "mm: enable device page migration from HMM pagewalk" documents
caller-side abort via migrate_hmm_range_setup(), does locking folios and
installing migration entries per-entry during the walk still leave three
regressions across the series when a range contains a mix of present
entries, entries requiring faults, or large folios?

1. Holding folio_lock across handle_mm_fault():
In hmm_vma_walk_pmd(), hmm_vma_handle_migrate_prepare() locks each
collected folio with folio_trylock(), pins it with folio_get(), and
installs a migration entry immediately. When a subsequent entry or hole in
the walk requires a page fault, hmm_vma_handle_pte() or hmm_vma_walk_hole()
returns HMM_FAULT_PENDING and hmm_range_fault_locked() calls hmm_do_fault()
-> handle_mm_fault() while the task still holds folio_lock on all folios
collected earlier in the range:

mm/hmm.c:hmm_range_fault_locked() {
    ...
    ret = walk_page_range(mm, hmm_vma_walk.last, range->end,
                          &hmm_walk_ops, &hmm_vma_walk);
    if (ret == HMM_FAULT_PENDING) {
        ret = hmm_do_fault(mm, &hmm_vma_walk);
        if (ret == HMM_FAULT_UNLOCKED) {
            ret = fatal_signal_pending(current) ? -EINTR :
                -EBUSY;
            break;
        }
    }
}

Holding folio_lock across handle_mm_fault() violates lock ordering
(folio_lock held before handle_mm_fault(), mapping->invalidate_lock, and
other folio locks) and deadlocks if handle_mm_fault() touches a locked
folio, or if hmm_do_fault() (which loops for addr < fault_end calling
handle_mm_fault() on every page up to the end of the pmd) encounters a
migration entry installed in an earlier pass/retry and blocks forever in
do_swap_page() -> migration_entry_wait() on a folio locked by current.

2. Orphaned migration entries when required_fault == 0:
In hmm_vma_handle_pte(), the check for !required_fault precedes the check
for softleaf_is_migration(entry):

mm/hmm.c:hmm_vma_handle_pte() {
    ...
    required_fault =
        hmm_pte_need_fault(hmm_vma_walk, pfn_req_flags, 0);
    if (!required_fault)
        goto out;
    ...
    if (softleaf_is_migration(entry)) {
        if (!hmm_select_migrate(range)) {
            ...
        }
        return 0;
    }
}

When required_fault == 0 (the default for migrate_vma_setup() without
MIGRATE_VMA_FAULT) and a walk re-visits an already-installed migration
entry (for example when !ptep resets addr = start and jumps to again:, or
when hmm_pfns_fill(addr, end, ..., HMM_PFN_ERROR) runs on a split failure
after a retry), hmm_vma_handle_pte() jumps to out:, which calls
hmm_vma_handle_migrate_prepare_rollback() (unlocking the folio and dropping
its migration reference) and overwrites *hmm_pfn with 0 while leaving the
migration entry installed in the page table.

3. Wrong rollback range on !ptep and missing rollback before fault:
In hmm_vma_walk_pmd(), if migrate_vma_split_folio() at addr > start drops
the page table lock and returns -EAGAIN, and pte_offset_map_lock()
subsequently sees the page table gone (!ptep), the rollback patch calls
hmm_vma_handle_migrate_prepare_rollback(walk->private, addr, end, hmm_pfns)
on [addr, end) instead of [start, addr) where the entries from the vanished
page table were actually collected:

mm/hmm.c:hmm_vma_walk_pmd() {
again:
    ptep = pte_offset_map_lock(walk->mm, pmdp, addr, &hmm_vma_walk->ptl);
    if (!ptep) {
        addr = start;
        goto again;
    }

    for (; addr < end; addr += PAGE_SIZE, ptep++, hmm_pfns++) {
        r = hmm_vma_handle_pte(walk, addr, end, pmdp, ptep, hmm_pfns);
        ...
        r = hmm_vma_handle_migrate_prepare(walk, pmdp, ptep, addr, hmm_pfns, &unmapped);
}

Furthermore, when a vanished page table becomes pmd_none (or an entry
requires a fault on retry) with required_fault != 0, hmm_vma_walk_hole()
and the fault: path in hmm_vma_handle_pte() return HMM_FAULT_PENDING
without rolling back previously collected entries in the range before
hmm_do_fault() runs.

> +	} else {
> +		folio_put(folio);
> +	}
> +out:
>  	return 0;
> +out_error:
> +	return -EFAULT;
>  }
>  
>  #else

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261021060506.2377545-1-mpenttil@redhat.com?part=5