Re: [PATCH 2/5] perf tools: Add bounds check to cpu__get_node()

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

On Fri, Jun 05, 2026 at 02:30:00PM +0000, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
> 
> Pre-existing issues:
> - [High] `builtin-kmem` omits the `.attr` callback in its `perf_tool`, which breaks processing of piped streams natively.
> - [High] Integer truncation in `set_max_cpu_num()` combined with missing bounds checks in `cpu__setup_cpunode_map()` causes heap out-of-bounds writes on systems with large sparse CPU topologies.
> - [High] `perf_mmap__aio_bind` triggers an out-of-bounds memory write and subsequent crash when a CPU's NUMA node is unknown or mapped to a dummy CPU.
> --

Thanks, added all these pre-existing conditions for the following
series,

- Arnaldo
 
> commit 84fade77fbbcea06a93ffe9275b1abcc29bbc6bc
> Author: Arnaldo Carvalho de Melo <acme@redhat.com>
> 
> perf tools: Add bounds check to cpu__get_node()
> 
> The patch adds a bounds check against max_cpu_num in cpu__get_node() to
> protect against untrusted or out-of-bounds CPU index values before accessing
> cpunode_map. This prevents out-of-bounds array indexing, particularly when
> analyzing cross-machine perf recordings where the recorded CPU count may
> exceed the host's CPU count.
> 
> > diff --git a/tools/perf/util/cpumap.c b/tools/perf/util/cpumap.c
> > index b1e5c29c6e3ec..d3432622b2adc 100644
> > --- a/tools/perf/util/cpumap.c
> > +++ b/tools/perf/util/cpumap.c
> > @@ -576,6 +576,10 @@ int cpu__get_node(struct perf_cpu cpu)
> >  		return -1;
> >  	}
> >  
> > +	/* cpunode_map allocated for max_cpu_num entries; input may be untrusted */
> > +	if (cpu.cpu < 0 || cpu.cpu >= max_cpu_num.cpu)
> 
> [Severity: High]
> This isn't a bug introduced by this patch, but parsing sysfs to determine
> the maximum CPU number introduces a memory corruption vulnerability.
> 
> In tools/perf/util/cpumap.c:set_max_cpu_num(), the max possible CPU number
> is parsed from sysfs into a local int max. This value is directly assigned to
> max_cpu_num.cpu, which is an int16_t. If max exceeds 32,767 (as is possible on
> large modern architectures or due to sparse APIC numbering), the assignment
> truncates the value, resulting in a small or negative integer.
> 
> Later, init_cpunode_map() allocates cpunode_map based on this truncated size,
> and cpu__setup_cpunode_map() writes to it without truncation:
> 
> tools/perf/util/cpumap.c:cpu__setup_cpunode_map() {
> ...
> 		while ((dent2 = readdir(dir2)) != NULL) {
> 			if (dent2->d_type != DT_LNK || sscanf(dent2->d_name, "cpu%u", &cpu) < 1)
> 				continue;
> 			cpunode_map[cpu] = mem;
> 		}
> ...
> 
> Could this result in an out-of-bounds write on the heap?
> 
> > +		return -1;
> 
> [Severity: High]
> This isn't a bug introduced by this patch, but returning -1 here causes a
> crash in perf_mmap__aio_bind() when a CPU's NUMA node is unknown or mapped to
> a dummy CPU.
> 
> tools/perf/util/mmap.c:perf_mmap__aio_bind() {
> ...
> 		node_index = cpu__get_node(cpu);
> 		node_mask = bitmap_zalloc(node_index + 1);
> 		if (!node_mask) {
> 			pr_err("Failed to allocate node mask for mbind: error %m\n");
> 			return -1;
> 		}
> 		__set_bit(node_index, node_mask);
> ...
> 
> When cpu__get_node() returns -1, it is implicitly cast to unsigned long
> node_index (ULONG_MAX). bitmap_zalloc(ULONG_MAX + 1) evaluates to
> bitmap_zalloc(0) and allocates a minimal buffer.
> 
> Does the subsequent __set_bit(ULONG_MAX, node_mask) perform a wild memory
> write vastly outside the allocated bitmap?
> 
> > +
> >  	return cpunode_map[cpu.cpu];
> >  }
> 
> [Severity: High]
> This isn't a bug introduced by this patch, but while reviewing
> builtin-kmem.c which is mentioned in the commit message as calling
> cpu__get_node(), it appears the perf_tool initialization omits the
> .attr callback:
> 
> tools/perf/builtin-kmem.c:cmd_kmem() {
> ...
> 	perf_tool__init(&perf_kmem, /*ordered_events=*/true);
> 	perf_kmem.sample	= process_sample_event;
> 	perf_kmem.comm		= perf_event__process_comm;
> 	perf_kmem.mmap		= perf_event__process_mmap;
> 	perf_kmem.mmap2		= perf_event__process_mmap2;
> 	perf_kmem.namespaces	= perf_event__process_namespaces;
> ...
> 
> In pipe mode (e.g., perf kmem record -o - | perf kmem stat -i -), perf.data
> headers are missing and events must be processed sequentially. Omitting the
> perf_event__process_attr callback causes perf_event_header_attr events to be
> silently discarded by the default stub.
> 
> Does this prevent the creation of evsels and the initialization of the
> perf_env, completely breaking the tool's ability to process piped data
> streams natively?
> 
> -- 
> Sashiko AI review · https://sashiko.dev/#/patchset/20260605121515.1725549-1-acme@kernel.org?part=2