Re: [RFC PATCH v2.1 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] Writing to the `enabled` sysfs parameter causes a kernel panic if the module failed to initialize during boot.
--

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

This commit updates the module to dynamically query the running status for
the enabled and kdamond_pid parameters. This avoids stale parameter values
if the kdamond daemon stops due to unintentional events like allocation
failures.

Cc: <stable@vger.kernel.org> # 6.0.x

> 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

[ ... ]

> @@ -430,42 +414,83 @@ module_param_cb(addr_unit, &addr_unit_param_ops, &addr_unit, 0600);

[ ... ]

>  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);

Could writing to this parameter cause a kernel panic if the module failed to
initialize during boot?

If damon_lru_sort_init() fails, the static ctx variable remains NULL. Because
the sysfs parameters are registered early, they remain exposed and writable
even after an initialization failure.

Writing true to the enabled parameter bypasses the damon_initialized() check
since it only checks the core DAMON cache, not this module's ctx.

Execution would then follow this path:

damon_lru_sort_enabled_store()
  damon_lru_sort_turn(true)
    damon_lru_sort_apply_parameters()
      damon_commit_ctx(ctx, param_ctx)

Since ctx is NULL, wouldn't damon_commit_ctx() crash when it tries to
dereference it? Should there be an explicit check for !ctx before attempting
to apply parameters or turn on the feature?

> +}

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