Re: [RFC PATCH v1 2/8] perf/core: Add BUILD_ID_OFFSET to UAPI

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [High] The commit introduces `PERF_SAMPLE_BUILD_ID_OFFSET` and `PERF_SAMPLE_CALLCHAIN_BUILD_ID_OFFSET` to the UAPI and bumps `PERF_SAMPLE_MAX`, but fails to implement the corresponding sample generation logic in the core kernel.
- [Medium] The UAPI header update is applied inconsistently between `include/` and `tools/include/`, causing divergence and leaving the canonical kernel header undocumented.
- [Medium] The ABI documentation comment references `struct build_id`, an undeclared structure in the canonical UAPI headers, resulting in an opaque and confusing binary layout.
--

--- Patch [2]: [RFC PATCH v1 2/8] perf/core: Add BUILD_ID_OFFSET to UAPI ---
commit 16454ab98fa038eb527938712933e7e488db3d27
Author: Ian Rogers <irogers@google.com>

perf/core: Add BUILD_ID_OFFSET to UAPI
    
This commit introduces PERF_SAMPLE_BUILD_ID_OFFSET and
PERF_SAMPLE_CALLCHAIN_BUILD_ID_OFFSET to the UAPI and bumps the internal
validation limit.

> diff --git a/include/uapi/linux/perf_event.h b/include/uapi/linux/perf_event.h
> index fd10aa8d697f2..f0712c1471d3d 100644
> --- a/include/uapi/linux/perf_event.h
> +++ b/include/uapi/linux/perf_event.h
> @@ -165,8 +165,10 @@ enum perf_event_sample_format {
>  	PERF_SAMPLE_DATA_PAGE_SIZE		= 1U << 22,
>  	PERF_SAMPLE_CODE_PAGE_SIZE		= 1U << 23,
>  	PERF_SAMPLE_WEIGHT_STRUCT		= 1U << 24,
> +	PERF_SAMPLE_BUILD_ID_OFFSET		= 1U << 25,
> +	PERF_SAMPLE_CALLCHAIN_BUILD_ID_OFFSET	= 1U << 26,
>  
> -	PERF_SAMPLE_MAX = 1U << 25,		/* non-ABI */
> +	PERF_SAMPLE_MAX = 1U << 27,		/* non-ABI */
>  };

[Severity: High]
Since PERF_SAMPLE_MAX is increased, perf_copy_attr() will now accept these
new sample types from userspace. However, is there missing core kernel logic
to actually process these bits?

Looking at the core kernel code, perf_output_sample() and
__perf_event_header_size() appear to lack handlers for the new bits. If a
userspace application requests these fields, it seems the kernel will not
allocate space for them or write them to the ring buffer.

When the userspace parser subsequently decodes the stream based on the
attr.sample_type bitmask, won't it read past the end of the truncated sample
and misinterpret the next event header as data?

> diff --git a/tools/include/uapi/linux/perf_event.h b/tools/include/uapi/linux/perf_event.h
> index 2479a38883d00..d5660e2d6b603 100644
> --- a/tools/include/uapi/linux/perf_event.h
> +++ b/tools/include/uapi/linux/perf_event.h

[ ... ]

> @@ -1054,6 +1056,13 @@ enum perf_event_type {
>  	 *	{ u64			code_page_size;} && PERF_SAMPLE_CODE_PAGE_SIZE
>  	 *	{ u64			size;
>  	 *	  char			data[size]; } && PERF_SAMPLE_AUX
> +	 *	{ struct build_id	bid;
> +	 *	  u64			offset; } && PERF_SAMPLE_BUILD_ID_OFFSET

[Severity: Medium]
It looks like this payload documentation block was only added to the tools
side header (tools/include/uapi/linux/perf_event.h). Was it intended to be
added to the canonical include/uapi/linux/perf_event.h as well?

Missing the canonical header causes divergence between the kernel and tools
headers, which might trigger check-headers.sh warnings and leave users relying
on the kernel headers without documentation for the new ABI.

[Severity: Medium]
Also, regarding the struct build_id payload documentation here, this structure
appears to be undeclared in the canonical kernel UAPI header.

Does the use of struct build_id here collide with the internal tools structure
of the same name? The internal tools/perf/util/build-id.h structure has a
layout of:

  u8 data[20];
  u8 size;

Whereas the ABI typically uses a different layout (e.g., u8 size; u8 res1;
u16 res2; u8 data[20];). Could this cause layout confusion for tool developers
attempting to parse the ABI?

> +	 *	{ u64			nr;
> +	 *	  struct {
> +	 *		struct build_id	bid;
> +	 *		u64 offset;
> +	 *	  }[nr]; } && PERF_SAMPLE_CALLCHAIN_BUILD_ID_OFFSET
>  	 * };
>  	 */
>  	PERF_RECORD_SAMPLE			= 9,

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260807071818.718751-1-irogers@google.com?part=2