Re: [PATCH v11 29/33] perf sched: Bounds check CPU in sched switch events

sashiko-bot
2026-04-13T02:30:10Z

Sashiko AI review found 2 potential issue(s):
- [Medium] Un-ratelimited warnings inside the per-sample processing loop can cause severe console spam and performance degradation.
- [Low] Type mismatch and format string errors cause compiler warnings that break the build under `-Werror`.
--

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

perf sched: Bounds check CPU in sched switch events

This commit adds bounds checking for the CPU index in various perf sched
event handlers to prevent out-of-bounds memory accesses.

> diff --git a/tools/perf/builtin-sched.c b/tools/perf/builtin-sched.c
> --- 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_warning("Out-of-bound sample CPU %d. Skipping sample\n", this_cpu);

Could printing a warning for every out-of-bounds sample cause severe
console spam and bottleneck trace processing?

Since these functions are called per-sample in the perf.data file, a trace
with corrupted data or recorded on a system with more than MAX_CPUS cores
might print millions of lines.

Would it be better to rate-limit this warning, aggregate the skipped counts,
or lower the log level to pr_debug()?

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

[ ... ]

> @@ -1813,6 +1818,11 @@ static int process_sched_runtime_event(const struct perf_tool *tool,
> {
> struct perf_sched *sched = container_of(tool, struct perf_sched, tool);
>
> + if (sample->cpu < 0 || sample->cpu >= MAX_CPUS) {
> + pr_warning("Out-of-bound sample CPU %d. Skipping sample\n", sample->cpu);

Since sample->cpu is an unsigned 32-bit integer, will the sample->cpu < 0
condition evaluate to always false and trigger a compiler warning?

Additionally, does using the %d format specifier for this unsigned value
trigger a format string warning, potentially breaking the build under -Werror?

> + return 0;
> + }
> +
> if (sched->tp_handler->runtime_event)

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