Re: [RFC PATCH v1.1 5/8] mm/damon/lru_sort: remove duplicated min_region_sz power_of_2() check

[SJ Park <sj@kernel.org>]

On Thu, 02 Jul 2026 21:20:05 +0000 sashiko-bot@kernel.org wrote:

> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [High] Removing the early validation of `min_region_sz` causes persistent corruption of the `monitor_region_start` and `monitor_region_end` module parameters on failure.
> --
> 
> commit 982a6b66f4f61086f3bd8de5a2da757081d4fa94
> Author: SJ Park <sj@kernel.org>
> 
> mm/damon/lru_sort: remove duplicated min_region_sz power_of_2() check
> 
> This commit removes an explicit power-of-two check on min_region_sz from
> damon_lru_sort_apply_parameters(), as the same validation is performed
> later in damon_commit_ctx().
> 
> > diff --git a/mm/damon/lru_sort.c b/mm/damon/lru_sort.c
> > index 32f41491b726b..2dd0cd0d26273 100644
> > --- a/mm/damon/lru_sort.c
> > +++ b/mm/damon/lru_sort.c
> > @@ -284,11 +284,6 @@ static int damon_lru_sort_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 validation allow execution to proceed with an invalid
> addr_unit?
> 
> If the monitor_region_start and monitor_region_end parameters are 0
> (the default, triggering auto-detection), they are overwritten in-place
> with system bounds scaled by the invalid addr_unit when this function
> is called later in damon_lru_sort_apply_parameters():
> 
> 	err = damon_set_region_system_rams_default(param_target,
> 					&monitor_region_start,
> 					&monitor_region_end,
> 					param_ctx->addr_unit,
> 					param_ctx->min_region_sz);
> 
> When damon_commit_ctx() later rejects the configuration and returns -EINVAL,
> these corrupted values are not reverted.
> 
> If a user then corrects addr_unit and applies again, will the now non-zero
> parameters bypass auto-detection and get scaled again? This could cause
> DAMON to monitor an incorrect and massive physical address range, forcing
> the kdamond thread into a long pfn_valid() loop that consumes excessive CPU.
> 
> Is the same state corruption possible in mm/damon/reclaim.c:
> damon_reclaim_apply_parameters() due to a similar removal of the early
> power-of-two check?

I think the user should recheck the parameters after failures.  No blocker.


Thanks,
SJ

[...]