Re: [PATCH v5 08/36] mm/mempolicy: skip private node folios when queueing for migration

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

Pre-existing issues:
- [Critical] Blind conversion of PMD to folio in queue_folios_pmd() causes out-of-bounds dereference on raw PFN mappings.
- [High] queue_folios_pte_range() sets walk->action = ACTION_AGAIN when pte_offset_map_lock() returns NULL, leading to an infinite retry loop for migration entries.
--

--- Patch [8]: [PATCH v5 08/36] mm/mempolicy: skip private node folios when queueing for migration ---
commit 0603242bf30dacf881c78a4de8f6d67872a88c83
Author: Gregory Price <[email protected]>
Subject: mm/mempolicy: skip private node folios when queueing for migration

Private nodes are already kept out of policy nodemasks (only N_MEMORY
nodes are allowed), but an mbind(MPOL_MF_MOVE) walk can still encounter a
private-node folio in the range.  Skip such folios so mempolicy-driven
migration never moves private-node memory.

> diff --git a/mm/mempolicy.c b/mm/mempolicy.c
> index 5720f7f54d942..8e8763f6e5f5b 100644
> --- a/mm/mempolicy.c
> +++ b/mm/mempolicy.c
[ ... ]
> @@ -668,6 +669,8 @@ static void queue_folios_pmd(pmd_t *pmd, struct mm_walk *walk)
>  	}
>  	if (!queue_folio_required(folio, qp))
>  		return;
> +	if (folio_is_private_node(folio))
> +		return;
>  	if (!(qp->flags & (MPOL_MF_MOVE | MPOL_MF_MOVE_ALL)) ||
>  	    !vma_migratable(walk->vma) ||
>  	    !migrate_folio_add(folio, qp->pagelist, qp->flags))

[Severity: Critical]
This is a pre-existing issue, but can this cause an out-of-bounds
dereference on raw PFN mappings?

Earlier in mm/mempolicy.c:queue_folios_pmd(), the code unconditionally
calls pmd_folio() and dereferences it:

	folio = pmd_folio(pmdval);
	if (is_huge_zero_folio(folio)) {

If an unprivileged user calls mbind(..., MPOL_MF_STRICT) on a memory
region mapped by a driver using raw PFNs via huge PMDs (e.g.,
vmf_insert_pfn_pmd() in a VM_PFNMAP VMA), pmd_trans_huge_lock() will
succeed. However, treating a raw PFN as a struct folio yields a bogus
pointer, leading to a kernel panic when accessed. Does this need a check
like pmd_special() to filter out raw PFN mappings?

> @@ -722,7 +725,7 @@ static int queue_folios_pte_range(pmd_t *pmd, unsigned long addr,
>  			continue;
>  		}
>  		folio = vm_normal_folio(vma, addr, ptent);
> -		if (!folio || folio_is_zone_device(folio))
> +		if (!folio || folio_is_private_managed(folio))
>  			continue;
>  		if (folio_test_large(folio) && max_nr != 1)
>  			nr = folio_pte_batch(folio, pte, ptent, max_nr);

[Severity: High]
This is a pre-existing issue, but can this lead to an infinite retry loop?

Just above this section in mm/mempolicy.c:queue_folios_pte_range(), there
is:

	mapped_pte = pte = pte_offset_map_lock(walk->mm, pmd, addr, &ptl);
	if (!pte) {
		walk->action = ACTION_AGAIN;
		return 0;
	}

Since pte_offset_map_lock() returns NULL non-transiently for migration
entries, setting walk->action = ACTION_AGAIN and returning 0 on this
failure instructs walk_pte_range() to retry the exact same PMD
indefinitely with no limit. Because this early return skips the
cond_resched() at the bottom of the function, it creates a tight CPU
livelock. Should this instead return 0 without setting ACTION_AGAIN to
gracefully skip the entry?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=8