Re: [RFC PATCH v1.3 15/19] mm/damon/core: implement damon_has_probe_weight()

[sashiko-bot@kernel.org]

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

New issues:
- [High] When probe weights are enabled, `damos_apply_target()` aggressively merges adjacent regions regardless of their `probe_hits`, permanently losing monitoring data.
- [High] Setting probe weights on unsupported operations backends (like DAMON_OPS_VADDR) silently disables all memory monitoring, breaking the DAMON core logic.

Pre-existing issues:
- [High] Calling `damon_call()` on an inactive DAMON context leads to an indefinite hang in `wait_for_completion()`, allowing a local DoS via sysfs.
--

--- Patch [15]: [RFC PATCH v1.3 15/19] mm/damon/core: implement damon_has_probe_weight() ---
commit 55c8b58c370f614fee60cf04b8ed7e90eb167302
Author: SJ Park <sj@kernel.org>

mm/damon/core: implement damon_has_probe_weight()

damon monitors only data attributes when probe weights are set.
However, the function for detecting it is intentionally returning false
always. Complete the mode detection function so that the mode can really
start working.

> diff --git a/mm/damon/core.c b/mm/damon/core.c
> index 91964336ce31e..806a67d02a6e9 100644
> --- a/mm/damon/core.c
> +++ b/mm/damon/core.c
> @@ -211,6 +211,12 @@ static struct damon_probe *damon_nth_probe(int n, struct damon_ctx *ctx)
>  
>  static bool damon_has_probe_weights(struct damon_ctx *c)
>  {
> +	struct damon_probe *p;
> +
> +	damon_for_each_probe(p, c) {
> +		if (p->weight)
> +			return true;

[Severity: High]
Does returning true here cause adjacent regions to be aggressively merged in
damos_apply_target(), resulting in the loss of monitoring data?

When access_check_disabled becomes true, nr_accesses remains 0. In
damos_apply_target(), regions with identical nr_accesses and age are merged
to repair quota-induced splits:

mm/damon/core.c:damos_apply_target() {
    ...
    prev_r->ar.end = r->ar.end;
    damon_destroy_region(r, t);
    ...
}

Since damos_apply_target() unconditionally merges these regions without
combining the probe_hits arrays, does this permanently destroy the
fine-grained probe_hits data?

[Severity: High]
Can returning true here also silently disable memory monitoring if the
operations backend does not support probes?

If a backend like vaddr or fvaddr is used, damon_valid_probe_params() does
not verify if ctx->ops.apply_probes is implemented before allowing probe
weights.

When this function returns true, standard access checks are bypassed:

mm/damon/core.c:kdamond_fn() {
    bool access_check_disabled = damon_has_probe_weights(ctx);
    ...
    if (!access_check_disabled && ctx->ops.prepare_access_checks)
        ctx->ops.prepare_access_checks(ctx);
}

Because ctx->ops.apply_probes is null, neither standard accesses nor probe
accesses are evaluated. Will the kdamond worker thread spin without
recording any memory activity?

> +	}
>  	return false;
>  }

[Severity: High]
This is a pre-existing issue, but is there a risk of an indefinite hang when
calling damon_call() from sysfs?

Looking at the sysfs store functions for reclaim and lru_sort, they invoke
damon_call() without verifying if the kdamond thread is actively running:

mm/damon/reclaim.c:damon_reclaim_commit_inputs_store() {
    ...
    if (!ctx)
        return -EINVAL;

    err = damon_call(ctx, &control);
    ...
}

If commit_inputs is written to while the module is disabled, damon_call()
will enqueue the request and block indefinitely on wait_for_completion()
because there is no kdamond thread to process the queue. Could we check
damon_is_running() before calling damon_call()?

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