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

[sashiko-bot@kernel.org]

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?

> [ ... ]
> @@ -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?

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