Re: [PATCH] perf/arm-cmn: Allow userspace to select the PMU's CPU

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] perf/arm-cmn: PMU context split data race via sysfs cpumask write
--

commit cd1fc8e509238a5fe44eb8583d42c30948f80fe1
Author: Haris Okanovic <harisokn@amazon.com>
Subject: perf/arm-cmn: Allow userspace to select the PMU's CPU

This commit allows userspace to migrate the PMU's CPU by making the 'cpumask'
sysfs attribute writable. It uses the existing arm_cmn_migrate() function to
move the PMU context and interrupts to the newly selected CPU, allowing
administrators to move background work away from latency-sensitive CPUs.

> diff --git a/drivers/perf/arm-cmn.c b/drivers/perf/arm-cmn.c
> index b162de3d9d169..77eed6dbb1bdb 100644
> --- a/drivers/perf/arm-cmn.c
> +++ b/drivers/perf/arm-cmn.c

[ ... ]

> @@ -1519,8 +1523,42 @@ static ssize_t arm_cmn_cpumask_show(struct device *dev,
>  	return sysfs_emit(buf, "%*pbl\n", cpumask_pr_args(cpumask_of(cmn->cpu)));
>  }
>  
> +static ssize_t arm_cmn_cpumask_store(struct device *dev,
> +				     struct device_attribute *attr,
> +				     const char *buf, size_t count)
> +{
> +	static DEFINE_MUTEX(cpumask_mutex);
> +
> +	struct arm_cmn *cmn = to_cmn(dev_get_drvdata(dev));
> +	unsigned int cpu;
> +	int err;
> +
> +	err = kstrtouint(buf, 0, &cpu);
> +	if (err)
> +		return err;
> +
> +	if (cpu >= nr_cpu_ids)
> +		return -EINVAL;
> +
> +	/* Serialises multiple writers against each other */
> +	mutex_lock(&cpumask_mutex);
> +	/* Blocks hotplug during write */
> +	cpus_read_lock();
> +
> +	if (!cpu_online(cpu))
> +		err = -EINVAL;
> +	else if (cpu != cmn->cpu)
> +		arm_cmn_migrate(cmn, cpu);

[Severity: High]
Is there a race condition here with concurrent perf_event_open() calls?

If a process calls perf_event_open() concurrently, arm_cmn_event_init() reads
cmn->cpu locklessly:

drivers/perf/arm-cmn.c:arm_cmn_event_init() {
	...
	event->cpu = cmn->cpu;
	if (event->cpu < 0)
		return -EINVAL;
	...
}

If this sysfs write changes cmn->cpu and migrates existing PMU contexts
to the new CPU right after arm_cmn_event_init() reads the old CPU,
the new event will be installed on the old CPU.

Since the old CPU remains fully online (unlike a true CPU hotplug event),
this splits the PMU context across multiple CPUs. PMU callbacks like
arm_cmn_event_add() can then execute simultaneously on different CPUs:

drivers/perf/arm-cmn.c:arm_cmn_event_add() {
	...
	while (dtm->pmu_config_low & CMN__PMEVCNT_PAIRED(dtm_idx))
	...
}

Could this lead to concurrent read-modify-writes to shared PMU hardware state
and array structures without locking?

> +
> +	cpus_read_unlock();
> +	mutex_unlock(&cpumask_mutex);
> +
> +	return err ?: count;
> +}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260929223244.2411400-1-harisokn@amazon.com?part=1