Re: [RFC PATCH 07/17] mm/damon/core: introduce damon_operations->prep_probes()

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Medium] prep_probes receives has_prep instead of access_check_disabled && has_prep, causing it to overwrite sampling_addr during normal access checking.
- [Medium] Uninitialized sampling_addr usage in apply_probes if prep_probes is not implemented.
--

--- Patch [7]: [RFC PATCH 07/17] mm/damon/core: introduce damon_operations->prep_probes() ---
commit 62e96fea7e813101fb4353abda03a93a90ef358a
Author: SJ Park <[email protected]>

mm/damon/core: introduce damon_operations->prep_probes()
    
damon_prep needs to be executed by the underlying DAMON operation set.
Extend the operation set callback list for the execution of damon_prep
actions.  If the underlying operation set implements the callback, DAMON
core executes it in the monitoring preparation time.

> diff --git a/mm/damon/core.c b/mm/damon/core.c
> index 127d3a7c27280..d5b1e0b5851a1 100644
> --- a/mm/damon/core.c
> +++ b/mm/damon/core.c

[ ... ]

> @@ -3878,6 +3891,8 @@ static int kdamond_fn(void *data)
>  
>  		if (!access_check_disabled && ctx->ops.prepare_access_checks)
>  			ctx->ops.prepare_access_checks(ctx);
> +		if (ctx->ops.prep_probes)
> +			ctx->ops.prep_probes(ctx, has_prep);

[Severity: Medium]
Could this overwrite the tracked sampling_addr during normal access checking?

If a configuration has a prep without probe weights, access_check_disabled
is false and has_prep is true. prepare_access_checks() sets sampling_addr 
and marks it for tracking. 

If prep_probes() is implemented by a backend, passing has_prep (true) here
instead of access_check_disabled && has_prep would cause it to generate
a new, unprepared sampling_addr that silently overwrites the tracked one.

>  
>  		kdamond_usleep(sample_interval);
>  		ctx->passed_sample_intervals++;
> @@ -3892,7 +3907,8 @@ static int kdamond_fn(void *data)
>  			else
>  				get_max_wsum = false;
>  			max_wsum = ctx->ops.apply_probes(ctx,
> -					access_check_disabled, get_max_wsum);
> +					access_check_disabled && !has_prep,
> +					get_max_wsum);

[Severity: Medium]
Does this result in uninitialized sampling_addr usage in apply_probes() if
the backend doesn't implement prep_probes?

If a probe has a weight and a prep, both access_check_disabled and has_prep
evaluate to true, which skips prepare_access_checks().

If the current operations do not implement prep_probes
(ctx->ops.prep_probes == NULL), that callback is also skipped.

When apply_probes() is called, set_samples evaluates to false (because
access_check_disabled && !has_prep is false). As a result, no function
initializes r->sampling_addr, and the uninitialized memory could be read
by apply_probes().

>  			if (get_max_wsum)
>  				max_merge_score = max_wsum;
>  		}

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