[RFC PATCH 0/8] mm/damon: move damos filter range arguments validation to core

[RFC PATCH 0/8] mm/damon: move damos filter range arguments validation to core

[SJ Park]

DAMOS filter range arguments are being validated in the DAMON sysfs
interface.  Some of those have minor time-of-check to time-of-use
(TOCTOU) bugs.  In future, other callers might have duplicated
validations with similar bugs.

Fix the bugs and further refactor the code to move the validation to the
core layer.  Also add kunit test cases for the validations.

Patches 1 and 2 fix the existing TOCTOU bugs.  Patches 3 and 4 adds the
validation in the core layer.  Patch 5 drops the replicated validation
in DAMON sysfs interface.  Patch 6 further cleanup the code.  Patches 7
and 8 extends kunit tests to test the validation.

SJ Park (8):
  mm/damon/sysfs-schemes: read sysfs_filter->addr_range only once
  mm/damon/sysfs-schemes: read sysfs_filter->sz_range only once
  mm/damon/core: return an error from damos_commit_filter_arg()
  mm/damon/core: disallow max < min damos filter range arguments commit
  mm/damon/sysfs-schemes: drop centralized filter range arg validations
  mm/damon/sysfs-schemes: use switch-case in add_scheme_filters()
  mm/damon/core-kunit: extend damos_commit_filter_for() for wrong input
  mm/damon/core-kunit: test invalid damos filter commits

 mm/damon/core.c             | 24 ++++++++++++++++-------
 mm/damon/sysfs-schemes.c    | 32 +++++++++++++------------------
 mm/damon/tests/core-kunit.h | 38 ++++++++++++++++++++++++++++---------
 3 files changed, 59 insertions(+), 35 deletions(-)


base-commit: ec1b9fab95fbfd9c5dbdfb841d91929c2cb63518
-- 
2.47.3

[RFC PATCH 1/8] mm/damon/sysfs-schemes: read sysfs_filter->addr_range only once

[SJ Park]

DAMON sysfs interface reads the user-provided address range arguments
for addr type DAMOS filter twice.  Once for validation, and once again
for assignments to the variable that will be passed to the core
layer. If the user updates the argument in parallel, an invalid address
range could be passed to the core layer.  Avoid it by doing the
assignments first, and then validating the assigned variables before
passing those to the core layer.

User impact of the bug should be trivial.  From the core layer's
perspective, the invalid address range is not really invalid.  It just
works as having a weird address range.  No critical issues such as a
crash or a leak could happen.  And sane users ain't do such parallel
arguments update anyway.  If they do, such racy behavior is arguably
somewhat expected and deserved.  That said, there is no reason to keep
such races.

Fixes: 2f1abcfccd86 ("mm/damon/sysfs-schemes: support address range type DAMOS filter")
Cc: <stable@vger.kernel.org> # 6.6.x
Signed-off-by: SJ Park <sj@kernel.org>
---
 mm/damon/sysfs-schemes.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/mm/damon/sysfs-schemes.c b/mm/damon/sysfs-schemes.c
index 3de4d804e049f..3c1c1cb387fec 100644
--- a/mm/damon/sysfs-schemes.c
+++ b/mm/damon/sysfs-schemes.c
@@ -2831,12 +2831,12 @@ static int damon_sysfs_add_scheme_filters(struct damos *scheme,
 				return err;
 			}
 		} else if (filter->type == DAMOS_FILTER_TYPE_ADDR) {
-			if (sysfs_filter->addr_range.end <
-					sysfs_filter->addr_range.start) {
+			filter->addr_range = sysfs_filter->addr_range;
+			if (filter->addr_range.end <
+					filter->addr_range.start) {
 				damos_destroy_filter(filter);
 				return -EINVAL;
 			}
-			filter->addr_range = sysfs_filter->addr_range;
 		} else if (filter->type == DAMOS_FILTER_TYPE_TARGET) {
 			filter->target_idx = sysfs_filter->target_idx;
 		} else if (filter->type == DAMOS_FILTER_TYPE_HUGEPAGE_SIZE) {
-- 
2.47.3

[RFC PATCH 2/8] mm/damon/sysfs-schemes: read sysfs_filter->sz_range only once

[SJ Park]

DAMON sysfs interface reads the user-provided size range arguments for
hugepage_size type DAMOS filter twice.  Once for validation, and once
again for assignments to the variable that will be passed to the core
layer.  If the user updates the arguments in parallel, an invalid size
range could be passed to the core layer.  Avoid it by doing the
assignments first, and then validating the assigned variables before
passing those to the core layer.

User impact of the bug should be trivial.  From the core layer's
perspective, the invalid size range is not really invalid.  It just
works as having a weird size range.  No critical issues such as a crash
or a leak could happen.  And sane users ain't do such parallel arguments
update anyway.  If they do, such racy behavior is arguably somewhat
expected and deserved.  That said, there is no reason to keep such
races.

Fixes: ea1f204ba29a ("mm/damon/sysfs-schemes: add files for setting damos_filter->sz_range")
Cc: <stable@vger.kernel.org> # 6.15.x
Signed-off-by: SJ Park <sj@kernel.org>
---
 mm/damon/sysfs-schemes.c | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/mm/damon/sysfs-schemes.c b/mm/damon/sysfs-schemes.c
index 3c1c1cb387fec..8c8ab82c8facc 100644
--- a/mm/damon/sysfs-schemes.c
+++ b/mm/damon/sysfs-schemes.c
@@ -2840,13 +2840,12 @@ static int damon_sysfs_add_scheme_filters(struct damos *scheme,
 		} else if (filter->type == DAMOS_FILTER_TYPE_TARGET) {
 			filter->target_idx = sysfs_filter->target_idx;
 		} else if (filter->type == DAMOS_FILTER_TYPE_HUGEPAGE_SIZE) {
-			if (sysfs_filter->range_min >
-					sysfs_filter->range_max) {
+			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;
 			}
-			filter->sz_range.min = sysfs_filter->range_min;
-			filter->sz_range.max = sysfs_filter->range_max;
 		} else if (filter->type == DAMOS_FILTER_TYPE_PROBE_HITS_WSUM) {
 			filter->range_min = sysfs_filter->range_min;
 			filter->range_max = sysfs_filter->range_max;
-- 
2.47.3

[RFC PATCH 3/8] mm/damon/core: return an error from damos_commit_filter_arg()

[SJ Park]

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.

Signed-off-by: SJ Park <sj@kernel.org>
---
 mm/damon/core.c | 18 +++++++++++-------
 1 file changed, 11 insertions(+), 7 deletions(-)

diff --git a/mm/damon/core.c b/mm/damon/core.c
index 0c98f7e267d07..3ad1fb2f21f3f 100644
--- a/mm/damon/core.c
+++ b/mm/damon/core.c
@@ -1317,7 +1317,7 @@ static struct damos_filter *damos_nth_ops_filter(int n, struct damos *s)
 	return NULL;
 }
 
-static void damos_commit_filter_arg(
+static int damos_commit_filter_arg(
 		struct damos_filter *dst, struct damos_filter *src)
 {
 	switch (dst->type) {
@@ -1340,28 +1340,32 @@ static void damos_commit_filter_arg(
 	default:
 		break;
 	}
+	return 0;
 }
 
-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);
 }
 
 static int damos_commit_core_filters(struct damos *dst, struct damos *src)
 {
 	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
+		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) {
-- 
2.47.3

[RFC PATCH 4/8] mm/damon/core: disallow max < min damos filter range arguments commit

[SJ Park]

damos_commit_filter_arg() receives range arguments for a few types of
DAMOS filters.  It allows any range including max < min range.  It is
fine for the logic, but makes no sense to support it.  Actually DAMON
sysfs interface is doing the validation on its own.  To avoid duplicated
validations in multiple DAMON API callers, it would be better to do the
validation in the core layer.  Add a validation of the given range.

Signed-off-by: SJ Park <sj@kernel.org>
---
 mm/damon/core.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/mm/damon/core.c b/mm/damon/core.c
index 3ad1fb2f21f3f..3dec1a8f7ed3a 100644
--- a/mm/damon/core.c
+++ b/mm/damon/core.c
@@ -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;
 		dst->addr_range = src->addr_range;
 		break;
 	case DAMOS_FILTER_TYPE_TARGET:
 		dst->target_idx = src->target_idx;
 		break;
 	case DAMOS_FILTER_TYPE_HUGEPAGE_SIZE:
+		if (src->sz_range.max < src->sz_range.min)
+			return -EINVAL;
 		dst->sz_range = src->sz_range;
 		break;
 	case DAMOS_FILTER_TYPE_PROBE_HITS_WSUM:
+		if (src->range_max < src->range_min)
+			return -EINVAL;
 		dst->range_min = src->range_min;
 		dst->range_max = src->range_max;
 		break;
-- 
2.47.3

[RFC PATCH 5/8] mm/damon/sysfs-schemes: drop centralized filter range arg validations

[SJ Park]

DAMON sysfs interface is validating wrong range arguments for DAMOS
filters.  Now the core layer is doing the same validation.  Drop the
duplicated validation in DAMON sysfs interface.

Signed-off-by: SJ Park <sj@kernel.org>
---
 mm/damon/sysfs-schemes.c | 13 -------------
 1 file changed, 13 deletions(-)

diff --git a/mm/damon/sysfs-schemes.c b/mm/damon/sysfs-schemes.c
index 8c8ab82c8facc..a4ec5d54cfbd1 100644
--- a/mm/damon/sysfs-schemes.c
+++ b/mm/damon/sysfs-schemes.c
@@ -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;
-			}
 		} 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;
-			}
 		} 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;
-			}
 		}
 
 		damos_add_filter(scheme, filter);
-- 
2.47.3

[RFC PATCH 6/8] mm/damon/sysfs-schemes: use switch-case in add_scheme_filters()

[SJ Park]

damon_sysfs_add_scheeme_filters() has long if-else chains for DAMOS
filter types.  Convert the code to use switch-case, which would be
cleaner and more efficient.

Signed-off-by: SJ Park <sj@kernel.org>
---
 mm/damon/sysfs-schemes.c | 18 +++++++++++++-----
 1 file changed, 13 insertions(+), 5 deletions(-)

diff --git a/mm/damon/sysfs-schemes.c b/mm/damon/sysfs-schemes.c
index a4ec5d54cfbd1..bfb6f0bc3f213 100644
--- a/mm/damon/sysfs-schemes.c
+++ b/mm/damon/sysfs-schemes.c
@@ -2822,7 +2822,8 @@ static int damon_sysfs_add_scheme_filters(struct damos *scheme,
 
 		if (!filter)
 			return -ENOMEM;
-		if (filter->type == DAMOS_FILTER_TYPE_MEMCG) {
+		switch (filter->type) {
+		case DAMOS_FILTER_TYPE_MEMCG:
 			err = damon_sysfs_memcg_path_to_id(
 					sysfs_filter->memcg_path,
 					&filter->memcg_id);
@@ -2830,16 +2831,23 @@ static int damon_sysfs_add_scheme_filters(struct damos *scheme,
 				damos_destroy_filter(filter);
 				return err;
 			}
-		} else if (filter->type == DAMOS_FILTER_TYPE_ADDR) {
+			break;
+		case DAMOS_FILTER_TYPE_ADDR:
 			filter->addr_range = sysfs_filter->addr_range;
-		} else if (filter->type == DAMOS_FILTER_TYPE_TARGET) {
+			break;
+		case DAMOS_FILTER_TYPE_TARGET:
 			filter->target_idx = sysfs_filter->target_idx;
-		} else if (filter->type == DAMOS_FILTER_TYPE_HUGEPAGE_SIZE) {
+			break;
+		case DAMOS_FILTER_TYPE_HUGEPAGE_SIZE:
 			filter->sz_range.min = sysfs_filter->range_min;
 			filter->sz_range.max = sysfs_filter->range_max;
-		} else if (filter->type == DAMOS_FILTER_TYPE_PROBE_HITS_WSUM) {
+			break;
+		case DAMOS_FILTER_TYPE_PROBE_HITS_WSUM:
 			filter->range_min = sysfs_filter->range_min;
 			filter->range_max = sysfs_filter->range_max;
+			break;
+		default:
+			break;
 		}
 
 		damos_add_filter(scheme, filter);
-- 
2.47.3

[RFC PATCH 7/8] mm/damon/core-kunit: extend damos_commit_filter_for() for wrong input

[SJ Park]

damos_commit_filter_for() supposes damos_commit_filter() to always
succeed with given inputs.  damos_commit_filter() could return an error
for invalid inputs.  Existing callers always pass only valid inputs, but
they may pass invalid inputs in future, for test purposes.  Extend the
function to be able to be used for wrong inputs-caused error testing.

Signed-off-by: SJ Park <sj@kernel.org>
---
 mm/damon/tests/core-kunit.h | 24 +++++++++++++++---------
 1 file changed, 15 insertions(+), 9 deletions(-)

diff --git a/mm/damon/tests/core-kunit.h b/mm/damon/tests/core-kunit.h
index a47a5cdf285c8..c9941f3bfb42b 100644
--- a/mm/damon/tests/core-kunit.h
+++ b/mm/damon/tests/core-kunit.h
@@ -1127,9 +1127,15 @@ static void damos_test_commit_dests(struct kunit *test)
 }
 
 static void damos_test_commit_filter_for(struct kunit *test,
-		struct damos_filter *dst, struct damos_filter *src)
+		struct damos_filter *dst, struct damos_filter *src,
+		bool expect_fail)
 {
-	damos_commit_filter(dst, src);
+	int err;
+
+	err = damos_commit_filter(dst, src);
+	KUNIT_EXPECT_EQ(test, err != 0, expect_fail);
+	if (expect_fail)
+		return;
 	KUNIT_EXPECT_EQ(test, dst->type, src->type);
 	KUNIT_EXPECT_EQ(test, dst->matching, src->matching);
 	KUNIT_EXPECT_EQ(test, dst->allow, src->allow);
@@ -1168,47 +1174,47 @@ static void damos_test_commit_filter(struct kunit *test)
 			.type = DAMOS_FILTER_TYPE_ANON,
 			.matching = true,
 			.allow = true,
-			});
+			}, false);
 	damos_test_commit_filter_for(test, &dst,
 			&(struct damos_filter){
 			.type = DAMOS_FILTER_TYPE_MEMCG,
 			.matching = false,
 			.allow = false,
 			.memcg_id = 123,
-			});
+			}, false);
 	damos_test_commit_filter_for(test, &dst,
 			&(struct damos_filter){
 			.type = DAMOS_FILTER_TYPE_YOUNG,
 			.matching = true,
 			.allow = true,
-			});
+			}, false);
 	damos_test_commit_filter_for(test, &dst,
 			&(struct damos_filter){
 			.type = DAMOS_FILTER_TYPE_HUGEPAGE_SIZE,
 			.matching = false,
 			.allow = false,
 			.sz_range = {.min = 234, .max = 345},
-			});
+			}, false);
 	damos_test_commit_filter_for(test, &dst,
 			&(struct damos_filter){
 			.type = DAMOS_FILTER_TYPE_UNMAPPED,
 			.matching = true,
 			.allow = true,
-			});
+			}, false);
 	damos_test_commit_filter_for(test, &dst,
 			&(struct damos_filter){
 			.type = DAMOS_FILTER_TYPE_ADDR,
 			.matching = false,
 			.allow = false,
 			.addr_range = {.start = 456, .end = 567},
-			});
+			}, false);
 	damos_test_commit_filter_for(test, &dst,
 			&(struct damos_filter){
 			.type = DAMOS_FILTER_TYPE_TARGET,
 			.matching = true,
 			.allow = true,
 			.target_idx = 6,
-			});
+			}, false);
 }
 
 static void damos_test_help_initailize_scheme(struct damos *scheme)
-- 
2.47.3

[RFC PATCH 8/8] mm/damon/core-kunit: test invalid damos filter commits

[SJ Park]

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

Signed-off-by: SJ Park <sj@kernel.org>
---
 mm/damon/tests/core-kunit.h | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/mm/damon/tests/core-kunit.h b/mm/damon/tests/core-kunit.h
index c9941f3bfb42b..5da84caf4124d 100644
--- a/mm/damon/tests/core-kunit.h
+++ b/mm/damon/tests/core-kunit.h
@@ -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,
@@ -1208,6 +1215,13 @@ static void damos_test_commit_filter(struct kunit *test)
 			.allow = false,
 			.addr_range = {.start = 456, .end = 567},
 			}, false);
+	damos_test_commit_filter_for(test, &dst,
+			&(struct damos_filter){
+			.type = DAMOS_FILTER_TYPE_ADDR,
+			.matching = false,
+			.allow = false,
+			.addr_range = {.start = 567, .end = 456},
+			}, true);
 	damos_test_commit_filter_for(test, &dst,
 			&(struct damos_filter){
 			.type = DAMOS_FILTER_TYPE_TARGET,
-- 
2.47.3

Re: [RFC PATCH 6/8] mm/damon/sysfs-schemes: use switch-case in add_scheme_filters()

[sashiko-bot]

> damon_sysfs_add_scheeme_filters() has long if-else chains for DAMOS
> filter types.  Convert the code to use switch-case, which would be
> cleaner and more efficient.
> 
> Signed-off-by: SJ Park <sj@kernel.org>

Sashiko has reviewed this patch and found no issues. It looks great!

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

Re: [RFC PATCH 2/8] mm/damon/sysfs-schemes: read sysfs_filter->sz_range only once

[sashiko-bot]

> DAMON sysfs interface reads the user-provided size range arguments for
> hugepage_size type DAMOS filter twice.  Once for validation, and once
> again for assignments to the variable that will be passed to the core
> layer.  If the user updates the arguments in parallel, an invalid size
> range could be passed to the core layer.  Avoid it by doing the
> assignments first, and then validating the assigned variables before
> passing those to the core layer.
> 
> User impact of the bug should be trivial.  From the core layer's
> perspective, the invalid size range is not really invalid.  It just
> works as having a weird size range.  No critical issues such as a crash
> or a leak could happen.  And sane users ain't do such parallel arguments
> update anyway.  If they do, such racy behavior is arguably somewhat
> expected and deserved.  That said, there is no reason to keep such
> races.
> 
> Fixes: ea1f204ba29a ("mm/damon/sysfs-schemes: add files for setting damos_filter->sz_range")
> Cc: <stable@vger.kernel.org> # 6.15.x
> Signed-off-by: SJ Park <sj@kernel.org>

Sashiko has reviewed this patch and found no issues. It looks great!

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

Re: [RFC PATCH 7/8] mm/damon/core-kunit: extend damos_commit_filter_for() for wrong input

[sashiko-bot]

> damos_commit_filter_for() supposes damos_commit_filter() to always
> succeed with given inputs.  damos_commit_filter() could return an error
> for invalid inputs.  Existing callers always pass only valid inputs, but
> they may pass invalid inputs in future, for test purposes.  Extend the
> function to be able to be used for wrong inputs-caused error testing.
> 
> Signed-off-by: SJ Park <sj@kernel.org>

Sashiko has reviewed this patch and found no issues. It looks great!

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

Re: [RFC PATCH 3/8] mm/damon/core: return an error from damos_commit_filter_arg()

[sashiko-bot]

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 <sj@kernel.org>

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.

> diff --git a/mm/damon/core.c b/mm/damon/core.c
> index 0c98f7e267d07..3ad1fb2f21f3f 100644
> --- a/mm/damon/core.c
> +++ b/mm/damon/core.c
[ ... ]
> -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.

>  static int damos_commit_core_filters(struct damos *dst, struct damos *src)
>  {
>  	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
> +		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?

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

Re: [RFC PATCH 4/8] mm/damon/core: disallow max < min damos filter range arguments commit

[sashiko-bot]

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 <sj@kernel.org>

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.

> diff --git a/mm/damon/core.c b/mm/damon/core.c
> index 3ad1fb2f21f3f..3dec1a8f7ed3a 100644
> --- a/mm/damon/core.c
> +++ b/mm/damon/core.c
> @@ -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.

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

[ ... ]

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

Re: [RFC PATCH 5/8] mm/damon/sysfs-schemes: drop centralized filter range arg validations

[sashiko-bot]

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 <sj@kernel.org>

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.

> diff --git a/mm/damon/sysfs-schemes.c b/mm/damon/sysfs-schemes.c
> index 8c8ab82c8facc..a4ec5d54cfbd1 100644
> --- a/mm/damon/sysfs-schemes.c
> +++ b/mm/damon/sysfs-schemes.c

[ ... ]

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

>  		} 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;
> -			}
>  		}
>  
>  		damos_add_filter(scheme, filter);

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

Re: [RFC PATCH 8/8] mm/damon/core-kunit: test invalid damos filter commits

[sashiko-bot]

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 <sj@kernel.org>

mm/damon/core-kunit: test invalid damos filter commits
    
Add test cases for testing the validation of damos filter arguments in
commit time.

> diff --git a/mm/damon/tests/core-kunit.h b/mm/damon/tests/core-kunit.h
> index 2b0931cf6fb32..527abc2570616 100644
> --- a/mm/damon/tests/core-kunit.h
> +++ b/mm/damon/tests/core-kunit.h
> @@ -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?

[ ... ]

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

Re: [RFC PATCH 1/8] mm/damon/sysfs-schemes: read sysfs_filter->addr_range only once

[sashiko-bot]

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

Pre-existing issues:
- [High] A double-read of `sysfs_goal->metric` in `damos_sysfs_add_quota_score()` causes uninitialized memory use in the core DAMOS quota auto-tuning logic.
--

commit 4b90a6724a684c24da0cc60a260ec280b6d479ac
Author: SJ Park <sj@kernel.org>
Subject: mm/damon/sysfs-schemes: read sysfs_filter->addr_range only once

This commit fixes a potential race in the DAMON sysfs interface where
sysfs_filter->addr_range is read twice. By copying the address range first
and validating the copy, it prevents invalid configurations from reaching
the core layer.

> diff --git a/mm/damon/sysfs-schemes.c b/mm/damon/sysfs-schemes.c
> index 3de4d804e049f..3c1c1cb387fec 100644
> --- a/mm/damon/sysfs-schemes.c
> +++ b/mm/damon/sysfs-schemes.c
> @@ -2831,12 +2831,12 @@ static int damon_sysfs_add_scheme_filters(struct damos *scheme,
>  				return err;
>  			}
>  		} else if (filter->type == DAMOS_FILTER_TYPE_ADDR) {
> -			if (sysfs_filter->addr_range.end <
> -					sysfs_filter->addr_range.start) {
> +			filter->addr_range = sysfs_filter->addr_range;
> +			if (filter->addr_range.end <
> +					filter->addr_range.start) {
>  				damos_destroy_filter(filter);
>  				return -EINVAL;
>  			}
> -			filter->addr_range = sysfs_filter->addr_range;
>  		} else if (filter->type == DAMOS_FILTER_TYPE_TARGET) {
>  			filter->target_idx = sysfs_filter->target_idx;
>  		} else if (filter->type == DAMOS_FILTER_TYPE_HUGEPAGE_SIZE) {

[ ... ]

While reviewing this fix for the double-read of addr_range, I noticed a
similar pattern in the same file:

mm/damon/sysfs-schemes.c:damos_sysfs_add_quota_score() {
    ...
		goal = damos_new_quota_goal(sysfs_goal->metric,
				sysfs_goal->target_value);
		if (!goal)
			return -ENOMEM;
		switch (sysfs_goal->metric) {
		case DAMOS_QUOTA_USER_INPUT:
			goal->current_value = sysfs_goal->current_value;
			break;
    ...
}

[Severity: High]
This is a pre-existing issue, but does this code double-read
sysfs_goal->metric?

The sysfs_goal->metric value is read first to allocate and initialize
goal->metric via damos_new_quota_goal(), and then read a second time
in the switch statement.

If the metric changes concurrently from DAMOS_QUOTA_USER_INPUT to another
value between these two reads, could the switch statement branch away and
skip the initialization of goal->current_value?

Since kmalloc does not zero memory, wouldn't goal->current_value be left
uninitialized, causing the core auto-tuning logic to read garbage data?

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

Re: [RFC PATCH 3/8] mm/damon/core: return an error from damos_commit_filter_arg()

[SJ Park]

On Sat, 12 Sep 2026 19:29:16 +0000 sashiko-bot@kernel.org wrote:

> 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 <sj@kernel.org>
> 
> 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.
> 
> > diff --git a/mm/damon/core.c b/mm/damon/core.c
> > index 0c98f7e267d07..3ad1fb2f21f3f 100644
> > --- a/mm/damon/core.c
> > +++ b/mm/damon/core.c
> [ ... ]
> > -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.

Yes, and the caller will destroy the filter, so no problem.

> 
> >  static int damos_commit_core_filters(struct damos *dst, struct damos *src)
> >  {
> >  	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
> > +		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?

Good finding, I will fix this in the next spin.

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

Good finding, I will fix this in the next spin.

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


Thanks,
SJ

Re: [RFC PATCH 4/8] mm/damon/core: disallow max < min damos filter range arguments commit

[SJ Park]

On Sat, 12 Sep 2026 19:29:42 +0000 sashiko-bot@kernel.org wrote:

> 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 <sj@kernel.org>
> 
> 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.
> 
> > diff --git a/mm/damon/core.c b/mm/damon/core.c
> > index 3ad1fb2f21f3f..3dec1a8f7ed3a 100644
> > --- a/mm/damon/core.c
> > +++ b/mm/damon/core.c
> > @@ -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.

Yes, and the caller will destroy it, so no issue.

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

Good findings, I will fix this in the next spin.

> 
> >  		dst->addr_range = src->addr_range;
> >  		break;
> 
> [ ... ]
> 
> -- 
> Sashiko AI review · https://sashiko.dev/#/patchset/20260912191807.138502-1-sj@kernel.org?part=4


Thanks,
SJ

Re: [RFC PATCH 5/8] mm/damon/sysfs-schemes: drop centralized filter range arg validations

[SJ Park]

On Sat, 12 Sep 2026 19:30:02 +0000 sashiko-bot@kernel.org wrote:

> 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 <sj@kernel.org>
> 
> 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.
> 
> > diff --git a/mm/damon/sysfs-schemes.c b/mm/damon/sysfs-schemes.c
> > index 8c8ab82c8facc..a4ec5d54cfbd1 100644
> > --- a/mm/damon/sysfs-schemes.c
> > +++ b/mm/damon/sysfs-schemes.c
> 
> [ ... ]
> 
> > @@ -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;
> > -			}
> >  		} 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?

Yes, I will fix those in the next spin.

> 
> >  		} 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;
> > -			}
> >  		}
> >  
> >  		damos_add_filter(scheme, filter);
> 
> -- 
> Sashiko AI review · https://sashiko.dev/#/patchset/20260912191807.138502-1-sj@kernel.org?part=5


Thanks,
SJ

Re: [RFC PATCH 8/8] mm/damon/core-kunit: test invalid damos filter commits

[SJ Park]

On Sat, 12 Sep 2026 19:36:40 +0000 sashiko-bot@kernel.org wrote:

> 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 <sj@kernel.org>
> 
> mm/damon/core-kunit: test invalid damos filter commits
>     
> Add test cases for testing the validation of damos filter arguments in
> commit time.
> 
> > diff --git a/mm/damon/tests/core-kunit.h b/mm/damon/tests/core-kunit.h
> > index 2b0931cf6fb32..527abc2570616 100644
> > --- a/mm/damon/tests/core-kunit.h
> > +++ b/mm/damon/tests/core-kunit.h
> > @@ -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.

Yes, I will fix in the next spin.

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

Yes, we have damon_ctx->maybe_corrupted to prevent this.  So, no issue.

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


Thanks,
SJ