Re: [RFC PATCH 2/2] mm/damonn/lru_sort: handle ctx allocation failure

[SeongJae Park <sj@kernel.org>]

On Thu, 28 May 2026 07:29:55 +0000 sashiko-bot@kernel.org wrote:

> Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
> 
> New issues:
> - [High] Module load parameter 'enabled' causes unconditional module load failure.
> 
> Pre-existing issues:
> - [High] TOCTOU race condition between sysfs store and module initialization leading to data corruption.
> --
> 
> commit 98350211104d3088f79c58844d29a85c013ac178
> Author: SeongJae Park <sj@kernel.org>
> 
> mm/damonn/lru_sort: handle ctx allocation failure
> 
> This commit attempts to fix a NULL pointer dereference in
> damon_lru_sort_enabled_store() by returning -ENOMEM if the damon_ctx object
> failed to allocate during initialization.
> 
> > diff --git a/mm/damon/lru_sort.c b/mm/damon/lru_sort.c
> > index eca88ed941b32..8298c6001fd09 100644
> > --- a/mm/damon/lru_sort.c
> > +++ b/mm/damon/lru_sort.c
> > @@ -476,6 +476,10 @@ static int damon_lru_sort_enabled_store(const char *val,
> >  	if (!damon_initialized())
> >  		return 0;
> >  
> > +	/* damon_modules_new_paddr_ctx_target() in the init function failed. */
> > +	if (!ctx)
> > +		return -ENOMEM;
> > +
> 
> [Severity: High]
> Will this cause an unconditional module load failure when passing the
> enabled parameter at load time?
> 
> When loading the module with modprobe damon_lru_sort enabled=Y, the kernel
> calls parse_args() before the module's init function damon_lru_sort_init()
> is executed.
> 
> At this point, damon_initialized() returns true because the damon_core
> dependency is already initialized, but ctx is still NULL since the module
> init function hasn't run yet.
> 
> Could this evaluate to true and return -ENOMEM, causing the entire module
> loading process to fail and abort?

DAMON_LRU_SORT is not a loadable but static module.  So this cannot happen.

> 
> >  	return damon_lru_sort_turn(enabled);
> >  }
> 
> [Severity: High]
> This isn't a bug introduced by this patch, but is there a TOCTOU race
> condition here between the sysfs store and module initialization?
> 
> The kernel exposes module parameter sysfs files via mod_sysfs_setup()
> before executing the module's initcall do_init_module().
> 
> If a userspace process writes to this sysfs file concurrently while
> damon_lru_sort_init() is executing, could it trigger the following sequence?
> 
> Userspace sysfs write happens right after damon_lru_sort_init() allocates
> ctx. This callback sees ctx != NULL and calls damon_lru_sort_turn(true).
> 
> Concurrently, damon_lru_sort_init() continues, reads enabled == true
> (updated by the sysfs store), and also calls damon_lru_sort_turn(true).
> 
> Does this concurrent execution of damon_lru_sort_apply_parameters() and
> damon_commit_ctx() on the same ctx without locks lead to list corruption
> or double frees?

Again, DAMON_LRU_SORT is not loadable.  So, no.


Thanks,
SJ

[...]