Re: [PATCH 4/4] perf sched: Fix comp_cpus heap overflow with cross-machine recordings

[Arnaldo Carvalho de Melo <acme@kernel.org>]

On Thu, Jun 04, 2026 at 08:26:05PM +0000, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [Medium] Removing the `sched->max_cpu.cpu` initialization introduces a severe off-by-one error, causing `perf sched map` to truncate the highest CPU from its output and print nothing for single-CPU traces.
> --
> 
> commit 4c1487bde6fa91cfb4bca7de01bfd611065c7c9a
> Author: Arnaldo Carvalho de Melo <acme@redhat.com>
> 
> perf sched: Fix comp_cpus heap overflow with cross-machine recordings
> 
> This commit fixes a heap buffer overflow by allocating comp_cpus with
> MAX_CPUS and the correct element size. It also removes the initial assignment
> of max_cpu from the host's CPU count.
> 
> > diff --git a/tools/perf/builtin-sched.c b/tools/perf/builtin-sched.c
> > index 36da451447b5e..80e42536400a9 100644
> > --- a/tools/perf/builtin-sched.c
> > +++ b/tools/perf/builtin-sched.c
> > @@ -3573,10 +3573,8 @@ static int perf_sched__lat(struct perf_sched *sched)
> >  
> >  static int setup_map_cpus(struct perf_sched *sched)
> >  {
> > -	sched->max_cpu.cpu  = sysconf(_SC_NPROCESSORS_CONF);
> > -
> 
> [Severity: Medium]
> Does removing this initialization cause an off-by-one error when rendering
> the schedule map?
> 
> Without this initialization, sched->max_cpu.cpu tracks the maximum 0-based
> CPU index seen in the trace. However, when compact mode is disabled,
> map_switch_event() assigns this 0-based index directly to cpus_nr:
> 
> tools/perf/builtin-sched.c:map_switch_event() {
>         ...
>         } else
>                 cpus_nr = sched->max_cpu.cpu;
>         ...
> }
> 
> Then print_sched_map() iterates strictly less than cpus_nr:
> 
> tools/perf/builtin-sched.c:print_sched_map() {
>         ...
>         for (int i = 0; i < cpus_nr; i++) {
>         ...
> }
> 
> Will this skip the highest CPU column entirely, and print nothing for 
> single-CPU traces where cpus_nr becomes 0?

Legit, v2 will have a fix for this, the sysconf() was just masking a
pre-existing problem.

- Arnaldo
 
> >  	if (sched->map.comp) {
> > -		sched->map.comp_cpus = calloc(sched->max_cpu.cpu, sizeof(int));
> > +		sched->map.comp_cpus = calloc(MAX_CPUS, sizeof(*sched->map.comp_cpus));
> >  		if (!sched->map.comp_cpus)
> >  			return -1;
> >  	}
> 
> -- 
> Sashiko AI review · https://sashiko.dev/#/patchset/20260604201119.1702338-1-acme@kernel.org?part=4