Re: [Patch v8 02/23] perf/x86: Move hybrid PMU initialization before x86_pmu_starting_cpu()

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

On 5/29/2026 4:51 PM, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
>
> Pre-existing issues:
> - [High] Use-after-free in `intel_pmu_cpu_dead()` triggered during error rollback if `perf_pmu_register()` fails.
> --
>
> commit a6b6c338247bf2e5f630f8fc0cce5709a4fbe801
> Author: Dapeng Mi <dapeng1.mi@linux.intel.com>
>
> perf/x86: Move hybrid PMU initialization before x86_pmu_starting_cpu()
>
> This patch moves the default hybrid PMU structure initialization before
> calling x86_pmu_starting_cpu(). This change prevents key fields like
> capabilities from being inadvertently overwritten during initialization.
>
>> diff --git a/arch/x86/events/core.c b/arch/x86/events/core.c
>> index 4b9e105309c6a..17e122e27e0b7 100644
>> --- a/arch/x86/events/core.c
>> +++ b/arch/x86/events/core.c
> [ ... ]
>> @@ -2195,8 +2195,20 @@ static int __init init_hw_perf_events(void)
>>  
>>  	pmu.attr_update = x86_pmu.attr_update;
>>  
>> -	if (!is_hybrid())
>> +	if (!is_hybrid()) {
>>  		x86_pmu_show_pmu_cap(NULL);
>> +	} else {
>> +		int i;
>> +
>> +		/*
>> +		 * Init default ops.
>> +		 * Must be called before registering x86_pmu_starting_cpu(),
>> +		 * otherwise some key PMU fields, e.g., capabilities
>> +		 * initialized in x86_pmu_starting_cpu(), would be overwritten.
>> +		 */
>> +		for (i = 0; i < x86_pmu.num_hybrid_pmus; i++)
>> +			x86_pmu.hybrid_pmu[i].pmu = pmu;
> [Severity: High]
> This is a pre-existing issue, but I noticed a potential use-after-free
> during error rollback.
>
> During init_hw_perf_events(), the CPU hotplug states are set up and
> x86_pmu_starting_cpu() sets cpuc->pmu to point to elements within the
> dynamically allocated x86_pmu.hybrid_pmu array.
>
> If the subsequent perf_pmu_register() loop fails, the error path frees the
> x86_pmu.hybrid_pmu array via kfree() and jumps to out2 to unregister the
> CPU hotplug states:
>
>     if (i < x86_pmu.num_hybrid_pmus) {
>         ...
>         kfree(x86_pmu.hybrid_pmu);
>         ...
>         goto out2;
>     }
>     ...
>     out:
>         cpuhp_remove_state(CPUHP_PERF_X86_PREPARE);
>
> Tearing down the CPUHP_PERF_X86_PREPARE state triggers x86_pmu_dead_cpu(),
> which eventually calls intel_pmu_cpu_dead().
>
> Could tearing down CPUHP_PERF_X86_PREPARE evaluate the dangling cpuc->pmu
> pointer in intel_pmu_cpu_dead(), causing cpumask_clear_cpu() to write to
> the array that was just freed?

Yes, it's a bug. Need to check if x86_pmu.num_hybrid_pmus is 0 before
accessing the cpuc->pmu, like this,

diff --git a/arch/x86/events/intel/core.c b/arch/x86/events/intel/core.c
index 5013c424a0a2..7aecc86b5788 100644
--- a/arch/x86/events/intel/core.c
+++ b/arch/x86/events/intel/core.c
@@ -6378,7 +6378,7 @@ static void intel_pmu_cpu_dead(int cpu)
        release_arch_pebs_buf_on_cpu(cpu);
        intel_cpuc_finish(cpuc);

-       if (is_hybrid() && cpuc->pmu)
+       if (is_hybrid() && x86_pmu.num_hybrid_pmus && cpuc->pmu)
                cpumask_clear_cpu(cpu, &hybrid_pmu(cpuc->pmu)->supported_cpus);
 }

BTW, I would extract these bug fixing and clean-up patches to a separate
patchset and avoid this patchset inflating too much.

Thanks.


>
>> +	}
>>  
>>  	if (!x86_pmu.read)
>>  		x86_pmu.read = _x86_pmu_read;