Re: [PATCH v1 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] The newly introduced `max_filename_len` clamping mechanism fails to prevent `-E2BIG` errors for long filenames because it does not account for alignment padding and the NUL byte.
--

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

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

Fix a critical logic bug in perf_event__synthesize_mmap2_build_id() where
the wrong union member structure size and offset boundaries were utilized.

> diff --git a/tools/perf/util/synthetic-events.c b/tools/perf/util/synthetic-events.c
> index 043f902f1bd2e..3a4030835a320 100644
> --- a/tools/perf/util/synthetic-events.c
> +++ b/tools/perf/util/synthetic-events.c
> @@ -2441,13 +2441,16 @@ 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.mmap2.filename) - (MAX_ID_HDR_ENTRIES * sizeof(__u64));
>  
> -	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));
>  
>  	if (ev_len + MAX_ID_HDR_ENTRIES * sizeof(__u64) > sizeof(ev))
>  		return -E2BIG;

[Severity: Medium]
Does this clamping logic fail to prevent -E2BIG errors for very long
filenames?

If filename_len is clamped to max_filename_len (4048 bytes), ev_len is
subsequently calculated as offsetof(struct perf_record_mmap2, filename) +
4048 + 1 (for the NUL byte), which equals 4129.

PERF_ALIGN(4129, sizeof(u64)) aligns this up to 4136.

When checking if the padded length plus the ID header trailer exceeds the
maximum event size:
4136 + 48 = 4184, which exceeds sizeof(ev) (4176).

This causes the function to return -E2BIG instead of successfully truncating
and processing the event. Should max_filename_len account for the NUL byte
and the 8-byte alignment padding to ensure the clamped size fits within
the boundaries?

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