Re: [RFC PATCH 13/27] mm/damon/core: introduce damon_report_exist()

[SJ Park <sj@kernel.org>]

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

> Thank you for your contribution! Sashiko AI review found 1 potential
> issue to consider:
> - [High] mm/damon/core: out-of-bounds and uninitialized read in
>   damon_report_exist()
> --
> 
> commit be7b07b7677627ddaaa72484649217b5172610a1
> Author: SJ Park <sj@kernel.org>
> 
> mm/damon/core: introduce damon_report_exist()
> 
> This commit adds damon_report_exist() to search damon_report_queue for a
> report matching a given type, address range, and minimum age in jiffies.
> 
> > diff --git a/mm/damon/core.c b/mm/damon/core.c
> > index c02c7fbc1d864..c60a6504d0a15 100644
> > --- a/mm/damon/core.c
> > +++ b/mm/damon/core.c
> > @@ -2386,6 +2386,52 @@ int damon_submit_report(struct damon_report *report)
> >  	return 0;
> >  }
> >  
> > +/**
> > + * damon_report_exist() - Find if a report of a given condition exists.
> > + *
> > + * @report_type:	Type of the report.
> > + * @addr_range:		Address range of the report.
> > + * @jiffies_old:	Oldest jiffies to find the report for.
> > + *
> > + * Returns true if the report is found, false otherwise.
> > + */
> > +bool damon_report_exist(enum damon_report_type report_type,
> > +		struct damon_addr_range *addr_range, unsigned long jiffies_old)
> > +{
> > +	struct damon_report_queue *queue;
> > +	int lock_old_val;
> > +	unsigned int idx, nr_read;
> > +	struct damon_report *report;
> > +	bool found = false;
> > +
> > +	queue = &damon_report_queue;
> > +	lock_old_val = atomic_cmpxchg(&queue->lock, 0, 1);
> > +
> > +	if (lock_old_val)
> > +		return false;
> > +	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;
> 
> [Severity: High]
> Can indexing queue->reports[idx] before decrementing and wrapping idx read
> uninitialized or out-of-bounds memory?
> 
> In damon_submit_report(), queue->tail is post-incremented after writing
> queue->reports[tail]:
> 
> mm/damon/core.c:damon_submit_report() {
> 	...
> 	tail = queue->tail;
> 	if (tail == queue->size)
> 		tail = 0;
> 	queue->reports[tail] = *report;
> 	queue->tail++;
> 	...
> }
> 
> Because queue->tail points one slot past the newest report, the first
> iteration of damon_report_exist() reads the unwritten slot at
> queue->reports[queue->tail] (which is allocated with kmalloc_objs() in
> damon_init() without zeroing) and never visits queue->reports[0].  Once
> queue->size (1000) reports have been submitted, queue->tail reaches 1000
> and the first iteration reads queue->reports[1000], one element past the
> end of the array.  Also, when damon_submit_report() wraps local tail to 0,
> queue->tail itself is not wrapped and increments past queue->size on
> subsequent submissions.
> 
> Should idx be decremented (wrapping from 0 to queue->size - 1) before
> indexing queue->reports[idx], with damon_submit_report() updating
> queue->tail = tail + 1?

Good catch.  I will move queue->reports[idx] after the idx decrement.

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


Thanks,
SJ