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

[SJ Park <sj@kernel.org>]

On Sun, 25 Oct 2026 17:37:55 +0000 sashiko-bot@kernel.org wrote:

> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [High] mm/damon: incomplete zero target_value check in damos_commit_quota_goal()
> --
> 
> commit ea716ea29282ef733822096401c5c8d92eaaef58
> Author: SJ Park <sj@kernel.org>
> 
> mm/damon/core: error damos_commit_quota_goal() for zero target_value
> 
> This commit validates that a DAMOS quota goal's target_value is non-zero in
> damos_commit_quota_goal(), returning -EINVAL and propagating the error through
> damos_commit_quota_goals() to prevent a division-by-zero in
> damos_quota_score().
> 
> 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/core.c b/mm/damon/core.c
> > index 4e6e3d7d0a5c0..8fa788b0d709c 100644
> > --- a/mm/damon/core.c
> > +++ b/mm/damon/core.c
> > @@ -807,15 +807,18 @@ static void damos_commit_quota_goal_union(
> >  	}
> >  }
> >  
> > -static void damos_commit_quota_goal(
> > +static int damos_commit_quota_goal(
> >  		struct damos_quota_goal *dst, struct damos_quota_goal *src)
> >  {
> > +	if (!src->target_value)
> > +		return  -EINVAL;
> 
> [Severity: High]
> Does checking for a zero target_value only in damos_commit_quota_goal() cover
> the path where DAMON is started via sysfs in this stable tree?
> 
> In mm/damon/sysfs-schemes.c, target_value_store() updates goal->target_value
> without holding damon_sysfs_lock:
> 
> mm/damon/sysfs-schemes.c:target_value_store() {
> 	struct damos_sysfs_quota_goal *goal = container_of(kobj, struct
> 			damos_sysfs_quota_goal, kobj);
> 	int err = kstrtoul(buf, 0, &goal->target_value);
> 
> 	return err ? err : count;
> }
> 
> Meanwhile, damos_sysfs_add_quota_score() reads sysfs_goal->target_value twice
> without synchronization against target_value_store():
> 
> mm/damon/sysfs-schemes.c:damos_sysfs_add_quota_score() {
> 		if (!sysfs_goal->target_value)
> 			continue;
> 
> 		goal = damos_new_quota_goal(sysfs_goal->metric,
> 				sysfs_goal->target_value);
> 		...
> 		damos_add_quota_goal(quota, goal);
> }
> 
> If target_value_store() writes 0 between those two reads,
> damos_new_quota_goal() allocates a goal with target_value == 0 and attaches it
> to quota->goals.
> 
> When starting DAMON via state_store() -> damon_sysfs_cmd_request_callback() ->
> damon_sysfs_handle_cmd() -> damon_sysfs_turn_damon_on(), this branch builds
> the context and starts kdamond directly without calling damon_commit_ctx() or
> damos_commit_quota_goals():
> 
> mm/damon/sysfs.c:damon_sysfs_turn_damon_on() {
> 	ctx = damon_sysfs_build_ctx(kdamond->contexts->contexts_arr[0]);
> 	if (IS_ERR(ctx)) {
> 		kfree(repeat_call_control);
> 		return PTR_ERR(ctx);
> 	}
> 	err = damon_start(&ctx, 1, false);
> }
> 
> Here damon_sysfs_build_ctx() populates scheme->quota.goals via
> damon_sysfs_apply_inputs() -> damon_sysfs_add_schemes() ->
> damon_sysfs_mk_scheme() -> damos_sysfs_add_quota_score(), bypassing
> damos_commit_quota_goal() entirely.
> 
> When kdamond subsequently runs kdamond_fn() -> kdamond_apply_schemes() ->
> damos_adjust_quota() -> damos_set_effective_quota() -> damos_quota_score():
> 
> mm/damon/core.c:damos_quota_score() {
> 		highest_score = max(highest_score,
> 				goal->current_value * 10000 /
> 				goal->target_value);
> }
> 
> can this still divide by zero if the race occurs during
> damon_sysfs_turn_damon_on()?

Nice catch.  I also just found this issue while working on 6.8.  Same issue in
the patch for 7.2.y [1].  I will withdraw this and that, and work on the next
spin.

[1] https://lore.kernel.org/20261025172434.161597-1-sj@kernel.org

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


Thanks,
SJ