mm/damon: move damos filter range arguments validation to core

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Unhandled error returns in DAMOS filter commit functions lead to uninitialized memory use and type confusion. --

commit f2946ab5f9d35b397a925510dc71e57f307f11d2
Author: SJ Park <[email protected]>

mm/damon/core: return an error from damos_commit_filter_arg()

damos_commit_filter_arg() is supposed to always succeed. It may not in future, for example, if the given filter is invalid. Prepare the case by modifying its signature to return an error when it failed. Also pipe the return value to its callers and let them handle the error.

Quoted 4 lines

mm/damon/core.c +0 -0

index 0c98f7e267d07..3ad1fb2f21f3f 100644

[ ... ]

mm/damon/core.c static void damos_commit_filter_arg( · line 1353

Quoted 10 lines

mm/damon/core.c +2 -2

-static void damos_commit_filter(
+static int damos_commit_filter(
 		struct damos_filter *dst, struct damos_filter *src)
 {
 	dst->type = src->type;
 	dst->matching = src->matching;
 	dst->allow = src->allow;
-	damos_commit_filter_arg(dst, src);
+	return damos_commit_filter_arg(dst, src);
 }

[Severity: High]
Does this code leave the target filter in a type-confused state if damos_commit_filter_arg() fails?

Since dst->type is modified before the arguments are committed, if an error is returned and subsequently ignored by the caller, the filter could have a new type but mismatched union arguments from the old type.

Quoted 12 lines

static int damos_commit_core_filters(struct damos *dst, struct damos *src) {

mm/damon/core.c +1 -4

 	struct damos_filter *dst_filter, *next, *src_filter, *new_filter;
-	int i = 0, j = 0;
+	int i = 0, j = 0, err;
 
 	damos_for_each_core_filter_safe(dst_filter, next, dst) {
 		src_filter = damos_nth_core_filter(i++, src);
-		if (src_filter)
-			damos_commit_filter(dst_filter, src_filter);
-		else

mm/damon/core.c static void damos_commit_filter_arg( · line 1371

Quoted 10 lines

mm/damon/core.c +6 -0

+		if (src_filter) {
+			err = damos_commit_filter(dst_filter, src_filter);
+			if (err)
+				return err;
+		} else {
 			damos_destroy_filter(dst_filter);
+		}
 	}
 
 	damos_for_each_core_filter_safe(src_filter, next, src) {

[Severity: High]
Does this code ignore the return value of damos_commit_filter_arg() when creating new core filters?

Further down in this loop:

mm/damon/core.c:damos_commit_core_filters() {
	...
	new_filter = damos_new_filter(
			src_filter->type, src_filter->matching,
			src_filter->allow);
	if (!new_filter)
		return -ENOMEM;
	damos_commit_filter_arg(new_filter, src_filter);
	damos_add_filter(dst, new_filter);
	...
}

If damos_commit_filter_arg() fails, does this silently add an uninitialized filter with garbage union arguments to the destination scheme?

[Severity: High]
Are there also missing error checks in damos_commit_ops_filters()?

It appears damos_commit_ops_filters() wasn't updated in this patch to check the new integer return values from damos_commit_filter() or damos_commit_filter_arg():

mm/damon/core.c:damos_commit_ops_filters() {
	...
	damos_for_each_ops_filter_safe(dst_filter, next, dst) {
		src_filter = damos_nth_ops_filter(i++, src);
		if (src_filter)
			damos_commit_filter(dst_filter, src_filter);
	...
	damos_for_each_ops_filter_safe(src_filter, next, src) {
		...
		damos_commit_filter_arg(new_filter, src_filter);
		damos_add_filter(dst, new_filter);
	...
}

Can ignoring these errors leave the ops filter in a type-confused state or silently add uninitialized filters to the destination scheme?

Signature · 2 lines

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

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] The newly introduced validation in damos_commit_filter_arg() returns -EINVAL on failure, but callers damos_commit_core_filters and damos_commit_ops_filters ignore this return value for new filters, adding uninitialized memory to active DAMON context.
- [High] damos_commit_filter() mutates the target filter's state (type, matching, allow) before validating its arguments, leaving the live filter in a corrupted state if validation subsequently fails.
--

commit b7aac655c61845e48301bed4669cfcf294cf84cb
Author: SJ Park <[email protected]>

mm/damon/core: disallow max < min damos filter range arguments commit

This commit adds validation to damos_commit_filter_arg() to reject invalid ranges where max is less than min. It returns -EINVAL when an invalid range is provided, avoiding duplicated validation across different DAMON API callers.

mm/damon/core.c static int damos_commit_filter_arg( · line 1329

Quoted 10 lines

mm/damon/core.c +2 -0

index 3ad1fb2f21f3f..3dec1a8f7ed3a 100644
@@ -1325,15 +1325,21 @@ static int damos_commit_filter_arg(
 		dst->memcg_id = src->memcg_id;
 		break;
 	case DAMOS_FILTER_TYPE_ADDR:
+		if (src->addr_range.end < src->addr_range.start)
+			return -EINVAL;

[Severity: High]
Does this code leave the target filter in a corrupted state on error?

Looking at damos_commit_filter(), the state is mutated before the newly introduced validation has a chance to execute:

mm/damon/core.c:damos_commit_filter() {
	dst->type = src->type;
	dst->matching = src->matching;
	dst->allow = src->allow;
	return damos_commit_filter_arg(dst, src);
}

If damos_commit_filter_arg() returns -EINVAL here, dst->type, dst->matching, and dst->allow have already been modified. If an existing ops filter receives an invalid range update, its type will be updated but its union arguments will belong to the previous type. When the filter executes, it might misinterpret the old union memory according to the new type, causing unintended matching behavior.

[Severity: High]
Can this validation error cause uninitialized memory to be added to the active DAMON context?

Looking at damos_commit_core_filters() and damos_commit_ops_filters(), the return value of damos_commit_filter_arg() is ignored when creating new filters:

mm/damon/core.c:damos_commit_core_filters() {
...

		new_filter = damos_new_filter(
				src_filter->type, src_filter->matching,
				src_filter->allow);
		if (!new_filter)
			return -ENOMEM;
		damos_commit_filter_arg(new_filter, src_filter);
		damos_add_filter(dst, new_filter);
	}

Because damos_new_filter() allocates memory without zero-initializing the union arguments, returning -EINVAL from damos_commit_filter_arg() means the union payload (like addr_range or sz_range) is left uninitialized. The callers ignore this error and add the filter to the system anyway, which could result in using kmalloc garbage and triggering undefined matching behavior.

Additionally, damos_commit_ops_filters() ignores the return value of damos_commit_filter() for existing ops filters:

mm/damon/core.c:damos_commit_ops_filters() {
...

	damos_for_each_ops_filter_safe(dst_filter, next, dst) {
		src_filter = damos_nth_ops_filter(i++, src);
		if (src_filter)
			damos_commit_filter(dst_filter, src_filter);

...

If an existing ops filter receives an invalid update, the validation error is swallowed entirely.

Quoted 3 lines

mm/damon/core.c +0 -0

 		dst->addr_range = src->addr_range;
 		break;

[ ... ]

Signature · 1 line

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

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Removing the filter range argument validation in damon_sysfs_add_scheme_filters() relies on the core layer to perform this validation. However, the core layer's commit functions (damos_commit_core_filters() and damos_commit_ops_filters()) silently ignore the return value of damos_commit_filter_arg() when adding new filters. When an invalid range is provided via sysfs, damos_commit_filter_arg() returns -EINVAL and aborts before copying the arguments. Because damos_new_filter() uses kmalloc_obj() (which does not zero memory), the new filter's union arguments (e.g., addr_range) are left completely uninitialized. The filter is then added to the active DAMON context, resulting in the scheme acting on an unpredictable memory range derived from uninitialized heap garbage. --

commit a45f05608fa3c7f9050ea4eac0fd8f19b22ee509
Author: SJ Park <[email protected]>

mm/damon/sysfs-schemes: drop centralized filter range arg validations

This patch drops the centralized filter range argument validations in the DAMON sysfs interface, relying on the core layer to perform this validation.

Quoted 4 lines

mm/damon/sysfs-schemes.c +0 -0

index 8c8ab82c8facc..a4ec5d54cfbd1 100644

[ ... ]

Quoted 8 lines

mm/damon/sysfs-schemes.c +0 -4

@@ -2832,27 +2832,14 @@ static int damon_sysfs_add_scheme_filters(struct damos *scheme,
 			}
 		} else if (filter->type == DAMOS_FILTER_TYPE_ADDR) {
 			filter->addr_range = sysfs_filter->addr_range;
-			if (filter->addr_range.end <
-					filter->addr_range.start) {
-				damos_destroy_filter(filter);
-				return -EINVAL;

mm/damon/sysfs-schemes.c static int damon_sysfs_add_scheme_filters(struct damos *scheme, · line 2848

Quoted 10 lines

mm/damon/sysfs-schemes.c +0 -5

-			}
 		} else if (filter->type == DAMOS_FILTER_TYPE_TARGET) {
 			filter->target_idx = sysfs_filter->target_idx;
 		} else if (filter->type == DAMOS_FILTER_TYPE_HUGEPAGE_SIZE) {
 			filter->sz_range.min = sysfs_filter->range_min;
 			filter->sz_range.max = sysfs_filter->range_max;
-			if (filter->range_min > filter->range_max) {
-				damos_destroy_filter(filter);
-				return -EINVAL;
-			}

[Severity: High]
Does removing this validation allow invalid ranges to bypass sysfs and reach the core layer logic where errors are silently ignored?

Looking at damos_commit_core_filters() in mm/damon/core.c, the return value of damos_commit_filter_arg() is not checked when adding new filters:

mm/damon/core.c:damos_commit_core_filters() {
    ...
    damos_commit_filter_arg(new_filter, src_filter);
    damos_add_filter(dst, new_filter);
    ...
}

A similar pattern exists for ops filters in damos_commit_ops_filters():

mm/damon/core.c:damos_commit_ops_filters() {
    ...
    if (src_filter)
        damos_commit_filter(dst_filter, src_filter);
    ...
    damos_commit_filter_arg(new_filter, src_filter);
    damos_add_filter(dst, new_filter);
    ...
}

When an invalid range is provided via sysfs, damos_commit_filter_arg() returns -EINVAL and aborts before copying the arguments. Because the new filter is allocated without zeroing its memory, the filter's union arguments (like addr_range) are left completely uninitialized.

Since the error is ignored and damos_add_filter() is still called, won't this result in the scheme acting on an unpredictable memory range derived from uninitialized heap garbage?

Quoted 10 lines

mm/damon/sysfs-schemes.c +0 -4

 		} else if (filter->type == DAMOS_FILTER_TYPE_PROBE_HITS_WSUM) {
 			filter->range_min = sysfs_filter->range_min;
 			filter->range_max = sysfs_filter->range_max;
-			if (filter->range_min > filter->range_max) {
-				damos_destroy_filter(filter);
-				return -EINVAL;
-			}
 		}
 

Quoted 1 line

mm/damon/sysfs-schemes.c +0 -0

 		damos_add_filter(scheme, filter);

Signature · 1 line

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

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

New issues:
- [High] Missing error handling for damos_commit_filter_arg() and damos_commit_filter() leads to uninitialized memory use for DAMON filters.

Pre-existing issues:
- [Critical] Memory allocation failure during damos_commit_dests() leaves the active scheme object partially torn down, leading to a deterministic NULL pointer dereference. --

commit 0ffd278b1f3947b2729c57dd026fc9283304d3af
Author: SJ Park <[email protected]>

mm/damon/core-kunit: test invalid damos filter commits

Add test cases for testing the validation of damos filter arguments in commit time.

Quoted 5 lines

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

index 2b0931cf6fb32..527abc2570616 100644

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

Quoted 10 lines

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

@@ -1184,6 +1184,13 @@ static void damos_test_commit_filter(struct kunit *test)
 			.allow = false,
 			.sz_range = {.min = 234, .max = 345},
 			}, false);
+	damos_test_commit_filter_for(test, &dst,
+			&(struct damos_filter){
+			.type = DAMOS_FILTER_TYPE_HUGEPAGE_SIZE,
+			.matching = false,
+			.allow = false,
+			.sz_range = {.min = 456, .max = 123},
+			}, true);

[Severity: High]
Does this patch series leave uninitialized memory in the payload of DAMON filters if an invalid range is provided?

While this patch adds tests for invalid filter ranges, it appears the error handling in earlier patches of the series might be incomplete. If a user provides an invalid range (e.g., max < min) via DAMON sysfs, the core validation returns -EINVAL, but this error is ignored when allocating a new core filter:

mm/damon/core.c:damos_commit_core_filters() {
    ...
	damos_for_each_core_filter_safe(src_filter, next, src) {
		if (j++ < i)
			continue;

		new_filter = damos_new_filter(
				src_filter->type, src_filter->matching,
				src_filter->allow);
		if (!new_filter)
			return -ENOMEM;
		damos_commit_filter_arg(new_filter, src_filter);
		damos_add_filter(dst, new_filter);
	}
    ...
}

Similarly, the return values of both damos_commit_filter() and damos_commit_filter_arg() are ignored when updating or allocating an ops filter:

mm/damon/core.c:damos_commit_ops_filters() {
    ...
	damos_for_each_ops_filter_safe(dst_filter, next, dst) {
		src_filter = damos_nth_ops_filter(i++, src);
		if (src_filter)
			damos_commit_filter(dst_filter, src_filter);
		else
			damos_destroy_filter(dst_filter);
	}

	damos_for_each_ops_filter_safe(src_filter, next, src) {
		if (j++ < i)
			continue;

		new_filter = damos_new_filter(
				src_filter->type, src_filter->matching,
				src_filter->allow);
		if (!new_filter)
			return -ENOMEM;
		damos_commit_filter_arg(new_filter, src_filter);
		damos_add_filter(dst, new_filter);
	}
    ...
}

Since damos_new_filter() allocates with kmalloc_obj() (which does not zero memory), ignoring the return value of damos_commit_filter_arg() could lead to the uninitialized heap data remaining in the filter's payload (e.g. sz_range or addr_range) when it is added to the active DAMON context.

[Severity: Critical]
This is a pre-existing issue, but can memory allocation failure during damos_commit_dests() leave the active scheme partially torn down and lead to a NULL pointer dereference?

If memory allocation fails for dst->weight_arr, damos_commit_dests() leaves dst->nr_dests with its old non-zero value, but assigns NULL to the array:

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;
		}
    ...
}

Because damon_commit_schemes() breaks its loop on -ENOMEM and returns, the partially modified scheme is left physically linked in the live context's scheme list:

mm/damon/core.c:damon_commit_schemes() {
    ...
		src_scheme = damon_nth_scheme(i++, src);
		if (src_scheme) {
			err = damos_commit(dst_scheme, src_scheme);
			if (err)
				return err;
		}
    ...
}

When the kdamond worker thread later executes damos_va_migrate_dests_add() on this corrupted scheme, it appears it will iterate up to dests->nr_dests and dereference the NULL weight_arr pointer:

mm/damon/vaddr.c:damos_va_migrate_dests_add() {
    ...
	for (i = 0; i < dests->nr_dests; i++)
		weight_total += dests->weight_arr[i];
    ...
}

Is there a mechanism preventing this kernel panic if memory allocation fails on the live destination context?

[ ... ]

Signature · 1 line

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

Add test cases for testing the validation of damos filter arguments in commit time.

Signed-off-by SJ Park <[email protected]>

---
 mm/damon/tests/core-kunit.h | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

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

index c9941f3bfb42b..5da84caf4124d 100644
@@ -1195,6 +1195,13 @@ static void damos_test_commit_filter(struct kunit *test)
 			.allow = false,
 			.sz_range = {.min = 234, .max = 345},
 			}, false);
+	damos_test_commit_filter_for(test, &dst,
+			&(struct damos_filter){
+			.type = DAMOS_FILTER_TYPE_HUGEPAGE_SIZE,
+			.matching = false,
+			.allow = false,
+			.sz_range = {.min = 456, .max = 123},
+			}, true);
 	damos_test_commit_filter_for(test, &dst,
 			&(struct damos_filter){
 			.type = DAMOS_FILTER_TYPE_UNMAPPED,

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

 	damos_test_commit_filter_for(test, &dst,
 			&(struct damos_filter){
 			.type = DAMOS_FILTER_TYPE_TARGET,

Signature · 1 line

2.47.3

Quoted 2 lines · On Sat, 12 Sep 2026 12:18:06 -0700 SJ Park <[email protected]> wrote:

Add test cases for testing the validation of damos filter arguments in commit time.

Could we also test an invalid PROBE_HITS_WSUM range here?

Patch 4 adds the same max < min validation for DAMOS_FILTER_TYPE_PROBE_HITS_WSUM, but this test only covers ADDR and HUGEPAGE_SIZE.

Hi Kunwu,

Quoted 4 lines · On Mon, 14 Sep 2026 23:57:27 +0800 Kunwu Chan <[email protected]> wrote:

Quoted 2 lines · On Sat, 12 Sep 2026 12:18:06 -0700 SJ Park <[email protected]> wrote:

Add test cases for testing the validation of damos filter arguments in commit time.

[...]

Quoted 5 lines

Could we also test an invalid PROBE_HITS_WSUM range here?

Patch 4 adds the same max < min validation for DAMOS_FILTER_TYPE_PROBE_HITS_WSUM, but this test only covers ADDR and HUGEPAGE_SIZE.

I agree it is better to add a test. But not necessarily together with this series, in my humble opinion. I am separately working on the test.

Thanks,
SJ

[...]