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

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

Pre-existing issues:
- [High] Missing validation in DAMON mtier module leading to division by zero
- [Medium] Memory leak of damon_ctx on damon_start() failure in sample modules
--

--- Patch [1]: [PATCH 6.18.y] samples/damon/mtier: fail early if address range parameters are invalid ---
commit 5cd52e0f168fe2427f3477f7a3e8f97bec2d9dca
Author: Zenghui Yu <[email protected]>

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

This commit adds a sanity check to damon_sample_mtier_build_ctx() to ensure
the starting address is less than the ending address. If the range is
invalid, it fails early to prevent triggering a WARN_ONCE inside
damon_new_region() when the module is enabled with uninitialized or
memoryless node addresses.

Link: https://lore.kernel.org/[email protected]

> diff --git a/samples/damon/mtier.c b/samples/damon/mtier.c
> index 775838a23d935..dab8fb10b617a 100644
> --- a/samples/damon/mtier.c
> +++ b/samples/damon/mtier.c
> @@ -120,6 +120,8 @@ 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;
>  	region = damon_new_region(addr.start, addr.end);
>  	if (!region)
>  		goto free_out;

[Severity: High]
This is a pre-existing issue, but since we are adding parameter validation
here to prevent panics, should we also validate node0_mem_used_bp and
node0_mem_free_bp later in this function?

If a user writes 0 to these sysfs parameters, they are passed directly to
damos_new_quota_goal() as target values:

    quota_goal = damos_new_quota_goal(
            promote ? DAMOS_QUOTA_NODE_MEM_USED_BP :
            DAMOS_QUOTA_NODE_MEM_FREE_BP,
            promote ? node0_mem_used_bp : node0_mem_free_bp);

When kdamond evaluates the quota score, does this zero target_value
cause a divide-by-zero kernel panic in mm/damon/core.c:damos_quota_score()?

    highest_score = max(highest_score,
            goal->current_value * 10000 /
            goal->target_value);

[Severity: Medium]
This is a pre-existing issue, but I noticed a potential resource leak
in the error handling path for the contexts built by this function.

When the contexts are started in damon_sample_mtier_start():

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

If another exclusive DAMON context is already running, damon_start()
can fail and return -EBUSY. Does returning this error directly leave
the newly allocated struct damon_ctx structures leaked, along with
their associated targets, regions, and schemes?

It looks like the same pattern might exist in damon_sample_prcl_start()
and damon_sample_wsse_start(). Should there be cleanup logic when
damon_start() fails?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=1