Re: [PATCH 6.18.y 2/2] mm/damon/lru_sort: detect and use fresh enabled and kdamond_pid values

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] NULL pointer dereference in damon_lru_sort_enabled_store if module initialization fails.
--

commit 1c4b8a292830197d1c3c21bb2ed871fd37b99120
Author: SeongJae Park <sj@kernel.org>

mm/damon/lru_sort: detect and use fresh enabled and kdamond_pid values

This patch updates DAMON_LRU_SORT to dynamically detect and use the fresh
status for the enabled and kdamond_pid parameters when requested. This
avoids stale parameter values if kdamond is stopped unintentionally.

Link: https://lore.kernel.org/20260419161003.79176-3-sj@kernel.org

> diff --git a/mm/damon/lru_sort.c b/mm/damon/lru_sort.c
> index 42b9a656f9ded..0c2274fefd763 100644
> --- a/mm/damon/lru_sort.c
> +++ b/mm/damon/lru_sort.c

[ ... ]

> @@ -330,42 +316,83 @@ module_param_cb(addr_unit, &addr_unit_param_ops, &addr_unit, 0600);
>  MODULE_PARM_DESC(addr_unit,
>  	"Scale factor for DAMON_LRU_SORT to ops address conversion (default: 1)");
>  
> +static bool damon_lru_sort_enabled(void)
> +{
> +	if (!ctx)
> +		return false;
> +	return damon_is_running(ctx);
> +}
> +
>  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);

Can this result in a NULL pointer dereference if the module initialization
fails?

If damon_lru_sort_init() fails during boot (for instance, due to memory
allocation failure), ctx remains NULL. Since CONFIG_DAMON_LRU_SORT
parameters are registered as built-in module parameters, writing 'Y' to
/sys/module/damon_lru_sort/parameters/enabled will invoke
damon_lru_sort_enabled_store().

In this scenario:
- damon_lru_sort_enabled() safely returns false (due to !ctx).
- The damon_lru_sort_enabled() == enabled check is bypassed (false != true).
- damon_initialized() returns true (since the DAMON core initialized).
- damon_lru_sort_turn(true) is executed.

This leads to damon_lru_sort_apply_parameters() calling
damon_commit_ctx(ctx, param_ctx), which unconditionally dereferences the
NULL ctx, leading to a kernel panic.

Should there be an explicit if (!ctx) check in
damon_lru_sort_enabled_store() to prevent this, similar to how it was
added to the read paths?

While this vulnerability is not strictly introduced by this patch, the patch
misses the opportunity to fix it while addressing ctx nullability in the
read paths. The same vulnerability also appears to be present in sibling
modules mm/damon/reclaim.c and mm/damon/stat.c.

> +}
>  
> -set_param_out:
> -	enabled = enable;
> -	return err;
> -}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/2026051240-quickly-effective-c901@gregkh?part=2