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

[Liew Rui Yan <aethernet65535@gmail.com>]

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.

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?

>  	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
#
# hkml [1] generated a draft of this mail.  It can be regenerated
# using below command:
#
#     hkml patch sashiko_dev --for_forwarding \
#             20260403052837.58063-2-aethernet65535@gmail.com
#
# [1] https://github.com/sjp38/hackermail