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

sashiko-bot
2026-04-11T07:30:21Z

Sashiko AI review found 1 potential issue(s):
- [Medium] Incomplete bounds checking for CPU indexes (commit message mismatch)
--

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

perf timechart: Bounds check CPU

This commit adds bounds checking for cpu_id in several perf timechart
event handlers to prevent out-of-bounds memory accesses in CPU state
arrays.

> Ensure the CPU is within expected bounds to avoid out of bound memory
> accesses in arrays tracking CPU state.

Does this update catch all of the out-of-bounds accesses in this file?

Looking at the other handlers in builtin-timechart.c, it appears that
process_sample_power_end() was skipped. It passes the unvalidated
sample->cpu directly to c_state_end(), which uses it to index the
fixed-size cpus_cstate_state and cpus_cstate_start_times arrays:

static int
process_sample_power_end(struct timechart *tchart,
struct perf_sample *sample,
const char *backtrace __maybe_unused)
{
c_state_end(tchart, sample->cpu, sample->time);
return 0;
}

Should process_sample_power_end() also bounds-check sample->cpu?

Similarly, process_sample_sched_switch() and process_sample_sched_wakeup()
pass unvalidated sample->cpu values. Could these be used later in cpu2y()
via svg_process() to index the heap-allocated topology_map, allowing
out-of-bounds reads?

Also, does end_sample_processing() need a check against MAX_CPUS?

It loops up to tchart->numcpus (which is populated from the trace file
header) and accesses cpus_pstate_state. If the header specifies more
than 4096 CPUs, could this loop cause a buffer overflow during
finalization?

static void end_sample_processing(struct timechart *tchart)
{
u64 cpu;
struct power_event *pwr;

for (cpu = 0; cpu <= tchart->numcpus; cpu++) {
...
pwr = p_state_end(tchart, cpu, tchart->last_time);
...
}
}

> diff --git a/tools/perf/builtin-timechart.c b/tools/perf/builtin-timechart.c
> index 64f69bd67c6b7..0db2a9bd5b911 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;
> + }
> if (state == (u32)PWR_EVENT_EXIT)
> c_state_end(tchart, cpu_id, sample->time);
> else

[ ... ]

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