Re: [PATCH v1 23/41] perf stat report: Display metrics for recorded events

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] perf stat: out-of-bounds read in process_stat_round_event() on empty thread map
--

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

perf stat report: Display metrics for recorded events

This commit updates perf stat report to display metrics for recorded events
by inspecting the recorded command line in struct perf_env and adding
support for --all-metrics, -M/--metrics, and --metric-only.

> diff --git a/tools/perf/builtin-stat.c b/tools/perf/builtin-stat.c
> index f81192cec4813..12004244af0e9 100644
> --- a/tools/perf/builtin-stat.c
> +++ b/tools/perf/builtin-stat.c
> @@ -2232,6 +2232,28 @@ static int process_stat_round_event(const struct perf_tool *tool __maybe_unused,
>  	struct perf_env *env = perf_session__env(session);
>  	const char **argv = env->cmdline_argv;
>  	int argc = env->nr_cmdline;
> +	static bool metrics_added;
> +
> +	if (!metrics_added) {
> +		struct perf_thread_map *threads = evlist__core(session->evlist)->threads;
> +		bool system_wide = !threads || perf_thread_map__pid(threads, /*idx=*/0) == -1;

[Severity: Medium]
Can perf_thread_map__pid(threads, 0) read out of bounds on map->map[0].pid if
a crafted or corrupted perf.data file contains a PERF_RECORD_THREAD_MAP event
with nr == 0?

When perf_session__process_user_event() validates PERF_RECORD_THREAD_MAP, it
permits event->thread_map.nr == 0:

tools/perf/builtin-stat.c:process_thread_map_event()
  -> thread_map__new_event()
     -> perf_thread_map__realloc(NULL, 0)

This allocates a non-NULL struct perf_thread_map with threads->nr == 0 and
zero elements in its trailing flexible array map[], which set_maps() attaches
to evlist__core(evsel_list)->threads.

When process_stat_round_event() subsequently runs, threads is non-NULL, so
perf_thread_map__pid(threads, 0) in tools/lib/perf/threadmap.c dereferences
map->map[0].pid past the end of the allocation.

Should this also check perf_thread_map__nr(threads) == 0 (or
perf_thread_map__Empty(threads)) before calling perf_thread_map__pid()?

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