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

[sashiko-bot@kernel.org]

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