All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 1/7] mm/damon/core: initialize damos->last_applied
  2026-07-17  0:30 SJ Park
@ 2026-07-17  0:30 ` SJ Park
  2026-07-17  0:52   ` sashiko-bot
  0 siblings, 1 reply; 22+ 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] 22+ 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; 22+ 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] 22+ 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; 22+ 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] 22+ messages in thread

* [PATCH 0/7] mm/damon: fix uninitialized DAMOS field and kunit exec expectation bugs
@ 2026-07-18  0:14 SJ Park
  2026-07-18  0:14 ` [PATCH 1/7] mm/damon/core: initialize damos->last_applied SJ Park
                   ` (7 more replies)
  0 siblings, 8 replies; 22+ messages in thread
From: SJ Park @ 2026-07-18  0:14 UTC (permalink / raw)
  To: Andrew Morton
  Cc: SJ Park, stable, Brendan Higgins, David Gow, SeongJae Park, damon,
	kunit-dev, linux-kernel, linux-kselftest, linux-mm

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.

The bugs are better to be fixed and eventually merged into stable@
kernel.  That said, the fixes are arguably not urgent.  Patch 1 only
introduces negligible DAMOS efficiency degradation in occasional
cases.  Kunit fixes could introduce quite bad consequences but those
are test code that affect only test run setups.

Changes from RFC v1.1
- RFC v1.1: https://lore.kernel.org/20260717143445.91207-1-sj@kernel.org
- Drop RFC.
Changes from RFC
- RFC: https://lore.kernel.org/20260717003022.119437-1-sj@kernel.org
  (Was not having RFC tag on the subject by a mistake)
- Add missed region split failure check in filter_out() test.
- Rebase to latest mm-new.

SJ Park (7):
  mm/damon/core: initialize damos->last_applied
  mm/damon/core-kunit: check region count before testing in split_at()
  mm/damon/vaddr-kunit: check region count in three_regions test
  mm/damon/core-kunit: handle region split failure in filter_out()
  mm/damon/core-kunit: skip wrong dest walk in commit_dests_for()
  mm/damon/core-kunit: skip wrong quota goal walk in
    commit_quota_goals()
  mm/damon/core-kunit: skip wrong region walk in commit_target_regions()

 mm/damon/core.c              |  1 +
 mm/damon/tests/core-kunit.h  | 30 ++++++++++++++++++++++++++++--
 mm/damon/tests/vaddr-kunit.h |  5 +++++
 3 files changed, 34 insertions(+), 2 deletions(-)


base-commit: 6bb81496f18939ccbc5f16840d5b2cf1af6c995e
-- 
2.47.3

^ permalink raw reply	[flat|nested] 22+ messages in thread

* [PATCH 1/7] mm/damon/core: initialize damos->last_applied
  2026-07-18  0:14 [PATCH 0/7] mm/damon: fix uninitialized DAMOS field and kunit exec expectation bugs SJ Park
@ 2026-07-18  0:14 ` SJ Park
  2026-07-18  0:32   ` sashiko-bot
  2026-07-18  0:14 ` [PATCH 2/7] mm/damon/core-kunit: check region count before testing in split_at() SJ Park
                   ` (6 subsequent siblings)
  7 siblings, 1 reply; 22+ messages in thread
From: SJ Park @ 2026-07-18  0:14 UTC (permalink / raw)
  To: Andrew Morton
  Cc: SJ Park, stable, 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] 22+ messages in thread

* [PATCH 2/7] mm/damon/core-kunit: check region count before testing in split_at()
  2026-07-18  0:14 [PATCH 0/7] mm/damon: fix uninitialized DAMOS field and kunit exec expectation bugs SJ Park
  2026-07-18  0:14 ` [PATCH 1/7] mm/damon/core: initialize damos->last_applied SJ Park
@ 2026-07-18  0:14 ` SJ Park
  2026-07-18  0:30   ` sashiko-bot
  2026-07-18  0:14 ` [PATCH 3/7] mm/damon/vaddr-kunit: check region count in three_regions test SJ Park
                   ` (5 subsequent siblings)
  7 siblings, 1 reply; 22+ messages in thread
From: SJ Park @ 2026-07-18  0:14 UTC (permalink / raw)
  To: Andrew Morton
  Cc: SJ Park, stable, 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] 22+ messages in thread

* [PATCH 3/7] mm/damon/vaddr-kunit: check region count in three_regions test
  2026-07-18  0:14 [PATCH 0/7] mm/damon: fix uninitialized DAMOS field and kunit exec expectation bugs SJ Park
  2026-07-18  0:14 ` [PATCH 1/7] mm/damon/core: initialize damos->last_applied SJ Park
  2026-07-18  0:14 ` [PATCH 2/7] mm/damon/core-kunit: check region count before testing in split_at() SJ Park
@ 2026-07-18  0:14 ` SJ Park
  2026-07-18  0:14 ` [PATCH 4/7] mm/damon/core-kunit: handle region split failure in filter_out() SJ Park
                   ` (4 subsequent siblings)
  7 siblings, 0 replies; 22+ messages in thread
From: SJ Park @ 2026-07-18  0:14 UTC (permalink / raw)
  To: Andrew Morton
  Cc: SJ Park, stable, 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] 22+ messages in thread

* [PATCH 4/7] mm/damon/core-kunit: handle region split failure in filter_out()
  2026-07-18  0:14 [PATCH 0/7] mm/damon: fix uninitialized DAMOS field and kunit exec expectation bugs SJ Park
                   ` (2 preceding siblings ...)
  2026-07-18  0:14 ` [PATCH 3/7] mm/damon/vaddr-kunit: check region count in three_regions test SJ Park
@ 2026-07-18  0:14 ` SJ Park
  2026-07-18  0:29   ` sashiko-bot
  2026-07-18  0:14 ` [PATCH 5/7] mm/damon/core-kunit: skip wrong dest walk in commit_dests_for() SJ Park
                   ` (3 subsequent siblings)
  7 siblings, 1 reply; 22+ messages in thread
From: SJ Park @ 2026-07-18  0:14 UTC (permalink / raw)
  To: Andrew Morton
  Cc: SJ Park, stable, 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 | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/mm/damon/tests/core-kunit.h b/mm/damon/tests/core-kunit.h
index 322a295ac2cec..eddf3a0484583 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);
@@ -1379,11 +1381,14 @@ static void damos_test_filter_out(struct kunit *test)
 	KUNIT_EXPECT_EQ(test, r->ar.start, 2);
 	KUNIT_EXPECT_EQ(test, r->ar.end, 6);
 	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, 6);
 	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] 22+ messages in thread

* [PATCH 5/7] mm/damon/core-kunit: skip wrong dest walk in commit_dests_for()
  2026-07-18  0:14 [PATCH 0/7] mm/damon: fix uninitialized DAMOS field and kunit exec expectation bugs SJ Park
                   ` (3 preceding siblings ...)
  2026-07-18  0:14 ` [PATCH 4/7] mm/damon/core-kunit: handle region split failure in filter_out() SJ Park
@ 2026-07-18  0:14 ` SJ Park
  2026-07-18  0:25   ` sashiko-bot
  2026-07-18  0:14 ` [PATCH 6/7] mm/damon/core-kunit: skip wrong quota goal walk in commit_quota_goals() SJ Park
                   ` (2 subsequent siblings)
  7 siblings, 1 reply; 22+ messages in thread
From: SJ Park @ 2026-07-18  0:14 UTC (permalink / raw)
  To: Andrew Morton
  Cc: SJ Park, stable, 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 eddf3a0484583..9b8c20a50ad53 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] 22+ messages in thread

* [PATCH 6/7] mm/damon/core-kunit: skip wrong quota goal walk in commit_quota_goals()
  2026-07-18  0:14 [PATCH 0/7] mm/damon: fix uninitialized DAMOS field and kunit exec expectation bugs SJ Park
                   ` (4 preceding siblings ...)
  2026-07-18  0:14 ` [PATCH 5/7] mm/damon/core-kunit: skip wrong dest walk in commit_dests_for() SJ Park
@ 2026-07-18  0:14 ` SJ Park
  2026-07-18  0:22   ` sashiko-bot
  2026-07-18  0:14 ` [PATCH 7/7] mm/damon/core-kunit: skip wrong region walk in commit_target_regions() SJ Park
  2026-07-18  1:13 ` [PATCH 0/7] mm/damon: fix uninitialized DAMOS field and kunit exec expectation bugs SJ Park
  7 siblings, 1 reply; 22+ messages in thread
From: SJ Park @ 2026-07-18  0:14 UTC (permalink / raw)
  To: Andrew Morton
  Cc: SJ Park, stable, 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 9b8c20a50ad53..fbf986aee2dea 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] 22+ messages in thread

* [PATCH 7/7] mm/damon/core-kunit: skip wrong region walk in commit_target_regions()
  2026-07-18  0:14 [PATCH 0/7] mm/damon: fix uninitialized DAMOS field and kunit exec expectation bugs SJ Park
                   ` (5 preceding siblings ...)
  2026-07-18  0:14 ` [PATCH 6/7] mm/damon/core-kunit: skip wrong quota goal walk in commit_quota_goals() SJ Park
@ 2026-07-18  0:14 ` SJ Park
  2026-07-18  1:13 ` [PATCH 0/7] mm/damon: fix uninitialized DAMOS field and kunit exec expectation bugs SJ Park
  7 siblings, 0 replies; 22+ messages in thread
From: SJ Park @ 2026-07-18  0:14 UTC (permalink / raw)
  To: Andrew Morton
  Cc: SJ Park, stable, 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 fbf986aee2dea..4a536d41cdb2d 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] 22+ messages in thread

* Re: [PATCH 6/7] mm/damon/core-kunit: skip wrong quota goal walk in commit_quota_goals()
  2026-07-18  0:14 ` [PATCH 6/7] mm/damon/core-kunit: skip wrong quota goal walk in commit_quota_goals() SJ Park
@ 2026-07-18  0:22   ` sashiko-bot
  2026-07-18  1:02     ` SJ Park
  0 siblings, 1 reply; 22+ messages in thread
From: sashiko-bot @ 2026-07-18  0:22 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] Out-of-bounds array read in damon_test_commit_target_regions_for() due to missing region count validation before traversing dst_target.
--

--- Patch [6]: [PATCH 6/7] mm/damon/core-kunit: skip wrong quota goal walk in commit_quota_goals() ---
commit 4bcb5123bbb5f6b4908f1d776a8608ef05a89501
Author: SJ Park <sj@kernel.org>
Subject: mm/damon/core-kunit: skip wrong quota goal walk in commit_quota_goals()

This commit addresses an out-of-bounds array access in the KUnit test
damos_test_commit_quota_goals_for(). If the number of created quota goals
does not match the expected count, blindly traversing them can cause the test
to read past the bounds of the expected values array. The patch adds a count
validation and an early exit to avoid this invalid traversal.

Link: https://lore.kernel.org/20260713144757.39740-1-sj@kernel.org

> diff --git a/mm/damon/tests/core-kunit.h b/mm/damon/tests/core-kunit.h
> index 9b8c20a50ad53..fbf986aee2dea 100644
> --- 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 isn't a bug introduced by this patch, but should a similar early exit
check be added to damon_test_commit_target_regions_for() to prevent an
out-of-bounds read?

Just like the issue fixed here, damon_test_commit_target_regions_for()
iterates over dst_target regions without validating the region count against
nr_expect_regions first:

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++;
	}
	KUNIT_EXPECT_EQ(test, damon_nr_regions(dst_target), nr_expect_regions);
    ...
}

If damon_commit_target_regions() incorrectly leaves more regions in dst_target
than nr_expect_regions, the loop will read past the end of the
expect_start_end[] array before reaching the KUNIT_EXPECT_EQ count validation
at the end of the function.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260718001442.87129-1-sj@kernel.org?part=6

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH 5/7] mm/damon/core-kunit: skip wrong dest walk in commit_dests_for()
  2026-07-18  0:14 ` [PATCH 5/7] mm/damon/core-kunit: skip wrong dest walk in commit_dests_for() SJ Park
@ 2026-07-18  0:25   ` sashiko-bot
  2026-07-18  1:01     ` SJ Park
  0 siblings, 1 reply; 22+ messages in thread
From: sashiko-bot @ 2026-07-18  0:25 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] If `damos_commit_dests()` fails to allocate the `weight_arr` array, it returns `-ENOMEM` but leaves the `dst` context partially modified with a NULL `weight_arr` and an incorrect `nr_dests`, leading to a NULL pointer dereference during DAMOS scheme execution.
- [Medium] Missing out-of-bounds array bounds checks in the test functions `damos_test_commit_quota_goals_for()` and `damon_test_commit_target_regions_for()` when iterating over tested items.
--

--- Patch [5]: [PATCH 5/7] mm/damon/core-kunit: skip wrong dest walk in commit_dests_for() ---
commit 937f902e215bffeba8d97bcc0725b5bfbb643c41
Author: SJ Park <sj@kernel.org>
Subject: mm/damon/core-kunit: skip wrong dest walk in commit_dests_for()

This patch fixes a potential out-of-bounds access in the KUnit tests for
damos_commit_dests(). It exits early in damos_test_commit_dests_for()
if the expected number of destinations does not match the actual number.

Link: https://lore.kernel.org/20260713144757.39740-1-sj@kernel.org

> diff --git a/mm/damon/tests/core-kunit.h b/mm/damon/tests/core-kunit.h
> index eddf3a0484583..9b8c20a50ad53 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;

[Severity: High]
This isn't a bug introduced by this patch, but looking at the function being
tested, could damos_commit_dests() leave the destination context in a
corrupted state on allocation failure?

If kmalloc_array() fails to allocate weight_arr, it returns -ENOMEM but
leaves dst->weight_arr as NULL without resetting dst->nr_dests:

mm/damon/core.c:damos_commit_dests() {
	...
		dst->weight_arr = kmalloc_array(src->nr_dests,
			sizeof(*dst->weight_arr), GFP_KERNEL);
		if (!dst->weight_arr) {
			/* ->node_id_arr will be freed by scheme destruction */
			return -ENOMEM;
		}
	}
	...
}

Would a subsequent execution in damos_va_migrate_dests_add() dereference this
NULL array when iterating over dst->nr_dests, leading to a system crash?

[Severity: Medium]
This is a pre-existing issue, but does the same out-of-bounds array access
vulnerability exist in other test functions in this file?

Similar to the fix applied here, the loops in
damos_test_commit_quota_goals_for() and
damon_test_commit_target_regions_for() iterate over items without bounds
checking, only verifying the length after the loop completes.

For example, in damos_test_commit_quota_goals_for():

mm/damon/tests/core-kunit.h:damos_test_commit_quota_goals_for() {
	...
	damos_for_each_quota_goal(goal, &dst->quota) {
		KUNIT_EXPECT_EQ(test, goal->metric, src_goals[i].metric);
		...
	}
	...
}

Could these loops exceed the bounds of arrays like src_goals[] or
expect_start_end[] if the tested function produces more elements than
expected?

>  	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]);

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260718001442.87129-1-sj@kernel.org?part=5

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH 4/7] mm/damon/core-kunit: handle region split failure in filter_out()
  2026-07-18  0:14 ` [PATCH 4/7] mm/damon/core-kunit: handle region split failure in filter_out() SJ Park
@ 2026-07-18  0:29   ` sashiko-bot
  2026-07-18  1:00     ` SJ Park
  0 siblings, 1 reply; 22+ messages in thread
From: sashiko-bot @ 2026-07-18  0:29 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:
- [High] In `damon_apply_min_nr_regions()`, `damon_split_regions_of()`, and `kdamond_split_regions()`, if `damon_split_region_at()` fails to allocate memory for a new region, the loop variable `r` is still unconditionally advanced to the next region. This leads to iterating past the end of the region list, reading from out-of-bounds memory, and potentially causing memory corruption.
--

--- Patch [4]: [PATCH 4/7] mm/damon/core-kunit: handle region split failure in filter_out() ---
commit 8274c3713b297d606d32992612a8cbd24ac04759
Author: SJ Park <sj@kernel.org>

mm/damon/core-kunit: handle region split failure in filter_out()

This commit modifies the kunit test damos_test_filter_out() to add early
exit checks when damon_nr_regions() does not return the expected value. This
prevents the test from dereferencing invalid pointers if
damon_split_region_at() fails. It fixes memory corruption that could occur
during test runs.

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..eddf3a0484583 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);

[ ... ]

[Severity: High]
This is a pre-existing issue, but I noticed a similar pattern in mm/damon/core.c
where damon_split_region_at() failures might not be handled safely.

In damon_apply_min_nr_regions():

mm/damon/core.c: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 to allocate memory due to memory pressure and
silently returns, does the unconditional call to damon_next_region(r) advance
past the end of the list?

If `r` was the last region in the target's list, could `r` become the list head
and point to memory before the struct damon_target?

If so, the condition while (damon_sz_region(r) > max_region_sz) could read
from out-of-bounds memory.

If the resulting garbage size exceeds max_region_sz, it might call
damon_split_region_at() on the invalid pointer, which could then corrupt the
heap by writing to r->ar.end.

The same pattern appears in damon_split_regions_of():

mm/damon/core.c:damon_split_regions_of() {
    ...
			damon_split_region_at(t, r, sz_sub);
			sz_region = sz_region - sz_sub;
			r = damon_next_region(r);
    ...
}

And also in kdamond_split_regions():

mm/damon/core.c:kdamond_split_regions() {
    ...
				damon_split_region_at(t, r, sz_sub);
				sz_region = sz_region - sz_sub;
				r = damon_next_region(r);
    ...
}

Can these paths also wrap around the list and cause out-of-bounds accesses
during memory pressure in the kdamond thread?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260718001442.87129-1-sj@kernel.org?part=4

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH 2/7] mm/damon/core-kunit: check region count before testing in split_at()
  2026-07-18  0:14 ` [PATCH 2/7] mm/damon/core-kunit: check region count before testing in split_at() SJ Park
@ 2026-07-18  0:30   ` sashiko-bot
  2026-07-18  0:55     ` SJ Park
  0 siblings, 1 reply; 22+ messages in thread
From: sashiko-bot @ 2026-07-18  0:30 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] Silent allocation failure inside `damon_split_region_at()` causes an improper pointer traversal into the list head in `damon_apply_min_nr_regions()`, resulting in a potential infinite loop and out-of-bounds memory corruption.
- [Medium] The patch fixes an invalid pointer dereference upon split allocation failure in `damon_test_split_at()`, but misses an identical vulnerability in `damos_test_filter_out()`.
--

--- Patch [2]: [PATCH 2/7] mm/damon/core-kunit: check region count before testing in split_at() ---
commit 97ba538eacadf2d58d517d9d0f86ecaac4b3851f
Author: SJ Park <sj@kernel.org>
Subject: mm/damon/core-kunit: check region count before testing in split_at()

This commit fixes an issue in the KUnit tests where damon_test_split_at()
could dereference invalid pointers if a region split fails to allocate
memory. The fix tests if the number of regions was changed as expected
and exits early upon 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 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);

[Severity: Medium]
This isn't a bug introduced by this patch, but this exact same vulnerability
seems to exist later in this file in damos_test_filter_out().

If damos_filter_match() internally fails to allocate memory during a region
split, the test continues and damon_next_region(r) returns the list head.

mm/damon/tests/core-kunit.h:damos_test_filter_out() {
...
	/* region started before the range */
	r->ar.start = 1;
	r->ar.end = 4;
	KUNIT_EXPECT_FALSE(test, damos_filter_match(NULL, t, r, f, 1));
	/* filter should have split the region */
	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);
	r2 = damon_next_region(r);
	KUNIT_EXPECT_EQ(test, r2->ar.start, 2);
	KUNIT_EXPECT_EQ(test, r2->ar.end, 4);
	damon_destroy_region(r2, t);
...
}

Should the test also be checking the region count and returning early here,
before we pass an invalid pointer to damon_destroy_region() and corrupt the
slab?

[Severity: High]
This is a pre-existing issue, but I'm also concerned about silent allocation
failures from damon_split_region_at() in core kernel paths.

Looking at damon_apply_min_nr_regions():

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() silently fails to allocate memory, it returns
without splitting the region. The while loop then incorrectly advances r
to the next region.

If this allocation failure occurs on the last region in the list,
damon_next_region(r) will return the list head.

The loop condition damon_sz_region(r) > max_region_sz will evaluate memory
located before the damon_target struct.

If those garbage values evaluate to true, could this result in an out-of-bounds
access loop or corrupt memory? Should damon_split_region_at() return an
error code that callers can check?

[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260718001442.87129-1-sj@kernel.org?part=2

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH 1/7] mm/damon/core: initialize damos->last_applied
  2026-07-18  0:14 ` [PATCH 1/7] mm/damon/core: initialize damos->last_applied SJ Park
@ 2026-07-18  0:32   ` sashiko-bot
  2026-07-18  0:53     ` SJ Park
  0 siblings, 1 reply; 22+ messages in thread
From: sashiko-bot @ 2026-07-18  0:32 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] Uninitialized memory use in DAMOS quota goals allocation (last_psi_total and nid).
- [High] damos_quota_init() modifies the passed quota struct, unintentionally resetting runtime quota statistics of actively running schemes.
--

--- Patch [1]: [PATCH 1/7] mm/damon/core: initialize damos->last_applied ---
commit 316d27ac2ea17607b5c9df656020658fa1b425df
Author: SJ Park <sj@kernel.org>

mm/damon/core: initialize damos->last_applied

This commit initializes the last_applied field of the damos scheme struct to
NULL in damon_new_scheme(). This prevents an uninitialized value from causing
a DAMON scheme to unexpectedly skip applying an action to a folio the very
first time.

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 is a pre-existing issue, but does damos_quota_init() unintentionally reset 
the runtime statistics of actively running schemes?

When updating a DAMON context via sysfs, damon_commit_ctx() calls 
damon_new_test_ctx() which iterates over running schemes and calls 
damon_new_scheme(), passing the active &src_scheme->quota pointer.

Inside damon_new_scheme(), it executes:
scheme->quota = *(damos_quota_init(quota));

If damos_quota_init() directly modifies the passed pointer:

mm/damon/core.c:damos_quota_init() {
    quota->esz = 0;
    quota->total_charged_sz = 0;
    quota->total_charged_ns = 0;
    quota->charged_sz = 0;
    ...
}

It appears this silently clears the tracking variables of the actively running 
scheme, potentially breaking the ability to track memory usage limits.

[Severity: High]
This isn't a bug introduced by this patch, but while reviewing initialization,
I noticed a potential issue with quota goals. Are the metric-specific fields in
damos_new_quota_goal() left uninitialized?

When allocating a new goal via sysfs or damon_commit_ctx():

mm/damon/core.c:damos_new_quota_goal() {
    ...
    goal = kmalloc_obj(*goal);
    ...
}

It seems fields like last_psi_total and nid are not zero-initialized.

Later, damos_commit_quota_goal_union() misses the switch case for 
DAMOS_QUOTA_NODE_ELIGIBLE_MEM_BP, leaving dst->nid uninitialized.

This can result in using garbage values when calculating current_value:

mm/damon/core.c:damos_set_quota_goal_current_value() {
    ...
    goal->current_value = damos_get_node_eligible_mem_bp(c, s, goal->nid);
    ...
}

And for PSI goals:

mm/damon/core.c:damos_set_quota_goal_current_value() {
    ...
    goal->current_value = now_psi_total - goal->last_psi_total;
    goal->last_psi_total = now_psi_total;
    ...
}

Could subtracting an uninitialized last_psi_total underflow and cause the 
quota tuner to aggressively zero out the quota?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260718001442.87129-1-sj@kernel.org?part=1

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH 1/7] mm/damon/core: initialize damos->last_applied
  2026-07-18  0:32   ` sashiko-bot
@ 2026-07-18  0:53     ` SJ Park
  0 siblings, 0 replies; 22+ messages in thread
From: SJ Park @ 2026-07-18  0:53 UTC (permalink / raw)
  To: sashiko-bot; +Cc: SJ Park, damon

On Sat, 18 Jul 2026 00:32:20 +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] Uninitialized memory use in DAMOS quota goals allocation (last_psi_total and nid).
> - [High] damos_quota_init() modifies the passed quota struct, unintentionally resetting runtime quota statistics of actively running schemes.
> --
> 
> --- Patch [1]: [PATCH 1/7] mm/damon/core: initialize damos->last_applied ---
> commit 316d27ac2ea17607b5c9df656020658fa1b425df
> Author: SJ Park <sj@kernel.org>
> 
> mm/damon/core: initialize damos->last_applied
> 
> This commit initializes the last_applied field of the damos scheme struct to
> NULL in damon_new_scheme(). This prevents an uninitialized value from causing
> a DAMON scheme to unexpectedly skip applying an action to a folio the very
> first time.
> 
> 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 is a pre-existing issue, but does damos_quota_init() unintentionally reset 
> the runtime statistics of actively running schemes?
> 
> When updating a DAMON context via sysfs, damon_commit_ctx() calls 
> damon_new_test_ctx() which iterates over running schemes and calls 
> damon_new_scheme(), passing the active &src_scheme->quota pointer.
> 
> Inside damon_new_scheme(), it executes:
> scheme->quota = *(damos_quota_init(quota));
> 
> If damos_quota_init() directly modifies the passed pointer:
> 
> mm/damon/core.c:damos_quota_init() {
>     quota->esz = 0;
>     quota->total_charged_sz = 0;
>     quota->total_charged_ns = 0;
>     quota->charged_sz = 0;
>     ...
> }
> 
> It appears this silently clears the tracking variables of the actively running 
> scheme, potentially breaking the ability to track memory usage limits.

No.  The damon_new_scheme() is for a case the commit is adding new scheme to
the destination context.  There is no data to erase.

> 
> [Severity: High]
> This isn't a bug introduced by this patch, but while reviewing initialization,
> I noticed a potential issue with quota goals. Are the metric-specific fields in
> damos_new_quota_goal() left uninitialized?
> 
> When allocating a new goal via sysfs or damon_commit_ctx():
> 
> mm/damon/core.c:damos_new_quota_goal() {
>     ...
>     goal = kmalloc_obj(*goal);
>     ...
> }
> 
> It seems fields like last_psi_total and nid are not zero-initialized.
> 
> Later, damos_commit_quota_goal_union() misses the switch case for 
> DAMOS_QUOTA_NODE_ELIGIBLE_MEM_BP, leaving dst->nid uninitialized.
> 
> This can result in using garbage values when calculating current_value:
> 
> mm/damon/core.c:damos_set_quota_goal_current_value() {
>     ...
>     goal->current_value = damos_get_node_eligible_mem_bp(c, s, goal->nid);
>     ...
> }

This is no issue.  The caller should set the nid.  And core functions using the
nid validates it before really using it.

> 
> And for PSI goals:
> 
> mm/damon/core.c:damos_set_quota_goal_current_value() {
>     ...
>     goal->current_value = now_psi_total - goal->last_psi_total;
>     goal->last_psi_total = now_psi_total;
>     ...
> }
> 
> Could subtracting an uninitialized last_psi_total underflow and cause the 
> quota tuner to aggressively zero out the quota?

Good finding.  I will separately fix this.

> 
> -- 
> Sashiko AI review · https://sashiko.dev/#/patchset/20260718001442.87129-1-sj@kernel.org?part=1


Thanks,
SJ

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH 2/7] mm/damon/core-kunit: check region count before testing in split_at()
  2026-07-18  0:30   ` sashiko-bot
@ 2026-07-18  0:55     ` SJ Park
  0 siblings, 0 replies; 22+ messages in thread
From: SJ Park @ 2026-07-18  0:55 UTC (permalink / raw)
  To: sashiko-bot; +Cc: SJ Park, damon

On Sat, 18 Jul 2026 00:30:26 +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] Silent allocation failure inside `damon_split_region_at()` causes an improper pointer traversal into the list head in `damon_apply_min_nr_regions()`, resulting in a potential infinite loop and out-of-bounds memory corruption.
> - [Medium] The patch fixes an invalid pointer dereference upon split allocation failure in `damon_test_split_at()`, but misses an identical vulnerability in `damos_test_filter_out()`.
> --
> 
> --- Patch [2]: [PATCH 2/7] mm/damon/core-kunit: check region count before testing in split_at() ---
> commit 97ba538eacadf2d58d517d9d0f86ecaac4b3851f
> Author: SJ Park <sj@kernel.org>
> Subject: mm/damon/core-kunit: check region count before testing in split_at()
> 
> This commit fixes an issue in the KUnit tests where damon_test_split_at()
> could dereference invalid pointers if a region split fails to allocate
> memory. The fix tests if the number of regions was changed as expected
> and exits early upon 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 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);
> 
> [Severity: Medium]
> This isn't a bug introduced by this patch, but this exact same vulnerability
> seems to exist later in this file in damos_test_filter_out().
> 
> If damos_filter_match() internally fails to allocate memory during a region
> split, the test continues and damon_next_region(r) returns the list head.
> 
> mm/damon/tests/core-kunit.h:damos_test_filter_out() {
> ...
> 	/* region started before the range */
> 	r->ar.start = 1;
> 	r->ar.end = 4;
> 	KUNIT_EXPECT_FALSE(test, damos_filter_match(NULL, t, r, f, 1));
> 	/* filter should have split the region */
> 	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);
> 	r2 = damon_next_region(r);
> 	KUNIT_EXPECT_EQ(test, r2->ar.start, 2);
> 	KUNIT_EXPECT_EQ(test, r2->ar.end, 4);
> 	damon_destroy_region(r2, t);
> ...
> }
> 
> Should the test also be checking the region count and returning early here,
> before we pass an invalid pointer to damon_destroy_region() and corrupt the
> slab?

This is fixed by a later patch of this series.

> 
> [Severity: High]
> This is a pre-existing issue, but I'm also concerned about silent allocation
> failures from damon_split_region_at() in core kernel paths.
> 
> Looking at damon_apply_min_nr_regions():
> 
> 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() silently fails to allocate memory, it returns
> without splitting the region. The while loop then incorrectly advances r
> to the next region.
> 
> If this allocation failure occurs on the last region in the list,
> damon_next_region(r) will return the list head.
> 
> The loop condition damon_sz_region(r) > max_region_sz will evaluate memory
> located before the damon_target struct.
> 
> If those garbage values evaluate to true, could this result in an out-of-bounds
> access loop or corrupt memory? Should damon_split_region_at() return an
> error code that callers can check?

I'm separately working [1] on it.

[1] https://lore.kernel.org/20260718004301.88883-1-sj@kernel.org

> 
> [ ... ]
> 
> -- 
> Sashiko AI review · https://sashiko.dev/#/patchset/20260718001442.87129-1-sj@kernel.org?part=2


Thanks,
SJ

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH 4/7] mm/damon/core-kunit: handle region split failure in filter_out()
  2026-07-18  0:29   ` sashiko-bot
@ 2026-07-18  1:00     ` SJ Park
  0 siblings, 0 replies; 22+ messages in thread
From: SJ Park @ 2026-07-18  1:00 UTC (permalink / raw)
  To: sashiko-bot; +Cc: SJ Park, damon

On Sat, 18 Jul 2026 00:29:09 +0000 sashiko-bot@kernel.org wrote:

> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> 
> Pre-existing issues:
> - [High] In `damon_apply_min_nr_regions()`, `damon_split_regions_of()`, and `kdamond_split_regions()`, if `damon_split_region_at()` fails to allocate memory for a new region, the loop variable `r` is still unconditionally advanced to the next region. This leads to iterating past the end of the region list, reading from out-of-bounds memory, and potentially causing memory corruption.
> --
> 
> --- Patch [4]: [PATCH 4/7] mm/damon/core-kunit: handle region split failure in filter_out() ---
> commit 8274c3713b297d606d32992612a8cbd24ac04759
> Author: SJ Park <sj@kernel.org>
> 
> mm/damon/core-kunit: handle region split failure in filter_out()
> 
> This commit modifies the kunit test damos_test_filter_out() to add early
> exit checks when damon_nr_regions() does not return the expected value. This
> prevents the test from dereferencing invalid pointers if
> damon_split_region_at() fails. It fixes memory corruption that could occur
> during test runs.
> 
> 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..eddf3a0484583 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);
> 
> [ ... ]
> 
> [Severity: High]
> This is a pre-existing issue, but I noticed a similar pattern in mm/damon/core.c
> where damon_split_region_at() failures might not be handled safely.
> 
> In damon_apply_min_nr_regions():
> 
> mm/damon/core.c: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 to allocate memory due to memory pressure and
> silently returns, does the unconditional call to damon_next_region(r) advance
> past the end of the list?
> 
> If `r` was the last region in the target's list, could `r` become the list head
> and point to memory before the struct damon_target?
> 
> If so, the condition while (damon_sz_region(r) > max_region_sz) could read
> from out-of-bounds memory.
> 
> If the resulting garbage size exceeds max_region_sz, it might call
> damon_split_region_at() on the invalid pointer, which could then corrupt the
> heap by writing to r->ar.end.

Yes, I'm separately working [1] on it.

> 
> The same pattern appears in damon_split_regions_of():
> 
> mm/damon/core.c:damon_split_regions_of() {
>     ...
> 			damon_split_region_at(t, r, sz_sub);
> 			sz_region = sz_region - sz_sub;
> 			r = damon_next_region(r);
>     ...
> }
> 
> And also in kdamond_split_regions():
> 
> mm/damon/core.c:kdamond_split_regions() {
>     ...
> 				damon_split_region_at(t, r, sz_sub);
> 				sz_region = sz_region - sz_sub;
> 				r = damon_next_region(r);
>     ...
> }
> 
> Can these paths also wrap around the list and cause out-of-bounds accesses
> during memory pressure in the kdamond thread?

No, the above two cases are hallucinations.  The example code snippet is
different from the real ones.

> 
> -- 
> Sashiko AI review · https://sashiko.dev/#/patchset/20260718001442.87129-1-sj@kernel.org?part=4

[1] https://lore.kernel.org/20260718004301.88883-1-sj@kernel.org


Thanks,
SJ

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH 5/7] mm/damon/core-kunit: skip wrong dest walk in commit_dests_for()
  2026-07-18  0:25   ` sashiko-bot
@ 2026-07-18  1:01     ` SJ Park
  0 siblings, 0 replies; 22+ messages in thread
From: SJ Park @ 2026-07-18  1:01 UTC (permalink / raw)
  To: sashiko-bot; +Cc: SJ Park, damon

On Sat, 18 Jul 2026 00:25:43 +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] If `damos_commit_dests()` fails to allocate the `weight_arr` array, it returns `-ENOMEM` but leaves the `dst` context partially modified with a NULL `weight_arr` and an incorrect `nr_dests`, leading to a NULL pointer dereference during DAMOS scheme execution.
> - [Medium] Missing out-of-bounds array bounds checks in the test functions `damos_test_commit_quota_goals_for()` and `damon_test_commit_target_regions_for()` when iterating over tested items.
> --
> 
> --- Patch [5]: [PATCH 5/7] mm/damon/core-kunit: skip wrong dest walk in commit_dests_for() ---
> commit 937f902e215bffeba8d97bcc0725b5bfbb643c41
> Author: SJ Park <sj@kernel.org>
> Subject: mm/damon/core-kunit: skip wrong dest walk in commit_dests_for()
> 
> This patch fixes a potential out-of-bounds access in the KUnit tests for
> damos_commit_dests(). It exits early in damos_test_commit_dests_for()
> if the expected number of destinations does not match the actual number.
> 
> Link: https://lore.kernel.org/20260713144757.39740-1-sj@kernel.org
> 
> > diff --git a/mm/damon/tests/core-kunit.h b/mm/damon/tests/core-kunit.h
> > index eddf3a0484583..9b8c20a50ad53 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;
> 
> [Severity: High]
> This isn't a bug introduced by this patch, but looking at the function being
> tested, could damos_commit_dests() leave the destination context in a
> corrupted state on allocation failure?
> 
> If kmalloc_array() fails to allocate weight_arr, it returns -ENOMEM but
> leaves dst->weight_arr as NULL without resetting dst->nr_dests:
> 
> mm/damon/core.c:damos_commit_dests() {
> 	...
> 		dst->weight_arr = kmalloc_array(src->nr_dests,
> 			sizeof(*dst->weight_arr), GFP_KERNEL);
> 		if (!dst->weight_arr) {
> 			/* ->node_id_arr will be freed by scheme destruction */
> 			return -ENOMEM;
> 		}
> 	}
> 	...
> }
> 
> Would a subsequent execution in damos_va_migrate_dests_add() dereference this
> NULL array when iterating over dst->nr_dests, leading to a system crash?

Yes, and another patch in this series fixes it.

> 
> [Severity: Medium]
> This is a pre-existing issue, but does the same out-of-bounds array access
> vulnerability exist in other test functions in this file?
> 
> Similar to the fix applied here, the loops in
> damos_test_commit_quota_goals_for() and
> damon_test_commit_target_regions_for() iterate over items without bounds
> checking, only verifying the length after the loop completes.
> 
> For example, in damos_test_commit_quota_goals_for():
> 
> mm/damon/tests/core-kunit.h:damos_test_commit_quota_goals_for() {
> 	...
> 	damos_for_each_quota_goal(goal, &dst->quota) {
> 		KUNIT_EXPECT_EQ(test, goal->metric, src_goals[i].metric);
> 		...
> 	}
> 	...
> }
> 
> Could these loops exceed the bounds of arrays like src_goals[] or
> expect_start_end[] if the tested function produces more elements than
> expected?

Yes, and another patch in this series fixes it.

> 
> >  	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]);
> 
> -- 
> Sashiko AI review · https://sashiko.dev/#/patchset/20260718001442.87129-1-sj@kernel.org?part=5


Thanks,
SJ

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH 6/7] mm/damon/core-kunit: skip wrong quota goal walk in commit_quota_goals()
  2026-07-18  0:22   ` sashiko-bot
@ 2026-07-18  1:02     ` SJ Park
  0 siblings, 0 replies; 22+ messages in thread
From: SJ Park @ 2026-07-18  1:02 UTC (permalink / raw)
  To: sashiko-bot; +Cc: SJ Park, damon

On Sat, 18 Jul 2026 00:22:57 +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] Out-of-bounds array read in damon_test_commit_target_regions_for() due to missing region count validation before traversing dst_target.
> --
> 
> --- Patch [6]: [PATCH 6/7] mm/damon/core-kunit: skip wrong quota goal walk in commit_quota_goals() ---
> commit 4bcb5123bbb5f6b4908f1d776a8608ef05a89501
> Author: SJ Park <sj@kernel.org>
> Subject: mm/damon/core-kunit: skip wrong quota goal walk in commit_quota_goals()
> 
> This commit addresses an out-of-bounds array access in the KUnit test
> damos_test_commit_quota_goals_for(). If the number of created quota goals
> does not match the expected count, blindly traversing them can cause the test
> to read past the bounds of the expected values array. The patch adds a count
> validation and an early exit to avoid this invalid traversal.
> 
> Link: https://lore.kernel.org/20260713144757.39740-1-sj@kernel.org
> 
> > diff --git a/mm/damon/tests/core-kunit.h b/mm/damon/tests/core-kunit.h
> > index 9b8c20a50ad53..fbf986aee2dea 100644
> > --- 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 isn't a bug introduced by this patch, but should a similar early exit
> check be added to damon_test_commit_target_regions_for() to prevent an
> out-of-bounds read?
> 
> Just like the issue fixed here, damon_test_commit_target_regions_for()
> iterates over dst_target regions without validating the region count against
> nr_expect_regions first:
> 
> 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++;
> 	}
> 	KUNIT_EXPECT_EQ(test, damon_nr_regions(dst_target), nr_expect_regions);
>     ...
> }
> 
> If damon_commit_target_regions() incorrectly leaves more regions in dst_target
> than nr_expect_regions, the loop will read past the end of the
> expect_start_end[] array before reaching the KUNIT_EXPECT_EQ count validation
> at the end of the function.

Yes, and another patch in this series fixes it.

> 
> -- 
> Sashiko AI review · https://sashiko.dev/#/patchset/20260718001442.87129-1-sj@kernel.org?part=6


Thanks,
SJ

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH 0/7] mm/damon: fix uninitialized DAMOS field and kunit exec expectation bugs
  2026-07-18  0:14 [PATCH 0/7] mm/damon: fix uninitialized DAMOS field and kunit exec expectation bugs SJ Park
                   ` (6 preceding siblings ...)
  2026-07-18  0:14 ` [PATCH 7/7] mm/damon/core-kunit: skip wrong region walk in commit_target_regions() SJ Park
@ 2026-07-18  1:13 ` SJ Park
  7 siblings, 0 replies; 22+ messages in thread
From: SJ Park @ 2026-07-18  1:13 UTC (permalink / raw)
  To: SJ Park
  Cc: Andrew Morton, stable, Brendan Higgins, David Gow, SeongJae Park,
	damon, kunit-dev, linux-kernel, linux-kselftest, linux-mm

On Fri, 17 Jul 2026 17:14:34 -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.

Sashiko found no blocker for this series.  Sashiko sent findings to damon@
mailing list [1], and I replied to all the comments.  Please read those for
details.

[1] https://lore.kernel.org/damon/


Thanks,
SJ

[...]

^ permalink raw reply	[flat|nested] 22+ messages in thread

end of thread, other threads:[~2026-07-18  1:13 UTC | newest]

Thread overview: 22+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-07-18  0:14 [PATCH 0/7] mm/damon: fix uninitialized DAMOS field and kunit exec expectation bugs SJ Park
2026-07-18  0:14 ` [PATCH 1/7] mm/damon/core: initialize damos->last_applied SJ Park
2026-07-18  0:32   ` sashiko-bot
2026-07-18  0:53     ` SJ Park
2026-07-18  0:14 ` [PATCH 2/7] mm/damon/core-kunit: check region count before testing in split_at() SJ Park
2026-07-18  0:30   ` sashiko-bot
2026-07-18  0:55     ` SJ Park
2026-07-18  0:14 ` [PATCH 3/7] mm/damon/vaddr-kunit: check region count in three_regions test SJ Park
2026-07-18  0:14 ` [PATCH 4/7] mm/damon/core-kunit: handle region split failure in filter_out() SJ Park
2026-07-18  0:29   ` sashiko-bot
2026-07-18  1:00     ` SJ Park
2026-07-18  0:14 ` [PATCH 5/7] mm/damon/core-kunit: skip wrong dest walk in commit_dests_for() SJ Park
2026-07-18  0:25   ` sashiko-bot
2026-07-18  1:01     ` SJ Park
2026-07-18  0:14 ` [PATCH 6/7] mm/damon/core-kunit: skip wrong quota goal walk in commit_quota_goals() SJ Park
2026-07-18  0:22   ` sashiko-bot
2026-07-18  1:02     ` SJ Park
2026-07-18  0:14 ` [PATCH 7/7] mm/damon/core-kunit: skip wrong region walk in commit_target_regions() SJ Park
2026-07-18  1:13 ` [PATCH 0/7] mm/damon: fix uninitialized DAMOS field and kunit exec expectation bugs SJ Park
  -- strict thread matches above, loose matches on Subject: below --
2026-07-17  0:30 SJ Park
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

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.