Re: [RFC PATCH 4/8] mm/damon/core: disallow max < min damos filter range arguments commit

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] The newly introduced validation in `damos_commit_filter_arg()` returns `-EINVAL` on failure, but callers `damos_commit_core_filters` and `damos_commit_ops_filters` ignore this return value for new filters, adding uninitialized memory to active DAMON context.
- [High] `damos_commit_filter()` mutates the target filter's state (type, matching, allow) before validating its arguments, leaving the live filter in a corrupted state if validation subsequently fails.
--

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

mm/damon/core: disallow max < min damos filter range arguments commit

This commit adds validation to damos_commit_filter_arg() to reject invalid
ranges where max is less than min. It returns -EINVAL when an invalid range
is provided, avoiding duplicated validation across different DAMON API callers.

> diff --git a/mm/damon/core.c b/mm/damon/core.c
> index 3ad1fb2f21f3f..3dec1a8f7ed3a 100644
> --- a/mm/damon/core.c
> +++ b/mm/damon/core.c
> @@ -1325,15 +1325,21 @@ static int damos_commit_filter_arg(
>  		dst->memcg_id = src->memcg_id;
>  		break;
>  	case DAMOS_FILTER_TYPE_ADDR:
> +		if (src->addr_range.end < src->addr_range.start)
> +			return -EINVAL;

[Severity: High]
Does this code leave the target filter in a corrupted state on error?

Looking at damos_commit_filter(), the state is mutated before the newly
introduced validation has a chance to execute:

mm/damon/core.c:damos_commit_filter() {
	dst->type = src->type;
	dst->matching = src->matching;
	dst->allow = src->allow;
	return damos_commit_filter_arg(dst, src);
}

If damos_commit_filter_arg() returns -EINVAL here, dst->type, dst->matching,
and dst->allow have already been modified. If an existing ops filter receives
an invalid range update, its type will be updated but its union arguments will
belong to the previous type. When the filter executes, it might misinterpret
the old union memory according to the new type, causing unintended matching
behavior.


[Severity: High]
Can this validation error cause uninitialized memory to be added to the active
DAMON context?

Looking at damos_commit_core_filters() and damos_commit_ops_filters(), the
return value of damos_commit_filter_arg() is ignored when creating new filters:

mm/damon/core.c:damos_commit_core_filters() {
...
		new_filter = damos_new_filter(
				src_filter->type, src_filter->matching,
				src_filter->allow);
		if (!new_filter)
			return -ENOMEM;
		damos_commit_filter_arg(new_filter, src_filter);
		damos_add_filter(dst, new_filter);
	}

Because damos_new_filter() allocates memory without zero-initializing the union
arguments, returning -EINVAL from damos_commit_filter_arg() means the union
payload (like addr_range or sz_range) is left uninitialized. The callers ignore
this error and add the filter to the system anyway, which could result in
using kmalloc garbage and triggering undefined matching behavior.

Additionally, damos_commit_ops_filters() ignores the return value of
damos_commit_filter() for existing ops filters:

mm/damon/core.c:damos_commit_ops_filters() {
...
	damos_for_each_ops_filter_safe(dst_filter, next, dst) {
		src_filter = damos_nth_ops_filter(i++, src);
		if (src_filter)
			damos_commit_filter(dst_filter, src_filter);
...

If an existing ops filter receives an invalid update, the validation error
is swallowed entirely.

>  		dst->addr_range = src->addr_range;
>  		break;

[ ... ]

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