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

[sashiko-bot@kernel.org]

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.

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

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260610011420.3018-1-sj@kernel.org?part=1