From: sashiko-bot@kernel.org
To: "Dapeng Mi" <dapeng1.mi@linux.intel.com>
Cc: linux-perf-users@vger.kernel.org
Subject: Re: [PATCH 3/7] perf/x86/intel: Clear cpuc->pmu on hybrid PMU init failure
Date: Fri, 10 Jul 2026 07:26:18 +0000 [thread overview]
Message-ID: <20260710072619.5E3011F000E9@smtp.kernel.org> (raw)
In-Reply-To: <20260710065128.1799838-4-dapeng1.mi@linux.intel.com>
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()?
> + 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?
[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?
> pr_info("%s PMU driver: ", pmu->name);
>
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260710065128.1799838-1-dapeng1.mi@linux.intel.com?part=3
next prev parent reply other threads:[~2026-07-10 7:26 UTC|newest]
Thread overview: 11+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-07-10 6:51 [PATCH 0/7] perf/x86: Miscellaneous PMU bug fixes and optimizations Dapeng Mi
2026-07-10 6:51 ` [PATCH 1/7] perf/x86: Unregister PMI handler on PMU init failure Dapeng Mi
2026-07-10 6:51 ` [PATCH 2/7] perf/x86: Free hybrid state " Dapeng Mi
2026-07-10 6:51 ` [PATCH 3/7] perf/x86/intel: Clear cpuc->pmu on hybrid " Dapeng Mi
2026-07-10 7:26 ` sashiko-bot [this message]
2026-07-10 8:01 ` Mi, Dapeng
2026-07-10 8:20 ` Mi, Dapeng
2026-07-10 6:51 ` [PATCH 4/7] perf/x86/intel: Unwind cpuc state if PEBS buffer setup fails Dapeng Mi
2026-07-10 6:51 ` [PATCH 5/7] perf/x86: Remove stale fixed counter helper and fix hybrid PMU access Dapeng Mi
2026-07-10 6:51 ` [PATCH 6/7] perf/x86/intel: Fix intel_cap handling on hybrid PMUs Dapeng Mi
2026-07-10 6:51 ` [PATCH 7/7] perf/x86: Optimize ACR handling in match_prev_assignment() Dapeng Mi
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20260710072619.5E3011F000E9@smtp.kernel.org \
--to=sashiko-bot@kernel.org \
--cc=dapeng1.mi@linux.intel.com \
--cc=linux-perf-users@vger.kernel.org \
--cc=sashiko-reviews@lists.linux.dev \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.