Re: [PATCH v4 4/4] perf synthetic-events: Fix bounds and union member access in mmap2 build_id synthesis

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] Latent buffer overflow and potential FORTIFY_SOURCE abort due to incorrect string length bounds check against union capacity instead of array size.
--

--- Patch [4]: [PATCH v4 4/4] perf synthetic-events: Fix bounds and union member access in mmap2 build_id synthesis ---
commit a02cc1d93b140eea5444d6b7ee95e98c02c00905
Author: Ian Rogers <irogers@google.com>

perf synthetic-events: Fix bounds and union member access in mmap2 build_id synthesis

This commit modifies bounds and union member access in mmap2 build_id
synthesis. It replaces the previous array-based bounds check with a new
calculation based on the size of the event union.

> diff --git a/tools/perf/util/synthetic-events.c b/tools/perf/util/synthetic-events.c
> index 63c9c85b02f37..8928c6964fc9d 100644
> --- a/tools/perf/util/synthetic-events.c
> +++ b/tools/perf/util/synthetic-events.c
[ ... ]
> @@ -2448,13 +2448,18 @@ int perf_event__synthesize_mmap2_build_id(const struct perf_tool *tool,
>  	size_t filename_len = strlen(filename);
>  	size_t ev_len;
>  	u64 sample_type = sample->evsel ? sample->evsel->core.attr.sample_type : 0;
> -	void *array;
> +	void *array = &ev;
>  	int ret;
> +	size_t max_filename_len;
>  
> -	if (filename_len >= sizeof(ev.mmap2.filename))
> -		return -EINVAL;
> +	max_filename_len = sizeof(ev) -
> +			   (MAX_ID_HDR_ENTRIES * sizeof(__u64)) -
> +			   offsetof(struct perf_record_mmap2, filename) - 1;

[Severity: Medium]
Will this calculation cause an issue if the size of the union perf_event
ever grows larger than struct perf_record_mmap2?

Since max_filename_len is derived from sizeof(ev) rather than the capacity of
the ev.mmap2.filename array, any future expansion of the union could cause
max_filename_len to exceed the actual array bounds.

>  
> -	ev_len = sizeof(ev.mmap2) - sizeof(ev.mmap2.filename) + filename_len + 1;
> +	if (filename_len > max_filename_len)
> +		filename_len = max_filename_len;
> +
> +	ev_len = offsetof(struct perf_record_mmap2, filename) + filename_len + 1;
>  	ev_len = PERF_ALIGN(ev_len, sizeof(u64));
[ ... ]
> @@ -2474,16 +2479,15 @@ int perf_event__synthesize_mmap2_build_id(const struct perf_tool *tool,
[ ... ]
> -	memcpy(ev.mmap2.filename, filename, min(strlen(filename), sizeof(ev.mmap.filename)));
> +	strlcpy(ev.mmap2.filename, filename, filename_len + 1);

[Severity: Medium]
If filename_len is bounded by sizeof(ev) and the union expands, could this
strlcpy write past the ev.mmap2.filename array capacity and trigger a
_FORTIFY_SOURCE abort?

When built with _FORTIFY_SOURCE=2, bounds checks evaluate against the sub-object
array capacity (ev.mmap2.filename) rather than the enclosing struct.

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