[PATCH] samples/damon/mtier: fail early if address range parameters are invalid

[PATCH] samples/damon/mtier: fail early if address range parameters are invalid

[Zenghui Yu]

The comment on top of `struct damon_region` clearly says that

    For any use case, @ar should be non-zero positive size.

which is now verified in damon_verify_new_region() if the kernel is built
with DAMON_DEBUG_SANITY.

The WARN_ONCE() can be triggered if the mtier sample module is enabled
before node{0,1}_{start,end}_addr have been properly initialized, which is
obviously not good.

 ------------[ cut here ]------------
 start 0 >= end 0
 WARNING: mm/damon/core.c:116 at damon_new_region+0xf0/0x118, CPU#39: bash/34144
 Call trace:
  damon_new_region+0xf0/0x118 (P)
  damon_sample_mtier_build_ctx+0xd4/0x368
  damon_sample_mtier_start+0x1c/0x90
  damon_sample_mtier_enable_store+0x98/0xb0
  param_attr_store+0xb4/0x128
  module_attr_store+0x2c/0x50
  sysfs_kf_write+0x58/0x90
  kernfs_fop_write_iter+0x16c/0x238
  vfs_write+0x2c0/0x370
  ksys_write+0x74/0x118
  __arm64_sys_write+0x24/0x38
  invoke_syscall+0xa8/0x118
  el0_svc_common.constprop.0+0x48/0xf0
  do_el0_svc+0x24/0x38
  el0_svc+0x54/0x370
  el0t_64_sync_handler+0xa0/0xe8
  el0t_64_sync+0x1ac/0x1b0
 ---[ end trace 0000000000000000 ]---

Fix it by checking the validity of parameters right before
damon_new_region() and fail early if they're invalid.

Signed-off-by: Zenghui Yu <yuzenghui@huawei.com>
---
 samples/damon/mtier.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/samples/damon/mtier.c b/samples/damon/mtier.c
index 775838a23d93..4a5d3fb12e1b 100644
--- a/samples/damon/mtier.c
+++ b/samples/damon/mtier.c
@@ -118,6 +118,9 @@ static struct damon_ctx *damon_sample_mtier_build_ctx(bool promote)
 	} else {
 		addr.start = promote ? node1_start_addr : node0_start_addr;
 		addr.end = promote ? node1_end_addr : node0_end_addr;
+
+		if (addr.start >= addr.end)
+			goto free_out;
 	}
 
 	region = damon_new_region(addr.start, addr.end);
-- 
2.33.0

Re: [PATCH] samples/damon/mtier: fail early if address range parameters are invalid

[sashiko-bot]

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

New issues:
- [Medium] The newly added validation for invalid address ranges bypasses the `detect_node_addresses` auto-detection path, leaving it vulnerable to the exact same `WARN_ONCE`.

Pre-existing issues:
- [High] Memory leak and potentially orphaned threads if `damon_start()` fails.
--

commit 08cd6a6fdfc8b3e6b6d9d4a3a29b99f99163573b
Author: Zenghui Yu <yuzenghui@huawei.com>

samples/damon/mtier: fail early if address range parameters are invalid

This commit adds a validation check in damon_sample_mtier_build_ctx() to
fail early if the start address is greater than or equal to the end address.
This prevents a warning from triggering when damon_new_region() is called
with invalid parameters before node addresses are properly initialized.

> diff --git a/samples/damon/mtier.c b/samples/damon/mtier.c
> index 775838a23d935..4a5d3fb12e1b3 100644
> --- a/samples/damon/mtier.c
> +++ b/samples/damon/mtier.c
> @@ -118,6 +118,9 @@ static struct damon_ctx *damon_sample_mtier_build_ctx(bool promote)
>  	} else {
>  		addr.start = promote ? node1_start_addr : node0_start_addr;
>  		addr.end = promote ? node1_end_addr : node0_end_addr;
> +
> +		if (addr.start >= addr.end)
> +			goto free_out;
>  	}
>  
>  	region = damon_new_region(addr.start, addr.end);

[Severity: Medium]
Does this leave the detect_node_addresses auto-detection path vulnerable to
the same warning?

If detect_node_addresses is true, nid_to_phys() is used to compute the
address ranges. For online memoryless nodes (like CPU-only NUMA nodes),
node_start_pfn equals node_end_pfn, which means addr.start == addr.end.

Since the new check is restricted to the else block, would the auto-detection
path bypass validation and still pass identical start and end values to
damon_new_region()?

[Severity: High]
This is a pre-existing issue, but I noticed a potential memory leak and
thread handling problem in damon_sample_mtier_start() when damon_start()
fails.

Looking at damon_sample_mtier_start():

    ctxs[1] = ctx;
    return damon_start(ctxs, 2, true);

If damon_start() fails (for example, returning -EBUSY because another
exclusive context is running), it returns an error code immediately without
calling damon_destroy_ctx(). Does this permanently leak the dynamically
allocated contexts?

Additionally, in mm/damon/core.c:damon_start():

    for (i = 0; i < nr_ctxs; i++) {
        err = __damon_start(ctxs[i]);
        if (err)
            break;
        nr_running_ctxs++;
    }

If damon_start() succeeds on ctxs[0] but fails on ctxs[1], it breaks out
and returns an error without stopping the already started ctxs[0]. Will
this leave ctxs[0] running indefinitely as an orphaned thread?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260608111534.264-1-yuzenghui@huawei.com?part=1

Re: [PATCH] samples/damon/mtier: fail early if address range parameters are invalid

[SeongJae Park]

On Mon, 08 Jun 2026 11:24:54 +0000 sashiko-bot@kernel.org wrote:

> Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
> 
> New issues:
> - [Medium] The newly added validation for invalid address ranges bypasses the `detect_node_addresses` auto-detection path, leaving it vulnerable to the exact same `WARN_ONCE`.
> 
> Pre-existing issues:
> - [High] Memory leak and potentially orphaned threads if `damon_start()` fails.

Nice fiindings, but not blockers of this patch.

I will work on these issues.


Thanks,
SJ

[...]

Re: [PATCH] samples/damon/mtier: fail early if address range parameters are invalid

[SeongJae Park]

On Mon, 8 Jun 2026 19:15:34 +0800 Zenghui Yu <yuzenghui@huawei.com> wrote:

> The comment on top of `struct damon_region` clearly says that
> 
>     For any use case, @ar should be non-zero positive size.
> 
> which is now verified in damon_verify_new_region() if the kernel is built
> with DAMON_DEBUG_SANITY.
> 
> The WARN_ONCE() can be triggered if the mtier sample module is enabled
> before node{0,1}_{start,end}_addr have been properly initialized, which is
> obviously not good.
> 
>  ------------[ cut here ]------------
>  start 0 >= end 0
>  WARNING: mm/damon/core.c:116 at damon_new_region+0xf0/0x118, CPU#39: bash/34144
>  Call trace:
>   damon_new_region+0xf0/0x118 (P)
>   damon_sample_mtier_build_ctx+0xd4/0x368
>   damon_sample_mtier_start+0x1c/0x90
>   damon_sample_mtier_enable_store+0x98/0xb0
>   param_attr_store+0xb4/0x128
>   module_attr_store+0x2c/0x50
>   sysfs_kf_write+0x58/0x90
>   kernfs_fop_write_iter+0x16c/0x238
>   vfs_write+0x2c0/0x370
>   ksys_write+0x74/0x118
>   __arm64_sys_write+0x24/0x38
>   invoke_syscall+0xa8/0x118
>   el0_svc_common.constprop.0+0x48/0xf0
>   do_el0_svc+0x24/0x38
>   el0_svc+0x54/0x370
>   el0t_64_sync_handler+0xa0/0xe8
>   el0t_64_sync+0x1ac/0x1b0
>  ---[ end trace 0000000000000000 ]---
> 
> Fix it by checking the validity of parameters right before
> damon_new_region() and fail early if they're invalid.

Good catch, thank you for this patch.

> 
> Signed-off-by: Zenghui Yu <yuzenghui@huawei.com>
> ---
>  samples/damon/mtier.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/samples/damon/mtier.c b/samples/damon/mtier.c
> index 775838a23d93..4a5d3fb12e1b 100644
> --- a/samples/damon/mtier.c
> +++ b/samples/damon/mtier.c
> @@ -118,6 +118,9 @@ static struct damon_ctx *damon_sample_mtier_build_ctx(bool promote)
>  	} else {
>  		addr.start = promote ? node1_start_addr : node0_start_addr;
>  		addr.end = promote ? node1_end_addr : node0_end_addr;
> +
> +		if (addr.start >= addr.end)
> +			goto free_out;
>  	}

Sashiko found [1] same issue can happen if detect_node_addresses is true, and
nodes 0 and 1 are both memoryless.  It shouldn't be a blocker of this patch,
but fixing it together can be very simple by moving this address check to the
out of the above if block, right here.  Zenghui, could you please update this
patch to do that?

Also, seems this patch is based on an old tree.  Could you please use
mm-new [2] as the base of your DAMON patches from next time?

[1] https://lore.kernel.org/20260608112455.274231F00893@smtp.kernel.org
[2] https://origin.kernel.org/doc/html/latest/mm/damon/maintainer-profile.html#scm-trees


Thanks,
SJ

[...]

Re: [PATCH] samples/damon/mtier: fail early if address range parameters are invalid

[Zenghui Yu]

Hi SeongJae,

On 6/8/26 10:32 PM, SeongJae Park wrote:
> On Mon, 8 Jun 2026 19:15:34 +0800 Zenghui Yu <yuzenghui@huawei.com> wrote:
> 
> > The comment on top of `struct damon_region` clearly says that
> >
> >     For any use case, @ar should be non-zero positive size.
> >
> > which is now verified in damon_verify_new_region() if the kernel is built
> > with DAMON_DEBUG_SANITY.
> >
> > The WARN_ONCE() can be triggered if the mtier sample module is enabled
> > before node{0,1}_{start,end}_addr have been properly initialized, which is
> > obviously not good.
> >
> >  ------------[ cut here ]------------
> >  start 0 >= end 0
> >  WARNING: mm/damon/core.c:116 at damon_new_region+0xf0/0x118, CPU#39: bash/34144
> >  Call trace:
> >   damon_new_region+0xf0/0x118 (P)
> >   damon_sample_mtier_build_ctx+0xd4/0x368
> >   damon_sample_mtier_start+0x1c/0x90
> >   damon_sample_mtier_enable_store+0x98/0xb0
> >   param_attr_store+0xb4/0x128
> >   module_attr_store+0x2c/0x50
> >   sysfs_kf_write+0x58/0x90
> >   kernfs_fop_write_iter+0x16c/0x238
> >   vfs_write+0x2c0/0x370
> >   ksys_write+0x74/0x118
> >   __arm64_sys_write+0x24/0x38
> >   invoke_syscall+0xa8/0x118
> >   el0_svc_common.constprop.0+0x48/0xf0
> >   do_el0_svc+0x24/0x38
> >   el0_svc+0x54/0x370
> >   el0t_64_sync_handler+0xa0/0xe8
> >   el0t_64_sync+0x1ac/0x1b0
> >  ---[ end trace 0000000000000000 ]---
> >
> > Fix it by checking the validity of parameters right before
> > damon_new_region() and fail early if they're invalid.
> 
> Good catch, thank you for this patch.
> 
> >
> > Signed-off-by: Zenghui Yu <yuzenghui@huawei.com>
> > ---
> >  samples/damon/mtier.c | 3 +++
> >  1 file changed, 3 insertions(+)
> >
> > diff --git a/samples/damon/mtier.c b/samples/damon/mtier.c
> > index 775838a23d93..4a5d3fb12e1b 100644
> > --- a/samples/damon/mtier.c
> > +++ b/samples/damon/mtier.c
> > @@ -118,6 +118,9 @@ static struct damon_ctx *damon_sample_mtier_build_ctx(bool promote)
> >  	} else {
> >  		addr.start = promote ? node1_start_addr : node0_start_addr;
> >  		addr.end = promote ? node1_end_addr : node0_end_addr;
> > +
> > +		if (addr.start >= addr.end)
> > +			goto free_out;
> >  	}
> 
> Sashiko found [1] same issue can happen if detect_node_addresses is true, and
> nodes 0 and 1 are both memoryless.  It shouldn't be a blocker of this patch,
> but fixing it together can be very simple by moving this address check to the
> out of the above if block, right here.  Zenghui, could you please update this
> patch to do that?

Yup, it's worth fixing. I will address the detect_node_addresses issue
in v2.

> 
> Also, seems this patch is based on an old tree.  Could you please use
> mm-new [2] as the base of your DAMON patches from next time?

Ah, I'm not familiar with the development process of DAMON and I created
this patch against mainline kernel. I'll re-test the whole thing on top
of mm-new. Thanks for the reminder!

Zenghu