mm/damon: fix uninitialized DAMOS field and kunit exec expectation bugs

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 <[email protected]>

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/[email protected] open here

mm/damon/core.c struct damos *damon_new_scheme(struct damos_access_pattern *pattern, · line 711

Quoted 12 lines

mm/damon/core.c +1 -0

index 603b102ff80f9..20d267c615faf 100644
@@ -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?

Signature · 2 lines

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=1

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 <[email protected]>
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/[email protected] open here

Quoted 5 lines

mm/damon/tests/core-kunit.h +0 -0

index eba643762132f..322a295ac2cec 100644

mm/damon/tests/core-kunit.h static void damon_test_split_at(struct kunit *test) · line 163

Quoted 9 lines

mm/damon/tests/core-kunit.h +4 -0

@@ -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?

[ ... ]

Signature · 1 line

Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=2

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 <[email protected]>

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/[email protected] open here

mm/damon/tests/core-kunit.h static void damos_test_filter_out(struct kunit *test) · line 1370

Quoted 11 lines

mm/damon/tests/core-kunit.h +2 -0

index 322a295ac2cec..eddf3a0484583 100644
@@ -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?

Signature · 2 lines

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=4

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 <[email protected]>
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/[email protected] open here

mm/damon/tests/core-kunit.h static void damos_test_commit_dests_for(struct kunit *test, · line 1012

Quoted 10 lines

mm/damon/tests/core-kunit.h +2 -0

index eddf3a0484583..9b8c20a50ad53 100644
@@ -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?

Quoted 4 lines

mm/damon/tests/core-kunit.h +0 -0

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

Signature · 1 line

Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=5

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 <[email protected]>
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/[email protected] open here

Quoted 9 lines

mm/damon/tests/core-kunit.h +1 -0

index 9b8c20a50ad53..fbf986aee2dea 100644
@@ -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)

mm/damon/tests/core-kunit.h static void damos_test_commit_quota_goals_for(struct kunit *test, · line 875

Quoted 10 lines

mm/damon/tests/core-kunit.h +7 -0

+		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.

Signature · 2 lines

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=6

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.