Re: [PATCH 3/7] perf/x86/intel: Clear cpuc->pmu on hybrid PMU init failure

["Mi, Dapeng" <dapeng1.mi@linux.intel.com>]

On 7/10/2026 3:26 PM, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
>
> New issues:
> - [High] Lockless double-read of `cpuc->pmu` in `x86_get_pmu()` leads to a TOCTOU race when racing with `cpuc->pmu = NULL`, causing a NULL pointer dereference.
>
> Pre-existing issues:
> - [High] The patch leaves a critical memory corruption vulnerability unhandled if a CPU hotplug rollback occurs before the hybrid PMU initialization runs.
> - [High] An unhandled #GP fault occurs in `intel_pmu_handle_irq` if a spurious NMI/PMI is received on a CPU where PMU initialization failed.
> --
>
> --- Patch [3]: [PATCH 3/7] perf/x86/intel: Clear cpuc->pmu on hybrid PMU init failure ---
> commit 0c199c356195e47c5b429eac88846944835a4589
> Author: Dapeng Mi <dapeng1.mi@linux.intel.com>
>
> perf/x86/intel: Clear cpuc->pmu on hybrid PMU init failure
>
> When init_hybrid_pmu() fails at check_hw_exists(), cpuc->pmu may still
> point to the default static PMU. The CPU hotplug rollback then runs
> intel_pmu_cpu_dead(). On hybrid systems, that path may call
> hybrid_pmu(cpuc->pmu), which is not valid for the static PMU pointer and
> can result in incorrect hybrid state access.
>
> Fix this by resetting cpuc->pmu to NULL on hybrid PMU init failure.
>
>> diff --git a/arch/x86/events/intel/core.c b/arch/x86/events/intel/core.c
>> index b39c6ce0efb5c..9d4774278b508 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]
> Can this assignment race with concurrent readers?  Looking at x86_get_pmu()
> in arch/x86/events/core.c, there is a lockless double-read of cpuc->pmu:
>
> x86_get_pmu(int cpu)
> {
>     ...
>     if (WARN_ON_ONCE(!cpuc->pmu))
>         return &pmu;
>
>     return cpuc->pmu;
> }
>
> If sys_perf_event_open() executes concurrently and calls x86_get_pmu()
> via reserve_lbr_buffers(), could the compiler emit two independent loads?
>
> If cpuc->pmu is non-NULL during the first load but is set to NULL by this
> patch before the second load, x86_get_pmu() would return NULL.  Does this
> lead to a NULL pointer dereference in reserve_lbr_buffers()?

Doubt if the issue could really happen. init_hybrid_pmu() is called on cpu
online phase. When it's called, there should no any perf event is created
and enabled. As the 3rd comment says, currently "If init_hybrid_pmu()
returns false here, x86_pmu_starting_cpu() ignores this
failure and returns 0, allowing the CPU to come online with a disabled
PMU." But If we enhance x86_pmu_starting_cpu() to propagate the error, then
the CPU won't be really online and no events can be run this cpu.


>
>> +		return false;
>> +	}
> [Severity: High]
> This isn't a bug introduced by this patch, but does this fully resolve the
> rollback vulnerability?
>
> If a CPU hotplug operation fails between the CPUHP_PERF_X86_PREPARE and
> CPUHP_AP_PERF_X86_STARTING states, the hotplug core rolls back and invokes
> intel_pmu_cpu_dead().
>
> Because init_hybrid_pmu() is bypassed entirely, cpuc->pmu remains pointing
> to the static default pmu struct.
>
> intel_pmu_cpu_dead() then executes:
>
>     if (is_hybrid() && cpuc->pmu)
>         cpumask_clear_cpu(cpu, &hybrid_pmu(cpuc->pmu)->supported_cpus);
>
> Since cpuc->pmu is non-NULL, hybrid_pmu() calculates a negative offset from
> the static pmu struct, and cpumask_clear_cpu() performs an out-of-bounds
> memory write. Is there a way to safely handle rollbacks that bypass
> initialization entirely?

Hmm, yes. Need to check if cpuc->pmu still points to the static pmu before
calling hybrid_pmu().


>
> [Severity: High]
> This is also a pre-existing issue, but what happens if a spurious NMI is
> received on a CPU where check_hw_exists() failed?
>
> If init_hybrid_pmu() returns false here, x86_pmu_starting_cpu() ignores this
> failure and returns 0, allowing the CPU to come online with a disabled PMU.
> If another CPU has active perf events, active_events is greater than 0.
>
> When an NMI broadcast hits this broken CPU, perf_event_nmi_handler() sees
> active_events > 0 and calls intel_pmu_handle_irq(), which executes:
>
>     __intel_pmu_disable_all(true);
>
> This unconditionally executes wrmsrq(MSR_CORE_PERF_GLOBAL_CTRL, 0).
> Since the PMU MSRs are unsupported on this CPU, will this unchecked wrmsrq
> trigger a fatal general protection fault?

Yes, need to propagate the error for x86_pmu_starting_cpu().

would fix this issue in v2. Thanks.



>
>>  	pr_info("%s PMU driver: ", pmu->name);
>>