* [PATCH 1/7] mm/damon/core: initialize damos->last_applied
2026-07-17 0:30 [PATCH 0/7] mm/damon: fix uninitialized DAMOS field and kunit exec expectation bugs SJ Park
@ 2026-07-17 0:30 ` SJ Park
2026-07-17 0:52 ` sashiko-bot
2026-07-17 0:30 ` [PATCH 2/7] mm/damon/core-kunit: check region count before testing in split_at() SJ Park
` (6 subsequent siblings)
7 siblings, 1 reply; 17+ messages in thread
From: SJ Park @ 2026-07-17 0:30 UTC (permalink / raw)
Cc: SJ Park, stable, Andrew Morton, Brendan Higgins, David Gow, damon,
kunit-dev, linux-kernel, linux-kselftest, linux-mm
Multiple DAMON regions could exist across a folio. If they fulfill the
condition to apply a DAMOS scheme, the scheme could be applied multiple
times to the folio. To avoid this, each DAMOS scheme stores the folio
that the scheme was applied to last time in the damos->last_applied
field and skips repeatedly applying the same scheme to the same folio.
The field is being used without initialization, though. Hence, the
mechanism could wrongly skip applying a scheme to a folio at the very
first time of DAMOS run.
The user impact is trivial. DAMON might unexpectedly skip applying
DAMOS action for one folio for the first time per scheme. In the
DAMON's best-effort world, this is never a real problem. No critical
consequences such as kernel panic or memory corruption happen.
It is a clear bug, though, and the fix is straightforward. Fix the
issue by initializing the field in DAMOS scheme creation function,
damon_new_scheme().
The issue was discovered [1] by Sashiko.
[1] https://lore.kernel.org/20260714055436.120034-1-sj@kernel.org
Fixes: 94ba17adaba0 ("mm/damon: avoid applying DAMOS action to same entity multiple times")
Cc: <stable@vger.kernel.org> # 6.15.x
Signed-off-by: SJ Park <sj@kernel.org>
---
mm/damon/core.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/mm/damon/core.c b/mm/damon/core.c
index f464b4f0976c3..60255f5cd715e 100644
--- a/mm/damon/core.c
+++ b/mm/damon/core.c
@@ -705,6 +705,7 @@ struct damos *damon_new_scheme(struct damos_access_pattern *pattern,
INIT_LIST_HEAD(&scheme->ops_filters);
scheme->stat = (struct damos_stat){};
scheme->max_nr_snapshots = 0;
+ scheme->last_applied = NULL;
INIT_LIST_HEAD(&scheme->list);
scheme->quota = *(damos_quota_init(quota));
--
2.47.3
^ permalink raw reply related [flat|nested] 17+ messages in thread* Re: [PATCH 1/7] mm/damon/core: initialize damos->last_applied
2026-07-17 0:30 ` [PATCH 1/7] mm/damon/core: initialize damos->last_applied SJ Park
@ 2026-07-17 0:52 ` sashiko-bot
2026-07-17 1:12 ` SJ Park
0 siblings, 1 reply; 17+ messages in thread
From: sashiko-bot @ 2026-07-17 0:52 UTC (permalink / raw)
To: SJ Park; +Cc: damon
Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
Pre-existing issues:
- [High] Incorrect PTE advancement for large folios in vaddr stat scanning and migration.
- [High] Commit of DAMON parameters corrupts the active scheme's quota state by zeroing dynamic fields in-place.
- [High] Failure in `damos_commit_dests` leaves the active scheme in an inconsistent state, leading to a NULL pointer dereference.
--
--- Patch [1]: [PATCH 1/7] mm/damon/core: initialize damos->last_applied ---
commit 90344a381581ba5ad7a7c87826b18bf948659516
Author: SJ Park <sj@kernel.org>
mm/damon/core: initialize damos->last_applied
This commit fixes an issue where the damos->last_applied field was being used
without initialization, leading to DAMON unexpectedly skipping the application
of DAMOS actions for a folio the first time per scheme. It addresses this by
initializing the field in damon_new_scheme().
Link: https://lore.kernel.org/20260714055436.120034-1-sj@kernel.org
> diff --git a/mm/damon/core.c b/mm/damon/core.c
> index 603b102ff80f9..20d267c615faf 100644
> --- a/mm/damon/core.c
> +++ b/mm/damon/core.c
> @@ -705,6 +705,7 @@ struct damos *damon_new_scheme(struct damos_access_pattern *pattern,
> INIT_LIST_HEAD(&scheme->ops_filters);
> scheme->stat = (struct damos_stat){};
> scheme->max_nr_snapshots = 0;
> + scheme->last_applied = NULL;
> INIT_LIST_HEAD(&scheme->list);
>
> scheme->quota = *(damos_quota_init(quota));
^^^^^^^^^^^^^^^^^^^^^^^^
[Severity: High]
This isn't a bug introduced by this patch, but does this in-place zeroing
corrupt the active scheme's quota state?
Looking at damon_new_scheme(), it passes the provided quota pointer to
damos_quota_init(), which zeros out dynamic fields directly:
mm/damon/core.c:damos_quota_init() {
...
quota->esz = 0;
quota->total_charged_sz = 0;
...
}
When damon_commit_ctx() applies new parameters to an active context, it copies
schemes by calling damon_new_scheme() with a pointer to the running context's
scheme quota.
Could this wipe out the current quota charge window progress and reset the
goal tuner's feedback loop (esz_bp) on every parameter commit, causing
inconsistent tuning pressure?
[Severity: High]
This is a pre-existing issue, but can the failure path in damos_commit_dests()
lead to a NULL pointer dereference?
In mm/damon/core.c:damos_commit_dests(), if memory allocation fails for
dst->node_id_arr or dst->weight_arr, the function returns -ENOMEM:
mm/damon/core.c:damos_commit_dests() {
...
dst->node_id_arr = kmalloc_array(src->nr_dests,
sizeof(*dst->node_id_arr), GFP_KERNEL);
if (!dst->node_id_arr) {
dst->weight_arr = NULL;
return -ENOMEM;
}
...
}
However, dst->nr_dests is updated later. If the allocation fails while
updating an active context during __damon_commit_ctx(), could the active
scheme be left with weight_arr == NULL but a non-zero nr_dests?
If so, would this cause a kernel panic when kdamond resumes and calls
damos_va_migrate_dests_add(), which iterates up to dests->nr_dests and
unconditionally dereferences dests->weight_arr[i]?
[Severity: High]
This is a pre-existing issue, but does the PTE iteration in
damos_va_stat_pmd_entry() and damos_va_migrate_pmd_entry() overshoot large
folios?
In both functions, when a valid folio is found, the loop increment is set to
the total number of pages in the folio:
mm/damon/vaddr.c:damos_va_stat_pmd_entry() {
...
if (!damos_va_filter_out(s, folio, vma, addr, pte, NULL))
*sz_filter_passed += folio_size(folio);
nr = folio_nr_pages(folio);
s->last_applied = folio;
...
}
If the current PTE maps a tail page of a large folio (for example, if the VMA
starts in the middle of a large folio), wouldn't advancing pte += nr and
addr += nr * PAGE_SIZE overshoot the end of the large folio?
Could this cause DAMON to skip subsequent PTEs in the mapped page table that
might belong to entirely different folios, missing stats and migration actions
for them?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260717003022.119437-1-sj@kernel.org?part=1
^ permalink raw reply [flat|nested] 17+ messages in thread* Re: [PATCH 1/7] mm/damon/core: initialize damos->last_applied
2026-07-17 0:52 ` sashiko-bot
@ 2026-07-17 1:12 ` SJ Park
0 siblings, 0 replies; 17+ messages in thread
From: SJ Park @ 2026-07-17 1:12 UTC (permalink / raw)
To: sashiko-bot; +Cc: SJ Park, damon
On Fri, 17 Jul 2026 00:52:19 +0000 sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
>
> Pre-existing issues:
> - [High] Incorrect PTE advancement for large folios in vaddr stat scanning and migration.
> - [High] Commit of DAMON parameters corrupts the active scheme's quota state by zeroing dynamic fields in-place.
> - [High] Failure in `damos_commit_dests` leaves the active scheme in an inconsistent state, leading to a NULL pointer dereference.
> --
>
> --- Patch [1]: [PATCH 1/7] mm/damon/core: initialize damos->last_applied ---
> commit 90344a381581ba5ad7a7c87826b18bf948659516
> Author: SJ Park <sj@kernel.org>
>
> mm/damon/core: initialize damos->last_applied
>
> This commit fixes an issue where the damos->last_applied field was being used
> without initialization, leading to DAMON unexpectedly skipping the application
> of DAMOS actions for a folio the first time per scheme. It addresses this by
> initializing the field in damon_new_scheme().
>
> Link: https://lore.kernel.org/20260714055436.120034-1-sj@kernel.org
>
> > diff --git a/mm/damon/core.c b/mm/damon/core.c
> > index 603b102ff80f9..20d267c615faf 100644
> > --- a/mm/damon/core.c
> > +++ b/mm/damon/core.c
> > @@ -705,6 +705,7 @@ struct damos *damon_new_scheme(struct damos_access_pattern *pattern,
> > INIT_LIST_HEAD(&scheme->ops_filters);
> > scheme->stat = (struct damos_stat){};
> > scheme->max_nr_snapshots = 0;
> > + scheme->last_applied = NULL;
> > INIT_LIST_HEAD(&scheme->list);
> >
> > scheme->quota = *(damos_quota_init(quota));
> ^^^^^^^^^^^^^^^^^^^^^^^^
>
> [Severity: High]
> This isn't a bug introduced by this patch, but does this in-place zeroing
> corrupt the active scheme's quota state?
>
> Looking at damon_new_scheme(), it passes the provided quota pointer to
> damos_quota_init(), which zeros out dynamic fields directly:
>
> mm/damon/core.c:damos_quota_init() {
> ...
> quota->esz = 0;
> quota->total_charged_sz = 0;
> ...
> }
>
> When damon_commit_ctx() applies new parameters to an active context, it copies
> schemes by calling damon_new_scheme() with a pointer to the running context's
> scheme quota.
>
> Could this wipe out the current quota charge window progress and reset the
> goal tuner's feedback loop (esz_bp) on every parameter commit, causing
> inconsistent tuning pressure?
No. damon_new_scheme() is called only for new scheme addition.
>
>
> [Severity: High]
> This is a pre-existing issue, but can the failure path in damos_commit_dests()
> lead to a NULL pointer dereference?
>
> In mm/damon/core.c:damos_commit_dests(), if memory allocation fails for
> dst->node_id_arr or dst->weight_arr, the function returns -ENOMEM:
>
> mm/damon/core.c:damos_commit_dests() {
> ...
> dst->node_id_arr = kmalloc_array(src->nr_dests,
> sizeof(*dst->node_id_arr), GFP_KERNEL);
> if (!dst->node_id_arr) {
> dst->weight_arr = NULL;
> return -ENOMEM;
> }
> ...
> }
>
> However, dst->nr_dests is updated later. If the allocation fails while
> updating an active context during __damon_commit_ctx(), could the active
> scheme be left with weight_arr == NULL but a non-zero nr_dests?
>
> If so, would this cause a kernel panic when kdamond resumes and calls
> damos_va_migrate_dests_add(), which iterates up to dests->nr_dests and
> unconditionally dereferences dests->weight_arr[i]?
No. If damon_commit_ctx() fails, kdamond stops due to 'maybe_corrupted' field.
There is no way to resume it.
>
>
> [Severity: High]
> This is a pre-existing issue, but does the PTE iteration in
> damos_va_stat_pmd_entry() and damos_va_migrate_pmd_entry() overshoot large
> folios?
>
> In both functions, when a valid folio is found, the loop increment is set to
> the total number of pages in the folio:
>
> mm/damon/vaddr.c:damos_va_stat_pmd_entry() {
> ...
> if (!damos_va_filter_out(s, folio, vma, addr, pte, NULL))
> *sz_filter_passed += folio_size(folio);
> nr = folio_nr_pages(folio);
> s->last_applied = folio;
> ...
> }
>
> If the current PTE maps a tail page of a large folio (for example, if the VMA
> starts in the middle of a large folio), wouldn't advancing pte += nr and
> addr += nr * PAGE_SIZE overshoot the end of the large folio?
>
> Could this cause DAMON to skip subsequent PTEs in the mapped page table that
> might belong to entirely different folios, missing stats and migration actions
> for them?
Known low priority issue.
>
> --
> Sashiko AI review · https://sashiko.dev/#/patchset/20260717003022.119437-1-sj@kernel.org?part=1
Thanks,
SJ
^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH 2/7] mm/damon/core-kunit: check region count before testing in split_at()
2026-07-17 0:30 [PATCH 0/7] mm/damon: fix uninitialized DAMOS field and kunit exec expectation bugs SJ Park
2026-07-17 0:30 ` [PATCH 1/7] mm/damon/core: initialize damos->last_applied SJ Park
@ 2026-07-17 0:30 ` SJ Park
2026-07-17 0:44 ` sashiko-bot
2026-07-17 0:30 ` [PATCH 3/7] mm/damon/vaddr-kunit: check region count in three_regions test SJ Park
` (5 subsequent siblings)
7 siblings, 1 reply; 17+ messages in thread
From: SJ Park @ 2026-07-17 0:30 UTC (permalink / raw)
Cc: SJ Park, stable, Andrew Morton, Brendan Higgins, David Gow,
SeongJae Park, damon, kunit-dev, linux-kernel, linux-kselftest,
linux-mm
damon_test_split_at() test next region that is assumed to be created by
damon_split_region_at() invocation. But the split might fail. In this
case, the succeeding test may dereference invalid pointers returned by
damon_next_region().
The invalid pointer may not cause a really bad user impact, because of
the implementation detail. It would only read wrong contents in the
belonging damon_target struct. Depending on the future change of the
offset from the link header to the accessing field, this could also be
really dangerous, though. Still, the realistic user impact would be
limited. It would affect only test run setups.
Fix it by testing if the number of regions was also changed as expected
and exit early for the failure.
The issue was discovered [1] by Sashiko.
[1] https://lore.kernel.org/20260714142352.100478-1-sj@kernel.org
Fixes: 17ccae8bb5c9 ("mm/damon: add kunit tests")
Cc: <stable@vger.kernel.org> # 5.15.x
Signed-off-by: SJ Park <sj@kernel.org>
---
mm/damon/tests/core-kunit.h | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/mm/damon/tests/core-kunit.h b/mm/damon/tests/core-kunit.h
index eba643762132f..322a295ac2cec 100644
--- a/mm/damon/tests/core-kunit.h
+++ b/mm/damon/tests/core-kunit.h
@@ -155,6 +155,10 @@ static void damon_test_split_at(struct kunit *test)
r->age = 10;
damon_add_region(r, t);
damon_split_region_at(t, r, 25);
+ KUNIT_EXPECT_EQ(test, damon_nr_regions(t), 2);
+ if (damon_nr_regions(t) != 2)
+ goto out;
+
KUNIT_EXPECT_EQ(test, r->ar.start, 0ul);
KUNIT_EXPECT_EQ(test, r->ar.end, 25ul);
@@ -166,6 +170,7 @@ static void damon_test_split_at(struct kunit *test)
KUNIT_EXPECT_EQ(test, r->last_nr_accesses, r_new->last_nr_accesses);
KUNIT_EXPECT_EQ(test, r->age, r_new->age);
+out:
damon_free_target(t);
}
--
2.47.3
^ permalink raw reply related [flat|nested] 17+ messages in thread* Re: [PATCH 2/7] mm/damon/core-kunit: check region count before testing in split_at()
2026-07-17 0:30 ` [PATCH 2/7] mm/damon/core-kunit: check region count before testing in split_at() SJ Park
@ 2026-07-17 0:44 ` sashiko-bot
2026-07-17 1:18 ` SJ Park
0 siblings, 1 reply; 17+ messages in thread
From: sashiko-bot @ 2026-07-17 0:44 UTC (permalink / raw)
To: SJ Park; +Cc: damon
Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
Pre-existing issues:
- [High] Kernel memory corruption in `damon_apply_min_nr_regions()` due to unhandled split failure.
- [Medium] Incomplete test fix for memory allocation failure in `damos_filter_match()` leading to test crash.
- [Medium] Missing upper bound checks in test loops leading to Out-Of-Bounds array reads.
--
--- Patch [2]: [PATCH 2/7] mm/damon/core-kunit: check region count before testing in split_at() ---
commit c03465ac93de9ac544447d782c3c2ea15bf60ab1
Author: SJ Park <sj@kernel.org>
mm/damon/core-kunit: check region count before testing in split_at()
damon_test_split_at() tests the next region that is assumed to be created by
damon_split_region_at(). But the split might fail. In this case, the test is
updated to check if the number of regions changed as expected and exit early.
Link: https://lore.kernel.org/20260714142352.100478-1-sj@kernel.org
> diff --git a/mm/damon/tests/core-kunit.h b/mm/damon/tests/core-kunit.h
> index eba643762132f..322a295ac2cec 100644
> --- a/mm/damon/tests/core-kunit.h
> +++ b/mm/damon/tests/core-kunit.h
> @@ -155,6 +155,10 @@ static void damon_test_split_at(struct kunit *test)
> r->age = 10;
> damon_add_region(r, t);
> damon_split_region_at(t, r, 25);
> + KUNIT_EXPECT_EQ(test, damon_nr_regions(t), 2);
> + if (damon_nr_regions(t) != 2)
> + goto out;
> +
[Severity: Medium]
This isn't a bug introduced by this patch, but it looks like the exact
same pattern being fixed here is present in damos_test_filter_out().
If damos_filter_match() fails to allocate memory, it returns without
splitting the region. The test logs a failure for the region count mismatch
but execution continues unconditionally:
mm/damon/tests/core-kunit.h:damos_test_filter_out() {
/* filter should have split the region */
...
KUNIT_EXPECT_EQ(test, damon_nr_regions(t), 2);
r2 = damon_next_region(r);
...
damon_destroy_region(r2, t);
}
Since r is the only item in the list, does damon_next_region(r) resolve to
an out-of-bounds pointer that is then passed to damon_destroy_region(),
causing list and slab corruption during test execution?
[Severity: High]
This is a pre-existing issue, but since we are handling split failures in
the tests, does mm/damon/core.c:damon_apply_min_nr_regions() suffer from a
similar problem in production code?
mm/damon/core.c:damon_apply_min_nr_regions() {
damon_for_each_target(t, ctx) {
damon_for_each_region_safe(r, next, t) {
while (damon_sz_region(r) > max_region_sz) {
damon_split_region_at(t, r, max_region_sz);
r = damon_next_region(r);
}
}
}
}
If damon_split_region_at() fails to allocate memory, it silently returns.
The loop then unconditionally advances r = damon_next_region(r). If r was
the last region, this returns a bogus pointer derived from the list head.
The loop condition damon_sz_region(r) then evaluates garbage memory. If it
happens to evaluate to greater than max_region_sz, the loop re-enters and
passes the bogus pointer to damon_split_region_at(). Could this lead to
out-of-bounds writes and list corruption under memory pressure?
[Severity: Medium]
This is also a pre-existing issue, but while reviewing the kunit tests, I
noticed several test loops lack upper bound checks against the destination
counts.
For example, in damos_test_commit_dests_for():
mm/damon/tests/core-kunit.h:damos_test_commit_dests_for() {
KUNIT_EXPECT_EQ(test, dst.nr_dests, src_nr_dests);
for (i = 0; i < dst.nr_dests; i++) {
KUNIT_EXPECT_EQ(test, dst.node_id_arr[i], src_node_id_arr[i]);
...
}
If the code under test incorrectly leaves dst.nr_dests greater than
src_nr_dests (for instance, when src_nr_dests is 0), the loop iterates out
of bounds and dereferences src_node_id_arr[i].
The same unbounded iteration pattern appears to exist in
damos_test_commit_quota_goals_for() and damon_test_commit_target_regions_for().
Could these also be fixed to prevent out-of-bounds reads during test failures?
[ ... ]
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260717003022.119437-1-sj@kernel.org?part=2
^ permalink raw reply [flat|nested] 17+ messages in thread* Re: [PATCH 2/7] mm/damon/core-kunit: check region count before testing in split_at()
2026-07-17 0:44 ` sashiko-bot
@ 2026-07-17 1:18 ` SJ Park
0 siblings, 0 replies; 17+ messages in thread
From: SJ Park @ 2026-07-17 1:18 UTC (permalink / raw)
To: sashiko-bot; +Cc: SJ Park, damon
On Fri, 17 Jul 2026 00:44:17 +0000 sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
>
> Pre-existing issues:
> - [High] Kernel memory corruption in `damon_apply_min_nr_regions()` due to unhandled split failure.
> - [Medium] Incomplete test fix for memory allocation failure in `damos_filter_match()` leading to test crash.
> - [Medium] Missing upper bound checks in test loops leading to Out-Of-Bounds array reads.
> --
>
> --- Patch [2]: [PATCH 2/7] mm/damon/core-kunit: check region count before testing in split_at() ---
> commit c03465ac93de9ac544447d782c3c2ea15bf60ab1
> Author: SJ Park <sj@kernel.org>
>
> mm/damon/core-kunit: check region count before testing in split_at()
>
> damon_test_split_at() tests the next region that is assumed to be created by
> damon_split_region_at(). But the split might fail. In this case, the test is
> updated to check if the number of regions changed as expected and exit early.
>
> Link: https://lore.kernel.org/20260714142352.100478-1-sj@kernel.org
>
> > diff --git a/mm/damon/tests/core-kunit.h b/mm/damon/tests/core-kunit.h
> > index eba643762132f..322a295ac2cec 100644
> > --- a/mm/damon/tests/core-kunit.h
> > +++ b/mm/damon/tests/core-kunit.h
> > @@ -155,6 +155,10 @@ static void damon_test_split_at(struct kunit *test)
> > r->age = 10;
> > damon_add_region(r, t);
> > damon_split_region_at(t, r, 25);
> > + KUNIT_EXPECT_EQ(test, damon_nr_regions(t), 2);
> > + if (damon_nr_regions(t) != 2)
> > + goto out;
> > +
>
> [Severity: Medium]
> This isn't a bug introduced by this patch, but it looks like the exact
> same pattern being fixed here is present in damos_test_filter_out().
>
> If damos_filter_match() fails to allocate memory, it returns without
> splitting the region. The test logs a failure for the region count mismatch
> but execution continues unconditionally:
>
> mm/damon/tests/core-kunit.h:damos_test_filter_out() {
> /* filter should have split the region */
> ...
> KUNIT_EXPECT_EQ(test, damon_nr_regions(t), 2);
> r2 = damon_next_region(r);
> ...
> damon_destroy_region(r2, t);
> }
>
> Since r is the only item in the list, does damon_next_region(r) resolve to
> an out-of-bounds pointer that is then passed to damon_destroy_region(),
> causing list and slab corruption during test execution?
That's what fixed by a later patch of this series.
>
> [Severity: High]
> This is a pre-existing issue, but since we are handling split failures in
> the tests, does mm/damon/core.c:damon_apply_min_nr_regions() suffer from a
> similar problem in production code?
>
> mm/damon/core.c:damon_apply_min_nr_regions() {
> damon_for_each_target(t, ctx) {
> damon_for_each_region_safe(r, next, t) {
> while (damon_sz_region(r) > max_region_sz) {
> damon_split_region_at(t, r, max_region_sz);
> r = damon_next_region(r);
> }
> }
> }
> }
>
> If damon_split_region_at() fails to allocate memory, it silently returns.
> The loop then unconditionally advances r = damon_next_region(r). If r was
> the last region, this returns a bogus pointer derived from the list head.
>
> The loop condition damon_sz_region(r) then evaluates garbage memory. If it
> happens to evaluate to greater than max_region_sz, the loop re-enters and
> passes the bogus pointer to damon_split_region_at(). Could this lead to
> out-of-bounds writes and list corruption under memory pressure?
Good finding, not necessarily a blocker of this series. I will separately work
on this.
>
> [Severity: Medium]
> This is also a pre-existing issue, but while reviewing the kunit tests, I
> noticed several test loops lack upper bound checks against the destination
> counts.
>
> For example, in damos_test_commit_dests_for():
>
> mm/damon/tests/core-kunit.h:damos_test_commit_dests_for() {
> KUNIT_EXPECT_EQ(test, dst.nr_dests, src_nr_dests);
> for (i = 0; i < dst.nr_dests; i++) {
> KUNIT_EXPECT_EQ(test, dst.node_id_arr[i], src_node_id_arr[i]);
> ...
> }
>
> If the code under test incorrectly leaves dst.nr_dests greater than
> src_nr_dests (for instance, when src_nr_dests is 0), the loop iterates out
> of bounds and dereferences src_node_id_arr[i].
>
> The same unbounded iteration pattern appears to exist in
> damos_test_commit_quota_goals_for() and damon_test_commit_target_regions_for().
> Could these also be fixed to prevent out-of-bounds reads during test failures?
Those are fixed by the later patches of this series.
>
> [ ... ]
>
> --
> Sashiko AI review · https://sashiko.dev/#/patchset/20260717003022.119437-1-sj@kernel.org?part=2
Thanks,
SJ
^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH 3/7] mm/damon/vaddr-kunit: check region count in three_regions test
2026-07-17 0:30 [PATCH 0/7] mm/damon: fix uninitialized DAMOS field and kunit exec expectation bugs SJ Park
2026-07-17 0:30 ` [PATCH 1/7] mm/damon/core: initialize damos->last_applied SJ Park
2026-07-17 0:30 ` [PATCH 2/7] mm/damon/core-kunit: check region count before testing in split_at() SJ Park
@ 2026-07-17 0:30 ` SJ Park
2026-07-17 0:30 ` [PATCH 4/7] mm/damon/core-kunit: handle region split failure in filter_out() SJ Park
` (4 subsequent siblings)
7 siblings, 0 replies; 17+ messages in thread
From: SJ Park @ 2026-07-17 0:30 UTC (permalink / raw)
Cc: SJ Park, stable, Andrew Morton, Brendan Higgins, David Gow,
SeongJae Park, damon, kunit-dev, linux-kernel, linux-kselftest,
linux-mm
damon_do_test_apply_three_regions() iterates regions after
damon_set_regions() call assuming the function would succeed at setting
the number of regions the same to the expected one. It might have
failed. In this case, __nth_region_of() in the iteration could return
NULL and NULL dereference can happen in the test.
The consequent user impact (NULL dereference) is quite bad. The
realistic user impact would be limited, though. It would affect only
test run setups.
Fix it by testing if the number of regions was also changed as expected
and exit early for the failure.
The issue was discovered [1] by Sashiko.
[1] https://lore.kernel.org/20260713144757.39740-1-sj@kernel.org
Fixes: 17ccae8bb5c9 ("mm/damon: add kunit tests")
Cc: <stable@vger.kernel.org> # 5.15.x
Signed-off-by: SJ Park <sj@kernel.org>
---
mm/damon/tests/vaddr-kunit.h | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/mm/damon/tests/vaddr-kunit.h b/mm/damon/tests/vaddr-kunit.h
index 61f844336ffb5..6a95441d193ae 100644
--- a/mm/damon/tests/vaddr-kunit.h
+++ b/mm/damon/tests/vaddr-kunit.h
@@ -158,12 +158,17 @@ static void damon_do_test_apply_three_regions(struct kunit *test,
kunit_skip(test, "second damon_set_regions() fail");
}
+ KUNIT_EXPECT_EQ(test, damon_nr_regions(t), nr_expected / 2);
+ if (damon_nr_regions(t) != nr_expected / 2)
+ goto out;
+
for (i = 0; i < nr_expected / 2; i++) {
r = __nth_region_of(t, i);
KUNIT_EXPECT_EQ(test, r->ar.start, expected[i * 2]);
KUNIT_EXPECT_EQ(test, r->ar.end, expected[i * 2 + 1]);
}
+out:
damon_destroy_target(t, NULL);
}
--
2.47.3
^ permalink raw reply related [flat|nested] 17+ messages in thread* [PATCH 4/7] mm/damon/core-kunit: handle region split failure in filter_out()
2026-07-17 0:30 [PATCH 0/7] mm/damon: fix uninitialized DAMOS field and kunit exec expectation bugs SJ Park
` (2 preceding siblings ...)
2026-07-17 0:30 ` [PATCH 3/7] mm/damon/vaddr-kunit: check region count in three_regions test SJ Park
@ 2026-07-17 0:30 ` SJ Park
2026-07-17 0:46 ` sashiko-bot
2026-07-17 0:30 ` [PATCH 5/7] mm/damon/core-kunit: skip wrong dest walk in commit_dests_for() SJ Park
` (3 subsequent siblings)
7 siblings, 1 reply; 17+ messages in thread
From: SJ Park @ 2026-07-17 0:30 UTC (permalink / raw)
Cc: SJ Park, stable, Andrew Morton, Brendan Higgins, David Gow, damon,
kunit-dev, linux-kernel, linux-kselftest, linux-mm
dasmon_test_filter_out() test checks if damos_filter_match() of an
address filter splits the region as expected under a given condition.
But, the test continued regardless of the split successes. As a result,
the later part of the test could dereference invalid pointers that
returned from damon_next_region(). Further, it could corrupt memory
from damon_destroy_region().
The consequent user impact (memory corruption) is quite bad. The
realistic user impact would be limited, though. It would affect only
test run setups.
Fix it by exiting early for the number of regions test failure.
The issue was discovered [1] by Sashiko.
[1] https://lore.kernel.org/20260714142352.100478-1-sj@kernel.org
Fixes: 26713c890875 ("mm/damon/core-test: add a unit test for __damos_filter_out()")
Cc: <stable@vger.kernel.org> # 6.6.x
Signed-off-by: SJ Park <sj@kernel.org>
---
mm/damon/tests/core-kunit.h | 3 +++
1 file changed, 3 insertions(+)
diff --git a/mm/damon/tests/core-kunit.h b/mm/damon/tests/core-kunit.h
index 322a295ac2cec..bd3bbd421392f 100644
--- a/mm/damon/tests/core-kunit.h
+++ b/mm/damon/tests/core-kunit.h
@@ -1365,6 +1365,8 @@ static void damos_test_filter_out(struct kunit *test)
KUNIT_EXPECT_EQ(test, r->ar.start, 1);
KUNIT_EXPECT_EQ(test, r->ar.end, 2);
KUNIT_EXPECT_EQ(test, damon_nr_regions(t), 2);
+ if (damon_nr_regions(t) != 2)
+ goto out;
r2 = damon_next_region(r);
KUNIT_EXPECT_EQ(test, r2->ar.start, 2);
KUNIT_EXPECT_EQ(test, r2->ar.end, 4);
@@ -1384,6 +1386,7 @@ static void damos_test_filter_out(struct kunit *test)
KUNIT_EXPECT_EQ(test, r2->ar.end, 8);
damon_destroy_region(r2, t);
+out:
damon_free_target(t);
damos_free_filter(f);
}
--
2.47.3
^ permalink raw reply related [flat|nested] 17+ messages in thread* Re: [PATCH 4/7] mm/damon/core-kunit: handle region split failure in filter_out()
2026-07-17 0:30 ` [PATCH 4/7] mm/damon/core-kunit: handle region split failure in filter_out() SJ Park
@ 2026-07-17 0:46 ` sashiko-bot
2026-07-17 1:22 ` SJ Park
0 siblings, 1 reply; 17+ messages in thread
From: sashiko-bot @ 2026-07-17 0:46 UTC (permalink / raw)
To: SJ Park; +Cc: damon
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
Pre-existing issues:
- [High] The patch adds an early exit check `if (damon_nr_regions(t) != 2) goto out;` to prevent an invalid pointer dereference when the first region split test fails, but misses the exact same vulnerability in the second region split test directly below it.
- [High] In `damon_apply_min_nr_regions`, if `damon_split_region_at` fails to allocate memory, the inner loop advances to the next list entry without inserting a new region, leading to an out-of-bounds read and write when it reaches the list head.
--
--- Patch [4]: [PATCH 4/7] mm/damon/core-kunit: handle region split failure in filter_out() ---
commit 705cf12a4e8501ec816031495df35460614c42c2
Author: SJ Park <sj@kernel.org>
mm/damon/core-kunit: handle region split failure in filter_out()
damos_test_filter_out() checks if damos_filter_match() of an address
filter splits the region as expected. But the test continued regardless of
whether the split succeeded, which could lead to dereferencing invalid
pointers or corrupting memory. This adds an early exit check on the first
test block to prevent continuing on failure.
Link: https://lore.kernel.org/20260714142352.100478-1-sj@kernel.org
> diff --git a/mm/damon/tests/core-kunit.h b/mm/damon/tests/core-kunit.h
> index 322a295ac2cec..bd3bbd421392f 100644
> --- a/mm/damon/tests/core-kunit.h
> +++ b/mm/damon/tests/core-kunit.h
> @@ -1365,6 +1365,8 @@ static void damos_test_filter_out(struct kunit *test)
> KUNIT_EXPECT_EQ(test, r->ar.start, 1);
> KUNIT_EXPECT_EQ(test, r->ar.end, 2);
> KUNIT_EXPECT_EQ(test, damon_nr_regions(t), 2);
> + if (damon_nr_regions(t) != 2)
> + goto out;
> r2 = damon_next_region(r);
> KUNIT_EXPECT_EQ(test, r2->ar.start, 2);
> KUNIT_EXPECT_EQ(test, r2->ar.end, 4);
> @@ -1384,6 +1386,7 @@ static void damos_test_filter_out(struct kunit *test)
> KUNIT_EXPECT_EQ(test, r2->ar.end, 8);
> damon_destroy_region(r2, t);
>
[Severity: High]
This isn't a bug introduced by this patch, but does this second region split
test have the same vulnerability as the one fixed above?
If damos_filter_match() fails to split the region in this second test block
(e.g. due to memory allocation failure), damon_nr_regions(t) would remain 1.
r2 would be assigned damon_next_region(r), which could incorrectly resolve
to the damon_target list head.
Calling damon_destroy_region(r2, t) would then call kmem_cache_free() on
this out-of-bounds pointer, potentially corrupting memory. Should another
early exit check be added here as well?
> +out:
> damon_free_target(t);
> damos_free_filter(f);
> }
[Severity: High]
This is a pre-existing issue, but while looking at region splits, could a
similar unchecked failure cause memory corruption in mm/damon/core.c?
In damon_apply_min_nr_regions():
while (damon_sz_region(r) > max_region_sz) {
damon_split_region_at(t, r, max_region_sz);
r = damon_next_region(r);
}
If damon_split_region_at() fails due to memory pressure, it does not split
the region. The call to damon_next_region(r) then erroneously advances r to
the next element.
If r was the last region, it becomes the list head (&t->regions_list). The
loop condition then evaluates damon_sz_region(r) on the list head, which
reads memory 32 bytes before the damon_target struct allocation. If this
difference exceeds max_region_sz, damon_split_region_at() could be called
on the list head, which might subsequently write to r->ar.end, corrupting
SLUB metadata or preceding objects.
Should damon_split_region_at() return an error code so callers can verify
if the split actually succeeded?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260717003022.119437-1-sj@kernel.org?part=4
^ permalink raw reply [flat|nested] 17+ messages in thread* Re: [PATCH 4/7] mm/damon/core-kunit: handle region split failure in filter_out()
2026-07-17 0:46 ` sashiko-bot
@ 2026-07-17 1:22 ` SJ Park
0 siblings, 0 replies; 17+ messages in thread
From: SJ Park @ 2026-07-17 1:22 UTC (permalink / raw)
To: sashiko-bot; +Cc: SJ Park, damon
On Fri, 17 Jul 2026 00:46:05 +0000 sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
>
> Pre-existing issues:
> - [High] The patch adds an early exit check `if (damon_nr_regions(t) != 2) goto out;` to prevent an invalid pointer dereference when the first region split test fails, but misses the exact same vulnerability in the second region split test directly below it.
> - [High] In `damon_apply_min_nr_regions`, if `damon_split_region_at` fails to allocate memory, the inner loop advances to the next list entry without inserting a new region, leading to an out-of-bounds read and write when it reaches the list head.
> --
>
> --- Patch [4]: [PATCH 4/7] mm/damon/core-kunit: handle region split failure in filter_out() ---
> commit 705cf12a4e8501ec816031495df35460614c42c2
> Author: SJ Park <sj@kernel.org>
>
> mm/damon/core-kunit: handle region split failure in filter_out()
>
> damos_test_filter_out() checks if damos_filter_match() of an address
> filter splits the region as expected. But the test continued regardless of
> whether the split succeeded, which could lead to dereferencing invalid
> pointers or corrupting memory. This adds an early exit check on the first
> test block to prevent continuing on failure.
>
> Link: https://lore.kernel.org/20260714142352.100478-1-sj@kernel.org
>
> > diff --git a/mm/damon/tests/core-kunit.h b/mm/damon/tests/core-kunit.h
> > index 322a295ac2cec..bd3bbd421392f 100644
> > --- a/mm/damon/tests/core-kunit.h
> > +++ b/mm/damon/tests/core-kunit.h
> > @@ -1365,6 +1365,8 @@ static void damos_test_filter_out(struct kunit *test)
> > KUNIT_EXPECT_EQ(test, r->ar.start, 1);
> > KUNIT_EXPECT_EQ(test, r->ar.end, 2);
> > KUNIT_EXPECT_EQ(test, damon_nr_regions(t), 2);
> > + if (damon_nr_regions(t) != 2)
> > + goto out;
> > r2 = damon_next_region(r);
> > KUNIT_EXPECT_EQ(test, r2->ar.start, 2);
> > KUNIT_EXPECT_EQ(test, r2->ar.end, 4);
> > @@ -1384,6 +1386,7 @@ static void damos_test_filter_out(struct kunit *test)
> > KUNIT_EXPECT_EQ(test, r2->ar.end, 8);
> > damon_destroy_region(r2, t);
> >
>
> [Severity: High]
> This isn't a bug introduced by this patch, but does this second region split
> test have the same vulnerability as the one fixed above?
>
> If damos_filter_match() fails to split the region in this second test block
> (e.g. due to memory allocation failure), damon_nr_regions(t) would remain 1.
> r2 would be assigned damon_next_region(r), which could incorrectly resolve
> to the damon_target list head.
>
> Calling damon_destroy_region(r2, t) would then call kmem_cache_free() on
> this out-of-bounds pointer, potentially corrupting memory. Should another
> early exit check be added here as well?
Good finding. I will fix this in the next revision of this series.
>
> > +out:
> > damon_free_target(t);
> > damos_free_filter(f);
> > }
>
> [Severity: High]
> This is a pre-existing issue, but while looking at region splits, could a
> similar unchecked failure cause memory corruption in mm/damon/core.c?
>
> In damon_apply_min_nr_regions():
>
> while (damon_sz_region(r) > max_region_sz) {
> damon_split_region_at(t, r, max_region_sz);
> r = damon_next_region(r);
> }
>
> If damon_split_region_at() fails due to memory pressure, it does not split
> the region. The call to damon_next_region(r) then erroneously advances r to
> the next element.
>
> If r was the last region, it becomes the list head (&t->regions_list). The
> loop condition then evaluates damon_sz_region(r) on the list head, which
> reads memory 32 bytes before the damon_target struct allocation. If this
> difference exceeds max_region_sz, damon_split_region_at() could be called
> on the list head, which might subsequently write to r->ar.end, corrupting
> SLUB metadata or preceding objects.
>
> Should damon_split_region_at() return an error code so callers can verify
> if the split actually succeeded?
Sashiko found this in a previous reply. I'm gonna separately work on this.
>
> --
> Sashiko AI review · https://sashiko.dev/#/patchset/20260717003022.119437-1-sj@kernel.org?part=4
Thanks,
SJ
^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH 5/7] mm/damon/core-kunit: skip wrong dest walk in commit_dests_for()
2026-07-17 0:30 [PATCH 0/7] mm/damon: fix uninitialized DAMOS field and kunit exec expectation bugs SJ Park
` (3 preceding siblings ...)
2026-07-17 0:30 ` [PATCH 4/7] mm/damon/core-kunit: handle region split failure in filter_out() SJ Park
@ 2026-07-17 0:30 ` SJ Park
2026-07-17 0:30 ` [PATCH 6/7] mm/damon/core-kunit: skip wrong quota goal walk in commit_quota_goals() SJ Park
` (2 subsequent siblings)
7 siblings, 0 replies; 17+ messages in thread
From: SJ Park @ 2026-07-17 0:30 UTC (permalink / raw)
Cc: SJ Park, stable, Andrew Morton, Brendan Higgins, David Gow, damon,
kunit-dev, linux-kernel, linux-kselftest, linux-mm
damos_test_commit_dests_for() traverse damos action destinations after
damos_commit_dests(). It assumes damos_commit_dests() made expected
numbers of destinations for source and destination structures. It might
not. Because the traversal is made based on destination struct length,
it could do out of bounds access for source value expectation.
The consequent user impact (out-of-bound access ) is quite bad. The
realistic user impact would be limited, though. It would affect only
test run setups.
Fix it by exiting early for the number of regions test failure.
The issue was discovered [1] by Sashiko.
[1] https://lore.kernel.org/20260713144757.39740-1-sj@kernel.org
Fixes: eec573b8dd65 ("mm/damon/tests/core-kunit: add damos_commit_dests() test")
Cc: <stable@vger.kernel.org> # 6.19.x
Signed-off-by: SJ Park <sj@kernel.org>
---
mm/damon/tests/core-kunit.h | 2 ++
1 file changed, 2 insertions(+)
diff --git a/mm/damon/tests/core-kunit.h b/mm/damon/tests/core-kunit.h
index bd3bbd421392f..ca6ac12a15b24 100644
--- a/mm/damon/tests/core-kunit.h
+++ b/mm/damon/tests/core-kunit.h
@@ -1008,6 +1008,8 @@ static void damos_test_commit_dests_for(struct kunit *test,
skip = false;
KUNIT_EXPECT_EQ(test, dst.nr_dests, src_nr_dests);
+ if (dst.nr_dests != src_nr_dests)
+ goto out;
for (i = 0; i < dst.nr_dests; i++) {
KUNIT_EXPECT_EQ(test, dst.node_id_arr[i], src_node_id_arr[i]);
KUNIT_EXPECT_EQ(test, dst.weight_arr[i], src_weight_arr[i]);
--
2.47.3
^ permalink raw reply related [flat|nested] 17+ messages in thread* [PATCH 6/7] mm/damon/core-kunit: skip wrong quota goal walk in commit_quota_goals()
2026-07-17 0:30 [PATCH 0/7] mm/damon: fix uninitialized DAMOS field and kunit exec expectation bugs SJ Park
` (4 preceding siblings ...)
2026-07-17 0:30 ` [PATCH 5/7] mm/damon/core-kunit: skip wrong dest walk in commit_dests_for() SJ Park
@ 2026-07-17 0:30 ` SJ Park
2026-07-17 0:44 ` sashiko-bot
2026-07-17 0:30 ` [PATCH 7/7] mm/damon/core-kunit: skip wrong region walk in commit_target_regions() SJ Park
2026-07-17 1:05 ` [PATCH 0/7] mm/damon: fix uninitialized DAMOS field and kunit exec expectation bugs SJ Park
7 siblings, 1 reply; 17+ messages in thread
From: SJ Park @ 2026-07-17 0:30 UTC (permalink / raw)
Cc: SJ Park, stable, Andrew Morton, Brendan Higgins, David Gow, damon,
kunit-dev, linux-kernel, linux-kselftest, linux-mm
damos_test_commit_quota_goals_for() traverses damos quota goals after
damos_commit_quota_goals() call. It assumes damos_commit_quota_goals()
made expected numbers of goals. It might not. Because the traversal is
made based on destination struct length, it could do out of bounds
access for source expectation value array.
The consequent user impact (out-of-bound access ) is quite bad. The
realistic user impact would be limited though. It would affect only
test run setups.
Fix it by testing if the number of goals was also changed as expected
and exit early for the failure.
The issue was discovered [1] by Sashiko.
[1] https://lore.kernel.org/20260713144757.39740-1-sj@kernel.org
Fixes: d9adfb8a28e7 ("mm/damon/tests/core-kunit: add damos_commit_quota_goals() test")
Cc: <stable@vger.kernel.org> # 6.19.x
Signed-off-by: SJ Park <sj@kernel.org>
---
mm/damon/tests/core-kunit.h | 9 +++++++++
1 file changed, 9 insertions(+)
diff --git a/mm/damon/tests/core-kunit.h b/mm/damon/tests/core-kunit.h
index ca6ac12a15b24..c0fc903a6a259 100644
--- a/mm/damon/tests/core-kunit.h
+++ b/mm/damon/tests/core-kunit.h
@@ -839,6 +839,7 @@ static void damos_test_commit_quota_goals_for(struct kunit *test,
struct damos_quota_goal *goal, *next;
bool skip = true;
int i;
+ int nr_dst = 0, nr_src = 0;
INIT_LIST_HEAD(&dst.goals);
INIT_LIST_HEAD(&src.goals);
@@ -861,6 +862,14 @@ static void damos_test_commit_quota_goals_for(struct kunit *test,
damos_commit_quota_goals(&dst, &src);
+ damos_for_each_quota_goal(goal, &dst)
+ nr_dst++;
+ damos_for_each_quota_goal(goal, &src)
+ nr_src++;
+ KUNIT_EXPECT_EQ(test, nr_dst, nr_src);
+ if (nr_dst != nr_src)
+ goto out;
+
i = 0;
damos_for_each_quota_goal(goal, (&dst)) {
KUNIT_EXPECT_EQ(test, goal->metric, src_goals[i].metric);
--
2.47.3
^ permalink raw reply related [flat|nested] 17+ messages in thread* Re: [PATCH 6/7] mm/damon/core-kunit: skip wrong quota goal walk in commit_quota_goals()
2026-07-17 0:30 ` [PATCH 6/7] mm/damon/core-kunit: skip wrong quota goal walk in commit_quota_goals() SJ Park
@ 2026-07-17 0:44 ` sashiko-bot
2026-07-17 1:23 ` SJ Park
0 siblings, 1 reply; 17+ messages in thread
From: sashiko-bot @ 2026-07-17 0:44 UTC (permalink / raw)
To: SJ Park; +Cc: damon
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
Pre-existing issues:
- [Medium] Missing error check in `damon_test_commit_target_regions_for()` leads to an out-of-bounds read if `damon_commit_target_regions()` fails.
--
--- Patch [6]: [PATCH 6/7] mm/damon/core-kunit: skip wrong quota goal walk in commit_quota_goals() ---
commit 9bf8891739c4d265498b06b272b3083da8b65ed2
Author: SJ Park <sj@kernel.org>
mm/damon/core-kunit: skip wrong quota goal walk in commit_quota_goals()
This commit fixes an out-of-bounds access in
damos_test_commit_quota_goals_for() by verifying the number of quota goals
matches the expected amount before traversing them. This prevents test setups
from crashing if damos_commit_quota_goals() creates an unexpected number of
goals.
Link: https://lore.kernel.org/20260713144757.39740-1-sj@kernel.org
> --- a/mm/damon/tests/core-kunit.h
> +++ b/mm/damon/tests/core-kunit.h
[ ... ]
> @@ -861,6 +862,14 @@ static void damos_test_commit_quota_goals_for(struct kunit *test,
>
> damos_commit_quota_goals(&dst, &src);
>
> + damos_for_each_quota_goal(goal, &dst)
> + nr_dst++;
> + damos_for_each_quota_goal(goal, &src)
> + nr_src++;
> + KUNIT_EXPECT_EQ(test, nr_dst, nr_src);
> + if (nr_dst != nr_src)
> + goto out;
> +
> i = 0;
> damos_for_each_quota_goal(goal, (&dst)) {
> KUNIT_EXPECT_EQ(test, goal->metric, src_goals[i].metric);
[Severity: Medium]
This is a pre-existing issue, but since this patch fixes this exact pattern
for quota goals, does damon_test_commit_target_regions_for() need a similar
fix for target regions?
If damon_commit_target_regions() fails (for example, due to a memory
allocation failure), it returns an error code and leaves dst_target's
regions unmodified. The test code currently ignores the return value and
blindly iterates over dst_target:
mm/damon/tests/core-kunit.h:damon_test_commit_target_regions_for() {
...
damon_commit_target_regions(dst_target, src_target, 1);
i = 0;
damon_for_each_region(r, dst_target) {
KUNIT_EXPECT_EQ(test, r->ar.start, expect_start_end[i][0]);
KUNIT_EXPECT_EQ(test, r->ar.end, expect_start_end[i][1]);
i++;
}
...
}
In a case where the original number of destination regions is larger than
the number of expected regions, could this loop execute more times than the
size of expect_start_end[] and cause an out-of-bounds read on the stack array?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260717003022.119437-1-sj@kernel.org?part=6
^ permalink raw reply [flat|nested] 17+ messages in thread* Re: [PATCH 6/7] mm/damon/core-kunit: skip wrong quota goal walk in commit_quota_goals()
2026-07-17 0:44 ` sashiko-bot
@ 2026-07-17 1:23 ` SJ Park
0 siblings, 0 replies; 17+ messages in thread
From: SJ Park @ 2026-07-17 1:23 UTC (permalink / raw)
To: sashiko-bot; +Cc: SJ Park, damon
On Fri, 17 Jul 2026 00:44:33 +0000 sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
>
> Pre-existing issues:
> - [Medium] Missing error check in `damon_test_commit_target_regions_for()` leads to an out-of-bounds read if `damon_commit_target_regions()` fails.
> --
>
> --- Patch [6]: [PATCH 6/7] mm/damon/core-kunit: skip wrong quota goal walk in commit_quota_goals() ---
> commit 9bf8891739c4d265498b06b272b3083da8b65ed2
> Author: SJ Park <sj@kernel.org>
>
> mm/damon/core-kunit: skip wrong quota goal walk in commit_quota_goals()
>
> This commit fixes an out-of-bounds access in
> damos_test_commit_quota_goals_for() by verifying the number of quota goals
> matches the expected amount before traversing them. This prevents test setups
> from crashing if damos_commit_quota_goals() creates an unexpected number of
> goals.
>
> Link: https://lore.kernel.org/20260713144757.39740-1-sj@kernel.org
>
> > --- a/mm/damon/tests/core-kunit.h
> > +++ b/mm/damon/tests/core-kunit.h
> [ ... ]
> > @@ -861,6 +862,14 @@ static void damos_test_commit_quota_goals_for(struct kunit *test,
> >
> > damos_commit_quota_goals(&dst, &src);
> >
> > + damos_for_each_quota_goal(goal, &dst)
> > + nr_dst++;
> > + damos_for_each_quota_goal(goal, &src)
> > + nr_src++;
> > + KUNIT_EXPECT_EQ(test, nr_dst, nr_src);
> > + if (nr_dst != nr_src)
> > + goto out;
> > +
> > i = 0;
> > damos_for_each_quota_goal(goal, (&dst)) {
> > KUNIT_EXPECT_EQ(test, goal->metric, src_goals[i].metric);
>
> [Severity: Medium]
> This is a pre-existing issue, but since this patch fixes this exact pattern
> for quota goals, does damon_test_commit_target_regions_for() need a similar
> fix for target regions?
>
> If damon_commit_target_regions() fails (for example, due to a memory
> allocation failure), it returns an error code and leaves dst_target's
> regions unmodified. The test code currently ignores the return value and
> blindly iterates over dst_target:
>
> mm/damon/tests/core-kunit.h:damon_test_commit_target_regions_for() {
> ...
> damon_commit_target_regions(dst_target, src_target, 1);
> i = 0;
> damon_for_each_region(r, dst_target) {
> KUNIT_EXPECT_EQ(test, r->ar.start, expect_start_end[i][0]);
> KUNIT_EXPECT_EQ(test, r->ar.end, expect_start_end[i][1]);
> i++;
> }
> ...
> }
>
> In a case where the original number of destination regions is larger than
> the number of expected regions, could this loop execute more times than the
> size of expect_start_end[] and cause an out-of-bounds read on the stack array?
Yes, and that is fixed by another patch in this series.
>
> --
> Sashiko AI review · https://sashiko.dev/#/patchset/20260717003022.119437-1-sj@kernel.org?part=6
Thanks,
SJ
^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH 7/7] mm/damon/core-kunit: skip wrong region walk in commit_target_regions()
2026-07-17 0:30 [PATCH 0/7] mm/damon: fix uninitialized DAMOS field and kunit exec expectation bugs SJ Park
` (5 preceding siblings ...)
2026-07-17 0:30 ` [PATCH 6/7] mm/damon/core-kunit: skip wrong quota goal walk in commit_quota_goals() SJ Park
@ 2026-07-17 0:30 ` SJ Park
2026-07-17 1:05 ` [PATCH 0/7] mm/damon: fix uninitialized DAMOS field and kunit exec expectation bugs SJ Park
7 siblings, 0 replies; 17+ messages in thread
From: SJ Park @ 2026-07-17 0:30 UTC (permalink / raw)
Cc: SJ Park, stable, Andrew Morton, Brendan Higgins, David Gow, damon,
kunit-dev, linux-kernel, linux-kselftest, linux-mm
damon_test_commit_target_regions_for() traverses expected values array
after damon_commit_target_regions() call. It assumes
damon_commit_target_regions() made expected number of regions. It might
not. Because the traversal is made based on the region count, it could
do out of bounds access to the expectation value array.
The consequent user impact (out-of-bound access) is quite bad. The
realistic user impact would be limited, though. It would affect only
test run setups.
Fix it by testing if the number of regions was also changed as expected
and exit early for the failure.
The issue was discovered [1] by Sashiko.
[1] https://lore.kernel.org/20260713144757.39740-1-sj@kernel.org
Fixes: 603f67eb91e0 ("mm/damon/tests/core-kunit: add damon_commit_target_regions() test")
Cc: <stable@vger.kernel.org> # 6.19.x
Signed-off-by: SJ Park <sj@kernel.org>
---
mm/damon/tests/core-kunit.h | 9 +++++++--
1 file changed, 7 insertions(+), 2 deletions(-)
diff --git a/mm/damon/tests/core-kunit.h b/mm/damon/tests/core-kunit.h
index c0fc903a6a259..805d15a64500d 100644
--- a/mm/damon/tests/core-kunit.h
+++ b/mm/damon/tests/core-kunit.h
@@ -1277,14 +1277,19 @@ static void damon_test_commit_target_regions_for(struct kunit *test,
kunit_skip(test, "src target setup fail");
}
damon_commit_target_regions(dst_target, src_target, 1);
+
+ KUNIT_EXPECT_EQ(test, damon_nr_regions(dst_target), nr_expect_regions);
+ if (damon_nr_regions(dst_target) != nr_expect_regions)
+ goto out;
+
i = 0;
damon_for_each_region(r, dst_target) {
KUNIT_EXPECT_EQ(test, r->ar.start, expect_start_end[i][0]);
KUNIT_EXPECT_EQ(test, r->ar.end, expect_start_end[i][1]);
i++;
}
- KUNIT_EXPECT_EQ(test, damon_nr_regions(dst_target), nr_expect_regions);
- KUNIT_EXPECT_EQ(test, i, nr_expect_regions);
+
+out:
damon_free_target(dst_target);
damon_free_target(src_target);
}
--
2.47.3
^ permalink raw reply related [flat|nested] 17+ messages in thread* Re: [PATCH 0/7] mm/damon: fix uninitialized DAMOS field and kunit exec expectation bugs
2026-07-17 0:30 [PATCH 0/7] mm/damon: fix uninitialized DAMOS field and kunit exec expectation bugs SJ Park
` (6 preceding siblings ...)
2026-07-17 0:30 ` [PATCH 7/7] mm/damon/core-kunit: skip wrong region walk in commit_target_regions() SJ Park
@ 2026-07-17 1:05 ` SJ Park
7 siblings, 0 replies; 17+ messages in thread
From: SJ Park @ 2026-07-17 1:05 UTC (permalink / raw)
To: SJ Park
Cc: stable, Andrew Morton, Brendan Higgins, David Gow, SeongJae Park,
damon, kunit-dev, linux-kernel, linux-kselftest, linux-mm
On Thu, 16 Jul 2026 17:30:14 -0700 SJ Park <sj@kernel.org> wrote:
> Fix a few Sashiko-found unurgent bugs. Patch 1 fixes use of
> uninitialized damos->last_applied field. Patches 2-7 fix DAMON kunit
> tests that do invalid memory access under test failures.
This was meant to be RFC, but I forgot adding the tag. Andrew, please don't
merge this as is. Sorry for confusing people.
Thanks,
SJ
[...]
^ permalink raw reply [flat|nested] 17+ messages in thread