All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v3] samples/damon/mtier: fail early if address range parameters are invalid
@ 2026-06-29 14:44 SJ Park
  2026-06-29 15:01 ` sashiko-bot
  0 siblings, 1 reply; 3+ messages in thread
From: SJ Park @ 2026-06-29 14:44 UTC (permalink / raw)
  To: Andrew Morton; +Cc: Zenghui Yu, SJ Park, damon, linux-kernel, linux-mm, stable

From: Zenghui Yu <yuzenghui@huawei.com>

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:217 at damon_new_region+0xf4/0x118, CPU#59: bash/341468
 Call trace:
  damon_new_region+0xf4/0x118 (P)
  damon_set_regions+0xfc/0x3c0
  damon_sample_mtier_build_ctx+0xe8/0x3a8
  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 ]---

Note that the same issue can happen if detect_node_addresses is true, and
node 0 or 1 is memoryless. Fix it together by checking the validity of
parameters right before damon_new_region() and fail early if they're
invalid.

Fixes: 82a08bde3cf7 ("samples/damon: implement a DAMON module for memory tiering")
Cc: <stable@vger.kernel.org> # 6.16.x
Signed-off-by: Zenghui Yu <yuzenghui@huawei.com>
Reviewed-by: SJ Park <sj@kernel.org>
Signed-off-by: SJ Park <sj@kernel.org>
---
Changes from v2
- v2: https://lore.kernel.org/20260609064653.1829-1-yuzenghui@huawei.com
- Collect R-b: from SJ.
- Add Fixes: and Cc: stable@.
- Rebase to latest mm-new.
Changes from v1
- v1: https://lore.kernel.org/20260608111534.264-1-yuzenghui@huawei.com
- take into account the detect_node_addresses case (Sashiko)
- based on mm-new (SJ)

This is a fix Cc-ing stable@.  Nonetheless, because it is for a sample
module, not necessarily "super hot".  7.3 should also be a reasonable
target, like the other sample module fixes [1].

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

 samples/damon/mtier.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/samples/damon/mtier.c b/samples/damon/mtier.c
index faaaaa12e6206..e567f4edd80ea 100644
--- a/samples/damon/mtier.c
+++ b/samples/damon/mtier.c
@@ -120,6 +120,9 @@ static struct damon_ctx *damon_sample_mtier_build_ctx(bool promote)
 		addr.end = promote ? node1_end_addr : node0_end_addr;
 	}
 
+	if (addr.start >= addr.end)
+		goto free_out;
+
 	range.start = addr.start;
 	range.end = addr.end;
 

base-commit: c1922f6a1a8b9a120b46051c3c5e3b81b4b75d92
-- 
2.47.3

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

* Re: [PATCH v3] samples/damon/mtier: fail early if address range parameters are invalid
  2026-06-29 14:44 [PATCH v3] samples/damon/mtier: fail early if address range parameters are invalid SJ Park
@ 2026-06-29 15:01 ` sashiko-bot
  2026-06-30  0:27   ` SJ Park
  0 siblings, 1 reply; 3+ messages in thread
From: sashiko-bot @ 2026-06-29 15:01 UTC (permalink / raw)
  To: SJ Park; +Cc: damon

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

Pre-existing issues:
- [High] The sysfs interface allows creating zero-sized regions, leading to a division by zero kernel panic in `damon_merge_two_regions()` when merging adjacent zero-sized regions.
--

commit 049fc008d25fe73be336de6b00bf5512c8f6ec71
Author: Zenghui Yu <yuzenghui@huawei.com>

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

This commit fixes an issue where invalid node address ranges could trigger a
WARN_ONCE. It prevents zero-sized or negatively-sized regions by adding an
early failure check if the start address is greater than or equal to the end
address.

> diff --git a/samples/damon/mtier.c b/samples/damon/mtier.c
> index faaaaa12e6206..e567f4edd80ea 100644
> --- a/samples/damon/mtier.c
> +++ b/samples/damon/mtier.c
[ ... ]
> @@ -120,6 +120,9 @@ static struct damon_ctx *damon_sample_mtier_build_ctx(bool promote)
>  		addr.end = promote ? node1_end_addr : node0_end_addr;
>  	}
>  
> +	if (addr.start >= addr.end)
> +		goto free_out;
> +

[Severity: High]
This is a pre-existing issue, but while you are fixing zero-sized regions
here, does the DAMON sysfs interface have a similar vulnerability that can
cause a kernel panic?

Looking at damon_sysfs_set_regions() in mm/damon/sysfs.c, the bounds check
only uses strictly greater than instead of greater than or equal to:

    if (sys_region->ar.start > sys_region->ar.end)
            goto out;

If a user configures two identical zero-sized regions via sysfs (e.g.,
start=4096 and end=4096), this check passes. Furthermore, the overlap
check allows consecutive identical zero-sized regions because exact boundary
matches evaluate to false.

When DAMON executes damon_merge_regions_of(), it identifies these as
perfectly adjacent and attempts to merge them in damon_merge_two_regions():

mm/damon/core.c:damon_merge_two_regions() {
    ...
    l->nr_accesses = (l->nr_accesses * sz_l + r->nr_accesses * sz_r) /
            (sz_l + sz_r);
    ...
}

Since both regions are zero-sized, sz_l + sz_r evaluates to 0, which would
cause a division by zero kernel panic. Should the sysfs interface also be
updated to use >= to prevent this?

>  	range.start = addr.start;
>  	range.end = addr.end;
>

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

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

* Re: [PATCH v3] samples/damon/mtier: fail early if address range parameters are invalid
  2026-06-29 15:01 ` sashiko-bot
@ 2026-06-30  0:27   ` SJ Park
  0 siblings, 0 replies; 3+ messages in thread
From: SJ Park @ 2026-06-30  0:27 UTC (permalink / raw)
  To: sashiko-bot; +Cc: SJ Park, damon

On Mon, 29 Jun 2026 15:01:15 +0000 sashiko-bot@kernel.org wrote:

> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> 
> Pre-existing issues:
> - [High] The sysfs interface allows creating zero-sized regions, leading to a division by zero kernel panic in `damon_merge_two_regions()` when merging adjacent zero-sized regions.
> --
> 
> commit 049fc008d25fe73be336de6b00bf5512c8f6ec71
> Author: Zenghui Yu <yuzenghui@huawei.com>
> 
> samples/damon/mtier: fail early if address range parameters are invalid
> 
> This commit fixes an issue where invalid node address ranges could trigger a
> WARN_ONCE. It prevents zero-sized or negatively-sized regions by adding an
> early failure check if the start address is greater than or equal to the end
> address.
> 
> > diff --git a/samples/damon/mtier.c b/samples/damon/mtier.c
> > index faaaaa12e6206..e567f4edd80ea 100644
> > --- a/samples/damon/mtier.c
> > +++ b/samples/damon/mtier.c
> [ ... ]
> > @@ -120,6 +120,9 @@ static struct damon_ctx *damon_sample_mtier_build_ctx(bool promote)
> >  		addr.end = promote ? node1_end_addr : node0_end_addr;
> >  	}
> >  
> > +	if (addr.start >= addr.end)
> > +		goto free_out;
> > +
> 
> [Severity: High]
> This is a pre-existing issue,

So no blocker of this patch.

> but while you are fixing zero-sized regions
> here, does the DAMON sysfs interface have a similar vulnerability that can
> cause a kernel panic?
> 
> Looking at damon_sysfs_set_regions() in mm/damon/sysfs.c, the bounds check
> only uses strictly greater than instead of greater than or equal to:
> 
>     if (sys_region->ar.start > sys_region->ar.end)
>             goto out;
> 
> If a user configures two identical zero-sized regions via sysfs (e.g.,
> start@96 and end@96), this check passes. Furthermore, the overlap
> check allows consecutive identical zero-sized regions because exact boundary
> matches evaluate to false.
> 
> When DAMON executes damon_merge_regions_of(), it identifies these as
> perfectly adjacent and attempts to merge them in damon_merge_two_regions():
> 
> mm/damon/core.c:damon_merge_two_regions() {
>     ...
>     l->nr_accesses = (l->nr_accesses * sz_l + r->nr_accesses * sz_r) /
>             (sz_l + sz_r);
>     ...
> }
> 
> Since both regions are zero-sized, sz_l + sz_r evaluates to 0, which would
> cause a division by zero kernel panic. Should the sysfs interface also be
> updated to use >= to prevent this?

Correct.  And we are working [1] on the fix.

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


Thanks,
SJ

[...]

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

end of thread, other threads:[~2026-06-30  0:27 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-06-29 14:44 [PATCH v3] samples/damon/mtier: fail early if address range parameters are invalid SJ Park
2026-06-29 15:01 ` sashiko-bot
2026-06-30  0:27   ` 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.