[RFC PATCH 0/8] mm/damon: validate all parameters in the core

[RFC PATCH 0/8] mm/damon: validate all parameters in the core

[SJ Park]

DAMON has a number of parameters.  Some of the parameters are validated
by DAMON, while some are validated by DAMON API callers.  Each caller
has their own set of parameters that are exposed to users.  Hence each
caller has their own ways to do the validation.  There is no clear lines
for the responsibilities.  It is confusing and easy to make mistakes at
validations.  Actually we found a few bugs in the class.

Define DAMON core as the place to do all the validations and implement
those.  damon_set_regions(), damon_start() and damon_commit_ctx() are
the three main DAMON core API functions for setup of DAMON parameters.
Make the three functions to do safe and holistic parameter checks.  The
first one is already providing the validation, so changes are only for
the last two functions.

This might add unnecessary validations for some use cases.  The overhead
should be negligible as parameters update is expected to only rarely
happen.  It reduces the number of places to check and fix for bugs of
the class from all callers to the single component.  The maintenance
efficiency gain is obvious.

Further cleanup documents and caller code.  Make the lines of validation
duties clearer.  Remove validations that are no more needed owing to the
core validations.

Patches Sequence
================

Patch 1 implements the core holistic parameters validation for
damon_commit_ctx().  Patch 2 extends the validation to damon_start().
Patch 3 removes the duplicated holistic parameters validation in DAMON
sysfs interface, which is now embedded into the core layer.  Patches 4
and 5 remove duplicated min_region_sz validation in DAMON modules.
Patches 6 updates kernel doc to clarify damon_set_regions() is doing the
range validation.  Patches 7 and 8 remove monitoring target range
validations that doesn't necessary thanks to the validation in
damon_set_regions().

Note that the patch for providing the validation from
damon_set_regions() is separately posted as a hotfix [1] but not yet
picked into mm.git as of this writing.

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

SJ Park (8):
  mm/damon/core: safely validate src on damon_commit_ctx()
  mm/damon/core: do parameter testing commit on damon_start()
  mm/damon/sysfs: remove duplicated commit input validity check
  mm/damon/reclaim: remove duplicated min_region_sz power of 2 check
  mm/damon/lru_sort: remove duplicated min_region_sz power_of_2() check
  mm/damon: document region size validation in damon_set_regions()
  mm/damon/core: remove start, end check in
    damon_set_region_system_rams()
  mm/damon/sysfs: remove region size validation

 include/linux/damon.h |  3 +-
 mm/damon/core.c       | 79 ++++++++++++++++++++++++++++++++-----------
 mm/damon/lru_sort.c   |  5 ---
 mm/damon/reclaim.c    |  5 ---
 mm/damon/sysfs.c      | 35 +------------------
 5 files changed, 63 insertions(+), 64 deletions(-)


base-commit: fe39036ef6a25f1ce6674c04188fd67fc53402e7
-- 
2.47.3

[RFC PATCH 1/8] mm/damon/core: safely validate src on damon_commit_ctx()

[SJ Park]

damon_commit_ctx() does its holistic parameter set validation while
applying the new parameter in the set one by one.  If it finds a
parameter is invalid, because some invalid parameters may already be
committed (it is called "commit" but not atomic and irreversable), it
stops the running DAMON context.

The callers of the function therefore have to validate the parameters
before calling it.  Because the function already embeds holistic
validation, DAMON_SYSFS reuses it in a safe way.  It creates a
test-purpose context that is not running but mimics the running one, and
calls damon_commit_ctx() against the test purpose context.  If it
succeeds, the parameters are considered valid, and a real
damon_commit_ctx() call against the running context is made with those.

Other callers such as DAMON_RECLAIM and DAMON_LRU_SORT do not expose
full parameters to users.  For efficiency, they validate only the known
set of parameters.  The efficiency gain is arguably small and doubtful,
though.  Meanwhile the maintenance overhead of the multiple different
validations is clearly high.  We actually found and fixed a few bugs in
the class.

Update damon_commit_ctx() to embed DAMON_SYSFS' safe and holistic
validation approach.  Callers can simply call damon_commit_ctx() without
worrying if their parameters are invalid.

Note that damon_commit_ctx() can still cause an unexpected stop of the
running context, if internal memory allocation fails.  It is arguably
unlikely since those internal allocations are too small to fail, but
theoretically possible.  It should also be better addressed, but not
necessarily a blocker of this small and incremental improvement effort.

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

diff --git a/mm/damon/core.c b/mm/damon/core.c
index 871c6f5257c9e..62c27002219cd 100644
--- a/mm/damon/core.c
+++ b/mm/damon/core.c
@@ -1664,20 +1664,7 @@ static int damon_commit_probes(struct damon_ctx *dst, struct damon_ctx *src)
 	return 0;
 }
 
-/**
- * damon_commit_ctx() - Commit parameters of a DAMON context to another.
- * @dst:	The commit destination DAMON context.
- * @src:	The commit source DAMON context.
- *
- * This function copies user-specified parameters from @src to @dst and update
- * the internal status and results accordingly.  Users should use this function
- * for context-level parameters update of running context, instead of manual
- * in-place updates.
- *
- * This function should be called from parameters-update safe context, like
- * damon_call().
- */
-int damon_commit_ctx(struct damon_ctx *dst, struct damon_ctx *src)
+static int __damon_commit_ctx(struct damon_ctx *dst, struct damon_ctx *src)
 {
 	int err;
 	struct damos *scheme;
@@ -1732,6 +1719,52 @@ int damon_commit_ctx(struct damon_ctx *dst, struct damon_ctx *src)
 	return 0;
 }
 
+static struct damon_ctx *damon_new_test_ctx(struct damon_ctx *dst)
+{
+	struct damon_ctx *test_ctx;
+	int err;
+
+	test_ctx = damon_new_ctx();
+	if (!test_ctx)
+		return NULL;
+	err = __damon_commit_ctx(test_ctx, dst);
+	if (err) {
+		damon_destroy_ctx(test_ctx);
+		return NULL;
+	}
+	return test_ctx;
+}
+
+/**
+ * damon_commit_ctx() - Commit parameters of a DAMON context to another.
+ * @dst:	The commit destination DAMON context.
+ * @src:	The commit source DAMON context.
+ *
+ * This function copies user-specified parameters from @src to @dst and update
+ * the internal status and results accordingly.  Users should use this function
+ * for context-level parameters update of running context, instead of manual
+ * in-place updates.
+ *
+ * This function should be called from parameters-update safe context, like
+ * damon_call().
+ */
+int damon_commit_ctx(struct damon_ctx *dst, struct damon_ctx *src)
+{
+	struct damon_ctx *test_ctx;
+	int err;
+
+	test_ctx = damon_new_test_ctx(dst);
+	if (!test_ctx)
+		return -ENOMEM;
+	err = __damon_commit_ctx(test_ctx, dst);
+	if (err)
+		goto out;
+	err = __damon_commit_ctx(dst, src);
+out:
+	damon_destroy_ctx(test_ctx);
+	return err;
+}
+
 /**
  * damon_nr_running_ctxs() - Return number of currently running contexts.
  */
-- 
2.47.3

Re: [RFC PATCH 1/8] mm/damon/core: safely validate src on damon_commit_ctx()

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Validation step incorrectly tests the existing context (`dst`) instead of the new parameters (`src`), completely neutralizing the patch's safety guarantees.
--

commit 4ac81fdba8a8c8576fad127fdd4347015f4dac51
Author: SJ Park <sj@kernel.org>

mm/damon/core: safely validate src on damon_commit_ctx()

This commit updates damon_commit_ctx() to validate parameters safely by
applying them to a test context before applying them to the running context.
If the test context validation succeeds, the real update proceeds.

> diff --git a/mm/damon/core.c b/mm/damon/core.c
> index 5febe58a07028..5ca176eb27e1d 100644
> --- a/mm/damon/core.c
> +++ b/mm/damon/core.c

[ ... ]

> @@ -1635,6 +1622,52 @@ int damon_commit_ctx(struct damon_ctx *dst, struct damon_ctx *src)

[ ... ]

> +int damon_commit_ctx(struct damon_ctx *dst, struct damon_ctx *src)
> +{
> +	struct damon_ctx *test_ctx;
> +	int err;
> +
> +	test_ctx = damon_new_test_ctx(dst);
> +	if (!test_ctx)
> +		return -ENOMEM;
> +	err = __damon_commit_ctx(test_ctx, dst);

[Severity: High]
Does this code incorrectly validate the existing context instead of the new
parameters?

test_ctx is initialized as a clone of dst via damon_new_test_ctx(dst). If
__damon_commit_ctx() applies dst to test_ctx again, it avoids testing the
new parameters in src entirely.

If src contains invalid parameters (like a non-power-of-two min_region_sz),
it will bypass this check. When the unvalidated src is subsequently applied
to the running context:

    err = __damon_commit_ctx(dst, src);
    
it can fail partway through. This leaves dst->maybe_corrupted = true and
causes the running kdamond monitoring thread to terminate unexpectedly.

Should the validation step be checking src instead?

    err = __damon_commit_ctx(test_ctx, src);

> +	if (err)
> +		goto out;
> +	err = __damon_commit_ctx(dst, src);
> +out:
> +	damon_destroy_ctx(test_ctx);
> +	return err;
> +}

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

Re: [RFC PATCH 1/8] mm/damon/core: safely validate src on damon_commit_ctx()

[SJ Park]

On Wed, 01 Jul 2026 15:01:10 +0000 sashiko-bot@kernel.org wrote:

> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [High] Validation step incorrectly tests the existing context (`dst`) instead of the new parameters (`src`), completely neutralizing the patch's safety guarantees.
> --
> 
> commit 4ac81fdba8a8c8576fad127fdd4347015f4dac51
> Author: SJ Park <sj@kernel.org>
> 
> mm/damon/core: safely validate src on damon_commit_ctx()
> 
> This commit updates damon_commit_ctx() to validate parameters safely by
> applying them to a test context before applying them to the running context.
> If the test context validation succeeds, the real update proceeds.
> 
> > diff --git a/mm/damon/core.c b/mm/damon/core.c
> > index 5febe58a07028..5ca176eb27e1d 100644
> > --- a/mm/damon/core.c
> > +++ b/mm/damon/core.c
> 
> [ ... ]
> 
> > @@ -1635,6 +1622,52 @@ int damon_commit_ctx(struct damon_ctx *dst, struct damon_ctx *src)
> 
> [ ... ]
> 
> > +int damon_commit_ctx(struct damon_ctx *dst, struct damon_ctx *src)
> > +{
> > +	struct damon_ctx *test_ctx;
> > +	int err;
> > +
> > +	test_ctx = damon_new_test_ctx(dst);
> > +	if (!test_ctx)
> > +		return -ENOMEM;
> > +	err = __damon_commit_ctx(test_ctx, dst);
> 
> [Severity: High]
> Does this code incorrectly validate the existing context instead of the new
> parameters?
> 
> test_ctx is initialized as a clone of dst via damon_new_test_ctx(dst). If
> __damon_commit_ctx() applies dst to test_ctx again, it avoids testing the
> new parameters in src entirely.
> 
> If src contains invalid parameters (like a non-power-of-two min_region_sz),
> it will bypass this check. When the unvalidated src is subsequently applied
> to the running context:
> 
>     err = __damon_commit_ctx(dst, src);
>     
> it can fail partway through. This leaves dst->maybe_corrupted = true and
> causes the running kdamond monitoring thread to terminate unexpectedly.
> 
> Should the validation step be checking src instead?
> 
>     err = __damon_commit_ctx(test_ctx, src);

Correct.  Maybe I made a copy-pasta.  I will fix this in the next revisssion.


Thanks,
SJ

[...]

[RFC PATCH 2/8] mm/damon/core: do parameter testing commit on damon_start()

[SJ Park]

damon_start() and damon_commit_ctx() are two main DAMON core API
functions for setting whole DAMON parameters.  While damon_commit_ctx()
does holistic parameters testing, damon_start() just believes the caller
validated the whole thing.  Embed the holistic parameter check that is
already in damon_commit_ctx() into damon_start().  After this change,
the callers can safely call damon_start() without validating the
parameters.

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

diff --git a/mm/damon/core.c b/mm/damon/core.c
index 62c27002219cd..6b30cb6007e54 100644
--- a/mm/damon/core.c
+++ b/mm/damon/core.c
@@ -1865,6 +1865,8 @@ static int __damon_start(struct damon_ctx *ctx)
 	return err;
 }
 
+static int __damon_commit_ctx(struct damon_ctx *dst, struct damon_ctx *src);
+
 /**
  * damon_start() - Starts the monitorings for a given group of contexts.
  * @ctxs:	an array of the pointers for contexts to start monitoring
@@ -1882,13 +1884,22 @@ static int __damon_start(struct damon_ctx *ctx)
  */
 int damon_start(struct damon_ctx **ctxs, int nr_ctxs, bool exclusive)
 {
+	struct damon_ctx *test_ctx;
 	int i;
 	int err = 0;
 
+	test_ctx = damon_new_ctx();
+	if (!test_ctx)
+		return -ENOMEM;
+
 	for (i = 0; i < nr_ctxs; i++) {
-		if (!is_power_of_2(ctxs[i]->min_region_sz))
-			return -EINVAL;
+		err = __damon_commit_ctx(test_ctx, ctxs[i]);
+		if (err) {
+			damon_destroy_ctx(test_ctx);
+			return err;
+		}
 	}
+	damon_destroy_ctx(test_ctx);
 
 	mutex_lock(&damon_lock);
 	if ((exclusive && nr_running_ctxs) ||
-- 
2.47.3

Re: [RFC PATCH 2/8] mm/damon/core: do parameter testing commit on damon_start()

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] `damon_commit_ctx()` incorrectly validates `dst` against `test_ctx` instead of `src`, bypassing the safety mechanism.
- [Medium] Reusing a single `test_ctx` across multiple loop iterations in `damon_start()` leaks state between contexts, leading to non-deterministic validation outcomes.
--

commit db4fd1a9680927efd94179ba2c47dd9fdd14f123
Author: SJ Park <sj@kernel.org>

mm/damon/core: do parameter testing commit on damon_start()

damon_start() and damon_commit_ctx() are the two main API functions for
setting DAMON parameters. This commit embeds the parameter check already
present in damon_commit_ctx() into damon_start(), allowing callers to
safely use damon_start() without manually validating parameters first.

The commit message mentions that damon_commit_ctx() does holistic
parameters testing. Looking at damon_commit_ctx() in the same file:

mm/damon/core.c:damon_commit_ctx() {
	...
	test_ctx = damon_new_test_ctx(dst);
	if (!test_ctx)
		return -ENOMEM;
	err = __damon_commit_ctx(test_ctx, dst);
	if (err)
		goto out;
	err = __damon_commit_ctx(dst, src);
	...
}

[Severity: High]
Does this bypass the safety validation mechanism in damon_commit_ctx()?

The validation incorrectly tests the active context dst against test_ctx
instead of testing the incoming parameters src. 

If invalid parameters are supplied or if a mid-commit allocation fails, the
incoming parameters src are directly applied to the active context dst
without any prior dry-run validation. This leaves the active context dst in a
corrupted state (maybe_corrupted = true), potentially leading to monitoring
thread crashes or unpredictable kernel behavior.

> diff --git a/mm/damon/core.c b/mm/damon/core.c
> index 5ca176eb27e1d..ace202f47dd7c 100644
> --- a/mm/damon/core.c
> +++ b/mm/damon/core.c

[ ... ]

> @@ -1785,13 +1787,22 @@ static int __damon_start(struct damon_ctx *ctx)
>   */
>  int damon_start(struct damon_ctx **ctxs, int nr_ctxs, bool exclusive)
>  {
> +	struct damon_ctx *test_ctx;
>  	int i;
>  	int err = 0;
>  
> +	test_ctx = damon_new_ctx();
> +	if (!test_ctx)
> +		return -ENOMEM;
> +
>  	for (i = 0; i < nr_ctxs; i++) {
> -		if (!is_power_of_2(ctxs[i]->min_region_sz))
> -			return -EINVAL;
> +		err = __damon_commit_ctx(test_ctx, ctxs[i]);

[Severity: Medium]
Does reusing a single test_ctx across multiple loop iterations leak state
between contexts?

If a caller invokes damon_start() with multiple contexts (nr_ctxs > 1),
test_ctx is mutated in the first iteration. In the second iteration,
__damon_commit_ctx() runs on the same, un-cleared test_ctx.

For example, if ctxs[1] has an obsolete target:

mm/damon/core.c:damon_commit_targets() {
	...
		/* target to remove has no matching dst */
		if (src_target->obsolete) {
			err = -EINVAL;
			goto out;
		}
	...
}

It evaluates against the residual targets of ctxs[0]. If ctxs[0] had enough
targets, the obsolete target incorrectly matches and passes validation. If
ctxs[0] had no targets, it fails with -EINVAL, producing non-deterministic
validation results.

> +		if (err) {
> +			damon_destroy_ctx(test_ctx);
> +			return err;
> +		}
>  	}
> +	damon_destroy_ctx(test_ctx);
>  
>  	mutex_lock(&damon_lock);
>  	if ((exclusive && nr_running_ctxs) ||

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

Re: [RFC PATCH 2/8] mm/damon/core: do parameter testing commit on damon_start()

[SJ Park]

On Wed, 01 Jul 2026 15:20:50 +0000 sashiko-bot@kernel.org wrote:

> Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
> - [High] `damon_commit_ctx()` incorrectly validates `dst` against `test_ctx` instead of `src`, bypassing the safety mechanism.
> - [Medium] Reusing a single `test_ctx` across multiple loop iterations in `damon_start()` leaks state between contexts, leading to non-deterministic validation outcomes.
> --
> 
> commit db4fd1a9680927efd94179ba2c47dd9fdd14f123
> Author: SJ Park <sj@kernel.org>
> 
> mm/damon/core: do parameter testing commit on damon_start()
> 
> damon_start() and damon_commit_ctx() are the two main API functions for
> setting DAMON parameters. This commit embeds the parameter check already
> present in damon_commit_ctx() into damon_start(), allowing callers to
> safely use damon_start() without manually validating parameters first.
> 
> The commit message mentions that damon_commit_ctx() does holistic
> parameters testing. Looking at damon_commit_ctx() in the same file:
> 
> mm/damon/core.c:damon_commit_ctx() {
> 	...
> 	test_ctx = damon_new_test_ctx(dst);
> 	if (!test_ctx)
> 		return -ENOMEM;
> 	err = __damon_commit_ctx(test_ctx, dst);
> 	if (err)
> 		goto out;
> 	err = __damon_commit_ctx(dst, src);
> 	...
> }
> 
> [Severity: High]
> Does this bypass the safety validation mechanism in damon_commit_ctx()?
> 
> The validation incorrectly tests the active context dst against test_ctx
> instead of testing the incoming parameters src. 
> 
> If invalid parameters are supplied or if a mid-commit allocation fails, the
> incoming parameters src are directly applied to the active context dst
> without any prior dry-run validation. This leaves the active context dst in a
> corrupted state (maybe_corrupted = true), potentially leading to monitoring
> thread crashes or unpredictable kernel behavior.

Correct.  Same finding was replied to the patch 1.  I promised to fix it in the
next revision.

> 
> > diff --git a/mm/damon/core.c b/mm/damon/core.c
> > index 5ca176eb27e1d..ace202f47dd7c 100644
> > --- a/mm/damon/core.c
> > +++ b/mm/damon/core.c
> 
> [ ... ]
> 
> > @@ -1785,13 +1787,22 @@ static int __damon_start(struct damon_ctx *ctx)
> >   */
> >  int damon_start(struct damon_ctx **ctxs, int nr_ctxs, bool exclusive)
> >  {
> > +	struct damon_ctx *test_ctx;
> >  	int i;
> >  	int err = 0;
> >  
> > +	test_ctx = damon_new_ctx();
> > +	if (!test_ctx)
> > +		return -ENOMEM;
> > +
> >  	for (i = 0; i < nr_ctxs; i++) {
> > -		if (!is_power_of_2(ctxs[i]->min_region_sz))
> > -			return -EINVAL;
> > +		err = __damon_commit_ctx(test_ctx, ctxs[i]);
> 
> [Severity: Medium]
> Does reusing a single test_ctx across multiple loop iterations leak state
> between contexts?
> 
> If a caller invokes damon_start() with multiple contexts (nr_ctxs > 1),
> test_ctx is mutated in the first iteration. In the second iteration,
> __damon_commit_ctx() runs on the same, un-cleared test_ctx.
> 
> For example, if ctxs[1] has an obsolete target:
> 
> mm/damon/core.c:damon_commit_targets() {
> 	...
> 		/* target to remove has no matching dst */
> 		if (src_target->obsolete) {
> 			err = -EINVAL;
> 			goto out;
> 		}
> 	...
> }
> 
> It evaluates against the residual targets of ctxs[0]. If ctxs[0] had enough
> targets, the obsolete target incorrectly matches and passes validation. If
> ctxs[0] had no targets, it fails with -EINVAL, producing non-deterministic
> validation results.

Good finding.  I will allocate text context for each user-given context in the
next revision.


Thanks,
SJ

[...]

[RFC PATCH 3/8] mm/damon/sysfs: remove duplicated commit input validity check

[SJ Park]

DAMON sysfs interface does parameters validation-purpose
damon_commit_ctx() calls for parameters update.  Now the same logic is
embedded inside damon_commit_ctx() itself.  Hence, the validation in
DAMON sysfs interface is just an unnecessary duplicate.  Remove it.

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

diff --git a/mm/damon/sysfs.c b/mm/damon/sysfs.c
index 6710b6d019bf5..e666dddf1feba 100644
--- a/mm/damon/sysfs.c
+++ b/mm/damon/sysfs.c
@@ -2098,26 +2098,6 @@ static int damon_sysfs_apply_inputs(struct damon_ctx *ctx,
 static struct damon_ctx *damon_sysfs_build_ctx(
 		struct damon_sysfs_context *sys_ctx);
 
-/*
- * Return a new damon_ctx for testing new parameters to commit.
- */
-static struct damon_ctx *damon_sysfs_new_test_ctx(
-		struct damon_ctx *running_ctx)
-{
-	struct damon_ctx *test_ctx;
-	int err;
-
-	test_ctx = damon_new_ctx();
-	if (!test_ctx)
-		return NULL;
-	err = damon_commit_ctx(test_ctx, running_ctx);
-	if (err) {
-		damon_destroy_ctx(test_ctx);
-		return NULL;
-	}
-	return test_ctx;
-}
-
 /*
  * damon_sysfs_commit_input() - Commit user inputs to a running kdamond.
  * @kdamond:	The kobject wrapper for the associated kdamond.
@@ -2127,7 +2107,7 @@ static struct damon_ctx *damon_sysfs_new_test_ctx(
 static int damon_sysfs_commit_input(void *data)
 {
 	struct damon_sysfs_kdamond *kdamond = data;
-	struct damon_ctx *param_ctx, *test_ctx;
+	struct damon_ctx *param_ctx;
 	int err;
 
 	if (!damon_sysfs_kdamond_running(kdamond))
@@ -2139,17 +2119,7 @@ static int damon_sysfs_commit_input(void *data)
 	param_ctx = damon_sysfs_build_ctx(kdamond->contexts->contexts_arr[0]);
 	if (IS_ERR(param_ctx))
 		return PTR_ERR(param_ctx);
-	test_ctx = damon_sysfs_new_test_ctx(kdamond->damon_ctx);
-	if (!test_ctx) {
-		damon_destroy_ctx(param_ctx);
-		return -ENOMEM;
-	}
-	err = damon_commit_ctx(test_ctx, param_ctx);
-	if (err)
-		goto out;
 	err = damon_commit_ctx(kdamond->damon_ctx, param_ctx);
-out:
-	damon_destroy_ctx(test_ctx);
 	damon_destroy_ctx(param_ctx);
 	return err;
 }
-- 
2.47.3

Re: [RFC PATCH 3/8] mm/damon/sysfs: remove duplicated commit input validity check

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Removing the explicit `test_ctx` parameter validation in `damon_sysfs_commit_input` exposes an underlying flaw in `damon_commit_ctx`, allowing invalid sysfs inputs to corrupt the running context and terminate the background kdamond thread.
--

Note: Due to system limitations regarding exact code recitation, this report
uses a free-form summary of the issue rather than inline patch quoting.

commit bec0828fbe945683bcef8cfdb56247414f2510d8
Author: SJ Park <sj@kernel.org>
Subject: mm/damon/sysfs: remove duplicated commit input validity check

This commit removes the explicit test_ctx validation inside
damon_sysfs_commit_input(), assuming the same validation logic is safely
embedded inside damon_commit_ctx().

[Severity: High]
Does removing the sysfs-level test_ctx wrapper expose the running context to
corruption from invalid user inputs?

In mm/damon/core.c:damon_commit_ctx(), it appears the underlying validation
logic tests 'dst' against the temporary 'test_ctx' instead of testing the
new 'src' parameters:

damon_commit_ctx()
   err = __damon_commit_ctx(test_ctx, dst);
   if (err)
       goto out;
   err = __damon_commit_ctx(dst, src);

Because damon_sysfs_commit_input() now relies entirely on damon_commit_ctx()
for validation, what happens if invalid configuration parameters (such as a
min_region_sz that is not a power of 2) are passed via the sysfs interface?

It looks like __damon_commit_ctx(dst, src) will fail halfway through, returning
-EINVAL but leaving the running context with maybe_corrupted = true. Can this
cause the background kdamond_fn thread to silently abort without recovering,
effectively resulting in a denial of service for the monitoring session?

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

Re: [RFC PATCH 3/8] mm/damon/sysfs: remove duplicated commit input validity check

[SJ Park]

On Wed, 01 Jul 2026 15:29: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 explicit `test_ctx` parameter validation in `damon_sysfs_commit_input` exposes an underlying flaw in `damon_commit_ctx`, allowing invalid sysfs inputs to corrupt the running context and terminate the background kdamond thread.
> --
> 
> Note: Due to system limitations regarding exact code recitation, this report
> uses a free-form summary of the issue rather than inline patch quoting.
> 
> commit bec0828fbe945683bcef8cfdb56247414f2510d8
> Author: SJ Park <sj@kernel.org>
> Subject: mm/damon/sysfs: remove duplicated commit input validity check
> 
> This commit removes the explicit test_ctx validation inside
> damon_sysfs_commit_input(), assuming the same validation logic is safely
> embedded inside damon_commit_ctx().
> 
> [Severity: High]
> Does removing the sysfs-level test_ctx wrapper expose the running context to
> corruption from invalid user inputs?
> 
> In mm/damon/core.c:damon_commit_ctx(), it appears the underlying validation
> logic tests 'dst' against the temporary 'test_ctx' instead of testing the
> new 'src' parameters:
> 
> damon_commit_ctx()
>    err = __damon_commit_ctx(test_ctx, dst);
>    if (err)
>        goto out;
>    err = __damon_commit_ctx(dst, src);
> 
> Because damon_sysfs_commit_input() now relies entirely on damon_commit_ctx()
> for validation, what happens if invalid configuration parameters (such as a
> min_region_sz that is not a power of 2) are passed via the sysfs interface?
> 
> It looks like __damon_commit_ctx(dst, src) will fail halfway through, returning
> -EINVAL but leaving the running context with maybe_corrupted = true. Can this
> cause the background kdamond_fn thread to silently abort without recovering,
> effectively resulting in a denial of service for the monitoring session?

Correct.  I will fix damon_commit_ctx() side validation in the next revision.


Thanks,
SJ

[...]

[RFC PATCH 4/8] mm/damon/reclaim: remove duplicated min_region_sz power of 2 check

[SJ Park]

DAMON_RECLAIM validates the user input for min_region_sz.  The same
validation is done inside damon_start() and damon_commit_ctx().  Remove
the duplicate.

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

diff --git a/mm/damon/reclaim.c b/mm/damon/reclaim.c
index 11b70d0a9a6f0..6469b25cc34f9 100644
--- a/mm/damon/reclaim.c
+++ b/mm/damon/reclaim.c
@@ -208,11 +208,6 @@ static int damon_reclaim_apply_parameters(void)
 	param_ctx->addr_unit = addr_unit;
 	param_ctx->min_region_sz = max(DAMON_MIN_REGION_SZ / addr_unit, 1);
 
-	if (!is_power_of_2(param_ctx->min_region_sz)) {
-		err = -EINVAL;
-		goto out;
-	}
-
 	if (!damon_reclaim_mon_attrs.aggr_interval) {
 		err = -EINVAL;
 		goto out;
-- 
2.47.3

Re: [RFC PATCH 4/8] mm/damon/reclaim: remove duplicated min_region_sz power of 2 check

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Removing the early `is_power_of_2()` check allows invalid `min_region_sz` inputs to reach `damon_commit_ctx()`, which exposes a bug introduced earlier in this series that corrupts the live context and silently terminates the DAMON background thread.
--

commit f5d47eadcc06079427c142dd59c5a8aa8721caec
Author: SJ Park <sj@kernel.org>

mm/damon/reclaim: remove duplicated min_region_sz power of 2 check

DAMON_RECLAIM validates the user input for min_region_sz.  The same
validation is done inside damon_start() and damon_commit_ctx().  Remove
the duplicate.

> diff --git a/mm/damon/reclaim.c b/mm/damon/reclaim.c
> index 11b70d0a9a6f0..6469b25cc34f9 100644
> --- a/mm/damon/reclaim.c
> +++ b/mm/damon/reclaim.c
> @@ -208,11 +208,6 @@ static int damon_reclaim_apply_parameters(void)
>  	param_ctx->addr_unit = addr_unit;
>  	param_ctx->min_region_sz = max(DAMON_MIN_REGION_SZ / addr_unit, 1);
>  
> -	if (!is_power_of_2(param_ctx->min_region_sz)) {
> -		err = -EINVAL;
> -		goto out;
> -	}

[Severity: High]
Does removing this early check expose a bug introduced earlier in this series?

If the user provides an invalid addr_unit (e.g., 3), min_region_sz will not be
a power of 2. By removing the check here, this invalid parameter is passed
to damon_commit_ctx().

Due to an issue in damon_commit_ctx() (from "mm/damon/core: safely validate
src on damon_commit_ctx()"), the dry-run validates dst instead of src:

mm/damon/core.c:damon_commit_ctx() {
	...
	err = __damon_commit_ctx(test_ctx, dst);
	if (err)
		goto out;
	err = __damon_commit_ctx(dst, src);
	...
}

When __damon_commit_ctx() is called with the live dst and invalid src, it
sets dst->maybe_corrupted before returning -EINVAL:

mm/damon/core.c:__damon_commit_ctx() {
	...
	dst->maybe_corrupted = true;
	if (!is_power_of_2(src->min_region_sz))
		return -EINVAL;
	...
}

Could this cause the background kdamond thread to see maybe_corrupted as true
on its next iteration and permanently terminate, silently turning a simple
configuration error into a denial of service for the subsystem?

> -
>  	if (!damon_reclaim_mon_attrs.aggr_interval) {
>  		err = -EINVAL;
>  		goto out;

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

Re: [RFC PATCH 4/8] mm/damon/reclaim: remove duplicated min_region_sz power of 2 check

[SJ Park]

On Wed, 01 Jul 2026 15:42:57 +0000 sashiko-bot@kernel.org wrote:

> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [High] Removing the early `is_power_of_2()` check allows invalid `min_region_sz` inputs to reach `damon_commit_ctx()`, which exposes a bug introduced earlier in this series that corrupts the live context and silently terminates the DAMON background thread.
> --
> 
> commit f5d47eadcc06079427c142dd59c5a8aa8721caec
> Author: SJ Park <sj@kernel.org>
> 
> mm/damon/reclaim: remove duplicated min_region_sz power of 2 check
> 
> DAMON_RECLAIM validates the user input for min_region_sz.  The same
> validation is done inside damon_start() and damon_commit_ctx().  Remove
> the duplicate.
> 
> > diff --git a/mm/damon/reclaim.c b/mm/damon/reclaim.c
> > index 11b70d0a9a6f0..6469b25cc34f9 100644
> > --- a/mm/damon/reclaim.c
> > +++ b/mm/damon/reclaim.c
> > @@ -208,11 +208,6 @@ static int damon_reclaim_apply_parameters(void)
> >  	param_ctx->addr_unit = addr_unit;
> >  	param_ctx->min_region_sz = max(DAMON_MIN_REGION_SZ / addr_unit, 1);
> >  
> > -	if (!is_power_of_2(param_ctx->min_region_sz)) {
> > -		err = -EINVAL;
> > -		goto out;
> > -	}
> 
> [Severity: High]
> Does removing this early check expose a bug introduced earlier in this series?
> 
> If the user provides an invalid addr_unit (e.g., 3), min_region_sz will not be
> a power of 2. By removing the check here, this invalid parameter is passed
> to damon_commit_ctx().
> 
> Due to an issue in damon_commit_ctx() (from "mm/damon/core: safely validate
> src on damon_commit_ctx()"), the dry-run validates dst instead of src:
> 
> mm/damon/core.c:damon_commit_ctx() {
> 	...
> 	err = __damon_commit_ctx(test_ctx, dst);
> 	if (err)
> 		goto out;
> 	err = __damon_commit_ctx(dst, src);
> 	...
> }
> 
> When __damon_commit_ctx() is called with the live dst and invalid src, it
> sets dst->maybe_corrupted before returning -EINVAL:
> 
> mm/damon/core.c:__damon_commit_ctx() {
> 	...
> 	dst->maybe_corrupted = true;
> 	if (!is_power_of_2(src->min_region_sz))
> 		return -EINVAL;
> 	...
> }
> 
> Could this cause the background kdamond thread to see maybe_corrupted as true
> on its next iteration and permanently terminate, silently turning a simple
> configuration error into a denial of service for the subsystem?

Yes, I will fix damon_commit_ctx() in the next revision.


Thanks,
SJ

[...]

[RFC PATCH 5/8] mm/damon/lru_sort: remove duplicated min_region_sz power_of_2() check

[SJ Park]

DAMON_LRU_SORT validates the user input for min_region_sz.  The same
validation is done inside damon_start() and damon_commit_ctx().  Remove
the unnecessary duplicate.

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

diff --git a/mm/damon/lru_sort.c b/mm/damon/lru_sort.c
index 32f41491b726b..2dd0cd0d26273 100644
--- a/mm/damon/lru_sort.c
+++ b/mm/damon/lru_sort.c
@@ -284,11 +284,6 @@ static int damon_lru_sort_apply_parameters(void)
 	param_ctx->addr_unit = addr_unit;
 	param_ctx->min_region_sz = max(DAMON_MIN_REGION_SZ / addr_unit, 1);
 
-	if (!is_power_of_2(param_ctx->min_region_sz)) {
-		err = -EINVAL;
-		goto out;
-	}
-
 	if (!damon_lru_sort_mon_attrs.sample_interval) {
 		err = -EINVAL;
 		goto out;
-- 
2.47.3

Re: [RFC PATCH 5/8] mm/damon/lru_sort: remove duplicated min_region_sz power_of_2() check

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Removing the `is_power_of_2()` validation exposes the system to undefined behavior via `ALIGN` macros and silently kills the `kdamond` thread (Denial of Service).
--

commit da14a5f653fa82832f40f8b6d0be4ad4c3036287
Author: SJ Park <sj@kernel.org>

mm/damon/lru_sort: remove duplicated min_region_sz power_of_2() check

DAMON_LRU_SORT validates the user input for min_region_sz.  The same
validation is done inside damon_start() and damon_commit_ctx().  Remove
the unnecessary duplicate.

> diff --git a/mm/damon/lru_sort.c b/mm/damon/lru_sort.c
> index 32f41491b726b..2dd0cd0d26273 100644
> --- a/mm/damon/lru_sort.c
> +++ b/mm/damon/lru_sort.c
> @@ -284,11 +284,6 @@ static int damon_lru_sort_apply_parameters(void)
>  	param_ctx->addr_unit = addr_unit;
>  	param_ctx->min_region_sz = max(DAMON_MIN_REGION_SZ / addr_unit, 1);
>  
> -	if (!is_power_of_2(param_ctx->min_region_sz)) {
> -		err = -EINVAL;
> -		goto out;
> -	}
> -

[Severity: High]
Does removing this early validation expose the rest of the DAMON subsystem to
a non-power-of-two min_region_sz?

If a user writes a non-power-of-two module parameter (for example,
addr_unit = 3), damon_lru_sort_apply_parameters() will calculate
min_region_sz as 349525.

This invalid size is then passed to damon_set_region_system_rams_default(),
and eventually to damon_set_regions(), where it is passed into the ALIGN()
and ALIGN_DOWN() macros:

mm/damon/core.c:damon_set_regions() {
    ...
    r = damon_new_region(
        ALIGN_DOWN(ranges[i].start, min_region_sz),
        ALIGN(ranges[i].end, min_region_sz));
    ...
}

These macros strictly require power-of-two alignment values to form bitwise
masks.

Can this result in meaningless garbage boundaries?

Furthermore, the invalid size propagates to damon_commit_ctx(). Due to a bug
in a prior patch in the series ('mm/damon/core: safely validate src on
damon_commit_ctx()'), the safe validation dry-run incorrectly validates dst
instead of src, leading directly to a call to __damon_commit_ctx():

mm/damon/core.c:__damon_commit_ctx() {
    ...
    dst->maybe_corrupted = true;
    if (!is_power_of_2(src->min_region_sz))
        return -EINVAL;
    ...
}

This function immediately sets dst->maybe_corrupted = true, evaluates
!is_power_of_2(src->min_region_sz), and returns -EINVAL early.

This leaves the running context's maybe_corrupted flag permanently set to
true, which causes the kdamond background thread to abruptly terminate itself
on its next iteration.

Could this lead to a denial of service by silently killing the kdamond thread?

>  	if (!damon_lru_sort_mon_attrs.sample_interval) {
>  		err = -EINVAL;
>  		goto out;

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

Re: [RFC PATCH 5/8] mm/damon/lru_sort: remove duplicated min_region_sz power_of_2() check

[SJ Park]

On Wed, 01 Jul 2026 15:56:23 +0000 sashiko-bot@kernel.org wrote:

> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [High] Removing the `is_power_of_2()` validation exposes the system to undefined behavior via `ALIGN` macros and silently kills the `kdamond` thread (Denial of Service).
> --
> 
> commit da14a5f653fa82832f40f8b6d0be4ad4c3036287
> Author: SJ Park <sj@kernel.org>
> 
> mm/damon/lru_sort: remove duplicated min_region_sz power_of_2() check
> 
> DAMON_LRU_SORT validates the user input for min_region_sz.  The same
> validation is done inside damon_start() and damon_commit_ctx().  Remove
> the unnecessary duplicate.
> 
> > diff --git a/mm/damon/lru_sort.c b/mm/damon/lru_sort.c
> > index 32f41491b726b..2dd0cd0d26273 100644
> > --- a/mm/damon/lru_sort.c
> > +++ b/mm/damon/lru_sort.c
> > @@ -284,11 +284,6 @@ static int damon_lru_sort_apply_parameters(void)
> >  	param_ctx->addr_unit = addr_unit;
> >  	param_ctx->min_region_sz = max(DAMON_MIN_REGION_SZ / addr_unit, 1);
> >  
> > -	if (!is_power_of_2(param_ctx->min_region_sz)) {
> > -		err = -EINVAL;
> > -		goto out;
> > -	}
> > -
> 
> [Severity: High]
> Does removing this early validation expose the rest of the DAMON subsystem to
> a non-power-of-two min_region_sz?
> 
> If a user writes a non-power-of-two module parameter (for example,
> addr_unit = 3), damon_lru_sort_apply_parameters() will calculate
> min_region_sz as 349525.
> 
> This invalid size is then passed to damon_set_region_system_rams_default(),
> and eventually to damon_set_regions(), where it is passed into the ALIGN()
> and ALIGN_DOWN() macros:
> 
> mm/damon/core.c:damon_set_regions() {
>     ...
>     r = damon_new_region(
>         ALIGN_DOWN(ranges[i].start, min_region_sz),
>         ALIGN(ranges[i].end, min_region_sz));
>     ...
> }
> 
> These macros strictly require power-of-two alignment values to form bitwise
> masks.
> 
> Can this result in meaningless garbage boundaries?
> 
> Furthermore, the invalid size propagates to damon_commit_ctx(). Due to a bug
> in a prior patch in the series ('mm/damon/core: safely validate src on
> damon_commit_ctx()'), the safe validation dry-run incorrectly validates dst
> instead of src,

Yes, I will fix the bug in the next revision.

> leading directly to a call to __damon_commit_ctx():
> 
> mm/damon/core.c:__damon_commit_ctx() {
>     ...
>     dst->maybe_corrupted = true;
>     if (!is_power_of_2(src->min_region_sz))
>         return -EINVAL;
>     ...
> }
> 
> This function immediately sets dst->maybe_corrupted = true, evaluates
> !is_power_of_2(src->min_region_sz), and returns -EINVAL early.
> 
> This leaves the running context's maybe_corrupted flag permanently set to
> true, which causes the kdamond background thread to abruptly terminate itself
> on its next iteration.
> 
> Could this lead to a denial of service by silently killing the kdamond thread?

So no issue should happen in the next revision.


Thanks,
SJ

[...]

[RFC PATCH 6/8] mm/damon: document region size validation in damon_set_regions()

[SJ Park]

The kernel doc comment of damon_region clearly specifies every region
should have positive size.  But it is unclear who should verify it.
damon_set_regions() is the recommended DAMON core function for setting
regions from the callers, and has the verification.  Update the comment
to clarify the callers should be ok to pass any values for region
addresses, as long as they use damon_set_regions().

Signed-off-by: SJ Park <sj@kernel.org>
---
 include/linux/damon.h | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/include/linux/damon.h b/include/linux/damon.h
index 616bdf0954b52..2661231c0ae82 100644
--- a/include/linux/damon.h
+++ b/include/linux/damon.h
@@ -49,7 +49,8 @@ struct damon_size_range {
  * @list:		List head for siblings.
  * @age:		Age of this region.
  *
- * For any use case, @ar should be non-zero positive size.
+ * For any use case, @ar should be non-zero positive size.  damon_set_regions()
+ * does the validation.
  *
  * @nr_accesses is reset to zero for every &damon_attrs->aggr_interval and be
  * increased for every &damon_attrs->sample_interval if an access to the region
-- 
2.47.3

Re: [RFC PATCH 6/8] mm/damon: document region size validation in damon_set_regions()

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] The documentation incorrectly claims `damon_set_regions()` validates region sizes, which led to the removal of input validation in callers, resulting in a kernel OOM panic when negative-sized regions are processed.
--

commit d4cdfb1871ad152ec6d79d41fca7f1497b7467a0
Author: SJ Park <sj@kernel.org>

mm/damon: document region size validation in damon_set_regions()

This commit updates the documentation for the damon_region struct. It
clarifies that callers can pass any values for region addresses because
damon_set_regions() validates that regions have a positive size.

> diff --git a/include/linux/damon.h b/include/linux/damon.h
> --- a/include/linux/damon.h
> +++ b/include/linux/damon.h
> @@ -51,7 +51,8 @@ struct damon_size_range {
>   * @list:		List head for siblings.
>   * @age:		Age of this region.
>   *
> - * For any use case, @ar should be non-zero positive size.
> + * For any use case, @ar should be non-zero positive size.  damon_set_regions()
> + * does the validation.

[Severity: High]
Does this documentation accurately reflect the implementation?

Looking at damon_set_regions() in mm/damon/core.c, it lacks checks for
start >= end before passing boundaries to damon_new_region():

mm/damon/core.c:damon_set_regions() {
    ...
			r = damon_new_region(
					ALIGN_DOWN(ranges[i].start,
						min_region_sz),
					ALIGN(ranges[i].end, min_region_sz));
    ...
}

If a region is created with start >= end, would damon_sz_region() underflow
to ULONG_MAX?

This enormous size could trigger an infinite memory allocation loop in
damon_apply_min_nr_regions() which repeatedly calls damon_split_region_at():

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

Since a subsequent patch in the series (mm/damon/sysfs: remove region size
validation) relies on this guarantee to remove the validation from sysfs,
could a user writing a region with start >= end to the DAMON sysfs interface
trigger an Out-of-Memory kernel panic?

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

Re: [RFC PATCH 6/8] mm/damon: document region size validation in damon_set_regions()

[SJ Park]

On Wed, 01 Jul 2026 16:04:18 +0000 sashiko-bot@kernel.org wrote:

> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [High] The documentation incorrectly claims `damon_set_regions()` validates region sizes, which led to the removal of input validation in callers, resulting in a kernel OOM panic when negative-sized regions are processed.
> --
> 
> commit d4cdfb1871ad152ec6d79d41fca7f1497b7467a0
> Author: SJ Park <sj@kernel.org>
> 
> mm/damon: document region size validation in damon_set_regions()
> 
> This commit updates the documentation for the damon_region struct. It
> clarifies that callers can pass any values for region addresses because
> damon_set_regions() validates that regions have a positive size.
> 
> > diff --git a/include/linux/damon.h b/include/linux/damon.h
> > --- a/include/linux/damon.h
> > +++ b/include/linux/damon.h
> > @@ -51,7 +51,8 @@ struct damon_size_range {
> >   * @list:		List head for siblings.
> >   * @age:		Age of this region.
> >   *
> > - * For any use case, @ar should be non-zero positive size.
> > + * For any use case, @ar should be non-zero positive size.  damon_set_regions()
> > + * does the validation.
> 
> [Severity: High]
> Does this documentation accurately reflect the implementation?
> 
> Looking at damon_set_regions() in mm/damon/core.c, it lacks checks for
> start >= end before passing boundaries to damon_new_region():

As I noted on the cover letter, the check is not yet merged into mm.git.

> 
> mm/damon/core.c:damon_set_regions() {
>     ...
> 			r = damon_new_region(
> 					ALIGN_DOWN(ranges[i].start,
> 						min_region_sz),
> 					ALIGN(ranges[i].end, min_region_sz));
>     ...
> }
> 
> If a region is created with start >= end, would damon_sz_region() underflow
> to ULONG_MAX?
> 
> This enormous size could trigger an infinite memory allocation loop in
> damon_apply_min_nr_regions() which repeatedly calls damon_split_region_at():
> 
> 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);
> 			}
>     ...
> }
> 
> Since a subsequent patch in the series (mm/damon/sysfs: remove region size
> validation) relies on this guarantee to remove the validation from sysfs,
> could a user writing a region with start >= end to the DAMON sysfs interface
> trigger an Out-of-Memory kernel panic?

So no issue.


Thanks,
SJ

[...]

[RFC PATCH 7/8] mm/damon/core: remove start, end check in damon_set_region_system_rams()

[SJ Park]

damon_set_region_system_rams() validates user inputs to avoid creating a
negative size region.  But DAMON core avoids zero size, too.  The check
is incomplete.  The complete check is done inside damon_set_regions(),
which is eventually called from damon_set_region_system_rams_default().
Drop the incomplete and unnecessary check.

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

diff --git a/mm/damon/core.c b/mm/damon/core.c
index 6b30cb6007e54..3caf1b67a7f98 100644
--- a/mm/damon/core.c
+++ b/mm/damon/core.c
@@ -3847,9 +3847,6 @@ int damon_set_region_system_rams_default(struct damon_target *t,
 {
 	struct damon_addr_range addr_range;
 
-	if (*start > *end)
-		return -EINVAL;
-
 	if (!*start && !*end &&
 		!damon_find_system_rams_range(start, end, addr_unit))
 		return -EINVAL;
-- 
2.47.3

Re: [RFC PATCH 7/8] mm/damon/core: remove start, end check in damon_set_region_system_rams()

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Removal of bounds validation permits inverted regions (start > end), leading to a massive integer underflow and guaranteed OOM kernel panic.
--

commit 03335779d391504c4265be49dedb155969cf1a7d
Author: SJ Park <sj@kernel.org>

mm/damon/core: remove start, end check in damon_set_region_system_rams()

This commit removes the explicit start > end validation in
damon_set_region_system_rams_default(), stating that damon_set_regions()
already performs a complete check.

> diff --git a/mm/damon/core.c b/mm/damon/core.c
> index ace202f47dd7c..f491a76c98bfc 100644
> --- a/mm/damon/core.c
> +++ b/mm/damon/core.c
> @@ -3794,9 +3794,6 @@ int damon_set_region_system_rams_default(struct damon_target *t,
>  {
>  	struct damon_addr_range addr_range;
>  
> -	if (*start > *end)
> -		return -EINVAL;
> -
[Severity: High]
The commit message states that the complete check is done inside
damon_set_regions(), but does damon_set_regions() actually validate that
start is less than or equal to end?

Looking at damon_set_regions(), it appears to blindly create the region
with underflowed sizes if start > end:

mm/damon/core.c:damon_set_regions() {
        ...
        r = damon_new_region(
                ALIGN_DOWN(ranges[i].start, min_region_sz),
                ALIGN(ranges[i].end, min_region_sz));
        ...
}

If an inverted region (e.g., [4096, 0)) is passed in, could this lead to a
massive integer underflow? The size of the region would underflow to roughly
ULONG_MAX.

Later in damon_apply_min_nr_regions(), this underflow seems to translate
into an effectively infinite loop:

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

Could this loop exhaust system memory by attempting to allocate quadrillions
of regions and cause an OOM kernel panic?

>  	if (!*start && !*end &&
>  		!damon_find_system_rams_range(start, end, addr_unit))
>  		return -EINVAL;

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

Re: [RFC PATCH 7/8] mm/damon/core: remove start, end check in damon_set_region_system_rams()

[SJ Park]

On Wed, 01 Jul 2026 16:22:20 +0000 sashiko-bot@kernel.org wrote:

> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [High] Removal of bounds validation permits inverted regions (start > end), leading to a massive integer underflow and guaranteed OOM kernel panic.
> --
> 
> commit 03335779d391504c4265be49dedb155969cf1a7d
> Author: SJ Park <sj@kernel.org>
> 
> mm/damon/core: remove start, end check in damon_set_region_system_rams()
> 
> This commit removes the explicit start > end validation in
> damon_set_region_system_rams_default(), stating that damon_set_regions()
> already performs a complete check.
> 
> > diff --git a/mm/damon/core.c b/mm/damon/core.c
> > index ace202f47dd7c..f491a76c98bfc 100644
> > --- a/mm/damon/core.c
> > +++ b/mm/damon/core.c
> > @@ -3794,9 +3794,6 @@ int damon_set_region_system_rams_default(struct damon_target *t,
> >  {
> >  	struct damon_addr_range addr_range;
> >  
> > -	if (*start > *end)
> > -		return -EINVAL;
> > -
> [Severity: High]
> The commit message states that the complete check is done inside
> damon_set_regions(), but does damon_set_regions() actually validate that
> start is less than or equal to end?
> 
> Looking at damon_set_regions(), it appears to blindly create the region
> with underflowed sizes if start > end:
> 
> mm/damon/core.c:damon_set_regions() {
>         ...
>         r = damon_new_region(
>                 ALIGN_DOWN(ranges[i].start, min_region_sz),
>                 ALIGN(ranges[i].end, min_region_sz));
>         ...
> }
> 
> If an inverted region (e.g., [4096, 0)) is passed in, could this lead to a
> massive integer underflow? The size of the region would underflow to roughly
> ULONG_MAX.
> 
> Later in damon_apply_min_nr_regions(), this underflow seems to translate
> into an effectively infinite loop:
> 
> 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);
>         }
>         ...
> }
> 
> Could this loop exhaust system memory by attempting to allocate quadrillions
> of regions and cause an OOM kernel panic?

My previous reply [1] should be applied here.

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


Thanks,
SJ

[...]

[RFC PATCH 8/8] mm/damon/sysfs: remove region size validation

[SJ Park]

DAMON_SYSFS validates user inputs for monitoring target regions to
disallow negative size regions.  DAMON core assumes only positive size
regions, though.  The validation is incomplete.  Fortunately
damon_set_regions(), which is eventually used by DAMON_SYSFS, does the
complete validation.  Remove the incomplete and unnecessary validation.

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

diff --git a/mm/damon/sysfs.c b/mm/damon/sysfs.c
index e666dddf1feba..b65651498e0d1 100644
--- a/mm/damon/sysfs.c
+++ b/mm/damon/sysfs.c
@@ -1987,9 +1987,6 @@ static int damon_sysfs_set_regions(struct damon_target *t,
 		struct damon_sysfs_region *sys_region =
 			sysfs_regions->regions_arr[i];
 
-		if (sys_region->ar.start > sys_region->ar.end)
-			goto out;
-
 		ranges[i].start = sys_region->ar.start;
 		ranges[i].end = sys_region->ar.end;
 		if (i == 0)
-- 
2.47.3

Re: [RFC PATCH 8/8] mm/damon/sysfs: remove region size validation

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Removing the `start > end` validation allows users to configure monitoring regions with negative sizes, causing integer underflow in region size calculations. This leads to infinite memory allocation loops (OOM), potential list corruption, and CPU soft lockups during DAMON region splitting and scheme operations.
--

commit 013d66b2108372ca2ada7fc6fcfd5e07f93fd78c
Author: SJ Park <sj@kernel.org>

mm/damon/sysfs: remove region size validation

This commit removes the region size validation in DAMON sysfs that checks
for negative size regions. The commit message states that the validation is
incomplete and that damon_set_regions() already performs complete
validation.

> diff --git a/mm/damon/sysfs.c b/mm/damon/sysfs.c
> index e666dddf1feba..b65651498e0d1 100644
> --- a/mm/damon/sysfs.c
> +++ b/mm/damon/sysfs.c
> @@ -1987,9 +1987,6 @@ static int damon_sysfs_set_regions(struct damon_target *t,
>  		struct damon_sysfs_region *sys_region =
>  			sysfs_regions->regions_arr[i];
>  
> -		if (sys_region->ar.start > sys_region->ar.end)
> -			goto out;
> -

[Severity: High]
The commit message mentions that damon_set_regions() does the complete
validation, but looking at damon_set_regions() in mm/damon/core.c, does it
actually validate that start <= end?

mm/damon/core.c:damon_set_regions() {
	...
	r = damon_new_region(
		ALIGN_DOWN(ranges[i].start, min_region_sz),
		ALIGN(ranges[i].end, min_region_sz));
	...
}

If a user configures a region where start > end, wouldn't this cause an
integer underflow when calculating the region size later? 

For example, could this underflow lead to an infinite memory allocation loop
in 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);
	}

Could this loop evaluate the underflowed size and attempt to allocate new
regions repeatedly until an OOM occurs?

Additionally, for newly split regions where start < end locally, could this
spin over a massive address range without yielding and cause a CPU soft
lockup in mm/damon/paddr.c:damon_pa_pageout():

	while (addr < damon_pa_phys_addr(r->ar.end, addr_unit)) {
		...
	}

>  		ranges[i].start = sys_region->ar.start;
>  		ranges[i].end = sys_region->ar.end;
>  		if (i == 0)

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

Re: [RFC PATCH 8/8] mm/damon/sysfs: remove region size validation

[SJ Park]

On Wed, 01 Jul 2026 16:34:54 +0000 sashiko-bot@kernel.org wrote:

> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [High] Removing the `start > end` validation allows users to configure monitoring regions with negative sizes, causing integer underflow in region size calculations. This leads to infinite memory allocation loops (OOM), potential list corruption, and CPU soft lockups during DAMON region splitting and scheme operations.
> --
> 
> commit 013d66b2108372ca2ada7fc6fcfd5e07f93fd78c
> Author: SJ Park <sj@kernel.org>
> 
> mm/damon/sysfs: remove region size validation
> 
> This commit removes the region size validation in DAMON sysfs that checks
> for negative size regions. The commit message states that the validation is
> incomplete and that damon_set_regions() already performs complete
> validation.
> 
> > diff --git a/mm/damon/sysfs.c b/mm/damon/sysfs.c
> > index e666dddf1feba..b65651498e0d1 100644
> > --- a/mm/damon/sysfs.c
> > +++ b/mm/damon/sysfs.c
> > @@ -1987,9 +1987,6 @@ static int damon_sysfs_set_regions(struct damon_target *t,
> >  		struct damon_sysfs_region *sys_region >  			sysfs_regions->regions_arr[i];
> >  
> > -		if (sys_region->ar.start > sys_region->ar.end)
> > -			goto out;
> > -
> 
> [Severity: High]
> The commit message mentions that damon_set_regions() does the complete
> validation, but looking at damon_set_regions() in mm/damon/core.c, does it
> actually validate that start <= end?
> 
> mm/damon/core.c:damon_set_regions() {
> 	...
> 	r = damon_new_region(
> 		ALIGN_DOWN(ranges[i].start, min_region_sz),
> 		ALIGN(ranges[i].end, min_region_sz));
> 	...
> }
> 
> If a user configures a region where start > end, wouldn't this cause an
> integer underflow when calculating the region size later? 
> 
> For example, could this underflow lead to an infinite memory allocation loop
> in 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);
> 	}
> 
> Could this loop evaluate the underflowed size and attempt to allocate new
> regions repeatedly until an OOM occurs?
> 
> Additionally, for newly split regions where start < end locally, could this
> spin over a massive address range without yielding and cause a CPU soft
> lockup in mm/damon/paddr.c:damon_pa_pageout():
> 
> 	while (addr < damon_pa_phys_addr(r->ar.end, addr_unit)) {
> 		...
> 	}

My previous reply [1] should again be applied here.

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


Thanks,
SJ

[...]