* [PATCH v2] mm/damon/vaddr: use a page-aligned address for the sampling walks
@ 2026-08-31 22:11 Nathan Gao
2026-08-31 22:39 ` sashiko-bot
2026-09-01 1:35 ` SJ Park
0 siblings, 2 replies; 6+ messages in thread
From: Nathan Gao @ 2026-08-31 22:11 UTC (permalink / raw)
To: sj, akpm; +Cc: damon, linux-mm, linux-kernel, baolin.wang, david, ryan.roberts
__damon_va_prepare_access_check() picks a random byte address within the
region and stores it in r->sampling_addr. There are two users of
r->sampling_addr in vaddr.c that pass it into a page table walk, and
both use it as the address of a page.
damon_va_mkold(mm, r->sampling_addr)
damon_va_walk_page_range(mm, addr, addr + 1)
damon_mkold_pmd_entry()
damon_ptep_mkold(pte, vma, addr)
ptep_test_and_clear_young(vma, addr, pte)
mmu_notifier_clear_young(mm, addr, addr + PAGE_SIZE)
damon_va_young(mm, r->sampling_addr, &folio_sz)
damon_va_walk_page_range(mm, addr, addr + 1)
damon_young_pmd_entry()
ptep_get(pte)
mmu_notifier_test_young(walk->mm, addr)
For arm64, before commit 6f0e1142173a ("arm64: mm: support batch
clearing of the young flag for large folios"), the contpte helper walked
exactly CONT_PTES entries from the aligned-down page table pointer and
used @addr only to pass down to each entry, so an unaligned value was
harmless:
ptep = contpte_align_down(ptep);
addr = ALIGN_DOWN(addr, CONT_PTE_SIZE);
for (i = 0; i < CONT_PTES; i++, ptep++, addr += PAGE_SIZE)
Now the range to walk is derived from @addr instead: end = addr +
nr * PAGE_SIZE, rounded up to CONT_PTE_SIZE. For a sample in the last
page of a contpte block, the sub-page offset puts end just past the
block boundary, so the round-up lands a whole block further and the
walk clears PTE_AF in CONT_PTES entries beyond the sampled block. For
the last block in a page table page, those entries are past the end of
that page, so the walk writes into the page that follows.
Triggered by the full 7.1/7.2 kernel selftest suite on arm64 (EC2
c/m6g.4xlarge). The kernel sometimes crashes at or shortly after the
DAMON test.
What the overrun does depends on the page that happens to follow the
page table, so there is no single signature. If that page is read-only,
the write faults in the sampling path itself:
Unable to handle kernel write to read-only memory at virtual address ffff0003c5d2d000
FSC = 0x0f: level 3 permission fault
CM = 0, WnR = 1, TnD = 0, TagAccess = 0
CPU: 10 UID: 0 PID: 3487 Comm: kdamond.2
pc : contpte_test_and_clear_young_ptes+0x70/0xc0
lr : damon_ptep_mkold+0x1e8/0x1f8
Call trace:
contpte_test_and_clear_young_ptes+0x70/0xc0 (P)
damon_mkold_pmd_entry+0x150/0x170
walk_pmd_range+0x110/0x2b0
walk_pud_range+0x10c/0x208
walk_pgd_range+0x134/0x258
__walk_page_range+0x98/0x1b0
walk_page_range_vma_unsafe+0x90/0x148
walk_page_range_vma+0x28/0x40
damon_va_walk_page_range+0x114/0x2b8
damon_va_prepare_access_checks+0xec/0x1a8
kdamond_fn+0x534/0x770
kthread+0x128/0x138
ret_from_fork+0x10/0x20
Otherwise the page is writable, the PTE_AF clearing succeeds silently
and the damage only surfaces later, in whatever happened to own the
page, so the backtrace is unrelated to DAMON and differs between runs.
Align the address down to a page boundary in damon_va_mkold() and
damon_va_young(), the two users that pass it into a page table walk. It
is the address of the page to sample, so this matches its intended
meaning. r->sampling_addr itself is left as is, so the sampling and
region bookkeeping semantics are unchanged.
Fixes: 6f0e1142173a ("arm64: mm: support batch clearing of the young flag for large folios")
Cc: Baolin Wang <baolin.wang@linux.alibaba.com>
Cc: David Hildenbrand (Arm) <david@kernel.org>
Cc: Ryan Roberts <ryan.roberts@arm.com>
Cc: stable@vger.kernel.org
Signed-off-by: Nathan Gao <zcgao@amazon.com>
---
V1 -> V2:
- Align inside damon_va_mkold() and damon_va_young(), the two users that
pass the address into a page table walk, rather than aligning
r->sampling_addr itself, so that sub-page sampling addresses remain
possible for future non-PTE access check primitives (SJ)
- Point Fixes: at 6f0e1142173a instead of 3f49584b262c, since the
unaligned address was harmless before that commit (SJ)
- Describe how the issue was noticed and what it does to the kernel (SJ)
- Reword the subject to match the narrower change
v1: https://lore.kernel.org/all/20260827193821.46115-1-zcgao@amazon.com/
mm/damon/vaddr.c | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/mm/damon/vaddr.c b/mm/damon/vaddr.c
index 2c1c1952c008d..e7aa18200088f 100644
--- a/mm/damon/vaddr.c
+++ b/mm/damon/vaddr.c
@@ -349,6 +349,9 @@ static void damon_va_mkold(struct mm_struct *mm, unsigned long addr)
.hugetlb_entry = damon_mkold_hugetlb_entry,
};
+ /* Arch helpers can derive a page range from @addr; align it down. */
+ addr = PAGE_ALIGN_DOWN(addr);
+
damon_va_walk_page_range(mm, addr, addr + 1, &damon_mkold_ops, NULL);
}
@@ -482,6 +485,9 @@ static bool damon_va_young(struct mm_struct *mm, unsigned long addr,
.hugetlb_entry = damon_young_hugetlb_entry,
};
+ /* Arch helpers can derive a page range from @addr; align it down. */
+ addr = PAGE_ALIGN_DOWN(addr);
+
damon_va_walk_page_range(mm, addr, addr + 1, &damon_young_ops, &arg);
return arg.young;
}
--
2.50.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH v2] mm/damon/vaddr: use a page-aligned address for the sampling walks
2026-08-31 22:11 [PATCH v2] mm/damon/vaddr: use a page-aligned address for the sampling walks Nathan Gao
@ 2026-08-31 22:39 ` sashiko-bot
2026-09-01 1:27 ` SJ Park
2026-09-01 1:35 ` SJ Park
1 sibling, 1 reply; 6+ messages in thread
From: sashiko-bot @ 2026-08-31 22:39 UTC (permalink / raw)
To: Nathan Gao; +Cc: damon
Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider:
Pre-existing issues:
- [High] The patch fails to fix the unaligned page table walk vulnerability for `r->ar.start` in DAMOS migration and stat paths.
- [High] Incorrect PTE advancement for large folios in `damos_va_migrate_pmd_entry` and `damos_va_stat_pmd_entry` causes unrelated PTEs to be silently skipped.
- [High] `damos_madvise` miscalculates the madvise range when region boundaries are not page-aligned, potentially applying actions to memory completely outside the region.
- [High] Unaligned huge page address passed to `mmu_notifier_clear_young()` corrupts adjacent huge pages' access bits.
--
commit fa91d6928f529249184a9980dfc55fc3ff427ea8
Author: Nathan Gao <zcgao@amazon.com>
mm/damon/vaddr: use a page-aligned address for the sampling walks
This commit aligns the sampling address passed to damon_va_walk_page_range()
down to a page boundary in damon_va_mkold() and damon_va_young(). This
prevents an out-of-bounds page table access on arm64 caused by clearing the
young flag on a batch of PTEs derived from an unaligned address.
> diff --git a/mm/damon/vaddr.c b/mm/damon/vaddr.c
> index 0648400b2d65b..fe76714d8e3d6 100644
> --- a/mm/damon/vaddr.c
> +++ b/mm/damon/vaddr.c
> @@ -350,6 +350,9 @@ static void damon_va_mkold(struct mm_struct *mm, unsigned long addr)
> .hugetlb_entry = damon_mkold_hugetlb_entry,
> };
>
> + /* Arch helpers can derive a page range from @addr; align it down. */
> + addr = PAGE_ALIGN_DOWN(addr);
> +
[Severity: High]
This is a pre-existing issue, but does this patch miss fixing the same
unaligned page table walk vulnerability for r->ar.start in the DAMOS
migration and stat paths?
While this commit fixes damon_va_mkold() and damon_va_young() by
aligning the address before walking the page table, DAMON region boundaries
(r->ar.start) can also be unaligned. damos_va_migrate() and
damos_va_stat() pass r->ar.start to damon_va_walk_page_range()
without aligning it.
When a YOUNG filter is applied, this unaligned address propagates through
damos_va_filter_out() to damon_ptep_mkold(..., addr):
mm/damon/vaddr.c:damos_va_migrate() {
...
damon_va_walk_page_range(mm, r->ar.start, r->ar.end, &walk_ops, &priv);
...
}
Will this trigger the exact same out-of-bounds PTE access bug on arm64
that this patch intends to fix?
[Severity: High]
This is a pre-existing issue, but can passing this PAGE_SIZE aligned
address cause state corruption for memory access tracking in secondary MMUs
(like KVM) for huge pages?
The page table walker calls damon_mkold_pmd_entry() and
damon_mkold_hugetlb_entry() with this page-aligned, but not necessarily
huge-page-aligned addr.
In damon_pmdp_mkold(), this unaligned addr is passed verbatim to
mmu_notifier_clear_young():
mm/damon/ops-common.c:damon_pmdp_mkold() {
...
young |= mmu_notifier_clear_young(vma->vm_mm, addr,
addr + HPAGE_PMD_SIZE);
...
}
Because addr is offset within the huge page, does the invalidation range
[addr, addr + HPAGE_PMD_SIZE) span past the end of the current huge page
and corrupt the access bits of the adjacent huge page?
> damon_va_walk_page_range(mm, addr, addr + 1, &damon_mkold_ops, NULL);
> }
[ ... ]
> @@ -476,6 +479,9 @@ static bool damon_va_young(struct mm_struct *mm, unsigned long addr)
> .hugetlb_entry = damon_young_hugetlb_entry,
> };
>
> + /* Arch helpers can derive a page range from @addr; align it down. */
> + addr = PAGE_ALIGN_DOWN(addr);
> +
[Severity: High]
This is a pre-existing issue, but does the PTE loop in
damos_va_migrate_pmd_entry() and damos_va_stat_pmd_entry() incorrectly
skip unrelated PTEs for large folios?
In these PMD entry callbacks, the advancement relies on
folio_nr_pages(folio):
mm/damon/vaddr.c:damos_va_migrate_pmd_entry() {
...
nr = folio_nr_pages(folio);
...
pte += nr;
addr += nr * PAGE_SIZE;
...
}
If the iteration starts on a tail subpage of a large folio (for example, if
the region boundary r->ar.start lands in the middle of a folio), the
remaining pages of that folio in the current PMD are fewer than
folio_nr_pages(folio). By advancing by the full folio_nr_pages(folio),
will this overstep the end of the folio and silently skip subsequent,
unrelated PTEs?
[Severity: High]
This is a pre-existing issue, but does damos_madvise() miscalculate the
range when region boundaries are not page-aligned, potentially applying
actions to memory completely outside the target region?
The boundaries are calculated using PAGE_ALIGN:
mm/damon/vaddr.c:damos_madvise() {
...
unsigned long start = PAGE_ALIGN(r->ar.start);
unsigned long len = PAGE_ALIGN(damon_sz_region(r));
...
}
If a region spans from 0x10005 to 0x11000 (size 0xFFB), start
becomes 0x11000 and len becomes 0x1000. The resulting do_madvise()
range is 0x11000 to 0x12000. Will this completely miss the target
region (which ends at 0x11000) and incorrectly apply destructive actions
like MADV_PAGEOUT or MADV_FREE to the page entirely after the region?
> damon_va_walk_page_range(mm, addr, addr + 1, &damon_young_ops, &arg);
> return arg.young;
> }
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260831221151.50561-1-zcgao@amazon.com?part=1
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2] mm/damon/vaddr: use a page-aligned address for the sampling walks
2026-08-31 22:39 ` sashiko-bot
@ 2026-09-01 1:27 ` SJ Park
0 siblings, 0 replies; 6+ messages in thread
From: SJ Park @ 2026-09-01 1:27 UTC (permalink / raw)
To: sashiko-bot; +Cc: SJ Park, Nathan Gao, damon
On Mon, 31 Aug 2026 22:39:36 +0000 sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider:
>
> Pre-existing issues:
> - [High] The patch fails to fix the unaligned page table walk vulnerability for `r->ar.start` in DAMOS migration and stat paths.
> - [High] Incorrect PTE advancement for large folios in `damos_va_migrate_pmd_entry` and `damos_va_stat_pmd_entry` causes unrelated PTEs to be silently skipped.
> - [High] `damos_madvise` miscalculates the madvise range when region boundaries are not page-aligned, potentially applying actions to memory completely outside the region.
> - [High] Unaligned huge page address passed to `mmu_notifier_clear_young()` corrupts adjacent huge pages' access bits.
> --
>
> commit fa91d6928f529249184a9980dfc55fc3ff427ea8
> Author: Nathan Gao <zcgao@amazon.com>
>
> mm/damon/vaddr: use a page-aligned address for the sampling walks
>
> This commit aligns the sampling address passed to damon_va_walk_page_range()
> down to a page boundary in damon_va_mkold() and damon_va_young(). This
> prevents an out-of-bounds page table access on arm64 caused by clearing the
> young flag on a batch of PTEs derived from an unaligned address.
>
> > diff --git a/mm/damon/vaddr.c b/mm/damon/vaddr.c
> > index 0648400b2d65b..fe76714d8e3d6 100644
> > --- a/mm/damon/vaddr.c
> > +++ b/mm/damon/vaddr.c
> > @@ -350,6 +350,9 @@ static void damon_va_mkold(struct mm_struct *mm, unsigned long addr)
> > .hugetlb_entry = damon_mkold_hugetlb_entry,
> > };
> >
> > + /* Arch helpers can derive a page range from @addr; align it down. */
> > + addr = PAGE_ALIGN_DOWN(addr);
> > +
>
> [Severity: High]
> This is a pre-existing issue, but does this patch miss fixing the same
> unaligned page table walk vulnerability for r->ar.start in the DAMOS
> migration and stat paths?
>
> While this commit fixes damon_va_mkold() and damon_va_young() by
> aligning the address before walking the page table, DAMON region boundaries
> (r->ar.start) can also be unaligned. damos_va_migrate() and
> damos_va_stat() pass r->ar.start to damon_va_walk_page_range()
> without aligning it.
>
> When a YOUNG filter is applied, this unaligned address propagates through
> damos_va_filter_out() to damon_ptep_mkold(..., addr):
>
> mm/damon/vaddr.c:damos_va_migrate() {
> ...
> damon_va_walk_page_range(mm, r->ar.start, r->ar.end, &walk_ops, &priv);
> ...
> }
>
> Will this trigger the exact same out-of-bounds PTE access bug on arm64
> that this patch intends to fix?
For vaddr case, no DAMON API callers oerride min_region_sz. So always regions
are aligned to default min_region_sz, which is PAGE_SIZE. So it should be
fine.
>
>
> [Severity: High]
> This is a pre-existing issue, but can passing this PAGE_SIZE aligned
> address cause state corruption for memory access tracking in secondary MMUs
> (like KVM) for huge pages?
>
> The page table walker calls damon_mkold_pmd_entry() and
> damon_mkold_hugetlb_entry() with this page-aligned, but not necessarily
> huge-page-aligned addr.
>
> In damon_pmdp_mkold(), this unaligned addr is passed verbatim to
> mmu_notifier_clear_young():
>
> mm/damon/ops-common.c:damon_pmdp_mkold() {
> ...
> young |= mmu_notifier_clear_young(vma->vm_mm, addr,
> addr + HPAGE_PMD_SIZE);
> ...
> }
>
> Because addr is offset within the huge page, does the invalidation range
> [addr, addr + HPAGE_PMD_SIZE) span past the end of the current huge page
> and corrupt the access bits of the adjacent huge page?
I believe clearing access bits more or less than needed should be fine, as long
as those are correctly access bits, since it is not corrupting memory. Let me
know if I'm missing something.
>
> > damon_va_walk_page_range(mm, addr, addr + 1, &damon_mkold_ops, NULL);
> > }
>
> [ ... ]
>
> > @@ -476,6 +479,9 @@ static bool damon_va_young(struct mm_struct *mm, unsigned long addr)
> > .hugetlb_entry = damon_young_hugetlb_entry,
> > };
> >
> > + /* Arch helpers can derive a page range from @addr; align it down. */
> > + addr = PAGE_ALIGN_DOWN(addr);
> > +
>
> [Severity: High]
> This is a pre-existing issue, but does the PTE loop in
> damos_va_migrate_pmd_entry() and damos_va_stat_pmd_entry() incorrectly
> skip unrelated PTEs for large folios?
>
> In these PMD entry callbacks, the advancement relies on
> folio_nr_pages(folio):
>
> mm/damon/vaddr.c:damos_va_migrate_pmd_entry() {
> ...
> nr = folio_nr_pages(folio);
> ...
> pte += nr;
> addr += nr * PAGE_SIZE;
> ...
> }
>
> If the iteration starts on a tail subpage of a large folio (for example, if
> the region boundary r->ar.start lands in the middle of a folio), the
> remaining pages of that folio in the current PMD are fewer than
> folio_nr_pages(folio). By advancing by the full folio_nr_pages(folio),
> will this overstep the end of the folio and silently skip subsequent,
> unrelated PTEs?
We are separately working [1] on this.
>
>
> [Severity: High]
> This is a pre-existing issue, but does damos_madvise() miscalculate the
> range when region boundaries are not page-aligned, potentially applying
> actions to memory completely outside the target region?
>
> The boundaries are calculated using PAGE_ALIGN:
>
> mm/damon/vaddr.c:damos_madvise() {
> ...
> unsigned long start = PAGE_ALIGN(r->ar.start);
> unsigned long len = PAGE_ALIGN(damon_sz_region(r));
> ...
> }
>
> If a region spans from 0x10005 to 0x11000 (size 0xFFB), start
> becomes 0x11000 and len becomes 0x1000. The resulting do_madvise()
> range is 0x11000 to 0x12000. Will this completely miss the target
> region (which ends at 0x11000) and incorrectly apply destructive actions
> like MADV_PAGEOUT or MADV_FREE to the page entirely after the region?
As I mentioned above, vaddr will always have PAGE_SIZE aligned regions, so this
issue doesn't happen.
>
> > damon_va_walk_page_range(mm, addr, addr + 1, &damon_young_ops, &arg);
> > return arg.young;
> > }
>
> --
> Sashiko AI review · https://sashiko.dev/#/patchset/20260831221151.50561-1-zcgao@amazon.com?part=1
>
[1] https://lore.kernel.org/20260831140254.72004-1-sj@kernel.org
Thanks,
SJ
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2] mm/damon/vaddr: use a page-aligned address for the sampling walks
2026-08-31 22:11 [PATCH v2] mm/damon/vaddr: use a page-aligned address for the sampling walks Nathan Gao
2026-08-31 22:39 ` sashiko-bot
@ 2026-09-01 1:35 ` SJ Park
2026-09-01 20:25 ` Nathan Gao
1 sibling, 1 reply; 6+ messages in thread
From: SJ Park @ 2026-09-01 1:35 UTC (permalink / raw)
To: Nathan Gao
Cc: SJ Park, akpm, damon, linux-mm, linux-kernel, baolin.wang, david,
ryan.roberts
On Mon, 31 Aug 2026 15:11:51 -0700 Nathan Gao <zcgao@amazon.com> wrote:
> __damon_va_prepare_access_check() picks a random byte address within the
> region and stores it in r->sampling_addr. There are two users of
> r->sampling_addr in vaddr.c that pass it into a page table walk, and
> both use it as the address of a page.
>
> damon_va_mkold(mm, r->sampling_addr)
> damon_va_walk_page_range(mm, addr, addr + 1)
> damon_mkold_pmd_entry()
> damon_ptep_mkold(pte, vma, addr)
> ptep_test_and_clear_young(vma, addr, pte)
> mmu_notifier_clear_young(mm, addr, addr + PAGE_SIZE)
>
> damon_va_young(mm, r->sampling_addr, &folio_sz)
> damon_va_walk_page_range(mm, addr, addr + 1)
> damon_young_pmd_entry()
> ptep_get(pte)
> mmu_notifier_test_young(walk->mm, addr)
>
> For arm64, before commit 6f0e1142173a ("arm64: mm: support batch
> clearing of the young flag for large folios"), the contpte helper walked
> exactly CONT_PTES entries from the aligned-down page table pointer and
> used @addr only to pass down to each entry, so an unaligned value was
> harmless:
>
> ptep = contpte_align_down(ptep);
> addr = ALIGN_DOWN(addr, CONT_PTE_SIZE);
> for (i = 0; i < CONT_PTES; i++, ptep++, addr += PAGE_SIZE)
>
> Now the range to walk is derived from @addr instead: end = addr +
> nr * PAGE_SIZE, rounded up to CONT_PTE_SIZE. For a sample in the last
> page of a contpte block, the sub-page offset puts end just past the
> block boundary, so the round-up lands a whole block further and the
> walk clears PTE_AF in CONT_PTES entries beyond the sampled block. For
> the last block in a page table page, those entries are past the end of
> that page, so the walk writes into the page that follows.
>
> Triggered by the full 7.1/7.2 kernel selftest suite on arm64 (EC2
> c/m6g.4xlarge). The kernel sometimes crashes at or shortly after the
> DAMON test.
>
> What the overrun does depends on the page that happens to follow the
> page table, so there is no single signature. If that page is read-only,
> the write faults in the sampling path itself:
>
> Unable to handle kernel write to read-only memory at virtual address ffff0003c5d2d000
> FSC = 0x0f: level 3 permission fault
> CM = 0, WnR = 1, TnD = 0, TagAccess = 0
> CPU: 10 UID: 0 PID: 3487 Comm: kdamond.2
> pc : contpte_test_and_clear_young_ptes+0x70/0xc0
> lr : damon_ptep_mkold+0x1e8/0x1f8
> Call trace:
> contpte_test_and_clear_young_ptes+0x70/0xc0 (P)
> damon_mkold_pmd_entry+0x150/0x170
> walk_pmd_range+0x110/0x2b0
> walk_pud_range+0x10c/0x208
> walk_pgd_range+0x134/0x258
> __walk_page_range+0x98/0x1b0
> walk_page_range_vma_unsafe+0x90/0x148
> walk_page_range_vma+0x28/0x40
> damon_va_walk_page_range+0x114/0x2b8
> damon_va_prepare_access_checks+0xec/0x1a8
> kdamond_fn+0x534/0x770
> kthread+0x128/0x138
> ret_from_fork+0x10/0x20
>
> Otherwise the page is writable, the PTE_AF clearing succeeds silently
> and the damage only surfaces later, in whatever happened to own the
> page, so the backtrace is unrelated to DAMON and differs between runs.
Urgh, this must have been a painful debugging. Sorry about that, and
appreciate your great work on this!
>
> Align the address down to a page boundary in damon_va_mkold() and
> damon_va_young(), the two users that pass it into a page table walk. It
> is the address of the page to sample, so this matches its intended
> meaning. r->sampling_addr itself is left as is, so the sampling and
> region bookkeeping semantics are unchanged.
I'm still wondering if it makes sense to restore unaligned address support in
contpte_test_and_clear_young_ptes() as a long term fix.
>
> Fixes: 6f0e1142173a ("arm64: mm: support batch clearing of the young flag for large folios")
> Cc: Baolin Wang <baolin.wang@linux.alibaba.com>
> Cc: David Hildenbrand (Arm) <david@kernel.org>
> Cc: Ryan Roberts <ryan.roberts@arm.com>
> Cc: stable@vger.kernel.org
> Signed-off-by: Nathan Gao <zcgao@amazon.com>
> ---
> V1 -> V2:
> - Align inside damon_va_mkold() and damon_va_young(), the two users that
> pass the address into a page table walk, rather than aligning
> r->sampling_addr itself, so that sub-page sampling addresses remain
> possible for future non-PTE access check primitives (SJ)
> - Point Fixes: at 6f0e1142173a instead of 3f49584b262c, since the
> unaligned address was harmless before that commit (SJ)
> - Describe how the issue was noticed and what it does to the kernel (SJ)
> - Reword the subject to match the narrower change
>
> v1: https://lore.kernel.org/all/20260827193821.46115-1-zcgao@amazon.com/
>
> mm/damon/vaddr.c | 6 ++++++
> 1 file changed, 6 insertions(+)
>
> diff --git a/mm/damon/vaddr.c b/mm/damon/vaddr.c
> index 2c1c1952c008d..e7aa18200088f 100644
> --- a/mm/damon/vaddr.c
> +++ b/mm/damon/vaddr.c
> @@ -349,6 +349,9 @@ static void damon_va_mkold(struct mm_struct *mm, unsigned long addr)
> .hugetlb_entry = damon_mkold_hugetlb_entry,
> };
>
> + /* Arch helpers can derive a page range from @addr; align it down. */
> + addr = PAGE_ALIGN_DOWN(addr);
> +
> damon_va_walk_page_range(mm, addr, addr + 1, &damon_mkold_ops, NULL);
Could we do the alignment just before passing the addr to
contpte_test_and_clear_young_ptes(), which is the exact function that disallows
the unaligned address?
> }
>
> @@ -482,6 +485,9 @@ static bool damon_va_young(struct mm_struct *mm, unsigned long addr,
> .hugetlb_entry = damon_young_hugetlb_entry,
> };
>
> + /* Arch helpers can derive a page range from @addr; align it down. */
> + addr = PAGE_ALIGN_DOWN(addr);
> +
> damon_va_walk_page_range(mm, addr, addr + 1, &damon_young_ops, &arg);
Ditto.
> return arg.young;
> }
> --
> 2.50.1
>
>
Thanks,
SJ
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2] mm/damon/vaddr: use a page-aligned address for the sampling walks
2026-09-01 1:35 ` SJ Park
@ 2026-09-01 20:25 ` Nathan Gao
2026-09-02 0:16 ` SJ Park
0 siblings, 1 reply; 6+ messages in thread
From: Nathan Gao @ 2026-09-01 20:25 UTC (permalink / raw)
To: SJ Park
Cc: Nathan Gao, akpm, damon, linux-mm, linux-kernel, baolin.wang,
david, ryan.roberts
On Mon, 31 Aug 2026 18:35:50 -0700 SJ Park <sj@kernel.org> wrote:
> On Mon, 31 Aug 2026 15:11:51 -0700 Nathan Gao <zcgao@amazon.com> wrote:
>
> > __damon_va_prepare_access_check() picks a random byte address within the
> > region and stores it in r->sampling_addr. There are two users of
> > r->sampling_addr in vaddr.c that pass it into a page table walk, and
> > both use it as the address of a page.
> >
> > damon_va_mkold(mm, r->sampling_addr)
> > damon_va_walk_page_range(mm, addr, addr + 1)
> > damon_mkold_pmd_entry()
> > damon_ptep_mkold(pte, vma, addr)
> > ptep_test_and_clear_young(vma, addr, pte)
> > mmu_notifier_clear_young(mm, addr, addr + PAGE_SIZE)
> >
> > damon_va_young(mm, r->sampling_addr, &folio_sz)
> > damon_va_walk_page_range(mm, addr, addr + 1)
> > damon_young_pmd_entry()
> > ptep_get(pte)
> > mmu_notifier_test_young(walk->mm, addr)
> >
> > For arm64, before commit 6f0e1142173a ("arm64: mm: support batch
> > clearing of the young flag for large folios"), the contpte helper walked
> > exactly CONT_PTES entries from the aligned-down page table pointer and
> > used @addr only to pass down to each entry, so an unaligned value was
> > harmless:
> >
> > ptep = contpte_align_down(ptep);
> > addr = ALIGN_DOWN(addr, CONT_PTE_SIZE);
> > for (i = 0; i < CONT_PTES; i++, ptep++, addr += PAGE_SIZE)
> >
> > Now the range to walk is derived from @addr instead: end = addr +
> > nr * PAGE_SIZE, rounded up to CONT_PTE_SIZE. For a sample in the last
> > page of a contpte block, the sub-page offset puts end just past the
> > block boundary, so the round-up lands a whole block further and the
> > walk clears PTE_AF in CONT_PTES entries beyond the sampled block. For
> > the last block in a page table page, those entries are past the end of
> > that page, so the walk writes into the page that follows.
> >
> > Triggered by the full 7.1/7.2 kernel selftest suite on arm64 (EC2
> > c/m6g.4xlarge). The kernel sometimes crashes at or shortly after the
> > DAMON test.
> >
> > What the overrun does depends on the page that happens to follow the
> > page table, so there is no single signature. If that page is read-only,
> > the write faults in the sampling path itself:
> >
> > Unable to handle kernel write to read-only memory at virtual address ffff0003c5d2d000
> > FSC = 0x0f: level 3 permission fault
> > CM = 0, WnR = 1, TnD = 0, TagAccess = 0
> > CPU: 10 UID: 0 PID: 3487 Comm: kdamond.2
> > pc : contpte_test_and_clear_young_ptes+0x70/0xc0
> > lr : damon_ptep_mkold+0x1e8/0x1f8
> > Call trace:
> > contpte_test_and_clear_young_ptes+0x70/0xc0 (P)
> > damon_mkold_pmd_entry+0x150/0x170
> > walk_pmd_range+0x110/0x2b0
> > walk_pud_range+0x10c/0x208
> > walk_pgd_range+0x134/0x258
> > __walk_page_range+0x98/0x1b0
> > walk_page_range_vma_unsafe+0x90/0x148
> > walk_page_range_vma+0x28/0x40
> > damon_va_walk_page_range+0x114/0x2b8
> > damon_va_prepare_access_checks+0xec/0x1a8
> > kdamond_fn+0x534/0x770
> > kthread+0x128/0x138
> > ret_from_fork+0x10/0x20
> >
> > Otherwise the page is writable, the PTE_AF clearing succeeds silently
> > and the damage only surfaces later, in whatever happened to own the
> > page, so the backtrace is unrelated to DAMON and differs between runs.
>
> Urgh, this must have been a painful debugging. Sorry about that, and
> appreciate your great work on this!
>
No problem at all! Had fun digging into this.
> >
> > Align the address down to a page boundary in damon_va_mkold() and
> > damon_va_young(), the two users that pass it into a page table walk. It
> > is the address of the page to sample, so this matches its intended
> > meaning. r->sampling_addr itself is left as is, so the sampling and
> > region bookkeeping semantics are unchanged.
>
> I'm still wondering if it makes sense to restore unaligned address support in
> contpte_test_and_clear_young_ptes() as a long term fix.
>
I'd appreciate Baolin's thoughts here. If it turns out callers are
expected to do the alignment, it would be better to have a WARN to expose
the issue.
> >
> > Fixes: 6f0e1142173a ("arm64: mm: support batch clearing of the young flag for large folios")
> > Cc: Baolin Wang <baolin.wang@linux.alibaba.com>
> > Cc: David Hildenbrand (Arm) <david@kernel.org>
> > Cc: Ryan Roberts <ryan.roberts@arm.com>
> > Cc: stable@vger.kernel.org
> > Signed-off-by: Nathan Gao <zcgao@amazon.com>
> > ---
> > V1 -> V2:
> > - Align inside damon_va_mkold() and damon_va_young(), the two users that
> > pass the address into a page table walk, rather than aligning
> > r->sampling_addr itself, so that sub-page sampling addresses remain
> > possible for future non-PTE access check primitives (SJ)
> > - Point Fixes: at 6f0e1142173a instead of 3f49584b262c, since the
> > unaligned address was harmless before that commit (SJ)
> > - Describe how the issue was noticed and what it does to the kernel (SJ)
> > - Reword the subject to match the narrower change
> >
> > v1: https://lore.kernel.org/all/20260827193821.46115-1-zcgao@amazon.com/
> >
> > mm/damon/vaddr.c | 6 ++++++
> > 1 file changed, 6 insertions(+)
> >
> > diff --git a/mm/damon/vaddr.c b/mm/damon/vaddr.c
> > index 2c1c1952c008d..e7aa18200088f 100644
> > --- a/mm/damon/vaddr.c
> > +++ b/mm/damon/vaddr.c
> > @@ -349,6 +349,9 @@ static void damon_va_mkold(struct mm_struct *mm, unsigned long addr)
> > .hugetlb_entry = damon_mkold_hugetlb_entry,
> > };
> >
> > + /* Arch helpers can derive a page range from @addr; align it down. */
> > + addr = PAGE_ALIGN_DOWN(addr);
> > +
> > damon_va_walk_page_range(mm, addr, addr + 1, &damon_mkold_ops, NULL);
>
> Could we do the alignment just before passing the addr to
> contpte_test_and_clear_young_ptes(), which is the exact function that disallows
> the unaligned address?
>
Sent a v3. It moves the alignment into damon_ptep_mkold(), the only place
in DAMON that reaches contpte_test_and_clear_young_ptes(), so it is as
close to that function as DAMON can get:
https://lore.kernel.org/all/20260901201001.33271-1-zcgao@amazon.com/
> > }
> >
> > @@ -482,6 +485,9 @@ static bool damon_va_young(struct mm_struct *mm, unsigned long addr,
> > .hugetlb_entry = damon_young_hugetlb_entry,
> > };
> >
> > + /* Arch helpers can derive a page range from @addr; align it down. */
> > + addr = PAGE_ALIGN_DOWN(addr);
> > +
> > damon_va_walk_page_range(mm, addr, addr + 1, &damon_young_ops, &arg);
>
> Ditto.
>
> > return arg.young;
> > }
> > --
> > 2.50.1
> >
> >
>
>
> Thanks,
> SJ
Thanks,
Nathan
Sent using hkml (https://github.com/sjp38/hackermail)
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2] mm/damon/vaddr: use a page-aligned address for the sampling walks
2026-09-01 20:25 ` Nathan Gao
@ 2026-09-02 0:16 ` SJ Park
0 siblings, 0 replies; 6+ messages in thread
From: SJ Park @ 2026-09-02 0:16 UTC (permalink / raw)
To: Nathan Gao
Cc: SJ Park, akpm, damon, linux-mm, linux-kernel, baolin.wang, david,
ryan.roberts
On Tue, 1 Sep 2026 13:25:36 -0700 Nathan Gao <zcgao@amazon.com> wrote:
> On Mon, 31 Aug 2026 18:35:50 -0700 SJ Park <sj@kernel.org> wrote:
>
> > On Mon, 31 Aug 2026 15:11:51 -0700 Nathan Gao <zcgao@amazon.com> wrote:
[...]
> > >
> > > Otherwise the page is writable, the PTE_AF clearing succeeds silently
> > > and the damage only surfaces later, in whatever happened to own the
> > > page, so the backtrace is unrelated to DAMON and differs between runs.
> >
> > Urgh, this must have been a painful debugging. Sorry about that, and
> > appreciate your great work on this!
> >
>
> No problem at all! Had fun digging into this.
Thank you for that kind ack!
>
> > >
> > > Align the address down to a page boundary in damon_va_mkold() and
> > > damon_va_young(), the two users that pass it into a page table walk. It
> > > is the address of the page to sample, so this matches its intended
> > > meaning. r->sampling_addr itself is left as is, so the sampling and
> > > region bookkeeping semantics are unchanged.
> >
> > I'm still wondering if it makes sense to restore unaligned address support in
> > contpte_test_and_clear_young_ptes() as a long term fix.
> >
>
> I'd appreciate Baolin's thoughts here. If it turns out callers are
> expected to do the alignment, it would be better to have a WARN to expose
> the issue.
I agree. If we conclude contpte helpers are not the right place to warn, we
should do that at least in DAMON internal helpers.
[...]
> > > --- a/mm/damon/vaddr.c
> > > +++ b/mm/damon/vaddr.c
> > > @@ -349,6 +349,9 @@ static void damon_va_mkold(struct mm_struct *mm, unsigned long addr)
> > > .hugetlb_entry = damon_mkold_hugetlb_entry,
> > > };
> > >
> > > + /* Arch helpers can derive a page range from @addr; align it down. */
> > > + addr = PAGE_ALIGN_DOWN(addr);
> > > +
> > > damon_va_walk_page_range(mm, addr, addr + 1, &damon_mkold_ops, NULL);
> >
> > Could we do the alignment just before passing the addr to
> > contpte_test_and_clear_young_ptes(), which is the exact function that disallows
> > the unaligned address?
> >
>
> Sent a v3. It moves the alignment into damon_ptep_mkold(), the only place
> in DAMON that reaches contpte_test_and_clear_young_ptes(), so it is as
> close to that function as DAMON can get:
>
> https://lore.kernel.org/all/20260901201001.33271-1-zcgao@amazon.com/
Thank you, I left comments. Please forgive me being picky there.
Thanks,
SJ
[...]
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2026-09-02 0:16 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-08-31 22:11 [PATCH v2] mm/damon/vaddr: use a page-aligned address for the sampling walks Nathan Gao
2026-08-31 22:39 ` sashiko-bot
2026-09-01 1:27 ` SJ Park
2026-09-01 1:35 ` SJ Park
2026-09-01 20:25 ` Nathan Gao
2026-09-02 0:16 ` SJ Park
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox