Re: [RFC PATCH v3 9/9] mm/damon/core: allow both primitives disabled when a perf probe is present

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] mm: destructive silent clearing of PTE accessed bits
- [High] mm: uninitialized sampling_addr read leads to out-of-bounds PFN lookup
--

commit 73703d94b2ed32c7f44c1db6e359a4902509dc91
Author: Ravi Jonnalagadda <ravis.opensrc@gmail.com>

mm/damon/core: allow both primitives disabled when a perf probe is present

The sample control validation requires exactly one software access check
primitive, page_table or page_fault, to be enabled.  A context carrying a
perf-event probe needs neither: the probe drives the access signal
through its report ring and the drain, so region access rates and ages
update with both primitives off.

> diff --git a/mm/damon/core.c b/mm/damon/core.c
> index c2c74d0daf957..04a1cccbc2f5d 100644
> --- a/mm/damon/core.c
> +++ b/mm/damon/core.c

[ ... ]

> @@ -2390,6 +2387,14 @@ static int __damon_commit_ctx(struct damon_ctx *dst, struct damon_ctx *src,
>  
>  	if (!damon_valid_probe_params(src))
>  		return -EINVAL;
> +	/*
> +	 * A probe-driven context needs no software primitive.  Validate this
> +	 * before anything is committed to @dst, so a rejected configuration
> +	 * cannot leave a live commit with its PMU events already armed.
> +	 */
> +	if (!damon_has_event_driven_probes(src) &&
> +	    damon_primitives_enabled_invalid(&src->sample_control.primitives_enabled))
> +		return -EINVAL;

[Severity: High]
Does this short-circuiting unintentionally allow both primitives to be enabled
concurrently when a probe is present?

Because damon_primitives_enabled_invalid() checks if both are enabled or
both are disabled, bypassing it allows both configurations.

If both page_table and page_fault are enabled, does this permanently destroy
system-wide page access history and break LRU memory reclaim algorithms?

In kdamond_fn(), the page_fault primitive takes precedence during the check
phase:

mm/damon/core.c:kdamond_fn() {
    ...
		if (ctx->sample_control.primitives_enabled.page_fault)
			max_merge_score = kdamond_apply_zero_access_report(ctx);
		else if (!access_check_disabled && ctx->ops.check_accesses)
			max_merge_score = ctx->ops.check_accesses(ctx);
    ...
}

Because the 'else if' is skipped, the hardware PTE accessed bits cleared
earlier by ctx->ops.prepare_access_checks() are never read back.

[Severity: High]
Does this relaxation also introduce an uninitialized read that leads to an
out-of-bounds PFN lookup?

If a context is configured with both primitives disabled and an event-driven
probe with a weight of 0, damon_has_probe_weights(ctx) returns false, which
causes access_check_disabled to evaluate to false.

Because page_table is false, ctx->ops.prepare_access_checks() skips
initialization, leaving r->sampling_addr completely uninitialized.

Since both are disabled, kdamond_fn() forces the execution of the access check
primitive:

mm/damon/core.c:kdamond_fn() {
    ...
		else if (!access_check_disabled && ctx->ops.check_accesses)
			max_merge_score = ctx->ops.check_accesses(ctx);
    ...
}

This calculates a physical address from this garbage memory in
__damon_pa_check_access():

mm/damon/paddr.c:__damon_pa_check_access() {
    ...
	phys_addr_t sampling_addr = damon_pa_phys_addr(
			r->sampling_addr, addr_unit);
    ...
}

Can this lead to out-of-bounds PFN lookups, unpredictable behavior, and
potential kernel crashes or information leaks?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261003-damon-perf-rfc-v3-send-2026-10-03-v3-0-0f00417b41bc@gmail.com?part=9