[PATCH 6.1.y] mm/damon/core: validate ranges in damon_set_regions()

[PATCH 6.1.y] mm/damon/core: validate ranges in damon_set_regions()

[SJ Park]

DAMON core logic assumes zero length regions don't exist.  However, a few
DAMON API callers including DAMON_SYSFS, DAMON_RECLAIM and DAMON_LRU_SORT
allow users to set empty monitoring target regions.  This could result in
WARN_ONCE() on CONFIG_DAMON_DEBUG_SANITY enabled kernel, and
divide-by-zero from damon_merge_two_regions().

For example, the WANR_ONCE() can be triggered like below.

    # grep DAMON_DEBUG_SANITY /boot/config-$(uname -r)
    # CONFIG_DAMON_DEBUG_SANITY=y
    # damo start
    # cd /sys/kernel/mm/damon/admin/kdamonds/0
    # echo 0 > contexts/0/targets/0/regions/0/start
    # echo 0 > contexts/0/targets/0/regions/0/end
    # echo commit > state
    # dmesg
    [....]
    [   73.705780] ------------[ cut here ]------------
    [   73.707552] start 0 >= end 0
    [   73.708452] WARNING: mm/damon/core.c:359 at damon_new_region+0x6e/0x80, CPU#1: kdamond.0/758
    [...]

All DAMON API callers eventually use damon_set_regions() to setup the
regions.  Add the validation logic in the function.

Link: https://lore.kernel.org/20260630035221.146458-1-sj@kernel.org
Fixes: 43b0536cb471 ("mm/damon: introduce DAMON-based Reclamation (DAMON_RECLAIM)")
Signed-off-by: SJ Park <sj@kernel.org>
Cc: Yang yingliang <yangyingliang@huawei.com>
Cc: <stable@vger.kernel.org> # 5.16.x
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
(cherry picked from commit 1292c0ecb1caefb8ca064a3639d5673991e8810c)
Signed-off-by: SJ Park <sj@kernel.org>
---
 mm/damon/core.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/mm/damon/core.c b/mm/damon/core.c
index 4b434ebd37c53..0a0bb033f28a4 100644
--- a/mm/damon/core.c
+++ b/mm/damon/core.c
@@ -212,6 +212,12 @@ int damon_set_regions(struct damon_target *t, struct damon_addr_range *ranges,
 	unsigned int i;
 	int err;
 
+	for (i = 0; i < nr_ranges; i++) {
+		if (ALIGN_DOWN(ranges[i].start, DAMON_MIN_REGION) >=
+				ALIGN(ranges[i].end, DAMON_MIN_REGION))
+			return -EINVAL;
+	}
+
 	/* Remove regions which are not in the new ranges */
 	damon_for_each_region_safe(r, next, t) {
 		for (i = 0; i < nr_ranges; i++) {
-- 
2.47.3

Re: [PATCH 6.1.y] mm/damon/core: validate ranges in damon_set_regions()

[sashiko-bot]

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

New issues:
- [High] The validation check fails to consistently reject unaligned zero-length regions, allowing them to bypass validation and silently expand.
- [High] The ALIGN() macro overflows to 0 when evaluating regions near the end of the 32-bit address space, causing valid regions to be erroneously rejected.

Pre-existing issues:
- [High] Pre-existing leak of `struct pid` references if target addition or context setup fails during sysfs input application.
--

--- Patch [1]: [PATCH 6.1.y] mm/damon/core: validate ranges in damon_set_regions() ---
commit a1c890d67da57c98800fdd95b91daeac6ab475ab
Author: SJ Park <sj@kernel.org>

mm/damon/core: validate ranges in damon_set_regions()

This commit adds a validation check in damon_set_regions() to ensure that the
provided region ranges are valid. It aims to prevent empty or negative length
regions from causing warnings and divide-by-zero errors in the core logic.

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

> diff --git a/mm/damon/core.c b/mm/damon/core.c
> index 4b434ebd37c53..0a0bb033f28a4 100644
> --- a/mm/damon/core.c
> +++ b/mm/damon/core.c
> @@ -212,6 +212,12 @@ int damon_set_regions(struct damon_target *t, struct damon_addr_range *ranges,
>  	unsigned int i;
>  	int err;
>  
> +	for (i = 0; i < nr_ranges; i++) {
> +		if (ALIGN_DOWN(ranges[i].start, DAMON_MIN_REGION) >=
> +				ALIGN(ranges[i].end, DAMON_MIN_REGION))

[Severity: High]
Does this logic successfully catch unaligned zero-length regions?

If a user configures a region with start and end both at an unaligned address
like 4097, ALIGN_DOWN(4097) evaluates to 4096, and ALIGN(4097) evaluates to
8192. The check 4096 >= 8192 is false, which allows the invalid empty region
to bypass validation and silently expand into a full page.

[Severity: High]
Will this ALIGN() check overflow on 32-bit systems when evaluating regions
near the end of the address space?

If the end address of the physical memory resource is ULONG_MAX (0xFFFFFFFF),
ALIGN(0xFFFFFFFF, 4096) overflows the 32-bit unsigned integer and evaluates
to 0. This makes the check ALIGN_DOWN(start) >= 0 unconditionally true, which
erroneously rejects valid memory regions and prevents DAMON initialization.

> +			return -EINVAL;
> +	}
> +
>  	/* Remove regions which are not in the new ranges */

[Severity: High]
This is a pre-existing issue, but does damon_destroy_ctx() leak struct pid
references if context setup fails?

Looking at damon_sysfs_build_ctx() in mm/damon/sysfs.c:

	err = damon_sysfs_apply_inputs(ctx, sys_ctx);
	if (err) {
		damon_destroy_ctx(ctx);
		return ERR_PTR(err);
	}

If damon_sysfs_apply_inputs() successfully adds targets (incrementing struct
pid refcounts) but a subsequent step fails, damon_destroy_ctx() is called.

Since vaddr ops do not define a .cleanup callback and the before_terminate
callback is never executed, the damon_target structures are freed without
calling put_pid(), permanently leaking the PID references.

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

Re: [PATCH 6.1.y] mm/damon/core: validate ranges in damon_set_regions()

[SJ Park]

On Thu, 30 Jul 2026 01:18:26 +0000 sashiko-bot@kernel.org wrote:

> Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
> 
> New issues:
> - [High] The validation check fails to consistently reject unaligned zero-length regions, allowing them to bypass validation and silently expand.
> - [High] The ALIGN() macro overflows to 0 when evaluating regions near the end of the 32-bit address space, causing valid regions to be erroneously rejected.
> 
> Pre-existing issues:
> - [High] Pre-existing leak of `struct pid` references if target addition or context setup fails during sysfs input application.
> --
> 
> --- Patch [1]: [PATCH 6.1.y] mm/damon/core: validate ranges in damon_set_regions() ---
> commit a1c890d67da57c98800fdd95b91daeac6ab475ab
> Author: SJ Park <sj@kernel.org>
> 
> mm/damon/core: validate ranges in damon_set_regions()
> 
> This commit adds a validation check in damon_set_regions() to ensure that the
> provided region ranges are valid. It aims to prevent empty or negative length
> regions from causing warnings and divide-by-zero errors in the core logic.
> 
> Link: https://lore.kernel.org/20260630035221.146458-1-sj@kernel.org
> 
> > diff --git a/mm/damon/core.c b/mm/damon/core.c
> > index 4b434ebd37c53..0a0bb033f28a4 100644
> > --- a/mm/damon/core.c
> > +++ b/mm/damon/core.c
> > @@ -212,6 +212,12 @@ int damon_set_regions(struct damon_target *t, struct damon_addr_range *ranges,
> >  	unsigned int i;
> >  	int err;
> >  
> > +	for (i = 0; i < nr_ranges; i++) {
> > +		if (ALIGN_DOWN(ranges[i].start, DAMON_MIN_REGION) >=
> > +				ALIGN(ranges[i].end, DAMON_MIN_REGION))
> 
> [Severity: High]
> Does this logic successfully catch unaligned zero-length regions?
> 
> If a user configures a region with start and end both at an unaligned address
> like 4097, ALIGN_DOWN(4097) evaluates to 4096, and ALIGN(4097) evaluates to
> 8192. The check 4096 >= 8192 is false, which allows the invalid empty region
> to bypass validation and silently expand into a full page.

No problem.  It is an intended behavior.

> 
> [Severity: High]
> Will this ALIGN() check overflow on 32-bit systems when evaluating regions
> near the end of the address space?
> 
> If the end address of the physical memory resource is ULONG_MAX (0xFFFFFFFF),
> ALIGN(0xFFFFFFFF, 4096) overflows the 32-bit unsigned integer and evaluates
> to 0. This makes the check ALIGN_DOWN(start) >= 0 unconditionally true, which
> erroneously rejects valid memory regions and prevents DAMON initialization.

For 32 bit systems having >4GiB memory, we provide addr_unit feature.  The user
could avoid the issue by using that.  So no problem.

> 
> > +			return -EINVAL;
> > +	}
> > +
> >  	/* Remove regions which are not in the new ranges */
> 
> [Severity: High]
> This is a pre-existing issue, but does damon_destroy_ctx() leak struct pid
> references if context setup fails?
> 
> Looking at damon_sysfs_build_ctx() in mm/damon/sysfs.c:
> 
> 	err = damon_sysfs_apply_inputs(ctx, sys_ctx);
> 	if (err) {
> 		damon_destroy_ctx(ctx);
> 		return ERR_PTR(err);
> 	}
> 
> If damon_sysfs_apply_inputs() successfully adds targets (incrementing struct
> pid refcounts) but a subsequent step fails, damon_destroy_ctx() is called.
> 
> Since vaddr ops do not define a .cleanup callback and the before_terminate
> callback is never executed, the damon_target structures are freed without
> calling put_pid(), permanently leaking the PID references.

Good finding.  Mainline tree avoided this issue by refactoring the code, with
commits including 7114bc5e01cf ("mm/damon/core: add cleanup_target() ops
callback").  Porting the all commits would be challenging.  I will work on this
with a dedicated fix.

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


Thanks,
SJ