Skip to content

Commit 26f775a

Browse files
sjp38akpm00
authored andcommitted
mm/damon/core: avoid use of half-online-committed context
One major usage of damon_call() is online DAMON parameters update. It is done by calling damon_commit_ctx() inside the damon_call() callback function. damon_commit_ctx() can fail for two reasons: 1) invalid parameters and 2) internal memory allocation failures. In case of failures, the damon_ctx that attempted to be updated (commit destination) can be partially updated (or, corrupted from a perspective), and therefore shouldn't be used anymore. The function only ensures the damon_ctx object can safely deallocated using damon_destroy_ctx(). The API callers are, however, calling damon_commit_ctx() only after asserting the parameters are valid, to avoid damon_commit_ctx() fails due to invalid input parameters. But it can still theoretically fail if the internal memory allocation fails. In the case, DAMON may run with the partially updated damon_ctx. This can result in unexpected behaviors including even NULL pointer dereference in case of damos_commit_dests() failure [1]. Such allocation failure is arguably too small to fail, so the real world impact would be rare. But, given the bad consequence, this needs to be fixed. Avoid such partially-committed (maybe-corrupted) damon_ctx use by saving the damon_commit_ctx() failure on the damon_ctx object. For this, introduce damon_ctx->maybe_corrupted field. damon_commit_ctx() sets it when it is failed. kdamond_call() checks if the field is set after each damon_call_control->fn() is executed. If it is set, ignore remaining callback requests and return. All kdamond_call() callers including kdamond_fn() also check the maybe_corrupted field right after kdamond_call() invocations. If the field is set, break the kdamond_fn() main loop so that DAMON sill doesn't use the context that might be corrupted. [[email protected]: let kdamond_call() with cancel regardless of maybe_corrupted] Link: https://lkml.kernel.org/r/[email protected] Link: https://sashiko.dev/#/patchset/20260319145218.86197-1-sj%40kernel.org Link: https://lkml.kernel.org/r/[email protected] Link: https://lore.kernel.org/[email protected] [1] Fixes: 3301f18 ("mm/damon/sysfs: handle commit command using damon_call()") Signed-off-by: SeongJae Park <[email protected]> Cc: <[email protected]> [6.15+] Signed-off-by: Andrew Morton <[email protected]>
1 parent 3a206a8 commit 26f775a

2 files changed

Lines changed: 14 additions & 0 deletions

File tree

include/linux/damon.h

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -810,6 +810,12 @@ struct damon_ctx {
810810
struct damos_walk_control *walk_control;
811811
struct mutex walk_control_lock;
812812

813+
/*
814+
* indicate if this may be corrupted. Currentonly this is set only for
815+
* damon_commit_ctx() failure.
816+
*/
817+
bool maybe_corrupted;
818+
813819
/* Working thread of the given DAMON context */
814820
struct task_struct *kdamond;
815821
/* Protects @kdamond field access */

mm/damon/core.c

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1252,6 +1252,7 @@ int damon_commit_ctx(struct damon_ctx *dst, struct damon_ctx *src)
12521252
{
12531253
int err;
12541254

1255+
dst->maybe_corrupted = true;
12551256
if (!is_power_of_2(src->min_region_sz))
12561257
return -EINVAL;
12571258

@@ -1277,6 +1278,7 @@ int damon_commit_ctx(struct damon_ctx *dst, struct damon_ctx *src)
12771278
dst->addr_unit = src->addr_unit;
12781279
dst->min_region_sz = src->min_region_sz;
12791280

1281+
dst->maybe_corrupted = false;
12801282
return 0;
12811283
}
12821284

@@ -2678,6 +2680,8 @@ static void kdamond_call(struct damon_ctx *ctx, bool cancel)
26782680
complete(&control->completion);
26792681
else if (control->canceled && control->dealloc_on_cancel)
26802682
kfree(control);
2683+
if (!cancel && ctx->maybe_corrupted)
2684+
break;
26812685
}
26822686

26832687
mutex_lock(&ctx->call_controls_lock);
@@ -2707,6 +2711,8 @@ static int kdamond_wait_activation(struct damon_ctx *ctx)
27072711
kdamond_usleep(min_wait_time);
27082712

27092713
kdamond_call(ctx, false);
2714+
if (ctx->maybe_corrupted)
2715+
return -EINVAL;
27102716
damos_walk_cancel(ctx);
27112717
}
27122718
return -EBUSY;
@@ -2790,6 +2796,8 @@ static int kdamond_fn(void *data)
27902796
* kdamond_merge_regions() if possible, to reduce overhead
27912797
*/
27922798
kdamond_call(ctx, false);
2799+
if (ctx->maybe_corrupted)
2800+
break;
27932801
if (!list_empty(&ctx->schemes))
27942802
kdamond_apply_schemes(ctx);
27952803
else

0 commit comments

Comments
 (0)