Re: [RFC PATCH v1.1 1/8] mm/damon/core: safely validate src on damon_commit_ctx()

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] damon_commit_ctx() silently corrupts the quota charging state of running schemes.
--

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

mm/damon/core: safely validate src on damon_commit_ctx()

damon_commit_ctx() does its holistic parameter set validation while
applying the new parameter in the set one by one.  If it finds a
parameter is invalid, because some invalid parameters may already be
committed (it is called "commit" but not atomic and irreversable), it
stops the running DAMON context.

> diff --git a/mm/damon/core.c b/mm/damon/core.c
> index c6fc35be633b3..b523a18a7b971 100644
> --- a/mm/damon/core.c
> +++ b/mm/damon/core.c

[ ... ]

> @@ -1702,6 +1689,52 @@ int damon_commit_ctx(struct damon_ctx *dst, struct damon_ctx *src)
>  	return 0;
>  }
>  
> +static struct damon_ctx *damon_new_test_ctx(struct damon_ctx *dst)
> +{
> +	struct damon_ctx *test_ctx;
> +	int err;
> +
> +	test_ctx = damon_new_ctx();
> +	if (!test_ctx)
> +		return NULL;
> +	err = __damon_commit_ctx(test_ctx, dst);

[Severity: High]
This is a pre-existing issue, but does this unintentionally corrupt the running
quota state?

When __damon_commit_ctx() is called with the running context (dst) as the
source, it copies the schemes via damon_commit_schemes(), which calls
damon_new_scheme() and passes a pointer to the active scheme's quota:

mm/damon/core.c:damon_commit_schemes() {
	...
	new_scheme = damon_new_scheme(&src_scheme->pattern,
				src_scheme->action,
				src_scheme->apply_interval_us,
				&src_scheme->quota, &src_scheme->wmarks,
				NUMA_NO_NODE);
	...
}

damon_new_scheme() then calls damos_quota_init() on that provided quota
pointer:

mm/damon/core.c:damon_new_scheme() {
	...
	scheme->quota = *(damos_quota_init(quota));
	...
}

damos_quota_init() zeroes out the fields in-place:

mm/damon/core.c:damos_quota_init() {
	...
	quota->esz = 0;
	quota->total_charged_sz = 0;
	...
}

This mutates the running scheme's quota state, which would make the quota
effectively "full" mid-interval and prevent the scheme from applying its
actions until the next reset interval.

Since this patch moves this test context logic into the core
damon_commit_ctx(), doesn't this expose all API users like damon_reclaim
and damon_lru_sort to this state corruption when parameters are committed?

> +	if (err) {
> +		damon_destroy_ctx(test_ctx);
> +		return NULL;
> +	}
> +	return test_ctx;
> +}

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