Re: [PATCH v12 27/33] perf synthetic-events: Bound check when synthesizing mmap2 and build_id events

sashiko-bot
2026-04-13T05:01:01Z

Sashiko AI review found 1 potential issue(s):
- [Low] Validation of `filename_len` in `perf_event__synthesize_build_id` uses the size of an unrelated union member's array.
--

commit ff006c7c376557f4e6b32e50c6f06e3663828f94
Author: Ian Rogers <irogers@google.com>

perf synthetic-events: Bound check when synthesize mmap2 and build_id events

This commit adds bounds checks when synthesizing mmap2 and build_id events
to prevent filename overflows from causing stack corruption.

> diff --git a/tools/perf/util/synthetic-events.c b/tools/perf/util/synthetic-events.c
> --- a/tools/perf/util/synthetic-events.c
> +++ b/tools/perf/util/synthetic-events.c
> @@ -2257,14 +2257,20 @@ int perf_event__synthesize_build_id(const struct perf_tool *tool,
> const char *filename)
> {
> union perf_event ev;
> - size_t len;
> + size_t len, filename_len = strlen(filename);
> u64 sample_type = sample->evsel ? sample->evsel->core.attr.sample_type : 0;
> void *array = &ev;
> int ret;
>
> - len = sizeof(ev.build_id) + strlen(filename) + 1;
> + if (filename_len >= sizeof(ev.mmap2.filename))

This isn't a bug, but is it intentional to use the mmap2 member's filename size
to validate the build_id event's filename?

While this effectively acts as a limit and prevents integer overflow, it
seems a bit unusual to reference an unrelated union member for the size check.
Would it be more clear to use PATH_MAX directly, or check the available space
in the union instead?

> + return -EINVAL;
> +
> + len = sizeof(ev.build_id) + filename_len + 1;

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