Re: [RFC PATCH v1.2 01/14] mm/damon/core: safely handle no region case in damon_set_regions()

[SeongJae Park <sj@kernel.org>]

On Thu, 21 May 2026 19:16:42 -0700 SeongJae Park <sj@kernel.org> wrote:

> On Thu, 21 May 2026 15:19:28 +0000 sashiko-bot@kernel.org wrote:
> 
> > Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider:
> > 
> > Pre-existing issues:
> > - [Critical] The ALIGN() macro can overflow when processing malicious or overly large user-provided region 'end' values, creating invalid regions where start > end.
> > - [High] min_region_sz can be a non-power-of-2 value, which corrupts region boundary calculations when passed to ALIGN() and ALIGN_DOWN().
> > - [High] Userspace can maliciously trigger a kernel warning (WARN_ONCE) by providing a region where start == end via sysfs.
> > - [High] A memory allocation failure during region creation results in a partial state update without a rollback, potentially leaving the running DAMON context corrupted.
> 
> No one is caused by this patch, so no blocker for this patch.
> 
> The last finding seems worthy to separately work for.  I will work on it.
> 
> Please read below for more details.
> 
> > --
> > 
> > commit fa588352eaf7b151171cca8c2acb0bb710a71c75
> > Author: SeongJae Park <sj@kernel.org>
> > 
> > mm/damon/core: safely handle no region case in damon_set_regions()
> > 
> > This commit refactors damon_set_regions() to explicitly handle the empty
> > region list case without depending on the internal implementation of
> > list_first_entry().
> > 
> > > diff --git a/mm/damon/core.c b/mm/damon/core.c
> > > index 4e223857a0f99..d0bdf6e01dff8 100644
> > > --- a/mm/damon/core.c
> > > +++ b/mm/damon/core.c
> > > @@ -356,6 +356,19 @@ int damon_set_regions(struct damon_target *t, struct damon_addr_range *ranges,
[...]
> > > +			if (!r)
> > > +				return -ENOMEM;
> > 
> > [Severity: High]
> > This is a pre-existing issue, but does this allocation failure leave the
> > DAMON context in a corrupted state?
> > 
> > If damon_new_region() returns NULL due to memory pressure, the function
> > immediately returns -ENOMEM without rolling back the regions that were
> > already removed or added in the previous loop.
> > 
> > Could this leave the running kdamond thread with a partially modified list
> > of regions?
> 
> Yes, and the caller should do damon_destroy_ctx() of the ctx.  And DAMON_SYSFS
> does that, while vaddr doesn't.  I will work on this.

Hmm, but the vaddr's damon_set_regions() calls are only for the automated
mapping detection based monitoring target address rangs setup.  In the mode,
the monitoring target address ranges setup is only the best effort, and
continuously retried for every operation set update interval.  Hence, ignoring
the error is actually intentional and fine.  So this is not a real issue and
therefore no fix is needed.

Let me know if I'm missing something, though.


Thanks,
SJ

[...]