Re: [PATCH v3 10/12] mm/madvise: Add MADV_CRASH_SECRET, MADV_CRASH_CACHE and MADV_CRASH_RESET

[sashiko-bot@kernel.org]

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 <linux@jaseg.de>

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-e511e9ee2329@jaseg.de?part=10