Re: [RFC PATCH 4/8] mm/damon/reclaim: remove duplicated min_region_sz power of 2 check

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Removing the early `is_power_of_2()` check allows invalid `min_region_sz` inputs to reach `damon_commit_ctx()`, which exposes a bug introduced earlier in this series that corrupts the live context and silently terminates the DAMON background thread.
--

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

mm/damon/reclaim: remove duplicated min_region_sz power of 2 check

DAMON_RECLAIM validates the user input for min_region_sz.  The same
validation is done inside damon_start() and damon_commit_ctx().  Remove
the duplicate.

> diff --git a/mm/damon/reclaim.c b/mm/damon/reclaim.c
> index 11b70d0a9a6f0..6469b25cc34f9 100644
> --- a/mm/damon/reclaim.c
> +++ b/mm/damon/reclaim.c
> @@ -208,11 +208,6 @@ static int damon_reclaim_apply_parameters(void)
>  	param_ctx->addr_unit = addr_unit;
>  	param_ctx->min_region_sz = max(DAMON_MIN_REGION_SZ / addr_unit, 1);
>  
> -	if (!is_power_of_2(param_ctx->min_region_sz)) {
> -		err = -EINVAL;
> -		goto out;
> -	}

[Severity: High]
Does removing this early check expose a bug introduced earlier in this series?

If the user provides an invalid addr_unit (e.g., 3), min_region_sz will not be
a power of 2. By removing the check here, this invalid parameter is passed
to damon_commit_ctx().

Due to an issue in damon_commit_ctx() (from "mm/damon/core: safely validate
src on damon_commit_ctx()"), the dry-run validates dst instead of src:

mm/damon/core.c:damon_commit_ctx() {
	...
	err = __damon_commit_ctx(test_ctx, dst);
	if (err)
		goto out;
	err = __damon_commit_ctx(dst, src);
	...
}

When __damon_commit_ctx() is called with the live dst and invalid src, it
sets dst->maybe_corrupted before returning -EINVAL:

mm/damon/core.c:__damon_commit_ctx() {
	...
	dst->maybe_corrupted = true;
	if (!is_power_of_2(src->min_region_sz))
		return -EINVAL;
	...
}

Could this cause the background kdamond thread to see maybe_corrupted as true
on its next iteration and permanently terminate, silently turning a simple
configuration error into a denial of service for the subsystem?

> -
>  	if (!damon_reclaim_mon_attrs.aggr_interval) {
>  		err = -EINVAL;
>  		goto out;

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