Re: [Patch v9 24/24] perf/x86/intel: Add sanity check for PEBS fragment size

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] Events without preceding valid records in a corrupted PEBS buffer are permanently stalled.
--

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

perf/x86/intel: Add sanity check for PEBS fragment size

This patch adds a sanity check for corrupted PEBS fragment sizes to prevent
potential infinite loops. If a corrupted fragment is detected, the entire
PEBS record and all subsequent records are dropped to ensure data integrity.

> diff --git a/arch/x86/events/intel/ds.c b/arch/x86/events/intel/ds.c
> index b38aed4f62b4c..7a8e619055393 100644
> --- a/arch/x86/events/intel/ds.c
> +++ b/arch/x86/events/intel/ds.c

[ ... ]

> @@ -3244,7 +3261,8 @@ static __always_inline void
>  __intel_pmu_handle_last_pebs_record(struct pt_regs *iregs,
>  				    struct pt_regs *regs,
>  				    struct perf_sample_data *data,
> -				    u64 mask, short *counts, void **last,
> +				    u64 mask, short *counts,
> +				    void **last, bool corrupted,
>  				    setup_fn setup_sample)
>  {
>  	struct cpu_hw_events *cpuc = this_cpu_ptr(&cpu_hw_events);
> @@ -3258,7 +3276,7 @@ __intel_pmu_handle_last_pebs_record(struct pt_regs *iregs,
>  		event = cpuc->events[bit];
>  
>  		__intel_pmu_pebs_last_event(event, iregs, regs, data, last[bit],
> -					    counts[bit], setup_sample);
> +					    counts[bit], corrupted, setup_sample);
>  	}
>  
>  }

[Severity: High]
This is a pre-existing issue, but does this logic permanently stall PMU
profiling for events without preceding valid records in a corrupted buffer?

In arch/x86/events/intel/ds.c:__intel_pmu_handle_last_pebs_record(), we skip
processing if the count is zero:

	for_each_set_bit(bit, (unsigned long *)&mask, X86_PMC_IDX_MAX) {
		if (!counts[bit])
			continue;
		...

If a corrupted PEBS record is detected (e.g., via the newly added at >= top
bounds checks) and the code exits the drain loop early, events that had no
valid records parsed prior to the corruption will have a count of zero.

Because these events are skipped here, __intel_pmu_pebs_last_event() is never
called, preventing intel_pmu_save_and_restart() from re-arming their hardware
counters. Furthermore, since handle_pmi_common() explicitly masks PEBS events
out of the global status, the generic NMI handler will also ignore them.

Could this lead to the hardware counters remaining disabled, stopping
profiling until the session is recreated?

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