mm/damon/{reclaim,lru_sort}: fix commit_inputs infinite hang

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 <[email protected]>

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 <[email protected]> # 7.2.x

Quoted 4 lines

mm/damon/reclaim.c +0 -0

index 09e941d75f67d..45d5557cc575a 100644

[ ... ]

Quoted 4 lines

mm/damon/reclaim.c +0 -0

@@ -291,11 +293,8 @@ static int damon_reclaim_commit_inputs_store(const char *val,
 	if (!commit_inputs_request)
 		return 0;
 

mm/damon/reclaim.c static int damon_reclaim_commit_inputs_store(const char *val, · line 300

Quoted 10 lines

mm/damon/reclaim.c +2 -5

-	/*
-	 * 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?

Signature · 2 lines

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=1

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 <[email protected]>

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.

Quoted 4 lines

mm/damon/lru_sort.c +0 -0

index 7e077084cb03a..7df45f9a0b3ae 100644

[ ... ]

Quoted 4 lines

mm/damon/lru_sort.c +0 -0

@@ -364,11 +366,8 @@ static int damon_lru_sort_commit_inputs_store(const char *val,
 	if (!commit_inputs_request)
 		return 0;
 

mm/damon/lru_sort.c static int damon_lru_sort_commit_inputs_store(const char *val, · line 373

Quoted 10 lines

mm/damon/lru_sort.c +2 -5

-	/*
-	 * 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?

Signature · 2 lines

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=2