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

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Medium] mm/damon/sysfs: ctx memory leak in damon_sysfs_turn_damon_on()
- [Low] mm/damon/sysfs: unknown member targe_value in damon_sysfs_valid_start_ctx()
--

commit 1635d9f393d64d22688141e9540fde6f98f5d6bb
Author: SJ Park <sj@kernel.org>

mm/damon/core: error damos_commit_quota_goal() for zero target_value

This patch rejects zero target_value in DAMOS quota goals during parameter
commit in damos_commit_quota_goal() and damos_commit_quota_goals() to avoid
division-by-zero errors. For this stable backport, it also adds a validation
helper before damon_start() 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..7895b11555d06 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->targe_value)
> +				return false;
> +		}
> +	}
> +	return true;
> +}

[Severity: Low]
Will mm/damon/sysfs.c fail to build in damon_sysfs_valid_start_ctx() due to
referencing g->targe_value instead of g->target_value?

In include/linux/damon.h, struct damos_quota_goal defines this field as:

struct damos_quota_goal {
	enum damos_quota_goal_metric metric;
	unsigned long target_value;
	unsigned long current_value;

> +
>  static int damon_sysfs_turn_damon_on(struct damon_sysfs_kdamond *kdamond)
>  {
>  	struct damon_ctx *ctx;
> @@ -1517,6 +1531,8 @@ 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))
> +		return -EINVAL;

[Severity: Medium]
Does damon_sysfs_turn_damon_on() leak ctx and its associated allocations when
damon_sysfs_valid_start_ctx(ctx) returns false?

When userspace writes "on" to the kdamond state sysfs file with a scheme
quota goal whose target_value is zero, damon_sysfs_build_ctx() allocates ctx
along with its targets, regions, schemes, quota goals, filters, and any
target struct pid references. Returning -EINVAL directly here skips
damon_destroy_ctx(ctx) (and target pid cleanup via
damon_sysfs_destroy_targets(ctx)), whereas the damon_start() error path right
below destroys ctx before returning:

	err = damon_start(&ctx, 1, false);
	if (err) {
		damon_destroy_ctx(ctx);
		return err;
	}

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