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

[SeongJae Park <sj@kernel.org>]

On Thu, 14 May 2026 01:56:20 +0000 sashiko-bot@kernel.org wrote:

> 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.

Sashiko reported [1] same thing before, and I'm working on it.

[1] https://lore.kernel.org/all/20260419014800.877-1-sj@kernel.org/


Thanks,
SJ

[...]