Re: [Patch v11 04/10] perf headers: Sync perf_event.h/perf_regs.h with the kernel headers

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] Sample parser in ASLR tool fails to consume the new SIMD register payload when PERF_SAMPLE_REGS_ABI_SIMD is set, leading to stream desynchronization.
- [Low] perf_event_attr__fprintf() prints the newly added SIMD fields out of struct layout order.
--

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

perf headers: Sync perf_event.h/perf_regs.h with the kernel headers

Sync the UAPI header changes of supporting SIMD/eGPRs/SSP sampling into
corresponding tools UAPI headers. Additionally, support the newly introduced
perf_event_attr fields in the perf_event_attr__fprintf and
perf_event__attr_swap() helpers.

> diff --git a/tools/include/uapi/linux/perf_event.h b/tools/include/uapi/linux/perf_event.h
> index fd10aa8d697f2..c49fc76292f79 100644
> --- a/tools/include/uapi/linux/perf_event.h
> +++ b/tools/include/uapi/linux/perf_event.h

[ ... ]

> @@ -1020,7 +1045,15 @@ enum perf_event_type {
>  	 *      } && PERF_SAMPLE_BRANCH_STACK
>  	 *
>  	 *	{ u64			abi; # enum perf_sample_regs_abi
> -	 *	  u64			regs[weight(mask)]; } && PERF_SAMPLE_REGS_USER
> +	 *	  u64			regs[weight(mask)];
> +	 *	  struct {
> +	 *		u64 nr_vectors;		# 0 ... weight(sample_simd_vec_reg_user)
> +	 *		u64 vector_qwords;	# 0 ... sample_simd_vec_reg_qwords
> +	 *		u64 nr_pred;		# 0 ... weight(sample_simd_pred_reg_user)
> +	 *		u64 pred_qwords;	# 0 ... sample_simd_pred_reg_qwords
> +	 *		u64 data[nr_vectors * vector_qwords + nr_pred * pred_qwords];
> +	 *	  } && (abi & PERF_SAMPLE_REGS_ABI_SIMD)
> +	 *	} && PERF_SAMPLE_REGS_USER
>  	 *

[Severity: High]
Does this new ABI payload cause stream desynchronization in
tools/perf/util/aslr.c?

When PERF_SAMPLE_REGS_ABI_SIMD is set, aslr_tool__process_sample() fails to 
advance the payload pointer over the extended SIMD struct:

tools/perf/util/aslr.c:aslr_tool__process_sample() {
	...
	if (orig_sample_type & PERF_SAMPLE_REGS_USER) {
		u64 abi;

		if (CHECK_BOUNDS(1, 0)) {
			ret = -EFAULT;
			goto out_put;
		}
		abi = in_array[i++];
		if (abi != PERF_SAMPLE_REGS_ABI_NONE) {
			u64 nr = hweight64(orig_regs_user);

			if (nr > max_i - i) {
				ret = -EFAULT;
				goto out_put;
			}
			i += nr;
		}
	}
	...
}

It appears the function skips standard registers by adding
hweight64(orig_regs_user) to the parser index, but misses parsing or skipping
the dynamically sized SIMD payload that follows. Can this leave unparsed
SIMD data in the stream, potentially corrupting subsequent fields?

A similar regression is present in the PERF_SAMPLE_REGS_INTR handling within
the same function.

Note that while other parsers like evsel.c were fixed in subsequent patches,
aslr.c appears to remain broken at the end of the series.

[ ... ]

> diff --git a/tools/perf/util/perf_event_attr_fprintf.c b/tools/perf/util/perf_event_attr_fprintf.c
> index cc817294ad3d6..2e8be9a357dce 100644
> --- a/tools/perf/util/perf_event_attr_fprintf.c
> +++ b/tools/perf/util/perf_event_attr_fprintf.c
> @@ -407,8 +407,17 @@ int perf_event_attr__fprintf(FILE *fp, struct perf_event_attr *attr,
>  	}
>  	if (attr_size >= offsetof(struct perf_event_attr, config4))
>  		PRINT_ATTRf(config3, p_hex);
> -	if (attr_size > offsetof(struct perf_event_attr, config4))
> +	if (attr_size >= offsetof(struct perf_event_attr, sample_simd_regs_enabled))
>  		PRINT_ATTRf(config4, p_hex);
> +	if (attr_size > offsetof(struct perf_event_attr, sample_simd_vec_reg_user)) {
> +		PRINT_ATTRf(sample_simd_regs_enabled, p_unsigned);
> +		PRINT_ATTRf(sample_simd_pred_reg_qwords, p_unsigned);
> +		PRINT_ATTRf(sample_simd_pred_reg_intr, p_hex);
> +		PRINT_ATTRf(sample_simd_pred_reg_user, p_hex);
> +		PRINT_ATTRf(sample_simd_vec_reg_qwords, p_unsigned);
> +		PRINT_ATTRf(sample_simd_vec_reg_intr, p_hex);
> +		PRINT_ATTRf(sample_simd_vec_reg_user, p_hex);
> +	}

[Severity: Low]
Does this print the newly added SIMD fields out of struct layout order?

The UAPI struct definition in tools/include/uapi/linux/perf_event.h places
sample_simd_vec_reg_qwords before sample_simd_pred_reg_intr:

tools/include/uapi/linux/perf_event.h:struct perf_event_attr {
	...
	__u16	sample_simd_regs_enabled;
	__u16	sample_simd_pred_reg_qwords;
	__u16	sample_simd_vec_reg_qwords;
	__u16	__reserved_4;

	__u32	sample_simd_pred_reg_intr;
	__u32	sample_simd_pred_reg_user;
	...
}

However, the fprintf code above prints sample_simd_vec_reg_qwords after both
sample_simd_pred_reg_intr and sample_simd_pred_reg_user. Could this cause
minor confusion when debugging raw perf_event_attr fields?

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