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