Re: [PATCH v8 25/29] perf sched: Bounds check CPU in sched switch events

sashiko-bot
2026-04-11T07:42:08Z

Sashiko AI review found 2 potential issue(s):
- [High] Missing CPU bounds check in the 'timehist' subcommand's sched switch and wakeup event handlers leads to out-of-bounds memory accesses.
- [Medium] Returning -1 on an out-of-bounds CPU aborts the entire trace processing session without an explicit error message.
--

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

perf sched: Bounds check CPU in sched switch events

This commit adds a bounds check on the CPU value extracted from perf samples
in the sched switch event handler. It prevents out-of-bounds array accesses
by checking if the CPU exceeds MAX_CPUS.

> diff --git a/tools/perf/builtin-sched.c b/tools/perf/builtin-sched.c
> index 45439e2af8ffa..6d0655d56e269 100644
> --- a/tools/perf/builtin-sched.c
> +++ b/tools/perf/builtin-sched.c
> @@ -1791,6 +1791,11 @@ static int process_sched_switch_event(const struct perf_tool *tool,
> u32 prev_pid = perf_sample__intval(sample, "prev_pid"),
> next_pid = perf_sample__intval(sample, "next_pid");
>
> + if (this_cpu < 0 || this_cpu >= MAX_CPUS) {
> + pr_debug("Out-of-bound sample CPU %d\n", this_cpu);
> + return -1;

Returning -1 here aborts the entire trace processing session. Because the
out-of-bounds condition is only logged via pr_debug(), normal users will
not see any explanation for the failure, and the command will simply exit
with a generic error message.

Could we either gracefully skip the unsupported sample by returning 0
(similar to the established pattern in replay_switch_event()), or use
pr_err() to explicitly inform the user why the session is being aborted?

Additionally, do the timehist subcommand event handlers also need CPU
bounds checks?

Looking at timehist_sched_switch_event() and timehist_sched_wakeup_event(),
they pass the unvalidated sample->cpu (a u32) to get_idle_thread(int cpu).
If the trace contains a corrupted value like 0xFFFFFFFF, it is cast to -1.

Since cpu is a signed int, the reallocation check bypasses reallocation if
idle_threads is already initialized (because -1 >= idle_max_cpu is false).
The code then attempts to read/write idle_threads[-1].

Can this result in a negative out-of-bounds memory access that crashes the
tool or causes memory corruption?

> + }
> +
> if (sched->curr_pid[this_cpu] != (u32)-1) {

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