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

[SeongJae Park <sj@kernel.org>]

On Sun, 19 Apr 2026 17:28:17 +0000 sashiko-bot@kernel.org wrote:

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

This is invalid.  Please read below for more details.

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

DAMON_LRU_SORT is builtin module.  It cannot be dynmically loaded.

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

Because DAMON_LRU_SORT cannot be dynamically loaded, the scenario cannot
happen.

Nonetheless, a similar issue can happen when the init function fails, which is
unlikely, but theoretically possible.  But that's not caused by this patch, so
no blocker of this patch.  I'm separately working on it.


Thanks,
SJ

[...]