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

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Critical] The validation logic in `perf_simd_reg_validate()` strictly checks unused, flag-gated syscall parameters outside of their respective flag scopes, breaking ABI compatibility.
--

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

perf/x86: Support XMM sampling using sample_simd_vec_reg_* fields

This patch adds support for sampling XMM registers using the new
sample_simd_vec_reg_* fields. It introduces ABI tracking for perf_regs
on x86 and implements validation logic for SIMD registers.

> diff --git a/arch/x86/kernel/perf_regs.c b/arch/x86/kernel/perf_regs.c
> index 81204cb7f7239..7b9b38c189de2 100644
> --- a/arch/x86/kernel/perf_regs.c
> +++ b/arch/x86/kernel/perf_regs.c
[ ... ]
> @@ -74,6 +77,71 @@ u64 perf_reg_value(struct pt_regs *regs, int idx)
[ ... ]
> +int perf_simd_reg_validate(u16 simd_enabled, u16 vec_qwords,
> +			   u64 vec_mask_intr, u64 vec_mask_user,
> +			   u16 pred_qwords, u32 pred_mask_intr,
> +			   u32 pred_mask_user)
> +{
> +	u64 size;
> +
> +	if (!simd_enabled) {
> +		if (vec_qwords || vec_mask_intr || vec_mask_user ||
> +		    pred_qwords || pred_mask_intr || pred_mask_user)
> +			return -EINVAL;

[Severity: Critical]
Are we unconditionally validating syscall parameters outside their flag scopes
here?

If userspace sets PERF_SAMPLE_REGS_USER but does not set
PERF_SAMPLE_REGS_INTR, the vec_mask_intr and pred_mask_intr parameters
could contain uninitialized stack garbage. Because userspace is not required
to zero-fill unused arguments, returning -EINVAL for garbage in these unused
masks could break ABI compatibility for programs recompiled against newer
kernel headers.

> +		return 0;
> +	}
> +
> +	if (!vec_qwords) {
> +		if (vec_mask_intr || vec_mask_user)
> +			return -EINVAL;
> +	} else {
> +		if (vec_qwords != PERF_X86_XMM_QWORDS)
> +			return -EINVAL;
> +		if ((!vec_mask_intr && !vec_mask_user) ||
> +		    (vec_mask_intr & ~PERF_X86_SIMD_VEC_MASK) ||
> +		    (vec_mask_user & ~PERF_X86_SIMD_VEC_MASK))
> +			return -EINVAL;

[Severity: Critical]
Similarly, could checking these masks cause false -EINVAL errors
if only one of the sampling flags (INTR or USER) is enabled?

If PERF_SAMPLE_REGS_USER is enabled but PERF_SAMPLE_REGS_INTR is disabled,
vec_mask_intr might contain random bits that fail the PERF_X86_SIMD_VEC_MASK
check. Should these validation checks be gated behind the presence of their
respective flags in the sample_type to prevent regressions?

> +	}
> +
> +	if (pred_qwords || pred_mask_intr || pred_mask_user)
> +		return -EINVAL;
> +
> +	size = ((vec_qwords * hweight64(vec_mask_intr)) +
> +		(vec_qwords * hweight64(vec_mask_user)) +
> +		(pred_qwords * hweight32(pred_mask_intr)) +
> +		(pred_qwords * hweight32(pred_mask_user))) * sizeof(u64);
> +	if (size > U16_MAX)
> +		return -EINVAL;
> +
> +	return 0;
> +}

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