Re: [RFC PATCH 6/8] Docs/mm/damon/design: update for hugepage_size probe filter

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Newly added probe filters of type DAMON_FILTER_TYPE_HUGEPAGE_SIZE are left with uninitialized `range_min` and `range_max` values, and bypass bounds validation.
--

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

Docs/mm/damon/design: update for hugepage_size probe filter

Update DAMON design document for the newly added hugepage_size data
attribute probe filter type.

> diff --git a/mm/damon/core.c b/mm/damon/core.c
> index 0c98f7e267d07..a3febdc355c1e 100644
> --- a/mm/damon/core.c
> +++ b/mm/damon/core.c
> @@ -1807,23 +1807,33 @@ static void damon_commit_filter(struct damon_filter *dst,
>  	case DAMON_FILTER_TYPE_MEMCG:
>  		dst->memcg_id = src->memcg_id;
>  		break;
> +	case DAMON_FILTER_TYPE_HUGEPAGE_SIZE:
> +		if (src->range_max < src->range_min)
> +			return -EINVAL;
> +		dst->range_min = src->range_min;
> +		dst->range_max = src->range_max;
> +		break;
>  	default:
>  		break;
>  	}
> +	return 0;
>  }
>  
>  static int damon_commit_filters(struct damon_probe *dst,
>  		struct damon_probe *src)
>  {
>  	struct damon_filter *dst_filter, *next, *src_filter, *new_filter;
> -	int i = 0, j = 0;
> +	int i = 0, j = 0, err;
>  
>  	damon_for_each_filter_safe(dst_filter, next, dst) {
>  		src_filter = damon_nth_filter(i++, src);
> -		if (src_filter)
> -			damon_commit_filter(dst_filter, src_filter);
> -		else
> +		if (src_filter) {
> +			err = damon_commit_filter(dst_filter, src_filter);
> +			if (err)
> +				return err;
> +		} else {
>  			damon_destroy_filter(dst_filter);
> +		}
>  	}
>  
>  	damon_for_each_filter_safe(src_filter, next, src) {

[ ... ]

> 		new_filter = damon_new_filter(src_filter->type,
> 				src_filter->matching, src_filter->allow);
> 		if (!new_filter)
> 			return -ENOMEM;
> 		switch (src_filter->type) {
> 		case DAMON_FILTER_TYPE_MEMCG:
> 			new_filter->memcg_id = src_filter->memcg_id;
> 			break;
> 		default:
> 			break;
> 		}
		^^^^

[Severity: High]
Does this code miss copying range_min and range_max when a HUGEPAGE_SIZE filter
is dynamically appended?

Since damon_new_filter() allocates new_filter without zeroing it, range_min and
range_max are left uninitialized for newly appended HUGEPAGE_SIZE filters.

Furthermore, because damon_commit_filter() isn't called here, the bounds
validation (src->range_max < src->range_min) added above is completely
bypassed. This leads to erratic memory filtering behavior since DAMON might
skip or apply actions based on memory garbage.

Should case DAMON_FILTER_TYPE_HUGEPAGE_SIZE be added to this switch block, or
perhaps the code could be refactored to reuse damon_commit_filter() here?

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