samples/damon/mtier: fail early if address range parameters are invalid
Subsystems (2)
PATCH 1/1 samples/damon/mtier: fail early if address range parameters are invalid
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.
--- samples/damon/mtier.c | 3 +++ 1 file changed, 3 insertions(+)
samples/damon/mtier.c +3 -0
@@ -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);
Signature · 1 line
2.33.0
[email protected]
AI reviewThank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
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 <[email protected]>
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.
Quoted 5 lines
samples/damon/mtier.c +0
-0
samples/damon/mtier.c static struct damon_ctx *damon_sample_mtier_build_ctx(bool promote) · line 126
Quoted 9 lines
samples/damon/mtier.c +3
-0@@ -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?
Signature · 1 line
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=1
SeongJae Park
maintainerQuoted 7 lines · On Mon, 08 Jun 2026 11:24:54 +0000 [email protected] 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 thedetect_node_addressesauto-detection path, leaving it vulnerable to the exact sameWARN_ONCE.Pre-existing issues:
- [High] Memory leak and potentially orphaned threads ifdamon_start()fails.
Nice fiindings, but not blockers of this patch.
I will work on these issues.
Thanks,
SJ
[...]
SeongJae Park
maintainerQuoted 34 lines · On Mon, 8 Jun 2026 19:15:34 +0800 Zenghui Yu <[email protected]> wrote:
The comment on top of
struct damon_regionclearly says thatFor any use case, @ar should be non-zero positive size.
which is now verified in
damon_verify_new_region()if the kernel is built withDAMON_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 ]---
Quoted 2 lines
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.
Quoted 11 lines
Signed-off-by Zenghui Yu <[email protected]>--- samples/damon/mtier.c | 3 +++ 1 file changed, 3 insertions(+)samples/damon/mtier.c +0
-0
samples/damon/mtier.c static struct damon_ctx *damon_sample_mtier_build_ctx(bool promote) · line 124
Quoted 7 lines
samples/damon/mtier.c +3
-0@@ -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/[email protected]
[2] https://origin.kernel.org/doc/html/latest/mm/damon/maintainer-profile.html#scm-trees
Thanks,
SJ
[...]
Hi SeongJae,
Quoted 57 lines · SeongJae Park
Quoted 34 lines · On Mon, 8 Jun 2026 19:15:34 +0800 Zenghui Yu <[email protected]> wrote:
The comment on top of
struct damon_regionclearly says thatFor any use case, @ar should be non-zero positive size.
which is now verified in
damon_verify_new_region()if the kernel is built withDAMON_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 ]---Quoted 2 lines
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.
Quoted 16 lines
Signed-off-by Zenghui Yu <[email protected]>--- samples/damon/mtier.c | 3 +++ 1 file changed, 3 insertions(+)samples/damon/mtier.c +2
-0@@ -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)
Quoted 8 lines
samples/damon/mtier.c static struct damon_ctx *damon_sample_mtier_build_ctx(bool promote) · line 124
Quoted 2 lines
samples/damon/mtier.c +1
-0+ goto free_out; }Sashiko found [1] same issue can happen if
detect_node_addressesis 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.
Quoted 3 lines
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