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

[SJ Park <sj@kernel.org>]

On Sun, 13 Sep 2026 17:24:53 +0000 sashiko-bot@kernel.org wrote:

> 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?

Yes, I will fix this in the next spin.

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


Thanks,
SJ