Re: [RFC PATCH 16/27] mm/damon/sysfs: support REPORT_MADE type probe filter setup

[SJ Park <sj@kernel.org>]

On Sun, 15 Nov 2026 20:41:12 +0000 sashiko-bot@kernel.org wrote:

> Thank you for your contribution! Sashiko AI review found 2 potential
> issues to consider:
> - [High] mm/damon: out-of-bounds and uninitialized access in
>   damon_report_exist()
> - [Medium] mm/damon: uninitialized report_type in damon_commit_filters()
> --
> 
> commit 6058e18a855d5b73c171dc4b94eb6031f06fdc63
> Author: SJ Park <sj@kernel.org>
> 
> mm/damon/sysfs: support REPORT_MADE type probe filter setup
> 
> This commit adds "report_made" to the DAMON sysfs probe filter type list
> and copies the filter's report_type when setting up probe filters.
> 
> > diff --git a/mm/damon/sysfs.c b/mm/damon/sysfs.c
> > index ca971cf1dcf6d..1c171af4590a8 100644
> > --- a/mm/damon/sysfs.c
> > +++ b/mm/damon/sysfs.c
> > @@ -1016,6 +1016,10 @@ damon_sysfs_filter_type_names[] = {
> >  		.type = DAMON_FILTER_TYPE_ACTIVE,
> >  		.name = "active",
> >  	},
> > +	{
> > +		.type = DAMON_FILTER_TYPE_REPORT_MADE,
> > +		.name = "report_made",
> > +	},
> >  };
> >  
> >  static ssize_t type_show(struct kobject *kobj,
> 
> [Severity: High]
> Can enabling DAMON_FILTER_TYPE_REPORT_MADE filters cause uninitialized and
> out-of-bounds accesses on queue->reports[]?
> 
> In damon_submit_report(), each report is stored at queue->reports[tail] and
> then queue->tail is incremented, so queue->tail points to the next slot to
> write rather than the most recent entry:
> 
> mm/damon/core.c:damon_submit_report() {
>     ...
> 	tail = queue->tail;
> 	if (tail == queue->size)
> 		tail = 0;
> 	queue->reports[tail] = *report;
> 	queue->tail++;
>     ...
> }
> 
> When damon_pa_filter_match() calls damon_report_exist() for a report_made
> filter, the loop reads queue->reports[idx] starting at idx = queue->tail
> before decrementing idx:
> 
> mm/damon/core.c:damon_report_exist() {
>     ...
> 	idx = queue->tail;
> 	for (nr_read = 0; nr_read < queue->nr_reports; nr_read++) {
> 		report = &queue->reports[idx];
> 		if (idx)
> 			idx--;
> 		else
> 			idx = queue->size - 1;
>     ...
> }
> 
> While queue->nr_reports < queue->size, the first iteration reads the
> unwritten slot at queue->reports[queue->tail] and the loop stops before
> checking queue->reports[0].  Once queue->tail reaches queue->size,
> damon_report_exist() reads out of bounds at queue->reports[queue->size],
> and the next damon_submit_report() wraps only the local tail variable
> before incrementing queue->tail past queue->size, causing subsequent
> submissions and lookups to write and read out of bounds on the heap.

Yes, I will fix submit_report() and report_exist() in earlier patches.

> 
> > @@ -2336,6 +2340,8 @@ static int damon_sysfs_set_filters(struct damon_probe *probe,
> >  		} else if (filter->type == DAMON_FILTER_TYPE_HUGEPAGE_SIZE) {
> >  			filter->range_min = sys_filter->range_min;
> >  			filter->range_max = sys_filter->range_max;
> > +		} else if (filter->type == DAMON_FILTER_TYPE_REPORT_MADE) {
> > +			filter->report_type = sys_filter->report_type;
> >  		}
> >  		damon_add_filter(probe, filter);
> >  	}
> 
> [Severity: Medium]
> Does committing a new DAMON_FILTER_TYPE_REPORT_MADE filter to a running
> context leave new_filter->report_type uninitialized?
> 
> When damon_sysfs_commit_input() commits the context built here via
> damon_commit_ctx() -> __damon_commit_ctx() -> damon_commit_probes() ->
> damon_commit_filters(), newly added filters are allocated with
> damon_new_filter(), which does not initialize the union in struct
> damon_filter, and the switch in damon_commit_filters() does not handle
> DAMON_FILTER_TYPE_REPORT_MADE:
> 
> mm/damon/core.c:damon_commit_filters() {
>     ...
> 		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;
> 		case DAMON_FILTER_TYPE_HUGEPAGE_SIZE:
> 			...
> 		default:
> 			break;
> 		}
>     ...
> }
> 
> As a result, damon_pa_filter_match() passes uninitialized slab memory in
> new_filter->report_type to damon_report_exist().

Yes, I will fix this in the earlier patch.

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


Thanks,
SJ