Re: [RFC PATCH v3 1/4] samples/damon/wsse: handle damon_start() failure

[SeongJae Park <sj@kernel.org>]

On Wed, 10 Jun 2026 01:29:12 +0000 sashiko-bot@kernel.org wrote:

> Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
> 
> Pre-existing issues:
> - [High] Unhandled failure of `damon_call()` leads to a permanent memory leak of the DAMON context.
> - [High] Concurrent execution of `module_init` and sysfs store causes Use-After-Free and Double Free on the global `ctx`.
> --
> 
> commit 6215f97f729272927aafd065cdad15da2af100a8
> Author: SeongJae Park <sj@kernel.org>
> 
> samples/damon/wsse: handle damon_start() failure
> 
> This commit fixes a memory leak in damon_sample_wsse_start() by explicitly
> freeing the DAMON context if damon_start() fails.
> 
> Link: https://lore.kernel.org/20260609145814.70163-1-sj@kernel.org
> 
> > diff --git a/samples/damon/wsse.c b/samples/damon/wsse.c
> > index 799ad44439434..bbd9392ab5b36 100644
> > --- a/samples/damon/wsse.c
> > +++ b/samples/damon/wsse.c
> > @@ -87,8 +87,10 @@ static int damon_sample_wsse_start(void)
> >  	target->pid = target_pidp;
> >  
> >  	err = damon_start(&ctx, 1, true);
> > -	if (err)
> > +	if (err) {
> > +		damon_destroy_ctx(ctx);
> >  		return err;
> > +	}
> 
> [Severity: High]
> This is a pre-existing issue, but does this new error path leave the global
> ctx pointer dangling?
> 
> If the module is loaded with enabled=1, damon_sample_wsse_init() calls
> damon_sample_wsse_start(). Concurrently, user space can write 0 to the
> enabled sysfs parameter, which triggers damon_sample_wsse_stop().
> 
> If damon_start() encounters an error, this new path calls
> damon_destroy_ctx(ctx) but ctx is not set to NULL. A concurrent
> damon_sample_wsse_stop() would see ctx != NULL and call
> damon_destroy_ctx(ctx) again, leading to a double free.
> 
> Similarly, if damon_sample_wsse_stop() executes while
> damon_sample_wsse_start() is running, stop() could call
> damon_destroy_ctx(ctx) while start() is actively using it, causing a
> use-after-free.

damon_sample_wsse cannot be dynamically loaded, so this issue cannot happen.

> 
> >  	repeat_call_control.data = ctx;
> >  	return damon_call(ctx, &repeat_call_control);
> >  }
> 
> [Severity: High]
> This is a pre-existing issue, but does this return path leak the newly
> created ctx if damon_call() fails?
> 
> If damon_call() returns an error (for example, -ECANCELED if the kdamond
> thread exits immediately and sets ctx->call_controls_obsolete to true), the
> error is returned directly to the caller.
> 
> Because the kdamond thread does not free the context itself when exiting,
> is damon_destroy_ctx(ctx) also needed here to prevent leaking the DAMON
> context and its associated objects?

Good finding.  I will fix this with another patch.


Thanks,
SJ

[...]