Re: [Patch v3 3/8] perf/x86: Guard intel_pmu_cpu_dead() against invalid hybrid PMU casts

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

New issues:
- [High] Setting `cpuc->pmu = NULL` on initialization failure triggers a user-triggerable `WARN_ON_ONCE` in `x86_get_pmu()`.

Pre-existing issues:
- [High] Unchecked MSR writes in NMI handler cause a #GP fault on asymmetric PMU configurations.
--

--- Patch [3]: [Patch v3 3/8] perf/x86: Guard intel_pmu_cpu_dead() against invalid hybrid PMU casts ---
commit 2ef12c59a0f0eb0a83a45dd11cf187ca55beecdf
Author: Dapeng Mi <dapeng1.mi@linux.intel.com>

perf/x86: Guard intel_pmu_cpu_dead() against invalid hybrid PMU casts
    
Fix invalid container conversions in failure paths by clearing cpuc->pmu to
NULL when check_hw_exists() fails, and validating cpuc->pmu in
intel_pmu_cpu_dead(). A new helper x86_get_static_pmu() is added.

> diff --git a/arch/x86/events/intel/core.c b/arch/x86/events/intel/core.c
> index b39c6ce0efb5c..a991fc4f1575f 100644
> --- a/arch/x86/events/intel/core.c
> +++ b/arch/x86/events/intel/core.c
> @@ -6329,8 +6329,10 @@ static bool init_hybrid_pmu(int cpu)
>  
>  	intel_pmu_check_hybrid_pmus(pmu);
>  
> -	if (!check_hw_exists(&pmu->pmu, pmu->cntr_mask, pmu->fixed_cntr_mask))
> +	if (!check_hw_exists(&pmu->pmu, pmu->cntr_mask, pmu->fixed_cntr_mask)) {
> +		cpuc->pmu = NULL;

[Severity: High]
Does setting cpuc->pmu to NULL here trigger a warning in x86_get_pmu()?

If check_hw_exists() fails (e.g., due to a hypervisor restricting PMU MSRs),
we now set cpuc->pmu to NULL but leave the CPU in the possible_cpus mask.

If a user later opens a perf event utilizing Architectural LBRs (like perf
record -b), x86_reserve_hardware() calls reserve_lbr_buffers() which
iterates over all possible CPUs:

arch/x86/events/intel/lbr.c:reserve_lbr_buffers() {
	...
	for_each_possible_cpu(cpu) {
		kmem_cache = x86_get_pmu(cpu)->task_ctx_cache;
	...
}

Because the failed CPU's cpuc->pmu is NULL, x86_get_pmu() hits its strict
WARN_ON_ONCE(!cpuc->pmu) assertion. If panic_on_warn=1 is set, could this
upgrade a graceful PMU initialization failure into a local panic?

Since intel_pmu_cpu_dead() now safely guards against the invalid cast with
cpuc->pmu != pmu, is this assignment necessary?

[Severity: High]
This is a pre-existing issue, but does failing check_hw_exists() leave the
CPU vulnerable to a #GP fault during a spurious NMI?

If a CPU is brought online but its PMU MSRs are disabled by a hypervisor,
initialization for that CPU aborts here. However, perf_event_nmi_handler()
is registered globally if at least one CPU succeeds.

If an active perf event runs on a healthy CPU (so active_events > 0), and a
spurious or broadcast NMI strikes the broken CPU, it will execute:

intel_pmu_handle_irq()
    __intel_pmu_disable_all(true)
        wrmsrq(MSR_CORE_PERF_GLOBAL_CTRL, 0);

Could accessing this non-existent MSR on the failed CPU trigger an unchecked
#GP fault and kernel panic?

>  		return false;
> +	}
>  
>  	pr_info("%s PMU driver: ", pmu->name);

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260717080342.1879573-1-dapeng1.mi@linux.intel.com?part=3