Re: [PATCH 2/4] mm/damon/sysfs: check contexts->nr before clear_schemes_tried_regions
From: Josh Law
Date: Fri Mar 20 2026 - 03:07:26 EST
On 20 March 2026 02:13:17 GMT, SeongJae Park <sj@xxxxxxxxxx> wrote:
>On Thu, 19 Mar 2026 15:57:40 +0000 Josh Law <objecting@xxxxxxxxxxxxx> wrote:
>
>> The CLEAR_SCHEMES_TRIED_REGIONS command accesses contexts_arr[0]
>> without verifying nr_contexts >= 1, causing a NULL pointer dereference
>> when no context is configured. Add the missing check.
>
>Nice catch, thank you!
>
>Privileged users can trigger this using DAMON sysfs interface. E.g.,
>
> # cd /sys/kernel/mm/damon/admin/kdamonds/
> # echo 1 > nr_kdamonds
> # echo clear_schemes_tried_regions > state
> killed
> # dmesg
> [...]
> [63541.377604] BUG: kernel NULL pointer dereference, address: 0000000000000000
> [...]
>
>Privileged users can do anything even worse than this, but they might also do
>this by a mistake.
>
>So this deserves Fixes: and Cc stable.
>
>>
>> Signed-off-by: Josh Law <objecting@xxxxxxxxxxxxx>
>> ---
>> mm/damon/sysfs.c | 2 ++
>> 1 file changed, 2 insertions(+)
>>
>> diff --git a/mm/damon/sysfs.c b/mm/damon/sysfs.c
>> index b573b9d60784..36ad2e8956c9 100644
>> --- a/mm/damon/sysfs.c
>> +++ b/mm/damon/sysfs.c
>> @@ -1769,6 +1769,8 @@ static int damon_sysfs_handle_cmd(enum damon_sysfs_cmd cmd,
>> case DAMON_SYSFS_CMD_UPDATE_SCHEMES_TRIED_REGIONS:
>> return damon_sysfs_update_schemes_tried_regions(kdamond, false);
>> case DAMON_SYSFS_CMD_CLEAR_SCHEMES_TRIED_REGIONS:
>> + if (kdamond->contexts->nr != 1)
>> + return -EINVAL;
>> return damon_sysfs_schemes_clear_regions(
>> kdamond->contexts->contexts_arr[0]->schemes);
>> case DAMON_SYSFS_CMD_UPDATE_SCHEMES_EFFECTIVE_QUOTAS:
>> --
>> 2.34.1
>
>So this patch looks good as an individual fix for the individual bug, but...
>
>Sashiko commented.
>
># review url: https://sashiko.dev/#/patchset/20260319155742.186627-3-objecting@xxxxxxxxxxxxx
>
>: Does this missing check also affect other manual commands?
>:
>: If a user writes UPDATE_SCHEMES_STATS, UPDATE_SCHEMES_EFFECTIVE_QUOTAS,
>: or UPDATE_TUNED_INTERVALS to the state file after setting nr_contexts
>: to 0, damon_sysfs_handle_cmd() queues the corresponding callback via
>: damon_sysfs_damon_call().
>:
>: When the kdamond thread executes the callback, it appears functions like
>: damon_sysfs_upd_schemes_stats() access contexts_arr[0] without verifying
>: contexts->nr:
>:
>: static int damon_sysfs_upd_schemes_stats(void *data)
>: {
>: struct damon_sysfs_kdamond *kdamond = data;
>: struct damon_ctx *ctx = kdamond->damon_ctx;
>:
>: damon_sysfs_schemes_update_stats(
>: kdamond->contexts->contexts_arr[0]->schemes, ctx);
>: return 0;
>: }
>:
>: Could this result in a similar NULL pointer dereference if these commands
>: are triggered while no context is configured?
>
>Sashiko is correct. Privileged users can trigger the issues like below.
>
># damo start
># cd /sys/kernel/mm/damon/admin/kdamonds/0
># echo 0 > contexts/nr_contexts
># echo update_schemes_stats > state
># echo update_schemes_effective_quotas > state
># echo update_tuned_intervals > state
>
>Not necessarily blocker of this patch, but seems all the issues are in a same
>category. The third patch of this series is also fixing one of the category
>bugs. How about fixing all at once by checking kdamond->contexts->nr at the
>beginning of damon_sysfs_handle_cmd(), like below?
>
>--- a/mm/damon/sysfs.c
>+++ b/mm/damon/sysfs.c
>@@ -2404,6 +2404,9 @@ static int damon_sysfs_update_schemes_tried_regions(
> static int damon_sysfs_handle_cmd(enum damon_sysfs_cmd cmd,
> struct damon_sysfs_kdamond *kdamond)
> {
>+ if (cmd != DAMON_SYSFS_CMD_OFF && kdamond->contexts->nr != 1)
>+ return -EINVAL;
>+
> switch (cmd) {
> case DAMON_SYSFS_CMD_ON:
> return damon_sysfs_turn_damon_on(kdamond);
>
>If we pick this, Fixes: would be deserve to the oldest buggy commit that
>introduced the first bug of this category. It is indeed quite old.
>
>Fixes: 0ac32b8affb5 ("mm/damon/sysfs: support DAMOS stats")
>Cc: <stable@xxxxxxxxxxxxxxx> # 5.18.x
>
>
>Thanks,
>SJ
Hello, did you give Reviewed by you? Or not..
V/R
Josh Law