Re: (sashiko review) [PATCH v3 1/2] mm/damon/lru_sort: validate min_region_size to be power of 2

[SeongJae Park <sj@kernel.org>]

Hello Liew,


I recently learned not everyone is interested [1] in sashiko review.  So when
I sharing sashiko review, I now try to drop recipients who are not necessarily
interested in the review.  My choices at the moment is the author of the patch,
maintainers of the subsystem, and the mailing lists of the subsystem.

So if I was sharing this review, I would dropped stable@ and yanquanmin1@ from
the Cc list.

Please consider doing similar when you share sashiko review.

Everyone including myself is learning how to use sashiko well, so it could be
confusing.  Sorry about that.  I'm planning to make a guideline for sashiko
review usage in DAMON subsystem, and update hkml to embed it, hoping that to
help reducing some of the confusion.

Anyway, seems sashiko found no blockers for this patch.

On Fri,  3 Apr 2026 16:31:25 +0800 Liew Rui Yan <aethernet65535@gmail.com> wrote:

> Forwarding Sashiko review for doing discussions via mails.
> 
> # review url: https://sashiko.dev/#/patchset/20260403052837.58063-2-aethernet65535@gmail.com
> # start of sashiko.dev inline review
> commit 525a0116b6d91612e679b30ae84e489a7540edbf
> Author: Liew Rui Yan <aethernet65535@gmail.com>
> 
> mm/damon/lru_sort: validate min_region_size to be power of 2
> 
> This commit adds an early check in damon_lru_sort_apply_parameters() to
> ensure min_region_sz is a power of 2. This prevents kdamond from unexpectedly
> terminating when given an invalid addr_unit.
> 
> > diff --git a/mm/damon/lru_sort.c b/mm/damon/lru_sort.c
> > index 554559d729760..3fd176ef9d9c1 100644
> > --- a/mm/damon/lru_sort.c
> > +++ b/mm/damon/lru_sort.c
> > @@ -294,6 +294,11 @@ 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;
> > +	}
> > +
> 
> Instead of adding this early check here, could this be resolved by fixing
> the stateless validation in damon_commit_ctx() directly?
> 
> Looking at damon_commit_ctx(), it currently sets dst->maybe_corrupted = true
> before checking if src->min_region_sz is a power of 2:
> 
> int damon_commit_ctx(struct damon_ctx *dst, struct damon_ctx *src)
> {
> 	int err;
> 
> 	dst->maybe_corrupted = true;
> 	if (!is_power_of_2(src->min_region_sz))
> 		return -EINVAL;
> ...
> }
> 
> When this validation fails, it leaves the active context marked as corrupted
> without making any state changes, which causes the kdamond thread to terminate
> permanently. Moving the stateless is_power_of_2() check above
> dst->maybe_corrupted = true would resolve the regression for all current and
> future callers without requiring duplicated validation logic at every
> call site.

We discussed this before, and decided not to add the check for now.

Keeping this behavior also makes the expectation for callers simpler:
damon_commit_ctx() failure always means kdamond will be terminated.

> 
> Does this missing validation also affect mm/damon/sysfs.c?
> 
> When a user starts DAMON via state_store() -> damon_sysfs_turn_damon_on(),
> the code calls damon_sysfs_build_ctx(). This calculates min_region_sz and
> then directly calls damon_start(), bypassing damon_commit_ctx() entirely.
> 
> Because this bypasses the is_power_of_2() checks, kdamond can actively run
> with a non-power-of-2 min_region_sz. Core DAMON operations like
> damon_set_regions() use ALIGN() and ALIGN_DOWN() with min_region_sz, which
> perform bitwise masking that requires the alignment parameter to be a
> power of 2.
> 
> Could this allow userspace to trigger incorrect address boundaries, massive
> unsigned underflows, or infinite loops during region splitting?

Good catch, but none of this patch's business.  I will work on this.

> 
> >  	if (!damon_lru_sort_mon_attrs.sample_interval) {
> >  		err = -EINVAL;
> >  		goto out;
> 
> 
> # end of sashiko.dev inline review
> # review url: https://sashiko.dev/#/patchset/20260403052837.58063-2-aethernet65535@gmail.com

[1] https://lore.kernel.org/20260331043441.67196-1-sj@kernel.org


Thanks,
SJ

[...]