Re: [Patch v9 01/24] perf/x86: Fix two error-path and hybrid PMU guard issues

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] The patch adds a check for `x86_pmu.num_hybrid_pmus` to prevent memory corruption in `intel_pmu_cpu_dead()` on error paths, but it is incomplete and still allows an out-of-bounds write during CPU hotplug rollback if the `CPUHP_AP_PERF_X86_STARTING` state setup fails.
- [High] If `alloc_arch_pebs_buf_on_cpu()` fails in `intel_pmu_cpu_prepare()`, resources allocated by `intel_cpuc_prepare()` are leaked.
- [High] Dangling `cpuc->pmu` pointer leads to Use-After-Free in `perf_event_print_debug()` when hybrid PMU registration fails.
- [High] Out-of-bounds read on the global `.bss` section in `perf_event_print_debug()` due to an invalid struct cast when `init_hw_perf_events()` fails early.
- [Medium] The dynamically allocated `x86_pmu.hybrid_pmu` array is permanently leaked on multiple error paths in `init_hw_perf_events()`.
--

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

perf/x86: Fix two error-path and hybrid PMU guard issues

Fix an NMI handler leak in init_hw_perf_events(). When PMU
initialization fails after register_nmi_handler(), the error path
exits without calling unregister_nmi_handler(), leaving a stale
NMI_LOCAL "PMI" handler registered. Add the missing call before
clearing x86_pmu state.

Also guard the hybrid PMU cpumask update in intel_pmu_cpu_dead()
with a check on x86_pmu.num_hybrid_pmus. Without this, hybrid_pmu()
may be called when the hybrid PMU array has not been allocated,
leading to an out-of-bounds access.

> diff --git a/arch/x86/events/core.c b/arch/x86/events/core.c
> index af0b67ffb43d8..872d07a5fa80d 100644
> --- a/arch/x86/events/core.c
> +++ b/arch/x86/events/core.c

[ ... ]

> @@ -2273,6 +2273,8 @@ static int __init init_hw_perf_events(void)
>  	cpuhp_remove_state(CPUHP_AP_PERF_X86_STARTING);
>  out:
>  	cpuhp_remove_state(CPUHP_PERF_X86_PREPARE);
> +pmi_unregister:
> +	unregister_nmi_handler(NMI_LOCAL, "PMI");
>  out_bad_pmu:
>  	memset(&x86_pmu, 0, sizeof(x86_pmu));

[Severity: Medium]
This isn't a bug introduced by this patch, but does this memset cause a
memory leak?

If cpuhp_setup_state() or check_hw_exists() fails during initialization,
the code jumps to the out_bad_pmu label and executes the memset without
freeing x86_pmu.hybrid_pmu. Could this permanently leak the dynamically
allocated hybrid PMU array?


[Severity: High]
This is a pre-existing issue, but could this error path lead to an
out-of-bounds read in perf_event_print_debug()?

If check_hw_exists() fails early during initialization, out_bad_pmu is
reached, but the perf_is_hybrid static key is never disabled. cpuc->pmu
defaults to the global &pmu.

If a privileged user triggers sysrq-p, perf_event_print_debug() uses
hybrid(cpuc->pmu, cntr_mask). Because is_hybrid() evaluates to true, it
casts the global &pmu to the larger struct x86_hybrid_pmu and attempts to
read cntr_mask, reading memory well beyond the object bounds in the .bss
section.


[Severity: High]
This is a pre-existing issue, but is there a use-after-free risk here if
hybrid PMU registration fails?

If perf_pmu_register() fails for a hybrid PMU inside init_hw_perf_events(),
the x86_pmu.hybrid_pmu array is freed via kfree. However, cpuc->pmu was
already assigned to elements of this array during intel_pmu_cpu_starting().

Since the teardown callbacks do not clear the cpuc->pmu pointers, they
remain dangling. A privileged user triggering sysrq-p will invoke
perf_event_print_debug(), which dereferences cpuc->pmu, potentially causing
a use-after-free.

>  	return err;
>  }

> diff --git a/arch/x86/events/intel/core.c b/arch/x86/events/intel/core.c
> index b39c6ce0efb5c..b8a6382dbb825 100644
> --- a/arch/x86/events/intel/core.c
> +++ b/arch/x86/events/intel/core.c

[ ... ]

> @@ -6479,7 +6479,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)

[Severity: High]
This is a pre-existing issue, but does this incomplete guard still allow an
out-of-bounds write during CPU hotplug rollback?

If CPUHP_AP_PERF_X86_STARTING hotplug setup fails, it rolls back the already
successful CPUHP_PERF_X86_PREPARE state by invoking intel_pmu_cpu_dead().

Because cpuc->pmu defaults to the statically allocated &pmu, the condition
evaluates to true. hybrid_pmu() casts the global pmu to the much larger
struct x86_hybrid_pmu, which could result in an out-of-bounds write when
modifying supported_cpus.

>  		cpumask_clear_cpu(cpu, &hybrid_pmu(cpuc->pmu)->supported_cpus);
>  }


[Severity: High]
This is a pre-existing issue, but can intel_pmu_cpu_prepare() leak memory
allocated by intel_cpuc_prepare() if alloc_arch_pebs_buf_on_cpu() fails?

When the CPU hotplug core attempts to online a CPU, it runs the prepare
callback. If alloc_arch_pebs_buf_on_cpu() fails, the state transitions are
aborted without calling the teardown callback intel_pmu_cpu_dead() for this
specific CPU.

Under memory pressure, could repeated CPU hotplug operations leak the
cpuc->shared_regs, cpuc->constraint_list, and cpuc->excl_cntrs structures?

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