Re: [RESEND Patch v10 23/23] perf/x86/intel: Add sanity check for PEBS record/fragment size

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

On 8/24/2026 5:08 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] setup_arch_pebs_sample_data() parses PEBS fragment payloads based on the presence of bit flags without verifying that the implied payload size fits within header->size or the buffer boundary.
> --
>
> --- Patch [23]: [RESEND Patch v10 23/23] perf/x86/intel: Add sanity check for PEBS record/fragment size ---
> commit 573a2a9becb54c34e2eef68662afc34b55c514f9
> Author: Dapeng Mi <dapeng1.mi@linux.intel.com>
>
> perf/x86/intel: Add sanity check for PEBS record/fragment size
>
> This patch adds sanity checks for corrupted PEBS records or fragment
> sizes to prevent potential infinite loops. If a corrupted fragment is
> detected, the entire PEBS record and all subsequent records are dropped.
>
>> diff --git a/arch/x86/events/intel/ds.c b/arch/x86/events/intel/ds.c
>> index 39028de75e4f6..4448144387158 100644
>> --- a/arch/x86/events/intel/ds.c
>> +++ b/arch/x86/events/intel/ds.c
>> @@ -2657,6 +2657,9 @@ static void setup_arch_pebs_sample_data(struct perf_event *event,
>>  
>>  again:
>>  	header = at;
>> +	if (!header->size)
>> +		return;
>> +
>>  	next_record = at + sizeof(struct arch_pebs_header);
>>  	if (header->basic) {
> [Severity: High]
> This is a pre-existing issue, but since this patch explicitly aims to safely
> handle corrupted PEBS records, could a corrupted record have a small
> header->size but multiple structural flags set?
>
> In setup_arch_pebs_sample_data(), next_record is advanced based on the
> presence of bit flags without checking against header->size or the buffer
> boundary:
>
>     if (header->gpr) {
>         gprs = next_record;
>         next_record = gprs + 1;
>         __setup_pebs_gpr_group(event, regs, ...);
>
> Would this lead to out-of-bounds memory reads if the hardware generates a
> corrupted record where the implied payload exceeds header->size? While
> intel_pmu_drain_arch_pebs() checks that header->size fits in the buffer,
> it doesn't seem to verify if the payload itself fits within header->size.

Yes, this concern could happen in theory. This patch mainly focus on the 0
size issue, the issue could be addressed by another patch if necessary. Thanks.


>