DAMON development mailing list
 help / color / mirror / Atom feed
* [PATCH 0/4] mm/damon: allow NULL or unstarted damon_ctx parameter for damon_call()
@ 2026-09-03  1:03 SJ Park
  2026-09-03  1:03 ` [PATCH 1/4] mm/damon/core: handle NULL ctx parameter in damon_call() SJ Park
                   ` (4 more replies)
  0 siblings, 5 replies; 11+ messages in thread
From: SJ Park @ 2026-09-03  1:03 UTC (permalink / raw)
  To: Andrew Morton; +Cc: SJ Park, damon, linux-kernel, linux-mm

Callers of damon_call() should validate the damon_ctx object parameter.
If it is NULL or never damon_start()-ed object, damon_call() could
dereference the NULL pointer or indefinitely hang.  Ensuring all callers
doing the validation correctly has turned out to be difficult.  Handle
the corner cases inside the core layer and remove callers' validations.

Patches 1 and 2 respectively allow passing NULL and not yet
damon_start()-ed ctx parameter to damon_call().  Patches 3 and 4 remove
the callers side validations in DMON_RECLAIM and DASMON_LRU_SORT,
respectively.

Changes from RFC
- RFC: https://lore.kernel.org/20260813035001.97364-1-sj@kernel.org
- Drop RFC tag.
- Rebase to lates mm-new.

SJ Park (4):
  mm/damon/core: handle NULL ctx parameter in damon_call()
  mm/damon/core: set ctx->call_controls_obsolete in damon_new_ctx()
  mm/damon/reclaim: remove unnecessary damon_call() param validation
  mm/damon/lru_sort: remove unnecessary damon_call() param validation

 mm/damon/core.c     | 7 +++----
 mm/damon/lru_sort.c | 8 --------
 mm/damon/reclaim.c  | 8 --------
 3 files changed, 3 insertions(+), 20 deletions(-)


base-commit: 7fe9a8a366a72a631bcb23c2b108669ab182f2f2
-- 
2.47.3

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH 1/4] mm/damon/core: handle NULL ctx parameter in damon_call()
  2026-09-03  1:03 [PATCH 0/4] mm/damon: allow NULL or unstarted damon_ctx parameter for damon_call() SJ Park
@ 2026-09-03  1:03 ` SJ Park
  2026-09-03  1:15   ` sashiko-bot
  2026-09-03  1:03 ` [PATCH 2/4] mm/damon/core: set ctx->call_controls_obsolete in damon_new_ctx() SJ Park
                   ` (3 subsequent siblings)
  4 siblings, 1 reply; 11+ messages in thread
From: SJ Park @ 2026-09-03  1:03 UTC (permalink / raw)
  To: Andrew Morton; +Cc: SJ Park, damon, linux-kernel, linux-mm

When NULL damon_ctx pointer parameter is passed, damon_call() could do
NULL dereference.  The caller is responsible to avoid that.  It is easy
to forget, and there are many damon_call() callers.  Meanwhile,
damon_call() is never meant to be performance critical.  It uses mutex
and completion.  Add the NULL pointer check inside damon_call() so that
callers can pass the parameter without NULL checks.

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

diff --git a/mm/damon/core.c b/mm/damon/core.c
index 85d29c76c00cf..03407cf7f9646 100644
--- a/mm/damon/core.c
+++ b/mm/damon/core.c
@@ -2206,6 +2206,8 @@ int damon_kdamond_pid(struct damon_ctx *ctx)
  */
 int damon_call(struct damon_ctx *ctx, struct damon_call_control *control)
 {
+	if (!ctx)
+		return -EINVAL;
 	if (!control->repeat)
 		init_completion(&control->completion);
 	control->canceled = false;
-- 
2.47.3

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH 2/4] mm/damon/core: set ctx->call_controls_obsolete in damon_new_ctx()
  2026-09-03  1:03 [PATCH 0/4] mm/damon: allow NULL or unstarted damon_ctx parameter for damon_call() SJ Park
  2026-09-03  1:03 ` [PATCH 1/4] mm/damon/core: handle NULL ctx parameter in damon_call() SJ Park
@ 2026-09-03  1:03 ` SJ Park
  2026-09-03  1:16   ` sashiko-bot
  2026-09-03  1:03 ` [PATCH 3/4] mm/damon/reclaim: remove unnecessary damon_call() param validation SJ Park
                   ` (2 subsequent siblings)
  4 siblings, 1 reply; 11+ messages in thread
From: SJ Park @ 2026-09-03  1:03 UTC (permalink / raw)
  To: Andrew Morton; +Cc: SJ Park, damon, linux-kernel, linux-mm

damon_ctx->call_controls_obsolete is used to disallow damon_call()
requests when the request cannot be served.  The field is unset and set
when the context execution is started and terminated, respectively.  The
intention is to allow damon_call() requests only while the context is
actively being executed.

damon_ctx constructor, damon_new_ctx() unsets the field, though.  As a
result, passing the damon_ctx parameter that never successfully
damon_start()-ed to damon_call() can indefinitely hang.  The callers
should ensure to avoid the case.  Such parameter validation is not
always simple.  Actually such bugs in DAMON_RECLAIM and DAMON_LRU_SORT
have been found and fixed [1].

Set the field in damon_new_ctx(), so that DAMON API callers can pass the
context parameter to damon_call() without the additional check.

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

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

diff --git a/mm/damon/core.c b/mm/damon/core.c
index 03407cf7f9646..56ec6f616fa9a 100644
--- a/mm/damon/core.c
+++ b/mm/damon/core.c
@@ -933,6 +933,7 @@ struct damon_ctx *damon_new_ctx(void)
 	INIT_LIST_HEAD(&ctx->adaptive_targets);
 	INIT_LIST_HEAD(&ctx->schemes);
 
+	ctx->call_controls_obsolete = true;
 	prandom_seed_state(&ctx->rnd_state, get_random_u64());
 
 	return ctx;
@@ -2196,10 +2197,6 @@ int damon_kdamond_pid(struct damon_ctx *ctx)
  * synchronization.  The return value of the function will be saved in
  * &damon_call_control->return_code.
  *
- * Note that this function should be called only after damon_start() with the
- * @ctx has succeeded.  Otherwise, this function could fall into an indefinite
- * wait.
- *
  * When this function is failed, the @ctx is guaranteed to be stopped.
  *
  * Return: 0 on success, negative error code otherwise.
-- 
2.47.3

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH 3/4] mm/damon/reclaim: remove unnecessary damon_call() param validation
  2026-09-03  1:03 [PATCH 0/4] mm/damon: allow NULL or unstarted damon_ctx parameter for damon_call() SJ Park
  2026-09-03  1:03 ` [PATCH 1/4] mm/damon/core: handle NULL ctx parameter in damon_call() SJ Park
  2026-09-03  1:03 ` [PATCH 2/4] mm/damon/core: set ctx->call_controls_obsolete in damon_new_ctx() SJ Park
@ 2026-09-03  1:03 ` SJ Park
  2026-09-03  1:10   ` sashiko-bot
  2026-09-03  1:03 ` [PATCH 4/4] mm/damon/lru_sort: " SJ Park
  2026-09-03  1:23 ` [PATCH 0/4] mm/damon: allow NULL or unstarted damon_ctx parameter for damon_call() SJ Park
  4 siblings, 1 reply; 11+ messages in thread
From: SJ Park @ 2026-09-03  1:03 UTC (permalink / raw)
  To: Andrew Morton; +Cc: SJ Park, damon, linux-kernel, linux-mm

DAMON_RECLAIM avoids passing NULL or unstarted damon_ctx to damon_call()
with its own validation.  The validation is no longer needed, because
the DAMON core layer now handles the corner cases itself.  Remove the
unnecessary check.

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

diff --git a/mm/damon/reclaim.c b/mm/damon/reclaim.c
index 45d5557cc575a..42a2c9cb13431 100644
--- a/mm/damon/reclaim.c
+++ b/mm/damon/reclaim.c
@@ -271,8 +271,6 @@ static int damon_reclaim_commit_inputs_fn(void *arg)
 	return damon_reclaim_apply_parameters();
 }
 
-static bool damon_reclaim_damon_has_started;
-
 static int damon_reclaim_commit_inputs_store(const char *val,
 					     const struct kernel_param *kp)
 {
@@ -293,10 +291,6 @@ static int damon_reclaim_commit_inputs_store(const char *val,
 	if (!commit_inputs_request)
 		return 0;
 
-	/* Skip damon_call() if ctx has not successfully started. */
-	if (!damon_reclaim_damon_has_started)
-		return -EINVAL;
-
 	err = damon_call(ctx, &control);
 
 	return err ? err : control.return_code;
@@ -343,8 +337,6 @@ static int damon_reclaim_turn(bool on)
 	err = damon_start(&ctx, 1, true);
 	if (err)
 		return err;
-	if (!damon_reclaim_damon_has_started)
-		damon_reclaim_damon_has_started = true;
 	return damon_call(ctx, &call_control);
 }
 
-- 
2.47.3

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH 4/4] mm/damon/lru_sort: remove unnecessary damon_call() param validation
  2026-09-03  1:03 [PATCH 0/4] mm/damon: allow NULL or unstarted damon_ctx parameter for damon_call() SJ Park
                   ` (2 preceding siblings ...)
  2026-09-03  1:03 ` [PATCH 3/4] mm/damon/reclaim: remove unnecessary damon_call() param validation SJ Park
@ 2026-09-03  1:03 ` SJ Park
  2026-09-03  1:10   ` sashiko-bot
  2026-09-03  1:23 ` [PATCH 0/4] mm/damon: allow NULL or unstarted damon_ctx parameter for damon_call() SJ Park
  4 siblings, 1 reply; 11+ messages in thread
From: SJ Park @ 2026-09-03  1:03 UTC (permalink / raw)
  To: Andrew Morton; +Cc: SJ Park, damon, linux-kernel, linux-mm

DAMON_LRU_SORT avoids passing NULL or unstarted damon_ctx to
damon_call() with its own validation.  The validation is no longer
needed, because the DAMON core layer now handles the corner cases
itself.  Remove the unnecessary check.

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

diff --git a/mm/damon/lru_sort.c b/mm/damon/lru_sort.c
index bd847829a9907..f25ee7326e87c 100644
--- a/mm/damon/lru_sort.c
+++ b/mm/damon/lru_sort.c
@@ -346,8 +346,6 @@ static int damon_lru_sort_commit_inputs_fn(void *arg)
 	return damon_lru_sort_apply_parameters();
 }
 
-static bool damon_lru_sort_damon_has_started;
-
 static int damon_lru_sort_commit_inputs_store(const char *val,
 					      const struct kernel_param *kp)
 {
@@ -368,10 +366,6 @@ static int damon_lru_sort_commit_inputs_store(const char *val,
 	if (!commit_inputs_request)
 		return 0;
 
-	/* Skip damon_call() if ctx has not successfully started. */
-	if (!damon_lru_sort_damon_has_started)
-		return -EINVAL;
-
 	err = damon_call(ctx, &control);
 
 	return err ? err : control.return_code;
@@ -422,8 +416,6 @@ static int damon_lru_sort_turn(bool on)
 	err = damon_start(&ctx, 1, true);
 	if (err)
 		return err;
-	if (!damon_lru_sort_damon_has_started)
-		damon_lru_sort_damon_has_started = true;
 	return damon_call(ctx, &call_control);
 }
 
-- 
2.47.3

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* Re: [PATCH 4/4] mm/damon/lru_sort: remove unnecessary damon_call() param validation
  2026-09-03  1:03 ` [PATCH 4/4] mm/damon/lru_sort: " SJ Park
@ 2026-09-03  1:10   ` sashiko-bot
  0 siblings, 0 replies; 11+ messages in thread
From: sashiko-bot @ 2026-09-03  1:10 UTC (permalink / raw)
  To: SJ Park; +Cc: damon

> DAMON_LRU_SORT avoids passing NULL or unstarted damon_ctx to
> damon_call() with its own validation.  The validation is no longer
> needed, because the DAMON core layer now handles the corner cases
> itself.  Remove the unnecessary check.
> 
> Signed-off-by: SJ Park <sj@kernel.org>

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

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


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 3/4] mm/damon/reclaim: remove unnecessary damon_call() param validation
  2026-09-03  1:03 ` [PATCH 3/4] mm/damon/reclaim: remove unnecessary damon_call() param validation SJ Park
@ 2026-09-03  1:10   ` sashiko-bot
  0 siblings, 0 replies; 11+ messages in thread
From: sashiko-bot @ 2026-09-03  1:10 UTC (permalink / raw)
  To: SJ Park; +Cc: damon

> DAMON_RECLAIM avoids passing NULL or unstarted damon_ctx to damon_call()
> with its own validation.  The validation is no longer needed, because
> the DAMON core layer now handles the corner cases itself.  Remove the
> unnecessary check.
> 
> Signed-off-by: SJ Park <sj@kernel.org>

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

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


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 1/4] mm/damon/core: handle NULL ctx parameter in damon_call()
  2026-09-03  1:03 ` [PATCH 1/4] mm/damon/core: handle NULL ctx parameter in damon_call() SJ Park
@ 2026-09-03  1:15   ` sashiko-bot
  0 siblings, 0 replies; 11+ messages in thread
From: sashiko-bot @ 2026-09-03  1:15 UTC (permalink / raw)
  To: SJ Park; +Cc: damon

> When NULL damon_ctx pointer parameter is passed, damon_call() could do
> NULL dereference.  The caller is responsible to avoid that.  It is easy
> to forget, and there are many damon_call() callers.  Meanwhile,
> damon_call() is never meant to be performance critical.  It uses mutex
> and completion.  Add the NULL pointer check inside damon_call() so that
> callers can pass the parameter without NULL checks.
> 
> Signed-off-by: SJ Park <sj@kernel.org>

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

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


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 2/4] mm/damon/core: set ctx->call_controls_obsolete in damon_new_ctx()
  2026-09-03  1:03 ` [PATCH 2/4] mm/damon/core: set ctx->call_controls_obsolete in damon_new_ctx() SJ Park
@ 2026-09-03  1:16   ` sashiko-bot
  2026-09-03  1:22     ` SJ Park
  0 siblings, 1 reply; 11+ messages in thread
From: sashiko-bot @ 2026-09-03  1:16 UTC (permalink / raw)
  To: SJ Park; +Cc: damon

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

Pre-existing issues:
- [High] The patch correctly fixes an indefinite wait vulnerability in damon_call() by initializing ctx->call_controls_obsolete to true, but fails to initialize the identical ctx->walk_control_obsolete flag. This leaves damos_walk() vulnerable to the exact same indefinite hang if called on an unstarted context.
--

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

mm/damon/core: set ctx->call_controls_obsolete in damon_new_ctx()

This patch sets ctx->call_controls_obsolete to true in damon_new_ctx() to
prevent indefinite hangs when damon_call() is used on an unstarted context.

Link: https://lore.kernel.org/20260803134646.16640-1-sj@kernel.org

> diff --git a/mm/damon/core.c b/mm/damon/core.c
> index f1f007d8dd1d9..610556abb129e 100644
> --- a/mm/damon/core.c
> +++ b/mm/damon/core.c
> @@ -933,6 +933,7 @@ struct damon_ctx *damon_new_ctx(void)
>  	INIT_LIST_HEAD(&ctx->adaptive_targets);
>  	INIT_LIST_HEAD(&ctx->schemes);
>  
> +	ctx->call_controls_obsolete = true;

[Severity: High]
This is a pre-existing issue, but does the same fix need to be applied to
ctx->walk_control_obsolete?

Because walk_control_obsolete is zero-initialized (false) by kzalloc_obj(),
calling damos_walk() on an unstarted context will bypass the obsolete check:

mm/damon/core.c:damos_walk() {
	if (ctx->walk_control_obsolete) {
		mutex_unlock(&ctx->walk_control_lock);
		return -ECANCELED;
	}
	...
	wait_for_completion(&control->completion);
}

This could lead to an indefinite hang in wait_for_completion(), since the
DAMON worker thread is not running to signal the completion. Should
ctx->walk_control_obsolete also be initialized to true here?

>  	prandom_seed_state(&ctx->rnd_state, get_random_u64());
>  
>  	return ctx;

[ ... ]

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

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 2/4] mm/damon/core: set ctx->call_controls_obsolete in damon_new_ctx()
  2026-09-03  1:16   ` sashiko-bot
@ 2026-09-03  1:22     ` SJ Park
  0 siblings, 0 replies; 11+ messages in thread
From: SJ Park @ 2026-09-03  1:22 UTC (permalink / raw)
  To: sashiko-bot; +Cc: SJ Park, damon

On Thu, 03 Sep 2026 01:16:20 +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] The patch correctly fixes an indefinite wait vulnerability in damon_call() by initializing ctx->call_controls_obsolete to true, but fails to initialize the identical ctx->walk_control_obsolete flag. This leaves damos_walk() vulnerable to the exact same indefinite hang if called on an unstarted context.
> --
> 
> commit 55412502962e64df1de83b2c9805595450e93fc7
> Author: SJ Park <sj@kernel.org>
> 
> mm/damon/core: set ctx->call_controls_obsolete in damon_new_ctx()
> 
> This patch sets ctx->call_controls_obsolete to true in damon_new_ctx() to
> prevent indefinite hangs when damon_call() is used on an unstarted context.
> 
> Link: https://lore.kernel.org/20260803134646.16640-1-sj@kernel.org
> 
> > diff --git a/mm/damon/core.c b/mm/damon/core.c
> > index f1f007d8dd1d9..610556abb129e 100644
> > --- a/mm/damon/core.c
> > +++ b/mm/damon/core.c
> > @@ -933,6 +933,7 @@ struct damon_ctx *damon_new_ctx(void)
> >  	INIT_LIST_HEAD(&ctx->adaptive_targets);
> >  	INIT_LIST_HEAD(&ctx->schemes);
> >  
> > +	ctx->call_controls_obsolete = true;
> 
> [Severity: High]
> This is a pre-existing issue, but does the same fix need to be applied to
> ctx->walk_control_obsolete?
> 
> Because walk_control_obsolete is zero-initialized (false) by kzalloc_obj(),
> calling damos_walk() on an unstarted context will bypass the obsolete check:
> 
> mm/damon/core.c:damos_walk() {
> 	if (ctx->walk_control_obsolete) {
> 		mutex_unlock(&ctx->walk_control_lock);
> 		return -ECANCELED;
> 	}
> 	...
> 	wait_for_completion(&control->completion);
> }
> 
> This could lead to an indefinite hang in wait_for_completion(), since the
> DAMON worker thread is not running to signal the completion. Should
> ctx->walk_control_obsolete also be initialized to true here?

The issue in damon_call() could happen because it has repeat mode.
damos_walk() doesn't have the repeat mode, so I don't think same issue exists
here.  I will consider doing so in future, for consistency.  But not something
need to be done now.

> 
> >  	prandom_seed_state(&ctx->rnd_state, get_random_u64());
> >  
> >  	return ctx;
> 
> [ ... ]
> 
> -- 
> Sashiko AI review · https://sashiko.dev/#/patchset/20260903010334.93622-1-sj@kernel.org?part=2


Thanks,
SJ

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 0/4] mm/damon: allow NULL or unstarted damon_ctx parameter for damon_call()
  2026-09-03  1:03 [PATCH 0/4] mm/damon: allow NULL or unstarted damon_ctx parameter for damon_call() SJ Park
                   ` (3 preceding siblings ...)
  2026-09-03  1:03 ` [PATCH 4/4] mm/damon/lru_sort: " SJ Park
@ 2026-09-03  1:23 ` SJ Park
  4 siblings, 0 replies; 11+ messages in thread
From: SJ Park @ 2026-09-03  1:23 UTC (permalink / raw)
  To: SJ Park; +Cc: Andrew Morton, damon, linux-kernel, linux-mm

On Wed,  2 Sep 2026 18:03:29 -0700 SJ Park <sj@kernel.org> wrote:

> Callers of damon_call() should validate the damon_ctx object parameter.
> If it is NULL or never damon_start()-ed object, damon_call() could
> dereference the NULL pointer or indefinitely hang.  Ensuring all callers
> doing the validation correctly has turned out to be difficult.  Handle
> the corner cases inside the core layer and remove callers' validations.

Sashiko found no blocker for this series.  Sashiko sent its findings to damon@
mailing list [1], and I replied to all the comments having findings other than
the absence of issues.  Please read those for details.

[1] https://lore.kernel.org/damon/


Thanks,
SJ

[...]

^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2026-09-03  1:24 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-09-03  1:03 [PATCH 0/4] mm/damon: allow NULL or unstarted damon_ctx parameter for damon_call() SJ Park
2026-09-03  1:03 ` [PATCH 1/4] mm/damon/core: handle NULL ctx parameter in damon_call() SJ Park
2026-09-03  1:15   ` sashiko-bot
2026-09-03  1:03 ` [PATCH 2/4] mm/damon/core: set ctx->call_controls_obsolete in damon_new_ctx() SJ Park
2026-09-03  1:16   ` sashiko-bot
2026-09-03  1:22     ` SJ Park
2026-09-03  1:03 ` [PATCH 3/4] mm/damon/reclaim: remove unnecessary damon_call() param validation SJ Park
2026-09-03  1:10   ` sashiko-bot
2026-09-03  1:03 ` [PATCH 4/4] mm/damon/lru_sort: " SJ Park
2026-09-03  1:10   ` sashiko-bot
2026-09-03  1:23 ` [PATCH 0/4] mm/damon: allow NULL or unstarted damon_ctx parameter for damon_call() SJ Park

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox