Re: [Patch v9 02/24] 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] Preexisting out-of-bounds read in `check_hw_exists()` on hybrid platforms due to an invalid `container_of()` cast on the global static `pmu` struct via the `hybrid()` macro.
--

commit 8216e613cee4854e16daaa11279146d17bc7f313
Author: Dapeng Mi <dapeng1.mi@linux.intel.com>

perf/x86: Move hybrid PMU initialization before x86_pmu_starting_cpu()

The current approach initializes hybrid PMU structures immediately before
registering them. This is risky as it can lead to key fields being
inadvertently overwritten. This patch mitigates this by moving the
default hybrid structure initialization before calling
x86_pmu_starting_cpu().

> diff --git a/arch/x86/events/core.c b/arch/x86/events/core.c
> index 872d07a5fa80d..0888d3b0923e1 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 does passing the global pmu struct to
check_hw_exists() earlier in this function cause an out-of-bounds read on
hybrid platforms?

In init_hw_perf_events(), check_hw_exists(&pmu, ...) is called after
perf_is_hybrid is enabled. Inside check_hw_exists(), it iterates over
fixed counters and calls fixed_counter_disabled():

arch/x86/events/perf_event.h:fixed_counter_disabled() {
    ...
    u64 intel_ctrl = hybrid(pmu, intel_ctrl);
    ...
}

The hybrid() macro expands to a container_of() cast:

    container_of(_pmu, struct x86_hybrid_pmu, pmu)->_field

Because the pmu passed in is the standalone global struct pmu (not embedded
within a struct x86_hybrid_pmu), this cast yields an invalid pointer.

Could this read garbage data past the end of the global pmu struct when
accessing intel_ctrl, potentially causing the BIOS hardware check to act on
garbage data or trigger a kernel panic if it crosses an unmapped page
boundary?

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