Re: [PATCH v9 26/29] perf timechart: Bounds check CPU

sashiko-bot
2026-04-11T19:54:54Z

Sashiko AI review found 2 potential issue(s):
- [High] Off-by-one heap out-of-bounds read in `end_sample_processing`.
- [High] Unbounded loop in `write_svg_file()` leading to arbitrary heap out-of-bounds read on `topology_map`.
--

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

perf timechart: Bounds check CPU

This commit adds bounds checking for CPU IDs during sample processing.
It limits the maximum CPU index to MAX_CPUS to prevent out-of-bounds
memory accesses in the arrays tracking CPU states.

> diff --git a/tools/perf/builtin-timechart.c b/tools/perf/builtin-timechart.c
> index 64f69bd67c6b7..64ff998c597a7 100644
> --- a/tools/perf/builtin-timechart.c
> +++ b/tools/perf/builtin-timechart.c
[ ... ]
> @@ -697,7 +725,7 @@ static void end_sample_processing(struct timechart *tchart)
> u64 cpu;
> struct power_event *pwr;
>
> - for (cpu = 0; cpu <= tchart->numcpus; cpu++) {
> + for (cpu = 0; cpu <= min_t(unsigned int, tchart->numcpus, MAX_CPUS); cpu++) {

Does this loop have an off-by-one regression?

If tchart->numcpus is greater than or equal to MAX_CPUS, min_t() returns
MAX_CPUS. Because the loop uses the <= operator, it will execute a final
iteration with cpu == MAX_CPUS.

Inside the loop, p_state_end() is called, which executes
pwr->state = cpus_pstate_state[cpu].

Since cpus_pstate_state is allocated with exactly MAX_CPUS elements,
accessing index MAX_CPUS performs a heap out-of-bounds read. This
uninitialized memory could then be leaked into the generated SVG output.

Could the upper bound be adjusted to MAX_CPUS - 1?

Is there an unbounded loop regression in write_svg_file() that can cause an
out-of-bounds access?

tchart->numcpus is initialized directly from the perf.data file header
without validation. In write_svg_file(), the code iterates up to
tchart->numcpus and calls svg_cpu_box(i):

tools/perf/builtin-timechart.c:write_svg_file() {
...
for (i = 0; i < tchart->numcpus; i++)
svg_cpu_box(i, tchart->max_freq, tchart->turbo_frequency);
...
}

The svg_cpu_box() function accesses the heap-allocated topology_map array,
which is sized based on env->nr_cpus_online (capped at MAX_NR_CPUS).
If a perf.data file sets nr_cpus_online to 1 but nr_cpus_avail to a very
large number, could this loop read past the end of the topology_map
allocation and cause a segmentation fault?

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