Linux-mm Archive on lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] samples/damon/mtier: fail early if address range parameters are invalid
@ 2026-06-08 11:15 Zenghui Yu
  2026-06-08 14:32 ` SeongJae Park
  0 siblings, 1 reply; 3+ messages in thread
From: Zenghui Yu @ 2026-06-08 11:15 UTC (permalink / raw)
  To: damon, linux-mm, linux-kernel
  Cc: sj, akpm, wangzhigang17, liqiqi23, 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



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

* Re: [PATCH] samples/damon/mtier: fail early if address range parameters are invalid
  2026-06-08 11:15 [PATCH] samples/damon/mtier: fail early if address range parameters are invalid Zenghui Yu
@ 2026-06-08 14:32 ` SeongJae Park
  2026-06-08 15:54   ` Zenghui Yu
  0 siblings, 1 reply; 3+ messages in thread
From: SeongJae Park @ 2026-06-08 14:32 UTC (permalink / raw)
  To: Zenghui Yu
  Cc: SeongJae Park, damon, linux-mm, linux-kernel, akpm, wangzhigang17,
	liqiqi23

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

[...]


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

* Re: [PATCH] samples/damon/mtier: fail early if address range parameters are invalid
  2026-06-08 14:32 ` SeongJae Park
@ 2026-06-08 15:54   ` Zenghui Yu
  0 siblings, 0 replies; 3+ messages in thread
From: Zenghui Yu @ 2026-06-08 15:54 UTC (permalink / raw)
  To: SeongJae Park
  Cc: Zenghui Yu, damon, linux-mm, linux-kernel, akpm, wangzhigang17,
	liqiqi23

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


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

end of thread, other threads:[~2026-06-08 15:54 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-06-08 11:15 [PATCH] samples/damon/mtier: fail early if address range parameters are invalid Zenghui Yu
2026-06-08 14:32 ` SeongJae Park
2026-06-08 15:54   ` Zenghui Yu

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox