[PATCH] mm/damon/core: handle region split failure in apply_min_nr_regions()

[PATCH] mm/damon/core: handle region split failure in apply_min_nr_regions()

[SJ Park]

damon_apply_min_nr_regions() repeatedly split each region until its size
becomes small enough to meet the user-defined low limit of the number of
regions.  The loop assumes the split operation (damon_split_region_at())
will always succeed and create the new region.  But the operation could
silently fail for memory allocation failures, for example.  If such
failure happens and the region was the last region, the linked
list-based next region fetching returns invalid pointer.  As a result,
invalid memory dereference and corruption could happen.  Fix by breaking
the loop in the corner case.

The allocation failure is unlikely since it is arguably too small to
fail.  But, it could still theoretically happen, and the consequence is
very bad.

This issue was discovered [1] by Sashiko.

[1] https://lore.kernel.org/20260717011834.120715-1-sj@kernel.org

Fixes: b1029f29eb1d ("mm/damon/core: split regions for min_nr_regions")
Cc: <stable@vger.kernel.org> # 7.1.x
Signed-off-by: SJ Park <sj@kernel.org>
---
Changes from RFC v1
- RFC v1: https://lore.kernel.org/20260718004301.88883-1-sj@kernel.org
- Drop RFC tag.
- Rebase to latest mm-new.

 mm/damon/core.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/mm/damon/core.c b/mm/damon/core.c
index 20d267c615faf..3d829e0ad63b5 100644
--- a/mm/damon/core.c
+++ b/mm/damon/core.c
@@ -1913,6 +1913,9 @@ static unsigned long damon_apply_min_nr_regions(struct damon_ctx *ctx)
 		damon_for_each_region_safe(r, next, t) {
 			while (damon_sz_region(r) > max_region_sz) {
 				damon_split_region_at(t, r, max_region_sz);
+				/* split might failed */
+				if (r == damon_last_region(t))
+					break;
 				r = damon_next_region(r);
 			}
 		}

base-commit: 5817557f6220462f0ca298b73e4b3f4bb91ccd1f
-- 
2.47.3

Re: [PATCH] mm/damon/core: handle region split failure in apply_min_nr_regions()

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

Pre-existing issues:
- [High] Incomplete bug fix in damon_apply_min_nr_regions() leads to O(N^2) allocator thrashing on allocation failure.
- [High] Silent failure of damon_split_region_at() bypasses DAMON quotas and bounds checks.
--

--- Patch [1]: [PATCH] mm/damon/core: handle region split failure in apply_min_nr_regions() ---
commit 51e7e9012c52dde83bac4be44f0bbfbdbe01572d
Author: SJ Park <sj@kernel.org>
Subject: mm/damon/core: handle region split failure in apply_min_nr_regions()

damon_apply_min_nr_regions() repeatedly split each region until its size
becomes small enough to meet the user-defined low limit of the number of
regions. The loop assumes the split operation (damon_split_region_at())
will always succeed and create the new region.

Link: https://lore.kernel.org/20260717011834.120715-1-sj@kernel.org

> diff --git a/mm/damon/core.c b/mm/damon/core.c
> index 20d267c615faf..3d829e0ad63b5 100644
> --- a/mm/damon/core.c
> +++ b/mm/damon/core.c
> @@ -1913,6 +1913,9 @@ static unsigned long damon_apply_min_nr_regions(struct damon_ctx *ctx)
>  		damon_for_each_region_safe(r, next, t) {
>  			while (damon_sz_region(r) > max_region_sz) {
>  				damon_split_region_at(t, r, max_region_sz);
> +				/* split might failed */
> +				if (r == damon_last_region(t))
> +					break;

[Severity: High]
This isn't a bug introduced by this patch, but does this fix leave an
O(N^2) allocator thrashing loop when an allocation fails on a non-last
region?

If a region fails to split due to memory pressure and is not the last
region, the inner loop unconditionally advances r = damon_next_region(r).
This moves to the next pre-existing region instead of a newly created
one, and the inner loop will attempt to split this next region, continuing
to the end of the list.

Then, the outer damon_for_each_region_safe() loop advances to its
pre-saved next pointer and repeats the entire process.

Could this severely thrash the page allocator with redundant
kmem_cache_alloc() calls and cause RCU stalls or lockups during an
OOM condition?

[Severity: High]
This is also a pre-existing issue and was not introduced by this patch, but
since we are handling split failures here, could the silent failure of
damon_split_region_at() bypass DAMON quotas in other callers?

Because damon_split_region_at() returns void, callers like
damos_apply_scheme() cannot detect if the split failed:

mm/damon/core.c:damos_apply_scheme() {
...
			damon_split_region_at(t, r, sz);
		}
		if (damos_core_filter_out(c, t, r, s))
			return;
...
		sz_applied = c->ops.apply_scheme(c, t, r, s,
				&sz_ops_filter_passed);
...
		damos_charge_quota(quota, sz, sz_applied);
}

If the split fails, the region remains oversized and the scheme is
erroneously applied to the entire original region.

The quota charging logic then only charges the intended small size (sz),
which permanently hides the limit bypass.

Could this cause DAMON schemes to violate their strict resource quota
limits and apply actions to much larger memory areas than permitted?

>  				r = damon_next_region(r);
>  			}
>  		}

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

Re: [PATCH] mm/damon/core: handle region split failure in apply_min_nr_regions()

[SJ Park]

On Sat, 18 Jul 2026 17:00:57 +0000 sashiko-bot@kernel.org wrote:

> Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
> 
> Pre-existing issues:
> - [High] Incomplete bug fix in damon_apply_min_nr_regions() leads to O(N^2) allocator thrashing on allocation failure.
> - [High] Silent failure of damon_split_region_at() bypasses DAMON quotas and bounds checks.
> --
> 
> --- Patch [1]: [PATCH] mm/damon/core: handle region split failure in apply_min_nr_regions() ---
> commit 51e7e9012c52dde83bac4be44f0bbfbdbe01572d
> Author: SJ Park <sj@kernel.org>
> Subject: mm/damon/core: handle region split failure in apply_min_nr_regions()
> 
> damon_apply_min_nr_regions() repeatedly split each region until its size
> becomes small enough to meet the user-defined low limit of the number of
> regions. The loop assumes the split operation (damon_split_region_at())
> will always succeed and create the new region.
> 
> Link: https://lore.kernel.org/20260717011834.120715-1-sj@kernel.org
> 
> > diff --git a/mm/damon/core.c b/mm/damon/core.c
> > index 20d267c615faf..3d829e0ad63b5 100644
> > --- a/mm/damon/core.c
> > +++ b/mm/damon/core.c
> > @@ -1913,6 +1913,9 @@ static unsigned long damon_apply_min_nr_regions(struct damon_ctx *ctx)
> >  		damon_for_each_region_safe(r, next, t) {
> >  			while (damon_sz_region(r) > max_region_sz) {
> >  				damon_split_region_at(t, r, max_region_sz);
> > +				/* split might failed */
> > +				if (r == damon_last_region(t))
> > +					break;
> 
> [Severity: High]
> This isn't a bug introduced by this patch, but does this fix leave an
> O(N^2) allocator thrashing loop when an allocation fails on a non-last
> region?
> 
> If a region fails to split due to memory pressure and is not the last
> region, the inner loop unconditionally advances r = damon_next_region(r).
> This moves to the next pre-existing region instead of a newly created
> one, and the inner loop will attempt to split this next region, continuing
> to the end of the list.
> 
> Then, the outer damon_for_each_region_safe() loop advances to its
> pre-saved next pointer and repeats the entire process.
> 
> Could this severely thrash the page allocator with redundant
> kmem_cache_alloc() calls and cause RCU stalls or lockups during an
> OOM condition?

The allocation is arguably too small to fail.  And the while loop is supposed
to run only occasionally.  Most regions in most case would not exceed
max_region_sz.  But good call.  I will separately work on making this more
safe.

> 
> [Severity: High]
> This is also a pre-existing issue and was not introduced by this patch, but
> since we are handling split failures here, could the silent failure of
> damon_split_region_at() bypass DAMON quotas in other callers?
> 
> Because damon_split_region_at() returns void, callers like
> damos_apply_scheme() cannot detect if the split failed:
> 
> mm/damon/core.c:damos_apply_scheme() {
> ...
> 			damon_split_region_at(t, r, sz);
> 		}
> 		if (damos_core_filter_out(c, t, r, s))
> 			return;
> ...
> 		sz_applied = c->ops.apply_scheme(c, t, r, s,
> 				&sz_ops_filter_passed);
> ...
> 		damos_charge_quota(quota, sz, sz_applied);
> }
> 
> If the split fails, the region remains oversized and the scheme is
> erroneously applied to the entire original region.
> 
> The quota charging logic then only charges the intended small size (sz),
> which permanently hides the limit bypass.
> 
> Could this cause DAMON schemes to violate their strict resource quota
> limits and apply actions to much larger memory areas than permitted?

Good finding.  Seems not urgent and not related to this patch.  I will
separately work on this.

> 
> >  				r = damon_next_region(r);
> >  			}
> >  		}
> 
> -- 
> Sashiko AI review · https://sashiko.dev/#/patchset/20260718163708.84113-1-sj@kernel.org?part=1


Thanks,
SJ

Re: [PATCH] mm/damon/core: handle region split failure in apply_min_nr_regions()

[SJ Park]

On Sat, 18 Jul 2026 10:15:29 -0700 SJ Park <sj@kernel.org> wrote:

> On Sat, 18 Jul 2026 17:00:57 +0000 sashiko-bot@kernel.org wrote:
> 
> > Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
> > 
> > Pre-existing issues:
> > - [High] Incomplete bug fix in damon_apply_min_nr_regions() leads to O(N^2) allocator thrashing on allocation failure.
> > - [High] Silent failure of damon_split_region_at() bypasses DAMON quotas and bounds checks.
> > --
> > 
> > --- Patch [1]: [PATCH] mm/damon/core: handle region split failure in apply_min_nr_regions() ---
> > commit 51e7e9012c52dde83bac4be44f0bbfbdbe01572d
> > Author: SJ Park <sj@kernel.org>
> > Subject: mm/damon/core: handle region split failure in apply_min_nr_regions()
> > 
> > damon_apply_min_nr_regions() repeatedly split each region until its size
> > becomes small enough to meet the user-defined low limit of the number of
> > regions. The loop assumes the split operation (damon_split_region_at())
> > will always succeed and create the new region.
> > 
> > Link: https://lore.kernel.org/20260717011834.120715-1-sj@kernel.org
> > 
> > > diff --git a/mm/damon/core.c b/mm/damon/core.c
> > > index 20d267c615faf..3d829e0ad63b5 100644
> > > --- a/mm/damon/core.c
> > > +++ b/mm/damon/core.c
> > > @@ -1913,6 +1913,9 @@ static unsigned long damon_apply_min_nr_regions(struct damon_ctx *ctx)
> > >  		damon_for_each_region_safe(r, next, t) {
> > >  			while (damon_sz_region(r) > max_region_sz) {
> > >  				damon_split_region_at(t, r, max_region_sz);
> > > +				/* split might failed */
> > > +				if (r == damon_last_region(t))
> > > +					break;
> > 
> > [Severity: High]
> > This isn't a bug introduced by this patch, but does this fix leave an
> > O(N^2) allocator thrashing loop when an allocation fails on a non-last
> > region?
> > 
> > If a region fails to split due to memory pressure and is not the last
> > region, the inner loop unconditionally advances r = damon_next_region(r).
> > This moves to the next pre-existing region instead of a newly created
> > one, and the inner loop will attempt to split this next region, continuing
> > to the end of the list.
> > 
> > Then, the outer damon_for_each_region_safe() loop advances to its
> > pre-saved next pointer and repeats the entire process.
> > 
> > Could this severely thrash the page allocator with redundant
> > kmem_cache_alloc() calls and cause RCU stalls or lockups during an
> > OOM condition?
> 
> The allocation is arguably too small to fail.  And the while loop is supposed
> to run only occasionally.  Most regions in most case would not exceed
> max_region_sz.  But good call.  I will separately work on making this more
> safe.

Hmm, but no reason to rush.  I will fix this together in the next revision.


Thanks,
SJ

[...]

Re: [PATCH] mm/damon/core: handle region split failure in apply_min_nr_regions()

[SJ Park]

On Sat, 18 Jul 2026 09:37:06 -0700 SJ Park <sj@kernel.org> wrote:

> damon_apply_min_nr_regions() repeatedly split each region until its size
> becomes small enough to meet the user-defined low limit of the number of
> regions.  The loop assumes the split operation (damon_split_region_at())
> will always succeed and create the new region.  But the operation could
> silently fail for memory allocation failures, for example.  If such
> failure happens and the region was the last region, the linked
> list-based next region fetching returns invalid pointer.  As a result,
> invalid memory dereference and corruption could happen.  Fix by breaking
> the loop in the corner case.

Sashiko found a room to improve in this patch.  I will post a new revision with
the improvement.  Please don't add this to mm-new.


Thanks,
SJ

[...]