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

[SJ Park <sj@kernel.org>]

On Wed, 01 Jul 2026 15:56:23 +0000 sashiko-bot@kernel.org wrote:

> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [High] Removing the `is_power_of_2()` validation exposes the system to undefined behavior via `ALIGN` macros and silently kills the `kdamond` thread (Denial of Service).
> --
> 
> commit da14a5f653fa82832f40f8b6d0be4ad4c3036287
> Author: SJ Park <sj@kernel.org>
> 
> mm/damon/lru_sort: remove duplicated min_region_sz power_of_2() check
> 
> DAMON_LRU_SORT validates the user input for min_region_sz.  The same
> validation is done inside damon_start() and damon_commit_ctx().  Remove
> the unnecessary duplicate.
> 
> > 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 expose the rest of the DAMON subsystem to
> a non-power-of-two min_region_sz?
> 
> If a user writes a non-power-of-two module parameter (for example,
> addr_unit = 3), damon_lru_sort_apply_parameters() will calculate
> min_region_sz as 349525.
> 
> This invalid size is then passed to damon_set_region_system_rams_default(),
> and eventually to damon_set_regions(), where it is passed into the ALIGN()
> and ALIGN_DOWN() macros:
> 
> mm/damon/core.c:damon_set_regions() {
>     ...
>     r = damon_new_region(
>         ALIGN_DOWN(ranges[i].start, min_region_sz),
>         ALIGN(ranges[i].end, min_region_sz));
>     ...
> }
> 
> These macros strictly require power-of-two alignment values to form bitwise
> masks.
> 
> Can this result in meaningless garbage boundaries?
> 
> Furthermore, the invalid size propagates to damon_commit_ctx(). Due to a bug
> in a prior patch in the series ('mm/damon/core: safely validate src on
> damon_commit_ctx()'), the safe validation dry-run incorrectly validates dst
> instead of src,

Yes, I will fix the bug in the next revision.

> leading directly to a call to __damon_commit_ctx():
> 
> mm/damon/core.c:__damon_commit_ctx() {
>     ...
>     dst->maybe_corrupted = true;
>     if (!is_power_of_2(src->min_region_sz))
>         return -EINVAL;
>     ...
> }
> 
> This function immediately sets dst->maybe_corrupted = true, evaluates
> !is_power_of_2(src->min_region_sz), and returns -EINVAL early.
> 
> This leaves the running context's maybe_corrupted flag permanently set to
> true, which causes the kdamond background thread to abruptly terminate itself
> on its next iteration.
> 
> Could this lead to a denial of service by silently killing the kdamond thread?

So no issue should happen in the next revision.


Thanks,
SJ

[...]