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

[sashiko-bot@kernel.org]

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?

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

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