Re: [PATCH 6.12.y] mm/damon/core: always put unsuccessfully committed target pids

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [Medium] PID references are leaked when damon_start() or late stages of context building fail in damon_sysfs_turn_damon_on().
--

--- Patch [1]: [PATCH 6.12.y] mm/damon/core: always put unsuccessfully committed target pids ---
commit d053b994cdbf8e3fe384c68bbdb8f593cb5a45e5
Author: SeongJae Park <sj@kernel.org>

mm/damon/core: always put unsuccessfully committed target pids

This commit fixes a PID leak by explicitly putting the PIDs of targets in
the commit operation when damon_commit_target() fails.

Link: https://lore.kernel.org/20260605013849.83750-1-sj@kernel.org
Link: https://lore.kernel.org/20260603112306.58490-1-akinobu.mita@gmail.com [1]
Link: https://lore.kernel.org/20260320020056.835-1-sj@kernel.org [2]

> The source context will do the put operations using the above explained
> routine.  However, let's suppose the destination context was not
> originally using [f]vaddr operation set and the commit failed before the
> ops of the source context is committed.  The destination does not have the
> cleanup_target() ops callback, so it cannot put the pids via the
> damon_destroy_ctx().
> 
> As a result, the pids are leaked.

[Severity: Medium]
This is a pre-existing issue, but does a similar PID leak happen in
mm/damon/sysfs.c when context building or starting fails?

For example, in damon_sysfs_build_ctx():

mm/damon/sysfs.c:damon_sysfs_build_ctx() {
	...
	err = damon_sysfs_apply_inputs(ctx, sys_ctx);
	if (err) {
		damon_destroy_ctx(ctx);
		return ERR_PTR(err);
	}
	...
}

If damon_sysfs_apply_inputs() fails during scheme addition (after targets
were successfully added and PIDs referenced), damon_destroy_ctx(ctx) is
called and the PIDs are leaked.

Similarly, in damon_sysfs_turn_damon_on():

mm/damon/sysfs.c:damon_sysfs_turn_damon_on() {
	...
	err = damon_start(&ctx, 1, false);
	if (err) {
		damon_destroy_ctx(ctx);
		return err;
	}
	...
}

If damon_start() fails (e.g., due to ENOMEM), the error path directly calls
damon_destroy_ctx(ctx). For vaddr operations, ctx->ops.cleanup is NULL,
and PID references are normally released by damon_sysfs_before_terminate().

However, since the thread is never started or the context is aborted early,
the termination callback is not called and the PIDs are never put.

Is this something that needs a similar fix to ensure these PIDs are
released?

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