Re: [Patch v8 12/23] 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:
- [Critical] Missing bounds checking on user-provided SIMD `qwords` fields allows an integer overflow of the 16-bit perf event header size, leading to ring buffer corruption and potential NMI lockups.
- [High] SIMD register parameters are evaluated and validated outside of their respective flag gates, which can falsely reject valid syscalls due to uninitialized memory.
--

commit eeb01562d09b920ab2f41841dd6b7a879ed10465
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 struct perf_event_attr. It adds a new ABI
format to dump the width and values of the requested SIMD registers, using
bitmasks and qword sizes configured by the user.

> diff --git a/kernel/events/core.c b/kernel/events/core.c
> index 2ce553db4dcb8..94bb034da9b9d 100644
> --- a/kernel/events/core.c
> +++ b/kernel/events/core.c

[ ... ]

> @@ -8596,6 +8664,33 @@ static __always_inline u64 __cond_set(u64 flags, u64 s, u64 d)
>  	return d * !!(flags & s);
>  }
>  
> +static u64 perf_update_xregs_size(struct perf_event *event, bool intr)
> +{
> +	u16 pred_qwords = event->attr.sample_simd_pred_reg_qwords;
> +	u16 vec_qwords = event->attr.sample_simd_vec_reg_qwords;
> +	u64 pred_mask;
> +	u64 mask;
> +	int size;
> +
> +	if (intr) {
> +		mask = event->attr.sample_simd_vec_reg_intr;
> +		pred_mask = event->attr.sample_simd_pred_reg_intr;
> +	} else {
> +		mask = event->attr.sample_simd_vec_reg_user;
> +		pred_mask = event->attr.sample_simd_pred_reg_user;
> +	}
> +
> +	/* nr_vectors, vector_qwords, nr_pred, pred_qwords */
> +	size = sizeof(u64) * 4;
> +	size += (hweight64(mask) * vec_qwords +
> +		 hweight64(pred_mask) * pred_qwords) * sizeof(u64);
> +

[Severity: Critical]
Is there any bounds checking on the user-provided vec_qwords and pred_qwords
fields here?

If userspace provides large values, could the multiplication with the bitmask
weights overflow the 16-bit perf event header size? 

If the size overflows, would perf_output_begin() allocate a smaller ring buffer
space than what perf_output_sample_simd_regs() subsequently writes, leading to
an out-of-bounds memory regression in NMI context?

> +	/* Warn if exceeding perf_event_header.size (u16). */
> +	WARN_ON_ONCE(size > U16_MAX);
> +
> +	return size;
> +}

[ ... ]

> @@ -13634,6 +13746,15 @@ 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_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: High]
When attr->sample_type & PERF_SAMPLE_REGS_USER is true but
PERF_SAMPLE_REGS_INTR is not set, does this validate the _intr fields that
might contain uninitialized garbage from userspace?

Since flag-gated syscall parameters are not required to be zero-filled when
the flag is absent, could evaluating these parameters outside their respective
flag gates falsely reject valid syscalls due to uninitialized memory?

[ ... ]

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