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

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

Sashiko AI review found 1 potential issue(s):
- [Medium] Returning -1 for out-of-bounds CPUs aborts trace processing entirely instead of gracefully skipping the sample.
--

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

perf timechart: Bounds check CPU

This commit adds bounds checks for the CPU ID in various sample processing
functions in the timechart tool to prevent out-of-bounds memory accesses.

> 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 abort the entire trace processing?

These handlers are called via evsel->handler() during event processing.
Returning a negative value from a tracepoint handler typically propagates up
and causes the perf session to immediately stop parsing the trace file.

Would it be better to return 0 here to gracefully skip the out-of-bounds
sample, allowing the rest of the trace to be processed?

> + }
> if (state == (u32)PWR_EVENT_EXIT)
> c_state_end(tchart, cpu_id, sample->time);
> else

[ ... ]

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