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

["Mi, Dapeng" <dapeng1.mi@linux.intel.com>]

On 7/6/2026 1:04 PM, sashiko-bot@kernel.org wrote:
> 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?

The issue won't really happen on real hardware. Fixing it would introduce
large complexity and seems unworthy ...


>