DAMON development mailing list
 help / color / mirror / Atom feed
* [RFC PATCH 0/2] mm/damon/{reclaim,lru_sort}: fix commit_inputs infinite hang
@ 2026-08-02 18:54 SJ Park
  2026-08-02 18:54 ` [RFC PATCH 1/2] mm/damon/reclaim: skip damon_call() if ctx has not started SJ Park
  2026-08-02 18:54 ` [RFC PATCH 2/2] mm/damon/lru_sort: " SJ Park
  0 siblings, 2 replies; 7+ messages in thread
From: SJ Park @ 2026-08-02 18:54 UTC (permalink / raw)
  Cc: SJ Park, stable, Andrew Morton, Liew Rui Yan, damon, linux-kernel,
	linux-mm

Writing 'Y' to commit_inputs parameters of DAMON_RECLAIM and
DAMON_LRU_SORT before the modules were ever turned on causes infinite
hang.  Fix those.

The issue was discovered [1] by Sashiko.

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

SJ Park (2):
  mm/damon/reclaim: skip damon_call() if ctx has not started
  mm/damon/lru_sort: skip damon_call() if ctx has not started

 mm/damon/lru_sort.c | 11 ++++++-----
 mm/damon/reclaim.c  | 11 ++++++-----
 2 files changed, 12 insertions(+), 10 deletions(-)


base-commit: d6580a342bc8c459628d2b68b313e9c0668bca59
-- 
2.47.3

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

* [RFC PATCH 1/2] mm/damon/reclaim: skip damon_call() if ctx has not started
  2026-08-02 18:54 [RFC PATCH 0/2] mm/damon/{reclaim,lru_sort}: fix commit_inputs infinite hang SJ Park
@ 2026-08-02 18:54 ` SJ Park
  2026-08-02 19:04   ` sashiko-bot
  2026-08-02 18:54 ` [RFC PATCH 2/2] mm/damon/lru_sort: " SJ Park
  1 sibling, 1 reply; 7+ messages in thread
From: SJ Park @ 2026-08-02 18:54 UTC (permalink / raw)
  Cc: SJ Park, stable, Andrew Morton, Liew Rui Yan, damon, linux-kernel,
	linux-mm

DAMON_RECLAIM calls damon_call() for commit_inputs parameter user input
if the DAMON context is initialized.  The context could be initialized,
but not yet successfully started.  In the case, damon_call() could
indefinitely hang.  Read the comment on damon_call() for more detail.
Fix the problem by memorizing if the DAMON context has ever successfully
started, and skip damon_call() if it has not.

This issue can easily be reproduced by writing Y to commit_inputs on a
system that DAMON_RECLAIM was not turned on before.

Fixes: de3c60e1c831 ("mm/damon: add synchronous commit for commit_inputs")
Cc: <stable@vger.kernel.org> # 7.2.x
Signed-off-by: SJ Park <sj@kernel.org>
---
 mm/damon/reclaim.c | 11 ++++++-----
 1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/mm/damon/reclaim.c b/mm/damon/reclaim.c
index 09e941d75f67d..45d5557cc575a 100644
--- a/mm/damon/reclaim.c
+++ b/mm/damon/reclaim.c
@@ -271,6 +271,8 @@ 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)
 {
@@ -291,11 +293,8 @@ static int damon_reclaim_commit_inputs_store(const char *val,
 	if (!commit_inputs_request)
 		return 0;
 
-	/*
-	 * Skip damon_call() if ctx is not initialized to avoid
-	 * NULL pointer dereference.
-	 */
-	if (!ctx)
+	/* Skip damon_call() if ctx has not successfully started. */
+	if (!damon_reclaim_damon_has_started)
 		return -EINVAL;
 
 	err = damon_call(ctx, &control);
@@ -344,6 +343,8 @@ 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] 7+ messages in thread

* [RFC PATCH 2/2] mm/damon/lru_sort: skip damon_call() if ctx has not started
  2026-08-02 18:54 [RFC PATCH 0/2] mm/damon/{reclaim,lru_sort}: fix commit_inputs infinite hang SJ Park
  2026-08-02 18:54 ` [RFC PATCH 1/2] mm/damon/reclaim: skip damon_call() if ctx has not started SJ Park
@ 2026-08-02 18:54 ` SJ Park
  2026-08-02 19:08   ` sashiko-bot
  1 sibling, 1 reply; 7+ messages in thread
From: SJ Park @ 2026-08-02 18:54 UTC (permalink / raw)
  Cc: SJ Park, stable, Andrew Morton, Liew Rui Yan, damon, linux-kernel,
	linux-mm

DAMON_LRU_SORT calls damon_call() for commit_inputs parameter user input
if the DAMON context is initialized.  The context could be initialized,
but not yet successfully started.  In the case, damon_call() could
indefinitely hang.  Read the comment on damon_call() for more detail.
Fix the problem by memorizing if the DAMON context has ever successfully
started, and skip damon_call() if it has not.

This issue can easily be reproduced by writing Y to the commit_inputs
parameter file on a system that DAMON_LRU_SORT has not turned on before.

Fixes: de3c60e1c831 ("mm/damon: add synchronous commit for commit_inputs")
Cc: <stable@vger.kernel.org> # 7.2.x
Signed-off-by: SJ Park <sj@kernel.org>
---
 mm/damon/lru_sort.c | 11 ++++++-----
 1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/mm/damon/lru_sort.c b/mm/damon/lru_sort.c
index 3920d5042f109..bd847829a9907 100644
--- a/mm/damon/lru_sort.c
+++ b/mm/damon/lru_sort.c
@@ -346,6 +346,8 @@ 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)
 {
@@ -366,11 +368,8 @@ static int damon_lru_sort_commit_inputs_store(const char *val,
 	if (!commit_inputs_request)
 		return 0;
 
-	/*
-	 * Skip damon_call() if ctx is not initialized to avoid
-	 * NULL pointer dereference.
-	 */
-	if (!ctx)
+	/* Skip damon_call() if ctx has not successfully started. */
+	if (!damon_lru_sort_damon_has_started)
 		return -EINVAL;
 
 	err = damon_call(ctx, &control);
@@ -423,6 +422,8 @@ 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] 7+ messages in thread

* Re: [RFC PATCH 1/2] mm/damon/reclaim: skip damon_call() if ctx has not started
  2026-08-02 18:54 ` [RFC PATCH 1/2] mm/damon/reclaim: skip damon_call() if ctx has not started SJ Park
@ 2026-08-02 19:04   ` sashiko-bot
  2026-08-02 19:07     ` SJ Park
  0 siblings, 1 reply; 7+ messages in thread
From: sashiko-bot @ 2026-08-02 19:04 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] This problem wasn't introduced by this patch, but the patch fixes an indefinite hang in `mm/damon/reclaim.c` when writing to `commit_inputs` before the DAMON context starts, while failing to apply the identical fix to `mm/damon/lru_sort.c`. This leaves `damon_lru_sort` vulnerable to the exact same bug.
--

--- Patch [1]: [RFC PATCH 1/2] mm/damon/reclaim: skip damon_call() if ctx has not started ---
commit 86e90c966d8636445486cfde6487a797b433cb75
Author: SJ Park <sj@kernel.org>

mm/damon/reclaim: skip damon_call() if ctx has not started

This commit addresses an indefinite hang in DAMON reclaim when writing to
commit_inputs before the context has fully started. It introduces a tracking
flag to determine whether the DAMON context has successfully started in order
to safely skip damon_call().

Cc: <stable@vger.kernel.org> # 7.2.x

> diff --git a/mm/damon/reclaim.c b/mm/damon/reclaim.c
> index 09e941d75f67d..45d5557cc575a 100644
> --- a/mm/damon/reclaim.c
> +++ b/mm/damon/reclaim.c
[ ... ]
> @@ -291,11 +293,8 @@ static int damon_reclaim_commit_inputs_store(const char *val,
>  	if (!commit_inputs_request)
>  		return 0;
>  
> -	/*
> -	 * Skip damon_call() if ctx is not initialized to avoid
> -	 * NULL pointer dereference.
> -	 */
> -	if (!ctx)
> +	/* Skip damon_call() if ctx has not successfully started. */
> +	if (!damon_reclaim_damon_has_started)
>  		return -EINVAL;
>  
>  	err = damon_call(ctx, &control);

[Severity: High]
This isn't a bug introduced by this patch, but does the same vulnerability
exist in mm/damon/lru_sort.c?

It looks like damon_lru_sort_commit_inputs_store() uses the exact same
logic that was fixed here in reclaim.c:

mm/damon/lru_sort.c:damon_lru_sort_commit_inputs_store() {
    ...
    /*
     * Skip damon_call() if ctx is not initialized to avoid
     * NULL pointer dereference.
     */
    if (!ctx)
        return -EINVAL;

    err = damon_call(ctx, &control);
    ...
}

If a user writes to commit_inputs for damon_lru_sort before the thread has
started, could damon_call() blindly invoke wait_for_completion() and hang
indefinitely without a running thread to signal it?

Should a similar fix be applied to mm/damon/lru_sort.c to prevent a task
from hanging in an uninterruptible sleep state?

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

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

* Re: [RFC PATCH 1/2] mm/damon/reclaim: skip damon_call() if ctx has not started
  2026-08-02 19:04   ` sashiko-bot
@ 2026-08-02 19:07     ` SJ Park
  0 siblings, 0 replies; 7+ messages in thread
From: SJ Park @ 2026-08-02 19:07 UTC (permalink / raw)
  To: sashiko-bot; +Cc: SJ Park, damon

On Sun, 02 Aug 2026 19:04:30 +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] This problem wasn't introduced by this patch, but the patch fixes an indefinite hang in `mm/damon/reclaim.c` when writing to `commit_inputs` before the DAMON context starts, while failing to apply the identical fix to `mm/damon/lru_sort.c`. This leaves `damon_lru_sort` vulnerable to the exact same bug.
> --
> 
> --- Patch [1]: [RFC PATCH 1/2] mm/damon/reclaim: skip damon_call() if ctx has not started ---
> commit 86e90c966d8636445486cfde6487a797b433cb75
> Author: SJ Park <sj@kernel.org>
> 
> mm/damon/reclaim: skip damon_call() if ctx has not started
> 
> This commit addresses an indefinite hang in DAMON reclaim when writing to
> commit_inputs before the context has fully started. It introduces a tracking
> flag to determine whether the DAMON context has successfully started in order
> to safely skip damon_call().
> 
> Cc: <stable@vger.kernel.org> # 7.2.x
> 
> > diff --git a/mm/damon/reclaim.c b/mm/damon/reclaim.c
> > index 09e941d75f67d..45d5557cc575a 100644
> > --- a/mm/damon/reclaim.c
> > +++ b/mm/damon/reclaim.c
> [ ... ]
> > @@ -291,11 +293,8 @@ static int damon_reclaim_commit_inputs_store(const char *val,
> >  	if (!commit_inputs_request)
> >  		return 0;
> >  
> > -	/*
> > -	 * Skip damon_call() if ctx is not initialized to avoid
> > -	 * NULL pointer dereference.
> > -	 */
> > -	if (!ctx)
> > +	/* Skip damon_call() if ctx has not successfully started. */
> > +	if (!damon_reclaim_damon_has_started)
> >  		return -EINVAL;
> >  
> >  	err = damon_call(ctx, &control);
> 
> [Severity: High]
> This isn't a bug introduced by this patch, but does the same vulnerability
> exist in mm/damon/lru_sort.c?
> 
> It looks like damon_lru_sort_commit_inputs_store() uses the exact same
> logic that was fixed here in reclaim.c:
> 
> mm/damon/lru_sort.c:damon_lru_sort_commit_inputs_store() {
>     ...
>     /*
>      * Skip damon_call() if ctx is not initialized to avoid
>      * NULL pointer dereference.
>      */
>     if (!ctx)
>         return -EINVAL;
> 
>     err = damon_call(ctx, &control);
>     ...
> }
> 
> If a user writes to commit_inputs for damon_lru_sort before the thread has
> started, could damon_call() blindly invoke wait_for_completion() and hang
> indefinitely without a running thread to signal it?
> 
> Should a similar fix be applied to mm/damon/lru_sort.c to prevent a task
> from hanging in an uninterruptible sleep state?

Yes, and the next patch of this series is fixing it.

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


Thanks,
SJ

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

* Re: [RFC PATCH 2/2] mm/damon/lru_sort: skip damon_call() if ctx has not started
  2026-08-02 18:54 ` [RFC PATCH 2/2] mm/damon/lru_sort: " SJ Park
@ 2026-08-02 19:08   ` sashiko-bot
  2026-08-02 19:16     ` SJ Park
  0 siblings, 1 reply; 7+ messages in thread
From: sashiko-bot @ 2026-08-02 19:08 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] A TOCTOU race between `damon_start()` and `damon_call()` can cause `damon_call()` to loop indefinitely.
--

--- Patch [2]: [RFC PATCH 2/2] mm/damon/lru_sort: skip damon_call() if ctx has not started ---
commit 9a410362ed5a4418640b6bf73c4ee49300c9dc16
Author: SJ Park <sj@kernel.org>

mm/damon/lru_sort: skip damon_call() if ctx has not started

This commit prevents damon_call() from hanging when the DAMON context is
initialized but not yet started. It introduces a boolean flag to track whether
the context has ever successfully started, and skips the call if it has not.

> diff --git a/mm/damon/lru_sort.c b/mm/damon/lru_sort.c
> index 7e077084cb03a..7df45f9a0b3ae 100644
> --- a/mm/damon/lru_sort.c
> +++ b/mm/damon/lru_sort.c
[ ... ]
> @@ -364,11 +366,8 @@ static int damon_lru_sort_commit_inputs_store(const char *val,
>  	if (!commit_inputs_request)
>  		return 0;
>  
> -	/*
> -	 * Skip damon_call() if ctx is not initialized to avoid
> -	 * NULL pointer dereference.
> -	 */
> -	if (!ctx)
> +	/* Skip damon_call() if ctx has not successfully started. */
> +	if (!damon_lru_sort_damon_has_started)
>  		return -EINVAL;
>  
>  	err = damon_call(ctx, &control);

[Severity: High]
This is a pre-existing issue, but could there be a TOCTOU race here between
damon_start() and damon_call() that causes damon_call() to loop indefinitely?

If a user starts DAMON (setting damon_lru_sort_damon_has_started = true),
stops it (leaving call_controls_obsolete = true), and then concurrently
starts DAMON again while calling damon_call() via commit_inputs, can
damon_call() get stuck?

Looking at mm/damon/core.c, __damon_start() sets ctx->kdamond before the new
thread clears ctx->call_controls_obsolete:

mm/damon/core.c:__damon_start() {
    ...
    ctx->kdamond = kthread_run(kdamond_fn, ctx, "kdamond.%d", nr_running_ctxs);
    ...
}

If damon_call() runs concurrently, it checks call_controls_obsolete without
holding kdamond_lock, sees the stale true value, and jumps to the canceled
label:

mm/damon/core.c:damon_call() {
    ...
    if (ctx->call_controls_obsolete) {
        ...
        goto canceled;
    }
    ...
canceled:
    while (damon_is_running(ctx))
        schedule_timeout_idle(msecs_to_jiffies(100));
    return -ECANCELED;
}

Since damon_is_running() returns true (because ctx->kdamond is set), and the
new healthy thread will run indefinitely, will this task loop forever in
TASK_IDLE?

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

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

* Re: [RFC PATCH 2/2] mm/damon/lru_sort: skip damon_call() if ctx has not started
  2026-08-02 19:08   ` sashiko-bot
@ 2026-08-02 19:16     ` SJ Park
  0 siblings, 0 replies; 7+ messages in thread
From: SJ Park @ 2026-08-02 19:16 UTC (permalink / raw)
  To: sashiko-bot; +Cc: SJ Park, damon

On Sun, 02 Aug 2026 19:08:37 +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] A TOCTOU race between `damon_start()` and `damon_call()` can cause `damon_call()` to loop indefinitely.
> --
> 
> --- Patch [2]: [RFC PATCH 2/2] mm/damon/lru_sort: skip damon_call() if ctx has not started ---
> commit 9a410362ed5a4418640b6bf73c4ee49300c9dc16
> Author: SJ Park <sj@kernel.org>
> 
> mm/damon/lru_sort: skip damon_call() if ctx has not started
> 
> This commit prevents damon_call() from hanging when the DAMON context is
> initialized but not yet started. It introduces a boolean flag to track whether
> the context has ever successfully started, and skips the call if it has not.
> 
> > diff --git a/mm/damon/lru_sort.c b/mm/damon/lru_sort.c
> > index 7e077084cb03a..7df45f9a0b3ae 100644
> > --- a/mm/damon/lru_sort.c
> > +++ b/mm/damon/lru_sort.c
> [ ... ]
> > @@ -364,11 +366,8 @@ static int damon_lru_sort_commit_inputs_store(const char *val,
> >  	if (!commit_inputs_request)
> >  		return 0;
> >  
> > -	/*
> > -	 * Skip damon_call() if ctx is not initialized to avoid
> > -	 * NULL pointer dereference.
> > -	 */
> > -	if (!ctx)
> > +	/* Skip damon_call() if ctx has not successfully started. */
> > +	if (!damon_lru_sort_damon_has_started)
> >  		return -EINVAL;
> >  
> >  	err = damon_call(ctx, &control);
> 
> [Severity: High]
> This is a pre-existing issue, but could there be a TOCTOU race here between
> damon_start() and damon_call() that causes damon_call() to loop indefinitely?
> 
> If a user starts DAMON (setting damon_lru_sort_damon_has_started = true),
> stops it (leaving call_controls_obsolete = true), and then concurrently
> starts DAMON again while calling damon_call() via commit_inputs, can
> damon_call() get stuck?

There is no such concurrent calls.

> 
> Looking at mm/damon/core.c, __damon_start() sets ctx->kdamond before the new
> thread clears ctx->call_controls_obsolete:
> 
> mm/damon/core.c:__damon_start() {
>     ...
>     ctx->kdamond = kthread_run(kdamond_fn, ctx, "kdamond.%d", nr_running_ctxs);
>     ...
> }
> 
> If damon_call() runs concurrently, it checks call_controls_obsolete without
> holding kdamond_lock, sees the stale true value, and jumps to the canceled
> label:
> 
> mm/damon/core.c:damon_call() {
>     ...
>     if (ctx->call_controls_obsolete) {
>         ...
>         goto canceled;
>     }
>     ...
> canceled:
>     while (damon_is_running(ctx))
>         schedule_timeout_idle(msecs_to_jiffies(100));
>     return -ECANCELED;
> }
> 
> Since damon_is_running() returns true (because ctx->kdamond is set), and the
> new healthy thread will run indefinitely, will this task loop forever in
> TASK_IDLE?

So the issue cannot happen in real.  But good point.  Maybe the API or the
documentation can be safer.  I will add this to my TODO list.

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


Thanks,
SJ

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

end of thread, other threads:[~2026-08-02 19:16 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-08-02 18:54 [RFC PATCH 0/2] mm/damon/{reclaim,lru_sort}: fix commit_inputs infinite hang SJ Park
2026-08-02 18:54 ` [RFC PATCH 1/2] mm/damon/reclaim: skip damon_call() if ctx has not started SJ Park
2026-08-02 19:04   ` sashiko-bot
2026-08-02 19:07     ` SJ Park
2026-08-02 18:54 ` [RFC PATCH 2/2] mm/damon/lru_sort: " SJ Park
2026-08-02 19:08   ` sashiko-bot
2026-08-02 19:16     ` SJ Park

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