Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [High] mm: MADV_CRASH_SECRET skips migration entries leaving secrets unprotected --
commit bd54b0316a2281079a7116bef1937bfde7198adf Author: Jan Sebastian Götte <[email protected]> mm/madvise: Add MADV_CRASH_SECRET, MADV_CRASH_CACHE and MADV_CRASH_RESET This commit introduces new madvise flags allowing userspace to mark pages containing secrets or unimportant cache data. These marks are registered in a bitmap for the kdump kernel to process during a crash dump. > diff --git a/mm/madvise.c b/mm/madvise.c > index 00b1be655a8b5..37f08334e95a8 100644 > --- a/mm/madvise.c > +++ b/mm/madvise.c [ ... ] > @@ -1173,6 +1175,98 @@ static long madvise_populate(struct madvise_behavior > *madv_behavior) [ ... ] > +static int crash_mark_pmd_entry(pmd_t *pmd, unsigned long addr, > + unsigned long end, struct mm_walk *walk) > +{ > + pte_t *start_pte, *pte; > + spinlock_t *ptl; > + > + if (fatal_signal_pending(current)) > + return -EINTR; > + > + ptl = pmd_trans_huge_lock(pmd, walk->vma); > + if (ptl) { > + pmd_t pmdval = *pmd; > + > + if (pmd_present(pmdval)) > + madvise_crash_mark_pfns(walk, > + pmd_pfn(pmdval) + ((addr & ~PMD_MASK) >> > PAGE_SHIFT), > + (end - addr) >> PAGE_SHIFT); > + spin_unlock(ptl); > + return 0; > + } [Severity: High] Does this code leave secrets unprotected if a migration is occurring concurrently? If a PMD is temporarily unmapped during a migration (such as background compaction or NUMA balancing), pmd_present evaluates to false. This code skips the PMD without marking it and returns 0, leaving the entire 2MB range unprotected in the crash memaction bitmap. Once the migration completes, the restored page remains permanently unmarked and will be dumped. > + > + start_pte = pte = pte_offset_map_lock(walk->mm, pmd, addr, &ptl); > + if (!start_pte) > + return 0; > + > + for (; addr < end; pte++, addr += PAGE_SIZE) { > + pte_t ptent = ptep_get(pte); > + > + if (pte_present(ptent)) > + madvise_crash_mark_pfns(walk, pte_pfn(ptent), 1); > + } [Severity: High] Similarly, does this loop silently skip non-present PTEs like migration and device-exclusive swap entries? When userspace calls madvise with MADV_CRASH_SECRET, it expects the memory range to be marked. Checking only pte_present skips resident physical pages that are temporarily unmapped for migration. Since the rmap hooks only see the flags as folios arrive, these skipped entries never get added to the bitmap, potentially leaking sensitive data in a crash dump. > + > + pte_unmap_unlock(start_pte, ptl); > + cond_resched(); > + return 0; > +} [ ... ] -- Sashiko AI review · https://sashiko.dev/#/patchset/20260928-crash-memaction-upstream-20260921-v3-0-e511e9ee2...@jaseg.de?part=10
