Re: [RFC PATCH v4 5/9] mm/damon: add perf_event prep type, core lifecycle, and PMU arm/disarm

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [High] mm/damon: PMU configuration bypasses perf_event_open security checks
- [Medium] mm/damon: perf_priv is permanently lost on kdamond stop
- [Low] mm/damon: uninitialized heap garbage in newly added damon_prep objects
--

commit bb2fe5a24d18d1ea5740c8beef1d58359ce0c535
Author: Ravi Jonnalagadda <ravis.opensrc@gmail.com>
Subject: mm/damon: add perf_event prep type, core lifecycle, and PMU arm/disarm

This patch adds a new DAMON_PREP_PERF_EVENT preparation action to allow DAMON
probes to be event-driven via a hardware PMU. It updates the core lifecycle
to manage PMU arming/disarming and handles both per-CPU and single-instance
models.

> diff --git a/mm/damon/core.c b/mm/damon/core.c
> index 9fc536238c2f0..c2acae6e19ec5 100644
> --- a/mm/damon/core.c
> +++ b/mm/damon/core.c
> @@ -2016,6 +2040,7 @@ static int damon_commit_targets(
>  static void damon_commit_prep(struct damon_prep *dst, struct damon_prep *src)
>  {
>  	dst->action = src->action;
> +	dst->perf = src->perf;
>  }

[Severity: Low]
Does this commit path properly initialize new damon_prep objects?

While the above handles updating existing prep objects, damon_commit_preps()
adds newly allocated prep objects for any new entries:

	new_prep = damon_new_prep(src_prep->action);
	if (!new_prep)
		return -ENOMEM;
	damon_add_prep(dst, new_prep);

Since damon_new_prep() uses kmalloc_obj() without zero-initialization, the
newly allocated object will contain uninitialized heap garbage in its perf
struct because the src_prep->perf values are never copied into new_prep.

[ ... ]

> @@ -4913,6 +5032,23 @@ static int kdamond_fn(void *data)
>  done:
>  	damon_destroy_targets(ctx);
>  
> +#ifdef CONFIG_DAMON_PERF_SOURCE
> +	/*
> +	 * Release perf-event probes here so a stopped kdamond leaves no event
> +	 * firing overflows into its report ring, and holds no PMU ownership.
> +	 */
> +	{
> +		struct damon_probe *p, *next_p;
> +
> +		damon_for_each_probe_safe(p, next_p, ctx) {
> +			if (p->perf_priv) {
> +				damon_perf_probe_teardown(ctx, p->perf_priv);
> +				p->perf_priv = NULL;

[Severity: Medium]
Can a stopped DAMON context ever be safely restarted?

When kdamond stops, this teardown path permanently nulls out p->perf_priv.
If a core API user subsequently restarts the context via damon_start(), the
context is left without its PMU configuration and will silently fail to
monitor any events.

While sysfs users aren't affected because they rebuild the context entirely
on restart, this breaks the core API's ability to reuse stopped contexts.

[ ... ]

> diff --git a/mm/damon/perf_source.c b/mm/damon/perf_source.c
> index 65bbfe850e708..4baa8aebcfa82 100644
> --- a/mm/damon/perf_source.c
> +++ b/mm/damon/perf_source.c
> @@ -285,12 +269,51 @@ int damon_perf_probe_setup(struct damon_ctx *ctx,
>  	perf = kzalloc_obj(*perf, GFP_KERNEL);
>  	if (!perf)
>  		goto release_owner;
> +	event->priv = perf;
> +
> +	/*
> +	 * A system-wide PMU is a single hardware unit rather than a per-CPU
> +	 * counter, so it needs exactly one counter: the cpuhp fan-out below
> +	 * would run one redundant sampler per CPU against the one device and
> +	 * corrupt its shared state.  Pin that counter to a fixed online CPU
> +	 * and bypass cpuhp.
> +	 *
> +	 * A kernel counter with no task must name a CPU.  If that CPU goes
> +	 * offline the counter stops and is not migrated.
> +	 */
> +	if (event->attr.single_instance) {
> +		struct perf_event_attr attr;
> +		int cpu = cpumask_first(cpu_online_mask);
> +
> +		damon_perf_event_init_attr(event, &attr);
> +		/*
> +		 * Pass @event as the overflow context, as the per-CPU path
> +		 * (damon_perf_cpu_online()) does: damon_perf_overflow() reads
> +		 * event->ctx via smp_load_acquire() for the teardown barrier.
> +		 */
> +		perf->single_event = perf_event_create_kernel_counter(&attr,
> +				cpu, NULL, damon_perf_overflow, event);

[Severity: High]
Does this allow userspace to bypass perf_event security policies?

The attributes copied in damon_perf_event_init_attr() (such as type, config,
and exclude_kernel) originate directly from user sysfs inputs.

By passing these raw user-supplied attributes directly into
perf_event_create_kernel_counter(), standard perf security checks like
perf_event_paranoid and LSM hooks are bypassed.

This could allow a user with sysfs write access to configure arbitrary PMU
counters and monitor kernel memory accesses.

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