Re: [PATCH 2/2] mm/damon/reclaim: fix potential memory leak

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

New issues:
- [Critical] The patch attempts to fix a memory leak by destroying the global `ctx` pointer on the error path, but fails to set it to `NULL`, leaving a dangling pointer that causes deterministic Use-After-Free vulnerabilities upon any sysfs interaction.

Pre-existing issues:
- [High] Pre-existing memory leak of `param_ctx` and its associated components when `damon_reclaim_apply_parameters()` succeeds.
--

commit 87008af2e1842854e4e34be35814335307b60672
Author: yahia ahmed <yahia.a.abdrabou@gmail.com>

mm/damon/reclaim: fix potential memory leak

Fix a potential memory leak if initialization encounters
an error, ctx won't be destroyed.

> diff --git a/mm/damon/reclaim.c b/mm/damon/reclaim.c
> index 15bc06ae75053..dfe52717d92cf 100644
> --- a/mm/damon/reclaim.c
> +++ b/mm/damon/reclaim.c
> @@ -481,6 +481,10 @@ static int __init damon_reclaim_init(void)
>  out:
>  	if (err && enabled)
>  		enabled = false;
> +
> +	/* Prevent potenial memory leakage */
> +	if (err && ctx)
> +		damon_destroy_ctx(ctx);

[Severity: Critical]
Does this leave a dangling pointer in the global ctx variable?

Because ctx is global, if it is freed here but not set to NULL, sysfs
callbacks like damon_reclaim_enabled() will bypass their safety checks:

mm/damon/reclaim.c:damon_reclaim_enabled() {
	if (!ctx)
		return false;
	return damon_is_running(ctx);
}

This would pass the freed context to damon_is_running(), which blindly
acquires the mutex:

mm/damon/core.c:damon_is_running() {
	...
	mutex_lock(&ctx->kdamond_lock);
	...
}

Can this trigger a use-after-free when sysfs parameters are accessed after
initialization fails?

>  	return err;
>  }

[Severity: High]
This is a pre-existing issue, but does damon_reclaim_apply_parameters() leak
param_ctx on the success path?

In mm/damon/reclaim.c:damon_reclaim_apply_parameters():

	err = damon_commit_ctx(ctx, param_ctx);

	if (!err)
		return 0;
out:
	damon_destroy_ctx(param_ctx);
	return err;

Since damon_commit_ctx() copies components and does not take ownership of
param_ctx, returning 0 directly without destroying param_ctx appears to leak
the locally allocated context and its attached components.

Could this lead to memory exhaustion if parameters are successfully applied
multiple times?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260619234606.221163-1-yahia.a.abdrabou@gmail.com?part=2