Re: [PATCH v5 32/36] mm/khugepaged: base private node collapse eligiblity on actor/cap bits

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Missing `folio_allows_collapse()` checks in `collapse_scan_file` allows khugepaged to silently migrate file-backed/shmem folios off private nodes.
--

--- Patch [32]: [PATCH v5 32/36] mm/khugepaged: base private node collapse eligiblity on actor/cap bits ---
commit e336e94fb86a5e84bcd43a7f17af3bf83d2b9126
Author: Gregory Price <[email protected]>

mm/khugepaged: base private node collapse eligiblity on actor/cap bits

Collapse can move base pages across nodes.  The khugepaged daemon does
this transparently, while MADV_COLLAPSE is a userland construct.

Isolate private nodes from khugepaged exactly like ZONE_DEVICE.
Never collapse private node folios silently (the owner may not
support migration).

> diff --git a/mm/khugepaged.c b/mm/khugepaged.c
> index fb4378cc17b10..5f20839857738 100644
> --- a/mm/khugepaged.c
> +++ b/mm/khugepaged.c
[ ... ]
> @@ -1687,7 +1696,8 @@ static enum scan_result collapse_scan_pmd(struct mm_struct *mm,
>  		}
>  
>  		page = vm_normal_page(vma, addr, pteval);
> -		if (unlikely(!page) || unlikely(page_is_private_managed(page))) {
> +		if (unlikely(!page) ||
> +		    unlikely(!page_allows_collapse(page, cc->is_khugepaged))) {
>  			result = SCAN_PAGE_NULL;
>  			goto out_unmap;
>  		}

[Severity: High]
Since this patch updates collapse_scan_pmd() to check
page_allows_collapse(), does collapse_scan_file() need a similar check?

Looking at collapse_scan_file(), the loop scans folios and tallies their
residency without verifying collapse eligibility:

mm/khugepaged.c:collapse_scan_file() {
    ...
		node = folio_nid(folio);
		if (collapse_scan_abort(node, cc)) {
			result = SCAN_SCAN_ABORT;
			folio_put(folio);
			break;
		}
		cc->node_load[node]++;
    ...
}

If an unprivileged process maps file-backed memory to a private node, this
allows the scan to proceed. alloc_charge_folio() will then deny allocation
on the private node (since is_khugepaged is true) and fall back to a normal
node via ALLOC_DEFAULT.

Once the fallback THP is allocated, collapse_file() copies the memory from
the private node to the newly allocated THP on the normal node. Could this
result in a silent migration that bypasses the intended isolation guarantees?

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