Re: [PATCH v15 08/11] mm/hmm: implement rollback for device page migration in HMM pagewalk

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [Critical] mm: memory corruption via uninitialized PFN unlock/put during rollback
- [High] mm: infinite loop in hmm_range_fault_locked on failed THP split
- [Medium] mm: lost per-PFN input flags on hmm rollback preventing correct retries
--

commit 773397c69855b5d4adbb7a21c81d362234a475a7
Author: Mika Penttilä <mpenttil@redhat.com>

mm/hmm: implement rollback for device page migration in HMM pagewalk

During the migration pagewalk, the PTE table could be cleared
and/or changed into PMD leaf or even another PTE table while
dropped locks.

In these cases the possibly inserted migration ptes are gone.
We have to however undo the collecting done so far, so unlock
the folios and drop reference taken.

[ ... ]

> diff --git a/include/linux/hmm.h b/include/linux/hmm.h
> index 4f56f3419cb46..b08ebc1343dd9 100644
> --- a/include/linux/hmm.h
> +++ b/include/linux/hmm.h
> @@ -111,6 +111,28 @@ static inline unsigned int hmm_pfn_to_map_order(unsigned long hmm_pfn)
>  	return (hmm_pfn >> HMM_PFN_ORDER_SHIFT) & 0x1F;
>  }
>  
> +/*
> + * hmm_pfn_collected() - is this pfn entry prepared for migration ?
> + * If collected the folio's refcount is increased and the folio
> + * is locked.
> + */
> +static inline bool hmm_pfn_collected(unsigned long hmm_pfn)
> +{
> +	return (hmm_pfn & (HMM_PFN_VALID | HMM_PFN_MIGRATE)) ==
> +		(HMM_PFN_VALID | HMM_PFN_MIGRATE);
> +}
> +
> +/*
> + * hmm_pfn_rollback_collected() - undoes the collection of hmm_pfn
> + *
> + * Note for total rollback the folio's refcount has to be put
> + * and folio has to be unlocked.
> + */
> +static inline unsigned long hmm_pfn_rollback_collected(unsigned long hmm_pfn)
> +{
> +	return hmm_pfn & ~(HMM_PFN_VALID | HMM_PFN_MIGRATE | HMM_PFN_COMPOUND);
> +}

[Severity: Medium]
Will hmm_pfn_rollback_collected() destroy the user's per-PFN request flags?

When pte_offset_map_lock() fails in hmm_vma_walk_pmd(),
hmm_vma_handle_migrate_prepare_rollback() is called on unprocessed entries.
This clears HMM_PFN_VALID and HMM_PFN_MIGRATE here. Because input flags
(HMM_PFN_REQ_FAULT and HMM_PFN_REQ_MIGRATE) share the exact same bit
definitions as the output flags, this effectively destroys the user's request
flags.

If the outer loop retries the walk, will it see no request flags for these
pages and silently skip them?

> +
>  /*
>   * struct hmm_range - track invalidation lock on virtual address range
>   *
> diff --git a/mm/hmm.c b/mm/hmm.c
> index 9fdd945cc0266..daf83f8091518 100644
> --- a/mm/hmm.c
> +++ b/mm/hmm.c

[ ... ]

> @@ -412,6 +421,9 @@ static int hmm_vma_handle_pte(struct mm_walk *walk, unsigned long addr,
>  
>  	new_pfn_flags = pte_pfn(pte) | cpu_flags;
>  out:
> +	hmm_vma_handle_migrate_prepare_rollback(hmm_vma_walk, addr,
> +						addr + PAGE_SIZE,
> +						hmm_pfn);
>  	*hmm_pfn = (*hmm_pfn & HMM_PFN_INOUT_FLAGS) | new_pfn_flags;
>  	return 0;

[Severity: Critical]
Can this regression lead to memory corruption via an uninitialized PFN
unlock/put?

Here hmm_vma_handle_migrate_prepare_rollback() is called before the
individual hmm_pfn entry is populated with new_pfn_flags, meaning it reads
and acts on the user's input flags.

If a user calls hmm_range_fault() to request migration using per-PFN flags
(setting HMM_PFN_REQ_MIGRATE | HMM_PFN_REQ_FAULT, which map directly to
HMM_PFN_MIGRATE | HMM_PFN_VALID), hmm_pfn_collected() will incorrectly
evaluate to true.

This causes hmm_vma_handle_migrate_prepare_rollback() to extract the PFN
part (typically 0), cast it to a folio, and attempt to unlock/put it:

hmm_vma_handle_migrate_prepare_rollback() {
	...
	folio = page_folio(hmm_pfn_to_page(hmm_pfn[i]));
	if (folio != fault_folio)
		folio_unlock(folio);
	folio_put(folio);
	...
}

[ ... ]

> @@ -1117,6 +1190,9 @@ static int hmm_vma_walk_pmd(pmd_t *pmdp,
>  		if (ptep) {
>  			lazy_mmu_mode_enable();
>  			hmm_vma_walk->ptelocked = true;
> +		} else {
> +			/* The pte table is gone */
> +			hmm_vma_handle_migrate_prepare_rollback(walk->private, addr, end, hmm_pfns);
>  		}
>  	} else {
>  		ptep = pte_offset_map(pmdp, addr);

[Severity: High]
Is there an infinite loop in hmm_range_fault_locked() when a THP split fails?

When hmm_vma_handle_migrate_prepare_pmd() attempts to split a PMD-mapped THP
via __migrate_vma_split_folio(), the split can fail and return -EBUSY. It
propagates this -EBUSY directly to hmm_vma_walk_pmd() without updating
hmm_vma_walk->last:

hmm_vma_handle_migrate_prepare_pmd() {
	...
	r = __migrate_vma_split_folio(folio, migrate->fault_page);
	if (r)
		return r;

	hmm_vma_walk->last = start;
	return -EBUSY;
}

However, hmm_vma_walk_pmd() handles -EBUSY as a success sentinel meaning
"split succeeded, please re-walk":

		/*
		 * hmm_vma_handle_migrate_prepare_pmd() splits the huge pmd in
		 * place when needed and returns -EBUSY to re-walk the range as
		 * PTEs; any other error means the split failed.
		 */
		if (r == -EBUSY)
			return -EBUSY;

This propagates the -EBUSY to walk_page_range(), and the outer loop in
hmm_range_fault_locked() restarts the walk from hmm_vma_walk.last (which was
not advanced). It encounters the exact same THP, fails to split it again,
and loops infinitely.

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