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

[sashiko-bot@kernel.org]

Sashiko AI review found 3 potential issue(s):
- [High] Unvalidated CPU values in `timehist` handlers cause out-of-bounds heap read/write
- [Medium] Missing bounds check in `process_sched_runtime_event` leads to tool crash via `BUG_ON`
- [Medium] Returning -1 for out-of-bounds CPUs causes premature abortion of the entire perf session
--

commit 063652e61f7d3a2993ab963919bbc194b73f6f39
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 in sched switch events
to prevent out of bound memory accesses when parsing samples.

> 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 cause a regression where the entire perf session
parsing aborts?

If a trace is recorded on a system with more than 4096 CPUs, returning a
negative value bubbles up to perf_session__process_events(), halting trace
parsing completely.

Would returning 0 gracefully skip the unsupported sample instead?

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

Since we are bounds checking process_sched_switch_event(), do we also need
similar protection in process_sched_runtime_event()?

If a malformed sched:sched_stat_runtime event contains an out-of-bounds CPU,
it is passed to latency_runtime_event() which executes
BUG_ON(cpu >= MAX_CPUS || cpu < 0). Could this crash the perf tool?

Similarly, do timehist handlers like timehist_sched_change_event()
need this bounds check to prevent out-of-bounds regressions?

If a corrupted trace provides sample->cpu = 0xFFFFFFFF, it gets passed to
get_idle_thread(int cpu) and cast to -1. This bypasses the expansion check
(-1 >= idle_max_cpu is false) and accesses idle_threads[-1].

Also, evsel__save_time() calculates __roundup_pow_of_two(cpu + 1),
which can overflow to 0, resulting in a realloc(..., 0) (effectively a free)
and an out-of-bounds write at last_time[cpu].

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