Re: [Patch v8 14/23] perf/x86: Support YMM sampling using sample_simd_vec_reg_* fields

[sashiko-bot@kernel.org]

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

New issues:
- [Medium] The `event_needs_ymm()` function evaluates flag-gated syscall parameters outside their corresponding flag gate (`PERF_SAMPLE_REGS_USER` / `PERF_SAMPLE_REGS_INTR`).

Pre-existing issues:
- [High] The function `perf_simd_reg_validate` improperly returns an error when unused, flag-gated parameters are not zero-filled, breaking ABI compatibility.
--

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

perf/x86: Support YMM sampling using sample_simd_vec_reg_* fields

This patch introduces support for sampling YMM registers via the
sample_simd_vec_reg_* fields. It updates the perf_simd_reg_value function
to assemble the XMM and YMMH halves into a complete YMM register and
sets the sample_simd_vec_reg_qwords to 4 for YMM sampling.

> diff --git a/arch/x86/events/core.c b/arch/x86/events/core.c
> --- a/arch/x86/events/core.c
> +++ b/arch/x86/events/core.c
[ ... ]
> @@ -1967,6 +1973,8 @@ static void x86_pmu_sample_xregs(struct perf_event *event,
>  
>  	if (event_needs_xmm(event))
>  		mask |= XFEATURE_MASK_SSE;
> +	if (event_needs_ymm(event))
> +		mask |= XFEATURE_MASK_YMM;
>  
>  	mask &= x86_pmu.ext_regs_mask;
>  	if (sample_type & PERF_SAMPLE_REGS_USER) {

[Severity: Medium]
Does this evaluate flag-gated syscall parameters outside their flag gate?

According to the syscall parameter trust boundaries, userspace is not
required to zero-fill unused fields, so they may contain arbitrary garbage
when their flag is absent.

The event_needs_ymm() helper checks event->attr.sample_simd_regs_enabled
and event->attr.sample_simd_vec_reg_qwords.

However, in x86_pmu_sample_xregs(), this check happens before we verify if
the sample type actually includes PERF_SAMPLE_REGS_USER or
PERF_SAMPLE_REGS_INTR.

Could this cause the kernel to read uninitialized userspace values when
those sample_type flags are not set?

> diff --git a/arch/x86/kernel/perf_regs.c b/arch/x86/kernel/perf_regs.c
> --- a/arch/x86/kernel/perf_regs.c
> +++ b/arch/x86/kernel/perf_regs.c
[ ... ]
> @@ -121,7 +128,8 @@ int perf_simd_reg_validate(u16 simd_enabled, u16 vec_qwords,
>  		if (vec_mask_intr || vec_mask_user)
>  			return -EINVAL;
>  	} else {
> -		if (vec_qwords != PERF_X86_XMM_QWORDS)
> +		if (vec_qwords != PERF_X86_XMM_QWORDS &&
> +		    vec_qwords != PERF_X86_YMM_QWORDS)
>  			return -EINVAL;

[Severity: High]
This isn't a bug introduced by this patch, but does the surrounding
validation code in perf_simd_reg_validate() inappropriately reject legal
syscalls?

When simd_enabled is 0, the function checks if vec_qwords, vec_mask_intr,
and other parameters are zero, returning -EINVAL if they are not.

Since userspace isn't required to zero-fill unused arguments outside their
flag gate, these parameters could contain garbage.

Is it possible that enforcing zero-fill on these unused arguments
functionally breaks ABI compatibility by failing requests that should be
valid under the trust boundary rules?

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