Skip to content

Commit 1b247cd

Browse files
sjp38gregkh
authored andcommitted
mm/damon/core: avoid use of half-online-committed context
commit 26f775a upstream. 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. [sj@kernel.org: let kdamond_call() with cancel regardless of maybe_corrupted] Link: https://lkml.kernel.org/r/20260320031553.2479-1-sj@kernel.org Link: https://sashiko.dev/#/patchset/20260319145218.86197-1-sj%40kernel.org Link: https://lkml.kernel.org/r/20260319145218.86197-1-sj@kernel.org Link: https://lore.kernel.org/20260319043309.97966-1-sj@kernel.org [1] Fixes: 3301f18 ("mm/damon/sysfs: handle commit command using damon_call()") Signed-off-by: SeongJae Park <sj@kernel.org> Cc: <stable@vger.kernel.org> [6.15+] Signed-off-by: Andrew Morton <akpm@linux-foundation.org> Signed-off-by: SeongJae Park <sj@kernel.org> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
1 parent 4d8d713 commit 1b247cd

2 files changed

Lines changed: 15 additions & 1 deletion

File tree

include/linux/damon.h

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -806,7 +806,14 @@ struct damon_ctx {
806806
struct damos_walk_control *walk_control;
807807
struct mutex walk_control_lock;
808808

809+
/*
810+
* indicate if this may be corrupted. Currentonly this is set only for
811+
* damon_commit_ctx() failure.
812+
*/
813+
bool maybe_corrupted;
814+
809815
/* public: */
816+
/* Working thread of the given DAMON context */
810817
struct task_struct *kdamond;
811818
struct mutex kdamond_lock;
812819

mm/damon/core.c

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1241,6 +1241,7 @@ int damon_commit_ctx(struct damon_ctx *dst, struct damon_ctx *src)
12411241
{
12421242
int err;
12431243

1244+
dst->maybe_corrupted = true;
12441245
if (!is_power_of_2(src->min_region_sz))
12451246
return -EINVAL;
12461247

@@ -1266,6 +1267,7 @@ int damon_commit_ctx(struct damon_ctx *dst, struct damon_ctx *src)
12661267
dst->addr_unit = src->addr_unit;
12671268
dst->min_region_sz = src->min_region_sz;
12681269

1270+
dst->maybe_corrupted = false;
12691271
return 0;
12701272
}
12711273

@@ -2610,10 +2612,11 @@ static void kdamond_call(struct damon_ctx *ctx, bool cancel)
26102612
complete(&control->completion);
26112613
} else if (control->canceled && control->dealloc_on_cancel) {
26122614
kfree(control);
2613-
continue;
26142615
} else {
26152616
list_add(&control->list, &repeat_controls);
26162617
}
2618+
if (!cancel && ctx->maybe_corrupted)
2619+
break;
26172620
}
26182621
control = list_first_entry_or_null(&repeat_controls,
26192622
struct damon_call_control, list);
@@ -2646,6 +2649,8 @@ static int kdamond_wait_activation(struct damon_ctx *ctx)
26462649
kdamond_usleep(min_wait_time);
26472650

26482651
kdamond_call(ctx, false);
2652+
if (ctx->maybe_corrupted)
2653+
return -EINVAL;
26492654
damos_walk_cancel(ctx);
26502655
}
26512656
return -EBUSY;
@@ -2731,6 +2736,8 @@ static int kdamond_fn(void *data)
27312736
* kdamond_merge_regions() if possible, to reduce overhead
27322737
*/
27332738
kdamond_call(ctx, false);
2739+
if (ctx->maybe_corrupted)
2740+
break;
27342741
if (!list_empty(&ctx->schemes))
27352742
kdamond_apply_schemes(ctx);
27362743
else

0 commit comments

Comments
 (0)