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
