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

sashiko-bot
2026-04-11T20:02:30Z

Sashiko AI review found 5 potential issue(s):
- [Critical] Out-of-bounds heap read/write in `perf sched timehist` due to unchecked `sample->cpu`.
- [Critical] Negative index and off-by-one bypass in `get_sample_cpu()` allows arbitrary heap read/write.
- [Critical] Unchecked `cpu_id` tracepoint payload leads to out-of-bounds heap writes in `builtin-timechart.c`.
- [High] Incomplete fix leaves `sched:sched_stat_runtime` events vulnerable to DoS via `BUG_ON()`.
- [Medium] Returning -1 for an out-of-bounds CPU fatally aborts the entire perf session with a hidden error message.
--

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

perf sched: Bounds check CPU in sched switch events

Ensure the CPU is within expected bounds to avoid out of bound memory
accesses.

Signed-off-by: Ian Rogers <irogers@google.com>

> 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;
> + }

Does returning -1 here fatally abort the entire perf session instead of just
skipping the corrupted sample?

Since a negative return propagates up and terminates the processing loop,
users might experience a sudden exit. Also, since this uses pr_debug, will
users without verbose mode enabled see a generic abort error without knowing
why the tool failed?

Does this fix also need to cover process_sched_runtime_event() to fully
prevent out of bound accesses?

When handling sched:sched_stat_runtime events, that function passes the
sample directly to latency_runtime_event(), where it extracts sample->cpu
and immediately hits BUG_ON(cpu >= MAX_CPUS || cpu < 0). Processing a
malformed perf.data file could still crash the tool.

Are there similar out-of-bounds regressions in perf sched timehist?

In perf_timehist__process_sample(), sample->cpu is assigned to a 16-bit
integer and passed to get_idle_thread(). Since an input like 0xFFFFFFFF wraps
to -1, it bypasses the upper bounds check and accesses idle_threads[-1],
which could lead to an out-of-bounds read and write.

Additionally, timehist_print_sample() tests the bit using
test_bit(sample->cpu, cpu_bitmap), potentially causing another out-of-bounds
read.

Could get_sample_cpu() in data-convert-bt.c also be bypassed?

The sample->cpu is assigned to a signed integer, meaning 0xFFFFFFFF wraps
to -1 and bypasses the check for cpu > cw->stream_cnt. The check also
evaluates to false if cpu is equal to cw->stream_cnt, creating an off-by-one
bypass.

This allows ctf_stream() to access cw->stream[-1] or
cw->stream[cw->stream_cnt], potentially allocating a new stream
out-of-bounds.

Is builtin-timechart.c similarly vulnerable to unchecked tracepoint payloads?

The process_sample_cpu_idle() and process_sample_cpu_frequency() functions
extract cpu_id and pass it directly to c_state_start() and p_state_change(),
where it is used as an index for static arrays bounded by MAX_CPUS. Supplying
a large cpu_id could result in out-of-bounds writes.

This regression was fixed later in the patch series by commit aa15bc3e9dfa
(perf timechart: Bounds check CPU).

> if (sched->curr_pid[this_cpu] != (u32)-1) {
> /*
> * Are we trying to switch away a PID that is

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