Re: [PATCH v9 05/22] perf: arm_pmuv3: Move counter allocation mask to per-CPU struct pmu_hw_events

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] perf: arm_pmuv3: info leak of KVM guest counters to host userspace
- [Low] perf: arm_pmu: incomplete migration to per-CPU cntr_mask in cpu_pm_pmu_setup()
--

commit 44148c2c0e2e2ea859f2a156493d6c82c8cca8a9
Author: Colton Lewis <coltonlewis@google.com>

perf: arm_pmuv3: Move counter allocation mask to per-CPU struct pmu_hw_events

This commit introduces a per-CPU counter allocation mask (`cntr_mask`) to
prepare for dynamic PMU partitioning with KVM. It initializes the per-CPU
mask from the global mask and updates the allocation functions to use it.

> diff --git a/drivers/perf/arm_pmu.c b/drivers/perf/arm_pmu.c
> index 1150695653892..344adcd3521d0 100644
> --- a/drivers/perf/arm_pmu.c
> +++ b/drivers/perf/arm_pmu.c
[ ... ]
> @@ -717,6 +719,7 @@ bool arm_pmu_irq_is_nmi(void)
>  static int arm_perf_starting_cpu(unsigned int cpu, struct hlist_node *node)
>  {
>  	struct arm_pmu *pmu = hlist_entry_safe(node, struct arm_pmu, node);
> +	struct pmu_hw_events *cpuc = per_cpu_ptr(pmu->hw_events, cpu);
>  	int irq;
>  
>  	if (!cpumask_test_cpu(cpu, &pmu->supported_cpus))
> @@ -724,6 +727,8 @@ static int arm_perf_starting_cpu(unsigned int cpu, struct hlist_node *node)
>  	if (pmu->reset)
>  		pmu->reset(pmu);
>  
> +	bitmap_copy(cpuc->cntr_mask, pmu->cntr_mask, ARMPMU_MAX_HWEVENTS);
> +
>  	irq = armpmu_get_cpu_irq(pmu, cpu);
>  	if (irq)
>  		per_cpu(cpu_irq_ops, cpu)->enable_pmuirq(irq);

[Severity: Low]
Is there a missing update to cpu_pm_pmu_setup() in this file?

Looking at cpu_pm_pmu_setup(), it continues to use the global mask rather
than the newly introduced per-CPU mask:

drivers/perf/arm_pmu.c:cpu_pm_pmu_setup() {
    ...
	for_each_set_bit(idx, armpmu->cntr_mask, ARMPMU_MAX_HWEVENTS) {
		event = hw_events->events[idx];
		if (!event)
			continue;
    ...
}

While hw_events->events[idx] safely evaluates to NULL for counters
partitioned to KVM, should this loop be updated to iterate over
hw_events->cntr_mask for consistency with the new per-CPU abstractions
introduced here?

> diff --git a/drivers/perf/arm_pmuv3.c b/drivers/perf/arm_pmuv3.c
> index 4a6c1f3bcea1f..49289e5993dd7 100644
> --- a/drivers/perf/arm_pmuv3.c
> +++ b/drivers/perf/arm_pmuv3.c
[ ... ]
> @@ -813,7 +813,7 @@ static void armv8pmu_enable_user_access(struct arm_pmu *cpu_pmu)
>  		write_pmuacr(mask);
>  	} else {
>  		/* Clear any unused counters to avoid leaking their contents */
> -		for_each_andnot_bit(i, cpu_pmu->cntr_mask, cpuc->used_mask,
> +		for_each_andnot_bit(i, cpuc->cntr_mask, cpuc->used_mask,
>  				    ARMPMU_MAX_HWEVENTS) {
>  			if (i == ARMV8_PMU_CYCLE_IDX)
>  				write_pmccntr(0);

[Severity: High]
Does this change create an information leak across VM boundaries?

On pre-v3.9 hardware, armv8pmu_enable_user_access() enables EL0 counter
access globally. It clears unused counters to prevent leaking kernel state.
However, by changing the loop to iterate over the per-CPU cpuc->cntr_mask
instead of the global cpu_pmu->cntr_mask, counters reserved for KVM (which
are dynamically removed from cpuc->cntr_mask) will be skipped.

Since KVM does not zero hardware counters on vcpu_put, won't this allow
unprivileged host userspace to read the leftover guest counter values?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260924172928.2110956-1-coltonlewis@google.com?part=5