Re: [PATCH v3 2/3] mm/damon/lru_sort: detect and use fresh enabled and kdamond_pid values

[sashiko-bot@kernel.org]

Sashiko AI review found 1 potential issue(s):
- [High] Loading the module with the `enabled=Y` parameter causes a NULL pointer dereference.
--

commit e25c111d55f5d7c99165dbc8b59a5261d640ada5
Author: SeongJae Park <sj@kernel.org>
mm/damon/lru_sort: detect and use fresh enabled and kdamond_pid values

This commit updates DAMON_LRU_SORT to dynamically detect and use the
current running status of the kdamond thread for the enabled and
kdamond_pid parameters. This prevents users from seeing stale values or
being unable to restart kdamond if it stops due to unintentional events.

> diff --git a/mm/damon/lru_sort.c b/mm/damon/lru_sort.c
> index 554559d729760..8494040b1ee48 100644
> --- a/mm/damon/lru_sort.c
> +++ b/mm/damon/lru_sort.c

[ ... ]

>  static int damon_lru_sort_enabled_store(const char *val,
>  		const struct kernel_param *kp)
>  {
> -	bool is_enabled = enabled;
> -	bool enable;
>  	int err;
>  
> -	err = kstrtobool(val, &enable);
> +	err = kstrtobool(val, &enabled);
>  	if (err)
>  		return err;
>  
> -	if (is_enabled == enable)
> +	if (damon_lru_sort_enabled() == enabled)
>  		return 0;
>  
>  	/* Called before init function.  The function will handle this. */
>  	if (!damon_initialized())
> -		goto set_param_out;
> +		return 0;
>  
> -	err = damon_lru_sort_turn(enable);
> -	if (err)
> -		return err;
> +	return damon_lru_sort_turn(enabled);
> +}

This isn't a bug introduced by this patch, but does this code cause a NULL
pointer dereference if the module is dynamically loaded with enabled=Y?

When the module is loaded as a dynamically loadable module with the
enabled=Y parameter, the kernel's module loader parses the parameters
before damon_lru_sort_init() runs.

This invokes damon_lru_sort_enabled_store(). Because damon_core (a module
dependency) is already loaded, damon_initialized() evaluates to true,
bypassing the safety guard.

The function proceeds to call damon_lru_sort_turn(true), which eventually
calls damon_lru_sort_apply_parameters(). This invokes:

    damon_commit_ctx(ctx, param_ctx);

At this point, the global ctx pointer is still NULL because it is only
allocated later in damon_lru_sort_init(). damon_commit_ctx() unconditionally
dereferences the pointer:

    dst->maybe_corrupted = true;

causing a direct NULL pointer dereference and a kernel panic.

Should this check use !ctx instead of !damon_initialized() to properly
defer execution to the init function?

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