Re: [PATCH v3] mm/damon/ops-common: use a page-aligned address in damon_ptep_mkold()

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] Unaligned addresses passed to mmu_notifier_clear_young and set_huge_pte_at for PMD and HugeTLB pages cause out-of-bounds clearing of accessed bits and page table corruption.
- [High] Incorrect advancement of PTE pointers for large folios in DAMON's vaddr operations skips unrelated PTEs.
--

commit e20303b2de84c12df7877f523196d088cede0a4e
Author: Nathan Gao <zcgao@amazon.com>

    mm/damon/ops-common: use a page-aligned address in damon_ptep_mkold()
    
    __damon_va_prepare_access_check() picks a random byte address within the
    region and stores it in r->sampling_addr. damon_va_mkold() passes it into
    a page table walk, which hands it to damon_ptep_mkold() as the address of
    the page to sample.

> diff --git a/mm/damon/ops-common.c b/mm/damon/ops-common.c
> index 7a2e40bc7baed..3a5f5cdeb2017 100644
> --- a/mm/damon/ops-common.c
> +++ b/mm/damon/ops-common.c
> @@ -46,6 +46,12 @@ void damon_ptep_mkold(pte_t *pte, struct vm_area_struct *vma, unsigned long addr
>  	bool young = false;
>  	unsigned long pfn;
>  
> +	/*
> +	 * Arch implementation of ptep_test_and_clear_young() may require
> +	 * aligned @addr
> +	 */
> +	addr = PAGE_ALIGN_DOWN(addr);
> +
>  	if (likely(pte_present(pteval)))
>  		pfn = pte_pfn(pteval);
>  	else

[Severity: High]
This isn't a bug introduced by this patch, but while this fixes the
unaligned address issue for damon_ptep_mkold(), the PMD and HugeTLB
paths are identically vulnerable.

In mm/damon/ops-common.c:damon_pmdp_mkold(), the unaligned address is
still passed directly to the MMU notifier:

	if (likely(pmd_present(pmdval)))
		young |= pmdp_test_and_clear_young(vma, addr, pmd);
	young |= mmu_notifier_clear_young(vma->vm_mm, addr, addr + HPAGE_PMD_SIZE);

Does this cause the cleared range to spill over into the next PMD mapping?

Similarly, in mm/damon/vaddr.c:damon_hugetlb_mkold():

	if (pte_young(entry)) {
		referenced = true;
		entry = pte_mkold(entry);
		set_huge_pte_at(mm, addr, pte, entry, psize);
	}

	if (mmu_notifier_clear_young(mm, addr,
				     addr + huge_page_size(hstate_vma(vma))))
		referenced = true;

Can passing the unaligned address to architecture-specific page table
modifiers like set_huge_pte_at() corrupt multi-entry contiguous blocks,
and does the MMU notifier suffer from the same out-of-bounds clearing here?

[Severity: High]
This is a pre-existing issue, but there appears to be a logic error with
how large folios are handled during page table walks in DAMON.

In mm/damon/vaddr.c:damos_va_stat_pmd_entry() and
damos_va_migrate_pmd_entry():

	for (; addr < next; pte += nr, addr += nr * PAGE_SIZE) {
		nr = 1;
		ptent = ptep_get(pte);
		
		if (pte_none(ptent) || !pte_present(ptent))
			continue;
		
		folio = vm_normal_folio(walk->vma, addr, ptent);
		if (!folio)
			continue;
		
		...
		nr = folio_nr_pages(folio);
	}

If the folio is partially mapped, or if the loop starts in the middle
of a large folio, wouldn't advancing by the entire folio size
(folio_nr_pages) overshoot the mapping bounds and silently skip
unrelated PTEs?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260901201001.33271-1-zcgao@amazon.com?part=1