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

[RFC PATCH v1.1 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

Changes from RFC v1
- RFC v1: https://lore.kernel.org/20260701144815.113325-1-sj@kernel.org
- Fix wrong dst testing of damon_commit_ctx().
- Use fresh test_ctx for each ctx in damon_start().

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       | 78 ++++++++++++++++++++++++++++++++-----------
 mm/damon/lru_sort.c   |  5 ---
 mm/damon/reclaim.c    |  5 ---
 mm/damon/sysfs.c      | 35 +------------------
 5 files changed, 62 insertions(+), 64 deletions(-)


base-commit: 77d522544edf9183183689ea38f01d6c03098200
-- 
2.47.3

[RFC PATCH v1.1 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..018dd5ff80324 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, src);
+	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 v1.1 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:

Pre-existing issues:
- [High] damon_commit_ctx() silently corrupts the quota charging state of running schemes.
--

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

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

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.

> diff --git a/mm/damon/core.c b/mm/damon/core.c
> index c6fc35be633b3..b523a18a7b971 100644
> --- a/mm/damon/core.c
> +++ b/mm/damon/core.c

[ ... ]

> @@ -1702,6 +1689,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);

[Severity: High]
This is a pre-existing issue, but does this unintentionally corrupt the running
quota state?

When __damon_commit_ctx() is called with the running context (dst) as the
source, it copies the schemes via damon_commit_schemes(), which calls
damon_new_scheme() and passes a pointer to the active scheme's quota:

mm/damon/core.c:damon_commit_schemes() {
	...
	new_scheme = damon_new_scheme(&src_scheme->pattern,
				src_scheme->action,
				src_scheme->apply_interval_us,
				&src_scheme->quota, &src_scheme->wmarks,
				NUMA_NO_NODE);
	...
}

damon_new_scheme() then calls damos_quota_init() on that provided quota
pointer:

mm/damon/core.c:damon_new_scheme() {
	...
	scheme->quota = *(damos_quota_init(quota));
	...
}

damos_quota_init() zeroes out the fields in-place:

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

This mutates the running scheme's quota state, which would make the quota
effectively "full" mid-interval and prevent the scheme from applying its
actions until the next reset interval.

Since this patch moves this test context logic into the core
damon_commit_ctx(), doesn't this expose all API users like damon_reclaim
and damon_lru_sort to this state corruption when parameters are committed?

> +	if (err) {
> +		damon_destroy_ctx(test_ctx);
> +		return NULL;
> +	}
> +	return test_ctx;
> +}

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

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

[SJ Park]

On Thu, 02 Jul 2026 21:21:42 +0000 sashiko-bot@kernel.org wrote:

> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> 
> Pre-existing issues:
> - [High] damon_commit_ctx() silently corrupts the quota charging state of running schemes.
> --
> 
> commit b274d76c061808ff0cd97c6e9433199f816cc1de
> Author: SJ Park <sj@kernel.org>
> 
> mm/damon/core: safely validate src on damon_commit_ctx()
> 
> 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.
> 
> > diff --git a/mm/damon/core.c b/mm/damon/core.c
> > index c6fc35be633b3..b523a18a7b971 100644
> > --- a/mm/damon/core.c
> > +++ b/mm/damon/core.c
> 
> [ ... ]
> 
> > @@ -1702,6 +1689,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);
> 
> [Severity: High]
> This is a pre-existing issue, but does this unintentionally corrupt the running
> quota state?
> 
> When __damon_commit_ctx() is called with the running context (dst) as the
> source, it copies the schemes via damon_commit_schemes(), which calls
> damon_new_scheme() and passes a pointer to the active scheme's quota:
> 
> mm/damon/core.c:damon_commit_schemes() {
> 	...
> 	new_scheme = damon_new_scheme(&src_scheme->pattern,
> 				src_scheme->action,
> 				src_scheme->apply_interval_us,
> 				&src_scheme->quota, &src_scheme->wmarks,
> 				NUMA_NO_NODE);
> 	...
> }
> 
> damon_new_scheme() then calls damos_quota_init() on that provided quota
> pointer:
> 
> mm/damon/core.c:damon_new_scheme() {
> 	...
> 	scheme->quota = *(damos_quota_init(quota));
> 	...
> }
> 
> damos_quota_init() zeroes out the fields in-place:
> 
> mm/damon/core.c:damos_quota_init() {
> 	...
> 	quota->esz = 0;
> 	quota->total_charged_sz = 0;
> 	...
> }
> 
> This mutates the running scheme's quota state, which would make the quota
> effectively "full" mid-interval and prevent the scheme from applying its
> actions until the next reset interval.

That doesn't matter because it is only testing purpose scheme, not the schemes
of the running context.

> 
> Since this patch moves this test context logic into the core
> damon_commit_ctx(), doesn't this expose all API users like damon_reclaim
> and damon_lru_sort to this state corruption when parameters are committed?

So, no problem.


Thanks,
SJ

[...]

[RFC PATCH v1.1 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 | 14 ++++++++++++--
 1 file changed, 12 insertions(+), 2 deletions(-)

diff --git a/mm/damon/core.c b/mm/damon/core.c
index 018dd5ff80324..ca301abcb9ec5 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
@@ -1886,8 +1888,16 @@ int damon_start(struct damon_ctx **ctxs, int nr_ctxs, bool exclusive)
 	int err = 0;
 
 	for (i = 0; i < nr_ctxs; i++) {
-		if (!is_power_of_2(ctxs[i]->min_region_sz))
-			return -EINVAL;
+		struct damon_ctx *test_ctx;
+
+		test_ctx = damon_new_ctx();
+		if (!test_ctx)
+			return -ENOMEM;
+
+		err = __damon_commit_ctx(test_ctx, ctxs[i]);
+		damon_destroy_ctx(test_ctx);
+		if (err)
+			return err;
 	}
 
 	mutex_lock(&damon_lock);
-- 
2.47.3

[RFC PATCH v1.1 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

[RFC PATCH v1.1 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

[RFC PATCH v1.1 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 v1.1 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 early validation of `min_region_sz` causes persistent corruption of the `monitor_region_start` and `monitor_region_end` module parameters on failure.
--

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

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

This commit removes an explicit power-of-two check on min_region_sz from
damon_lru_sort_apply_parameters(), as the same validation is performed
later in damon_commit_ctx().

> 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 allow execution to proceed with an invalid
addr_unit?

If the monitor_region_start and monitor_region_end parameters are 0
(the default, triggering auto-detection), they are overwritten in-place
with system bounds scaled by the invalid addr_unit when this function
is called later in damon_lru_sort_apply_parameters():

	err = damon_set_region_system_rams_default(param_target,
					&monitor_region_start,
					&monitor_region_end,
					param_ctx->addr_unit,
					param_ctx->min_region_sz);

When damon_commit_ctx() later rejects the configuration and returns -EINVAL,
these corrupted values are not reverted.

If a user then corrects addr_unit and applies again, will the now non-zero
parameters bypass auto-detection and get scaled again? This could cause
DAMON to monitor an incorrect and massive physical address range, forcing
the kdamond thread into a long pfn_valid() loop that consumes excessive CPU.

Is the same state corruption possible in mm/damon/reclaim.c:
damon_reclaim_apply_parameters() due to a similar removal of the early
power-of-two check?

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

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

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

[SJ Park]

On Thu, 02 Jul 2026 21:20:05 +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 validation of `min_region_sz` causes persistent corruption of the `monitor_region_start` and `monitor_region_end` module parameters on failure.
> --
> 
> commit 982a6b66f4f61086f3bd8de5a2da757081d4fa94
> Author: SJ Park <sj@kernel.org>
> 
> mm/damon/lru_sort: remove duplicated min_region_sz power_of_2() check
> 
> This commit removes an explicit power-of-two check on min_region_sz from
> damon_lru_sort_apply_parameters(), as the same validation is performed
> later in damon_commit_ctx().
> 
> > 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 allow execution to proceed with an invalid
> addr_unit?
> 
> If the monitor_region_start and monitor_region_end parameters are 0
> (the default, triggering auto-detection), they are overwritten in-place
> with system bounds scaled by the invalid addr_unit when this function
> is called later in damon_lru_sort_apply_parameters():
> 
> 	err = damon_set_region_system_rams_default(param_target,
> 					&monitor_region_start,
> 					&monitor_region_end,
> 					param_ctx->addr_unit,
> 					param_ctx->min_region_sz);
> 
> When damon_commit_ctx() later rejects the configuration and returns -EINVAL,
> these corrupted values are not reverted.
> 
> If a user then corrects addr_unit and applies again, will the now non-zero
> parameters bypass auto-detection and get scaled again? This could cause
> DAMON to monitor an incorrect and massive physical address range, forcing
> the kdamond thread into a long pfn_valid() loop that consumes excessive CPU.
> 
> Is the same state corruption possible in mm/damon/reclaim.c:
> damon_reclaim_apply_parameters() due to a similar removal of the early
> power-of-two check?

I think the user should recheck the parameters after failures.  No blocker.


Thanks,
SJ

[...]

[RFC PATCH v1.1 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 v1.1 6/8] mm/damon: document region size validation in damon_set_regions()

[sashiko-bot]

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

Pre-existing issues:
- [High] `damon_set_regions()` creates duplicate and overlapping regions if input ranges share aligned boundaries, corrupting DAMON's internal state.
- [Medium] `damon_set_regions()` inconsistently bypasses size validation, silently accepting and re-aligning negative-sized (`start > end`) ranges instead of rejecting them.
--

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

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

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.

> diff --git a/include/linux/damon.h b/include/linux/damon.h
> index 805e089ff4f2f..b3c0ac5bf9b88 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.

[Severity: High]
This is a pre-existing issue, but if callers rely on damon_set_regions() to
do the validation, can it create duplicate and overlapping regions when input
ranges share aligned boundaries?

If userspace passes multiple valid non-overlapping ranges (e.g., [0, 100] and
[100, 200]) that align to the same min_region_sz (e.g., 4096), and the target
is empty, the loop in mm/damon/core.c:damon_set_regions() blindly adds
aligned ranges without checking for previous alignments:

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 (!r)
				return -ENOMEM;
			damon_add_region(r, t);
...
}

When regions exist, the iterator is not reset per input range:

mm/damon/core.c:damon_set_regions() {
...
	r = damon_first_region(t);
	/* Add new regions or resize existing regions to fit in the ranges */
	for (i = 0; i < nr_ranges; i++) {
...
		/* Get the first/last regions intersecting with the range */
		damon_for_each_region_from(r, t) {
...
}

It advances past the first intersection, fails to find the same region for the
second range, and creates a duplicate [0, 4096] region, which corrupts
DAMON's internal lists.

[Severity: Medium]
This isn't a bug introduced by this patch, but does damon_set_regions()
actually fully validate the size?

The validation check in mm/damon/core.c:damon_set_regions() uses aligned
boundaries:

mm/damon/core.c:damon_set_regions() {
...
		if (ALIGN_DOWN(ranges[i].start, min_region_sz) >=
				ALIGN(ranges[i].end, min_region_sz))
			return -EINVAL;
...
}

If a user writes a reversed range (e.g., start=4000, end=1000) via sysfs,
since 0 >= 4096 evaluates to false, wouldn't the invalid input be silently
accepted and transformed into [0, 4096], bypassing the intended size
restrictions?

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

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

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

[SJ Park]

On Thu, 02 Jul 2026 21:19:54 +0000 sashiko-bot@kernel.org wrote:

> Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
> 
> Pre-existing issues:
> - [High] `damon_set_regions()` creates duplicate and overlapping regions if input ranges share aligned boundaries, corrupting DAMON's internal state.
> - [Medium] `damon_set_regions()` inconsistently bypasses size validation, silently accepting and re-aligning negative-sized (`start > end`) ranges instead of rejecting them.
> --
> 
> commit ded29c13e7c58ad791814e55b8139da57477556b
> Author: SJ Park <sj@kernel.org>
> 
> mm/damon: document region size validation in damon_set_regions()
> 
> 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.
> 
> > diff --git a/include/linux/damon.h b/include/linux/damon.h
> > index 805e089ff4f2f..b3c0ac5bf9b88 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.
> 
> [Severity: High]
> This is a pre-existing issue, but if callers rely on damon_set_regions() to
> do the validation, can it create duplicate and overlapping regions when input
> ranges share aligned boundaries?
> 
> If userspace passes multiple valid non-overlapping ranges (e.g., [0, 100] and
> [100, 200]) that align to the same min_region_sz (e.g., 4096), and the target
> is empty, the loop in mm/damon/core.c:damon_set_regions() blindly adds
> aligned ranges without checking for previous alignments:
> 
> 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 (!r)
> 				return -ENOMEM;
> 			damon_add_region(r, t);
> ...
> }
> 
> When regions exist, the iterator is not reset per input range:
> 
> mm/damon/core.c:damon_set_regions() {
> ...
> 	r = damon_first_region(t);
> 	/* Add new regions or resize existing regions to fit in the ranges */
> 	for (i = 0; i < nr_ranges; i++) {
> ...
> 		/* Get the first/last regions intersecting with the range */
> 		damon_for_each_region_from(r, t) {
> ...
> }
> 
> It advances past the first intersection, fails to find the same region for the
> second range, and creates a duplicate [0, 4096] region, which corrupts
> DAMON's internal lists.

Yes, and we are working for this with a separate hotfix [1].

> 
> [Severity: Medium]
> This isn't a bug introduced by this patch, but does damon_set_regions()
> actually fully validate the size?
> 
> The validation check in mm/damon/core.c:damon_set_regions() uses aligned
> boundaries:
> 
> mm/damon/core.c:damon_set_regions() {
> ...
> 		if (ALIGN_DOWN(ranges[i].start, min_region_sz) >=
> 				ALIGN(ranges[i].end, min_region_sz))
> 			return -EINVAL;
> ...
> }
> 
> If a user writes a reversed range (e.g., start=4000, end=1000) via sysfs,
> since 0 >= 4096 evaluates to false, wouldn't the invalid input be silently
> accepted and transformed into [0, 4096], bypassing the intended size
> restrictions?

This is an intended behavior.

Yes, this might be confusing.  But why a sane user would do this?  Maybe we can
update documentation for reducing the confusion, but not necessarily a blocker
of this patch.

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


Thanks,
SJ

[...]

[RFC PATCH v1.1 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 ca301abcb9ec5..3c5a46e1c079a 100644
--- a/mm/damon/core.c
+++ b/mm/damon/core.c
@@ -3846,9 +3846,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 v1.1 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:
- [Medium] Removing the explicit `*start > *end` check introduces inconsistent input validation, allowing inverted or zero-size ranges to be silently accepted.
--

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

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

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.

> diff --git a/mm/damon/core.c b/mm/damon/core.c
> index 179ce6b0a542d..dba53a41a1025 100644
> --- a/mm/damon/core.c
> +++ b/mm/damon/core.c
> @@ -3812,9 +3812,6 @@ int damon_set_region_system_rams_default(struct damon_target *t,
>  {
>  	struct damon_addr_range addr_range;
>  
> -	if (*start > *end)
> -		return -EINVAL;
> -

[Severity: Medium]
Does removing this check allow invalid inverted or zero-size unaligned bounds
to be silently accepted?

The commit message claims that a complete check is done inside
damon_set_regions(). However, looking at damon_set_regions(), the validation
acts on aligned values:

mm/damon/core.c:damon_set_regions() {
    ...
	for (i = 0; i < nr_ranges; i++) {
		if (ALIGN_DOWN(ranges[i].start, min_region_sz) >=
				ALIGN(ranges[i].end, min_region_sz))
			return -EINVAL;
	}
    ...
}

If unaligned invalid bounds are passed (for example, start=4095 and end=20,
or start=20 and end=20), they are rounded to valid blocks (e.g., 0 to 4096)
by ALIGN_DOWN and ALIGN.

Because they evaluate to a valid positive range after alignment, they escape
the >= check and are silently accepted.

Should this explicit check be kept to ensure unaligned invalid bounds are
correctly rejected before alignment?

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

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

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

[SJ Park]

On Thu, 02 Jul 2026 21:16:52 +0000 sashiko-bot@kernel.org wrote:

> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [Medium] Removing the explicit `*start > *end` check introduces inconsistent input validation, allowing inverted or zero-size ranges to be silently accepted.
> --
> 
> commit ad7c7a8a7766992d5ad4dbe1c23874e807f45071
> Author: SJ Park <sj@kernel.org>
> 
> mm/damon/core: remove start, end check in damon_set_region_system_rams()
> 
> 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.
> 
> > diff --git a/mm/damon/core.c b/mm/damon/core.c
> > index 179ce6b0a542d..dba53a41a1025 100644
> > --- a/mm/damon/core.c
> > +++ b/mm/damon/core.c
> > @@ -3812,9 +3812,6 @@ int damon_set_region_system_rams_default(struct damon_target *t,
> >  {
> >  	struct damon_addr_range addr_range;
> >  
> > -	if (*start > *end)
> > -		return -EINVAL;
> > -
> 
> [Severity: Medium]
> Does removing this check allow invalid inverted or zero-size unaligned bounds
> to be silently accepted?
> 
> The commit message claims that a complete check is done inside
> damon_set_regions(). However, looking at damon_set_regions(), the validation
> acts on aligned values:
> 
> mm/damon/core.c:damon_set_regions() {
>     ...
> 	for (i = 0; i < nr_ranges; i++) {
> 		if (ALIGN_DOWN(ranges[i].start, min_region_sz) >=
> 				ALIGN(ranges[i].end, min_region_sz))
> 			return -EINVAL;
> 	}
>     ...
> }
> 
> If unaligned invalid bounds are passed (for example, start=4095 and end=20,
> or start=20 and end=20), they are rounded to valid blocks (e.g., 0 to 4096)
> by ALIGN_DOWN and ALIGN.
> 
> Because they evaluate to a valid positive range after alignment, they escape
> the >= check and are silently accepted.
> 
> Should this explicit check be kept to ensure unaligned invalid bounds are
> correctly rejected before alignment?

Both are no issues.  Refer to my reply to patch 6 Sashiko reply for more
details.


Thanks,
SJ

[...]

[RFC PATCH v1.1 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 v1.1 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 negative-sized regions to bypass subsequent validation, resulting in overlapping regions and eventually negative-sized regions in the DAMON core, leading to unbounded memory allocation (OOM).
--

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

mm/damon/sysfs: remove region size validation

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.

> 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]
Does removing this validation allow negative-sized regions to bypass the
subsequent overlap checks in damon_sysfs_set_regions()?

If we have an inverted region (e.g., start=2000, end=1000) followed by another
region (e.g., start=1500, end=3000), the overlap check in
damon_sysfs_set_regions() evaluates:

	if (ranges[i - 1].end > ranges[i].start)

Which results in ranges[0].end (1000) > ranges[1].start (1500).

This is false, meaning the inverted region bypasses the overlap check.

Will this inverted region also bypass the alignment-based bounds check in
mm/damon/core.c:damon_set_regions()?

mm/damon/core.c:damon_set_regions() {
	...
	for (i = 0; i < nr_ranges; i++) {
		if (ALIGN_DOWN(ranges[i].start, min_region_sz) >=
				ALIGN(ranges[i].end, min_region_sz))
			return -EINVAL;
	}

If the regions fall within the same alignment block (e.g., start=2000 and
end=1000 with min_region_sz=4096 align to 0 and 4096), the check evaluates
0 >= 4096, which is false, passing the invalid input to the core.

When the negative-sized region reaches mm/damon/core.c:damon_apply_min_nr_regions(),
can this cause an out-of-memory 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);
			}

The inverted region size would underflow to a very large number, which appears
to cause this loop to continuously split the region, allocating region structs
until the system runs out of memory.

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

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

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

[SJ Park]

On Thu, 02 Jul 2026 21:15:39 +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 negative-sized regions to bypass subsequent validation, resulting in overlapping regions and eventually negative-sized regions in the DAMON core, leading to unbounded memory allocation (OOM).
> --
> 
> commit d23d0a5af36c320aac75064c21548cc9de76c4f1
> Author: SJ Park <sj@kernel.org>
> 
> mm/damon/sysfs: remove region size validation
> 
> 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.
> 
> > 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]
> Does removing this validation allow negative-sized regions to bypass the
> subsequent overlap checks in damon_sysfs_set_regions()?
> 
> If we have an inverted region (e.g., start=2000, end=1000) followed by another
> region (e.g., start=1500, end=3000), the overlap check in
> damon_sysfs_set_regions() evaluates:
> 
> 	if (ranges[i - 1].end > ranges[i].start)
> 
> Which results in ranges[0].end (1000) > ranges[1].start (1500).
> 
> This is false, meaning the inverted region bypasses the overlap check.
> 
> Will this inverted region also bypass the alignment-based bounds check in
> mm/damon/core.c:damon_set_regions()?
> 
> mm/damon/core.c:damon_set_regions() {
> 	...
> 	for (i = 0; i < nr_ranges; i++) {
> 		if (ALIGN_DOWN(ranges[i].start, min_region_sz) >=
> 				ALIGN(ranges[i].end, min_region_sz))
> 			return -EINVAL;
> 	}
> 
> If the regions fall within the same alignment block (e.g., start=2000 and
> end=1000 with min_region_sz=4096 align to 0 and 4096), the check evaluates
> 0 >= 4096, which is false, passing the invalid input to the core.
> 
> When the negative-sized region reaches mm/damon/core.c:damon_apply_min_nr_regions(),
> can this cause an out-of-memory 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);
> 			}
> 
> The inverted region size would underflow to a very large number, which appears
> to cause this loop to continuously split the region, allocating region structs
> until the system runs out of memory.

We are working on a fix [1] of this in a separate thread.

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


Thanks,
SJ

[...]