Re: [RFC PATCH 6.12.y v2] mm/damon/core: error damos_commit_quota_goal() for zero target_value

[SJ Park <sj@kernel.org>]

On Mon, 26 Oct 2026 14:11:30 +0000 sashiko-bot@kernel.org wrote:

> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [Medium] mm/damon/sysfs: pid leak in damon_sysfs_turn_damon_on() on invalid start ctx
> --
> 
> commit dc23c827b1a01b5ed768b0a218bb6a67def40106
> Author: SJ Park <sj@kernel.org>
> 
> mm/damon/core: error damos_commit_quota_goal() for zero target_value
> 
> This commit validates that DAMOS quota goal target_value is non-zero in
> damos_commit_quota_goal() and damos_commit_quota_goals() to prevent a
> division-by-zero error. For the stable backport, it also adds
> damon_sysfs_valid_start_ctx() to reject zero target_value before starting
> DAMON in damon_sysfs_turn_damon_on().
> 
> Link: https://lore.kernel.org/20260903010722.94244-1-sj@kernel.org
> Link: https://lore.kernel.org/20260903010722.94244-2-sj@kernel.org
> Link: https://lore.kernel.org/20260803134034.15217-1-sj@kernel.org [1]
> 
> > diff --git a/mm/damon/sysfs.c b/mm/damon/sysfs.c
> > index 5aa23c6545cb8..2c8749b62c621 100644
> > --- a/mm/damon/sysfs.c
> > +++ b/mm/damon/sysfs.c
> > @@ -1497,6 +1497,20 @@ static struct damon_ctx *damon_sysfs_build_ctx(
> >  	return ctx;
> >  }
> >  
> > +static bool damon_sysfs_valid_start_ctx(struct damon_ctx *ctx)
> > +{
> > +	struct damos *s;
> > +	struct damos_quota_goal *g;
> > +
> > +	damon_for_each_scheme(s, ctx) {
> > +		damos_for_each_quota_goal(g, (&s->quota)) {
> > +			if (!g->target_value)
> > +				return false;
> > +		}
> > +	}
> > +	return true;
> > +}
> > +
> >  static int damon_sysfs_turn_damon_on(struct damon_sysfs_kdamond *kdamond)
> >  {
> >  	struct damon_ctx *ctx;
> > @@ -1517,6 +1531,10 @@ static int damon_sysfs_turn_damon_on(struct damon_sysfs_kdamond *kdamond)
> >  	ctx = damon_sysfs_build_ctx(kdamond->contexts->contexts_arr[0]);
> >  	if (IS_ERR(ctx))
> >  		return PTR_ERR(ctx);
> > +	if (!damon_sysfs_valid_start_ctx(ctx)) {
> > +		damon_destroy_ctx(ctx);
> > +		return -EINVAL;
> > +	}
> 
> [Severity: Medium]
> Does this error path leak the target struct pid references (t->pid) when
> damon_target_has_pid(ctx) is true?
> 
> When damon_sysfs_build_ctx() constructs ctx via damon_sysfs_apply_inputs() ->
> damon_sysfs_add_targets() -> damon_sysfs_add_target(), it acquires a struct
> pid reference for each target:
> 
> mm/damon/sysfs.c:damon_sysfs_add_target() {
> 	...
> 	if (damon_target_has_pid(ctx)) {
> 		t->pid = find_get_pid(sys_target->pid);
> 		if (!t->pid)
> 			goto destroy_targets_out;
> 	}
> 	...
> }
> 
> Because damon_sysfs_valid_start_ctx(ctx) fails before damon_start() is
> called, kdamond never runs and ctx->callback.before_terminate
> (damon_sysfs_before_terminate()) is never invoked to release those
> references. Calling damon_destroy_ctx(ctx) invokes damon_destroy_targets(ctx),
> which frees each target without calling put_pid(t->pid):
> 
> mm/damon/core.c:damon_destroy_targets() {
> 	struct damon_target *t, *next_t;
> 
> 	if (ctx->ops.cleanup) {
> 		ctx->ops.cleanup(ctx);
> 		return;
> 	}
> 
> 	damon_for_each_target_safe(t, next_t, ctx)
> 		damon_destroy_target(t);
> }
> 
> In comparison, damon_sysfs_commit_input() cleans up an unstarted context from
> damon_sysfs_build_ctx() by calling damon_sysfs_destroy_targets() before
> damon_destroy_ctx():
> 
> mm/damon/sysfs.c:damon_sysfs_commit_input() {
> 	...
> 	err = damon_commit_ctx(kdamond->damon_ctx, param_ctx);
> 	damon_sysfs_destroy_targets(param_ctx);
> 	damon_destroy_ctx(param_ctx);
> 	return err;
> }
> 
> Should damon_sysfs_turn_damon_on() also call damon_sysfs_destroy_targets(ctx)
> before damon_destroy_ctx(ctx) here?

Nice catch.  I will fix this in the next revision.

FYI, this patch for 6.18 and later kernels doesn't have this problem, since
they have commit 7114bc5e01cf ("mm/damon/core: add cleanup_target() ops
callback").  The commit made damon_destroy_ctx() to do the pid reference drop
by itself, and merged into 6.17.

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


Thanks,
SJ