Re: [PATCH v11 30/33] perf timechart: Bounds check CPU

sashiko-bot
2026-04-13T02:07:09Z

Sashiko AI review found 2 potential issue(s):
- [High] Returning `-1` on out-of-bounds CPU IDs causes the entire perf session processing to abort silently.
- [High] Out-of-bounds read on the `topology_map` array when offline CPUs are present.
--

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

perf timechart: Bounds check CPU

This commit adds bounds checking for CPU IDs across multiple sample
processing callbacks in perf timechart to prevent out-of-bounds memory
accesses when updating CPU state arrays.

> diff --git a/tools/perf/builtin-timechart.c b/tools/perf/builtin-timechart.c
> index 64f69bd67c6b7..64903b54e8a73 100644
> --- a/tools/perf/builtin-timechart.c
> +++ b/tools/perf/builtin-timechart.c

[ ... ]

> @@ -607,6 +607,10 @@ process_sample_cpu_idle(struct timechart *tchart __maybe_unused,
> u32 state = perf_sample__intval(sample, "state");
> u32 cpu_id = perf_sample__intval(sample, "cpu_id");
>
> + if (cpu_id >= MAX_CPUS) {
> + pr_debug("Out-of-bounds cpu_id %u\n", cpu_id);
> + return -1;

Does returning -1 here cause the entire perf session processing to abort?

In the perf tool subsystem, returning a negative error code from a trace
handler propagates up to perf_session__process_events(), which aborts
parsing the rest of the trace file.

Should this log a warning and return 0 instead to skip the invalid sample,
so that a single corrupted sample or an unsupported high CPU ID does not
prevent the generation of SVG output for the valid samples?

Additionally, using pr_debug() hides the out-of-bounds notification from
the user unless verbose mode is enabled.

[ ... ]

> @@ -1517,6 +1544,8 @@ static int process_header(struct perf_file_section *section __maybe_unused,
> switch (feat) {
> case HEADER_NRCPUS:
> tchart->numcpus = ph->env.nr_cpus_avail;
> + if (tchart->numcpus > MAX_CPUS)
> + tchart->numcpus = MAX_CPUS;
> break;
>
> case HEADER_CPU_TOPOLOGY:

Does capping tchart->numcpus here fully prevent out-of-bounds reads when
offline CPUs are present?

While this bounds tchart->numcpus to MAX_CPUS, tchart->numcpus is
initialized to ph->env.nr_cpus_avail. The topology_map is allocated in
svg_build_topology_map() based on env->nr_cpus_online. On systems with
offline CPUs, nr_cpus_avail is strictly greater than nr_cpus_online.

Will iterating up to tchart->numcpus during rendering result in cpu2y()
accessing the topology_map[] array out of bounds?

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