[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <467513ae-3d02-4639-97e9-a5f0d3451a7b@linux.intel.com>
Date: Fri, 9 May 2025 13:01:09 -0400
From: "Liang, Kan" <kan.liang@...ux.intel.com>
To: Adrian Hunter <adrian.hunter@...el.com>,
Peter Zijlstra <peterz@...radead.org>
Cc: Ingo Molnar <mingo@...hat.com>,
Alexander Shishkin <alexander.shishkin@...ux.intel.com>,
Arnaldo Carvalho de Melo <acme@...nel.org>, Jiri Olsa <jolsa@...nel.org>,
Namhyung Kim <namhyung@...nel.org>, Ian Rogers <irogers@...gle.com>,
linux-kernel@...r.kernel.org, linux-perf-users@...r.kernel.org
Subject: Re: [PATCH] perf/x86/intel: Fix segfault with PEBS-via-PT with
sample_freq
On 2025-05-08 9:44 a.m., Adrian Hunter wrote:
> Currently, using PEBS-via-PT with a sample frequency instead of a sample
> period, causes a segfault. For example:
>
> [ 103.607823] BUG: kernel NULL pointer dereference, address: 0000000000000195
> [ 103.607876] <NMI>
> [ 103.607879] ? __die_body.cold+0x19/0x27
> [ 103.607885] ? page_fault_oops+0xca/0x290
> [ 103.607891] ? exc_page_fault+0x7e/0x1b0
> [ 103.607897] ? asm_exc_page_fault+0x26/0x30
> [ 103.607901] ? intel_pmu_pebs_event_update_no_drain+0x40/0x60
> [ 103.607903] ? intel_pmu_pebs_event_update_no_drain+0x32/0x60
> [ 103.607905] intel_pmu_drain_pebs_icl+0x333/0x350
> [ 103.607910] handle_pmi_common+0x272/0x3c0
> [ 103.607919] intel_pmu_handle_irq+0x10a/0x2e0
> [ 103.607922] perf_event_nmi_handler+0x2a/0x50
>
> That happens because intel_pmu_pebs_event_update_no_drain() assumes all the
> pebs_enabled bits represent counter indexes, which is not always the case.
> In this particular case, bits 60 and 61 are set for PEBS-via-PT purposes.
>
> The behaviour of PEBS-via-PT with sample frequency is questionable because
> although a PMI is generated (PEBS_PMI_AFTER_EACH_RECORD), the period is not
> adjusted anyway.
>
> Putting that aside, fix intel_pmu_pebs_event_update_no_drain() by passing
> the mask of counter bits instead of 'size'. Note, prior to the Fixes
> commit, 'size' would be limited to the maximum counter index, so the issue
> was not hit.
>
> Fixes: 722e42e45c2f1 ("perf/x86: Support counter mask")
> Signed-off-by: Adrian Hunter <adrian.hunter@...el.com>
Thanks for the fix.
Reviewed-by: Kan Liang <kan.liang@...ux.intel.com>
Thanks,
Kan
> ---
> arch/x86/events/intel/ds.c | 9 +++++----
> 1 file changed, 5 insertions(+), 4 deletions(-)
>
> diff --git a/arch/x86/events/intel/ds.c b/arch/x86/events/intel/ds.c
> index adb2e44761b2..8da1105a419f 100644
> --- a/arch/x86/events/intel/ds.c
> +++ b/arch/x86/events/intel/ds.c
> @@ -2469,8 +2469,9 @@ static void intel_pmu_drain_pebs_core(struct pt_regs *iregs, struct perf_sample_
> setup_pebs_fixed_sample_data);
> }
>
> -static void intel_pmu_pebs_event_update_no_drain(struct cpu_hw_events *cpuc, int size)
> +static void intel_pmu_pebs_event_update_no_drain(struct cpu_hw_events *cpuc, u64 mask)
> {
> + u64 pebs_enabled = cpuc->pebs_enabled & mask;
> struct perf_event *event;
> int bit;
>
> @@ -2481,7 +2482,7 @@ static void intel_pmu_pebs_event_update_no_drain(struct cpu_hw_events *cpuc, int
> * It needs to call intel_pmu_save_and_restart_reload() to
> * update the event->count for this case.
> */
> - for_each_set_bit(bit, (unsigned long *)&cpuc->pebs_enabled, size) {
> + for_each_set_bit(bit, (unsigned long *)&pebs_enabled, X86_PMC_IDX_MAX) {
> event = cpuc->events[bit];
> if (event->hw.flags & PERF_X86_EVENT_AUTO_RELOAD)
> intel_pmu_save_and_restart_reload(event, 0);
> @@ -2516,7 +2517,7 @@ static void intel_pmu_drain_pebs_nhm(struct pt_regs *iregs, struct perf_sample_d
> }
>
> if (unlikely(base >= top)) {
> - intel_pmu_pebs_event_update_no_drain(cpuc, size);
> + intel_pmu_pebs_event_update_no_drain(cpuc, mask);
> return;
> }
>
> @@ -2630,7 +2631,7 @@ static void intel_pmu_drain_pebs_icl(struct pt_regs *iregs, struct perf_sample_d
> (hybrid(cpuc->pmu, fixed_cntr_mask64) << INTEL_PMC_IDX_FIXED);
>
> if (unlikely(base >= top)) {
> - intel_pmu_pebs_event_update_no_drain(cpuc, X86_PMC_IDX_MAX);
> + intel_pmu_pebs_event_update_no_drain(cpuc, mask);
> return;
> }
>
Powered by blists - more mailing lists