All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 6.12.y] mm/damon/core: validate ranges in damon_set_regions()
       [not found] <2026072900-emit-mortified-8331@gregkh>
@ 2026-07-30  0:56 ` SJ Park
  2026-07-30  1:15   ` sashiko-bot
  0 siblings, 1 reply; 3+ messages in thread
From: SJ Park @ 2026-07-30  0:56 UTC (permalink / raw)
  To: stable; +Cc: damon, SJ Park, Yang yingliang, Andrew Morton

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 7911599c37f22..7217bb2ef51ec 100644
--- a/mm/damon/core.c
+++ b/mm/damon/core.c
@@ -214,6 +214,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


^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH 6.12.y] mm/damon/core: validate ranges in damon_set_regions()
  2026-07-30  0:56 ` [PATCH 6.12.y] mm/damon/core: validate ranges in damon_set_regions() SJ Park
@ 2026-07-30  1:15   ` sashiko-bot
  2026-07-30  3:57     ` SJ Park
  0 siblings, 1 reply; 3+ messages in thread
From: sashiko-bot @ 2026-07-30  1:15 UTC (permalink / raw)
  To: SJ Park; +Cc: damon

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

New issues:
- [High] The newly added validation check incorrectly uses alignment macros, allowing unaligned zero-length or inverted regions to silently bypass validation.
- [Medium] The validation check unconditionally rejects valid monitoring ranges ending near the top of the address space due to an integer overflow in the ALIGN macro.

Pre-existing issues:
- [High] Alignment rounding combined with a flawed iteration loop creates overlapping monitoring regions from valid, non-overlapping user inputs.
--

--- Patch [1]: [PATCH 6.12.y] mm/damon/core: validate ranges in damon_set_regions() ---
commit b519271ef73c8bbed5092bcf809ea6bc504693b6
Author: SJ Park <sj@kernel.org>
Date:   Thu Jul 30 00:56:39 2026 +0000

mm/damon/core: validate ranges in damon_set_regions()
    
This commit attempts to validate regions in damon_set_regions() to reject 
zero length monitoring target regions, which could previously trigger a 
divide-by-zero or WARN_ONCE.

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

> diff --git a/mm/damon/core.c b/mm/damon/core.c
> index 7911599c37f22..7217bb2ef51ec 100644
> --- a/mm/damon/core.c
> +++ b/mm/damon/core.c
> @@ -214,6 +214,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;
> +	}
> +

[Severity: High]
Does this validation logic allow unaligned zero-length or inverted regions
to bypass the check?

If a user configures an unaligned empty region (e.g., start = 4097, 
end = 4097) via sysfs, ALIGN_DOWN evaluates to 4096 and ALIGN evaluates to
8192. The condition 4096 >= 8192 evaluates to false, which completely 
bypasses the rejection of the invalid region.

[Severity: Medium]
Will this check unconditionally reject valid monitoring ranges ending near the
top of the address space?

If a caller specifies a monitoring region ending near ULONG_MAX, the
ALIGN(ranges[i].end, DAMON_MIN_REGION) operation could wrap around to 0.
This would cause the unsigned comparison ALIGN_DOWN(start) >= 0 to always
evaluate to true, resulting in a false positive -EINVAL rejection.

[Severity: High]
This is a pre-existing issue, but does alignment rounding combined with the
iteration loop create overlapping monitoring regions from valid,
non-overlapping user inputs?

mm/damon/core.c:damon_set_regions() {
    ...
    if (!first) {
        /* no region intersects with this range */
        newr = damon_new_region(
                ALIGN_DOWN(range->start,
                    DAMON_MIN_REGION),
                ALIGN(range->end, DAMON_MIN_REGION));
        if (!newr)
            return -ENOMEM;
        damon_insert_region(newr, damon_prev_region(r), r, t);
    } else {
    ...
}

If a user configures adjacent, non-overlapping unaligned ranges (e.g., 
start = 0, end = 4097 and start = 4098, end = 8192), ALIGN_DOWN and ALIGN 
expand these to [0, 8192] and [4096, 8192], which overlap.

Furthermore, because the loop cursor is not reset, intersection checks are 
skipped for subsequent ranges once the list end is reached. Could this cause 
these overlapping regions to be blindly inserted, potentially corrupting 
core data structures?

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

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH 6.12.y] mm/damon/core: validate ranges in damon_set_regions()
  2026-07-30  1:15   ` sashiko-bot
@ 2026-07-30  3:57     ` SJ Park
  0 siblings, 0 replies; 3+ messages in thread
From: SJ Park @ 2026-07-30  3:57 UTC (permalink / raw)
  To: sashiko-bot; +Cc: SJ Park, damon

On Thu, 30 Jul 2026 01:15:54 +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 newly added validation check incorrectly uses alignment macros, allowing unaligned zero-length or inverted regions to silently bypass validation.
> - [Medium] The validation check unconditionally rejects valid monitoring ranges ending near the top of the address space due to an integer overflow in the ALIGN macro.
> 
> Pre-existing issues:
> - [High] Alignment rounding combined with a flawed iteration loop creates overlapping monitoring regions from valid, non-overlapping user inputs.
> --
> 
> --- Patch [1]: [PATCH 6.12.y] mm/damon/core: validate ranges in damon_set_regions() ---
> commit b519271ef73c8bbed5092bcf809ea6bc504693b6
> Author: SJ Park <sj@kernel.org>
> Date:   Thu Jul 30 00:56:39 2026 +0000
> 
> mm/damon/core: validate ranges in damon_set_regions()
>     
> This commit attempts to validate regions in damon_set_regions() to reject 
> zero length monitoring target regions, which could previously trigger a 
> divide-by-zero or WARN_ONCE.
> 
> Link: https://lore.kernel.org/20260630035221.146458-1-sj@kernel.org
> 
> > diff --git a/mm/damon/core.c b/mm/damon/core.c
> > index 7911599c37f22..7217bb2ef51ec 100644
> > --- a/mm/damon/core.c
> > +++ b/mm/damon/core.c
> > @@ -214,6 +214,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;
> > +	}
> > +
> 
> [Severity: High]
> Does this validation logic allow unaligned zero-length or inverted regions
> to bypass the check?
> 
> If a user configures an unaligned empty region (e.g., start = 4097, 
> end = 4097) via sysfs, ALIGN_DOWN evaluates to 4096 and ALIGN evaluates to
> 8192. The condition 4096 >= 8192 evaluates to false, which completely 
> bypasses the rejection of the invalid region.

Yes, and no problem.  That's the intended behavior.

> 
> [Severity: Medium]
> Will this check unconditionally reject valid monitoring ranges ending near the
> top of the address space?
> 
> If a caller specifies a monitoring region ending near ULONG_MAX, the
> ALIGN(ranges[i].end, DAMON_MIN_REGION) operation could wrap around to 0.
> This would cause the unsigned comparison ALIGN_DOWN(start) >= 0 to always
> evaluate to true, resulting in a false positive -EINVAL rejection.

Too weired user setup.  I will ignore this unless someone reports different
opinions.

> 
> [Severity: High]
> This is a pre-existing issue, but does alignment rounding combined with the
> iteration loop create overlapping monitoring regions from valid,
> non-overlapping user inputs?
> 
> mm/damon/core.c:damon_set_regions() {
>     ...
>     if (!first) {
>         /* no region intersects with this range */
>         newr = damon_new_region(
>                 ALIGN_DOWN(range->start,
>                     DAMON_MIN_REGION),
>                 ALIGN(range->end, DAMON_MIN_REGION));
>         if (!newr)
>             return -ENOMEM;
>         damon_insert_region(newr, damon_prev_region(r), r, t);
>     } else {
>     ...
> }
> 
> If a user configures adjacent, non-overlapping unaligned ranges (e.g., 
> start = 0, end = 4097 and start = 4098, end = 8192), ALIGN_DOWN and ALIGN 
> expand these to [0, 8192] and [4096, 8192], which overlap.

In the case, the overlapping regions check at the beginning of the function
should returned an error.

> 
> Furthermore, because the loop cursor is not reset, intersection checks are 
> skipped for subsequent ranges once the list end is reached. Could this cause 
> these overlapping regions to be blindly inserted, potentially corrupting 
> core data structures?

So this couldn't happen.

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


Thanks,
SJ

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2026-07-30  3:57 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
     [not found] <2026072900-emit-mortified-8331@gregkh>
2026-07-30  0:56 ` [PATCH 6.12.y] mm/damon/core: validate ranges in damon_set_regions() SJ Park
2026-07-30  1:15   ` sashiko-bot
2026-07-30  3:57     ` SJ Park

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.