Re: [PATCH v14 07/12] mm/hmm: migrate collection in HMM pagewalk - pmd level

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [Critical] mm: set_pmd_migration_entry() uses wrong subpage for >2MB folios
- [High] mm: pmd_folio() dereferences bogus pointer for non-present PMD leaf entries
- [High] mm: unreachable device-private PMD branch causes infinite fault loops
--

commit 191891c0dfbd0707d81f83e9fa9b79a00e1693ac
Author: Mika Penttilä <mpenttil@redhat.com>

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

Implement hmm_vma_handle_migrate_prepare_pmd() for HMM pagewalk migration.
It makes migration decisions, possibly splits folios, and inserts migration
pmds.

> diff --git a/mm/hmm.c b/mm/hmm.c
> index b3b79d13c7978..a1459bd34c04e 100644
> --- a/mm/hmm.c
> +++ b/mm/hmm.c
> @@ -552,8 +552,100 @@ static int hmm_vma_handle_migrate_prepare_pmd(const struct mm_walk *walk,
[ ... ]
> +	if (pmd_trans_huge(*pmdp)) {
> +		if (!(minfo & MIGRATE_VMA_SELECT_SYSTEM))
> +			goto out;
> +
> +		folio = pmd_folio(*pmdp);

[Severity: High]
Does this code handle non-present PMD leaf entries correctly?

When pmd_trans_huge() is called, it can return true for non-present PMD leaf
entries like migration or device-private swap entries because they have the
huge bit set. If that happens, this path will unconditionally call
pmd_folio(), which can produce a bogus struct page pointer because swap
metadata is interpreted as a PFN. Could this lead to an invalid memory access
if userspace triggers HMM migration on memory regions with these entries?

> +		if (is_huge_zero_folio(folio))
> +			return hmm_pfns_fill(start, end, hmm_vma_walk, 0);
> +
> +	} else if (!pmd_present(*pmdp)) {

[Severity: High]
Is this branch actually reachable for device-private PMDs?

According to the MM subsystem guide, pmd_present() returns true for
non-present PMD leaf entries, which includes device-private entries. That
would make !pmd_present(*pmdp) evaluate to false. If device-private PMDs
bypass the huge PMD check and fail this condition, they will fall through to
the final else block and return -EBUSY. Returning -EBUSY causes the outer
walk_page_range() loop in hmm_range_fault_locked() to retry the same address
indefinitely. Can this cause a denial of service via an infinite loop?

> +		const softleaf_t entry = softleaf_from_pmd(*pmdp);
[ ... ]
> +	if (thp_migration_supported() &&
> +	    (migrate->flags & MIGRATE_VMA_SELECT_COMPOUND) &&
> +	    (IS_ALIGNED(start, HPAGE_PMD_SIZE) &&
> +	     IS_ALIGNED(end, HPAGE_PMD_SIZE))) {
> +		struct page_vma_mapped_walk pvmw = {
> +			.ptl = hmm_vma_walk->ptl,
> +			.address = start,
> +			.pmd = pmdp,
> +			.vma = walk->vma,
> +		};
> +
> +		hmm_pfn[0] |= HMM_PFN_MIGRATE | HMM_PFN_COMPOUND;
> +
> +		r = set_pmd_migration_entry(&pvmw, folio_page(folio, 0));

[Severity: Critical]
Will this use the correct subpage for folios larger than 2MB?

For file-backed folios that are larger than PMD size (e.g., 1GB), they can
be mapped by multiple PMDs. Using folio_page(folio, 0) always evaluates to
the head page of the large folio, regardless of which 2MB chunk the PMD
actually maps. Since set_pmd_migration_entry() uses page_to_pfn() on the
provided page, all PMD migration entries for the large folio might encode
the PFN of the head chunk. Upon migration restoration, could this silently
corrupt data by mapping all PMDs to the first 2MB chunk's physical memory?
Would it be safer to use pmd_page(*pmdp) here to get the specific subpage?

> +		if (r) {
> +			hmm_pfn[0] &= ~(HMM_PFN_MIGRATE | HMM_PFN_COMPOUND);
> +			r = -ENOENT;  // fallback

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