Re: [Patch v9 13/24] perf: Add sampling support for SIMD registers

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

On 7/6/2026 10:34 AM, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
>
> New issues:
> - [Medium] Uninitialized, flag-gated syscall parameters are unconditionally passed to and evaluated by the validation function `perf_simd_reg_validate()`, breaking trust boundaries.
>
> Pre-existing issues:
> - [High] The dynamic size added by PERF_SAMPLE_REGS_INTR (and exacerbated by SIMD registers) bypasses the size capping applied during PERF_SAMPLE_STACK_USER, leading to a U16_MAX integer overflow and ring buffer corruption.
> --
>
> commit 8a8cf6dc3b640f823c8a7990b8c46e78cd38719e
> Author: Kan Liang <kan.liang@linux.intel.com>
>
> perf: Add sampling support for SIMD registers
>
> This commit introduces support for sampling SIMD registers during profiling
> by adding new attribute fields to the perf_event_attr structure. It provides
> a new output format and the PERF_SAMPLE_REGS_ABI_SIMD ABI to represent the
> number and width of these registers based on user configuration.
>
>> diff --git a/kernel/events/core.c b/kernel/events/core.c
>> index 0239864029bec..ce93c23b3a33f 100644
>> --- a/kernel/events/core.c
>> +++ b/kernel/events/core.c
>> @@ -8773,6 +8877,10 @@ void perf_prepare_sample(struct perf_sample_data *data,
>>  			u64 mask = event->attr.sample_regs_intr;
>>  
>>  			size += hweight64(mask) * sizeof(u64);
>> +			if (event_has_simd_regs(event)) {
>> +				size += perf_update_xregs_size(event, true);
>> +				data->regs_intr.abi |= PERF_SAMPLE_REGS_ABI_SIMD;
>> +			}
>>  		}
>>  
>>  		data->dyn_size += size;
> [Severity: High]
> This is a pre-existing issue, but does adding this new SIMD register size
> cause a buffer overrun by overflowing data->dyn_size past USHRT_MAX?
>
> Earlier in perf_prepare_sample(), when PERF_SAMPLE_STACK_USER is processed,
> perf_sample_ustack_size() caps the stack dump size so that the total dynamic
> size precisely reaches USHRT_MAX. 
>
> Since PERF_SAMPLE_REGS_INTR is unconditionally processed after
> PERF_SAMPLE_STACK_USER, adding its size (which can now be significantly
> larger with SIMD registers) to data->dyn_size will exceed 65535 bytes.
>
> Could this truncate the 16-bit header size and cause perf_output_sample()
> to write far more data than was allocated in the ring buffer?

The function perf_simd_reg_validate() would check if SIMD register size
exceeds USHRT_MAX. If so, the event creation would fail before sampling.


>
>> [ ... ]
>> @@ -13694,6 +13809,16 @@ static int perf_copy_attr(struct perf_event_attr __user *uattr,
>>  		ret = perf_reg_validate(attr->sample_regs_user);
>>  		if (ret)
>>  			return ret;
>> +		ret = perf_simd_reg_validate(attr->sample_type,
>> +					     attr->sample_simd_regs_enabled,
>> +					     attr->sample_simd_vec_reg_qwords,
>> +					     attr->sample_simd_vec_reg_intr,
>> +					     attr->sample_simd_vec_reg_user,
>> +					     attr->sample_simd_pred_reg_qwords,
>> +					     attr->sample_simd_pred_reg_intr,
>> +					     attr->sample_simd_pred_reg_user);
>> +		if (ret)
>> +			return ret;
>>  	}
> [Severity: Medium]
> Could this pass uninitialized syscall parameters to the validation logic?
>
> When PERF_SAMPLE_REGS_USER is set but PERF_SAMPLE_REGS_INTR is not, the user
> is not required to initialize the sample_simd_vec_reg_intr or
> sample_simd_pred_reg_intr fields. 
>
> If perf_simd_reg_validate() evaluates these uninitialized fields:
>
> arch/x86/kernel/perf_regs.c:perf_simd_reg_validate() {
>     if (!vec_mask_intr && !vec_mask_user)
>         return -EINVAL;
> }
>
> Does this allow an invalid configuration where vec_mask_user is zero to
> bypass validation because vec_mask_intr contains non-zero garbage data?

Hmm, it looks a overkill here. It would be fair enough to check REGS_USER
and REGS_INTR separately. Thanks.


>