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

On Sat, 25 Jul 2026 21:14:14 +0000 [email protected] wrote:

> 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.

Good check.

> 
> >  
> >  		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().

Again, good finding.

I will fix above two like below.

'''
--- a/mm/damon/core.c
+++ b/mm/damon/core.c
@@ -4251,7 +4251,7 @@ static int kdamond_fn(void *data)
                unsigned long next_ops_update_sis = ctx->next_ops_update_sis;
                unsigned long sample_interval = ctx->attrs.sample_interval;
                bool access_check_disabled = damon_has_probe_weights(ctx);
-               bool has_prep = damon_has_prep(ctx);
+               bool do_prep = damon_has_prep(ctx) && ctx->ops.prep_probes;
                unsigned int max_merge_score = 0, max_wsum;
                bool get_max_wsum;

@@ -4260,8 +4260,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);
+               if (do_prep)
+                       ctx->ops.prep_probes(ctx, access_check_disabled);

                kdamond_usleep(sample_interval);
                ctx->passed_sample_intervals++;
@@ -4283,7 +4283,7 @@ static int kdamond_fn(void *data)
                        else
                                get_max_wsum = false;
                        max_wsum = ctx->ops.apply_probes(ctx,
-                                       access_check_disabled && !has_prep,
+                                       access_check_disabled && !do_prep,
                                        get_max_wsum);
                        if (get_max_wsum)
                                max_merge_score = max_wsum;
'''

> 
> >  			if (get_max_wsum)
> >  				max_merge_score = max_wsum;
> >  		}
> 
> -- 
> Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=7


Thanks,
SJ