Re: [PATCH 03/11] perf tools: Use perf_env__get_cpu_topology() in machine__resolve()

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] Out-of-bounds read on the global `symbol_conf.parallelism_filter` bitmap due to unclamped `al->parallelism`.
--

commit 02d9d799b673cb65dd2a323c5686a0235ee2e563
Author: Arnaldo Carvalho de Melo <acme@redhat.com>

perf tools: Use perf_env__get_cpu_topology() in machine__resolve()

This commit fixes an out-of-bounds heap read in machine__resolve() by properly
validating al->cpu against env->nr_cpus_avail before accessing the CPU
topology. It also adds a bounds check before casting to struct perf_cpu to
prevent silent truncation on large CPU values.

> diff --git a/tools/perf/util/event.c b/tools/perf/util/event.c
> --- a/tools/perf/util/event.c
> +++ b/tools/perf/util/event.c

[ ... ]

> @@ -836,8 +837,18 @@ int machine__resolve(struct machine *machine, struct addr_location *al,
>  	if (al->cpu >= 0) {
>  		struct perf_env *env = machine->env;
>  
> -		if (env && env->cpu)
> -			al->socket = env->cpu[al->cpu].socket_id;
> +		/*
> +		 * Bounds-check al->cpu (s32) before casting to struct perf_cpu
> +		 * (int16_t): without this, e.g. 65536 truncates to 0 and silently
> +		 * returns CPU 0's topology.  Can go once perf_cpu.cpu is widened.
> +		 */
> +		if (env && al->cpu < env->nr_cpus_avail) {
> +			struct cpu_topology_map *topo;
> +
> +			topo = perf_env__get_cpu_topology(env, (struct perf_cpu){ al->cpu });
> +			if (topo)
> +				al->socket = topo->socket_id;
> +		}
>  	}
>  
>  	/* Account for possible out-of-order switch events. */
>  	al->parallelism = max(1, min(machine->parallelism, machine__nr_cpus_avail(machine)));
>  	if (test_bit(al->parallelism, symbol_conf.parallelism_filter))

[Severity: High]
This is a pre-existing issue, but does this test_bit() allow an out-of-bounds
read on the global symbol_conf.parallelism_filter bitmap?

Since al->parallelism is computed using machine__nr_cpus_avail(machine),
which depends on the HEADER_NRCPUS value in the perf.data file, a crafted
file could specify an extremely large number of CPUs.

If numerous PERF_RECORD_SWITCH events artificially inflate
machine->parallelism, the unclamped al->parallelism value could far exceed
the bounds of symbol_conf.parallelism_filter, which is statically sized to
MAX_NR_CPUS + 1 bits (typically 4097 bits).

Could reading so far beyond the mapped boundaries of the global symbol_conf
structure cause a segmentation fault when parsing an untrusted file?

>  		al->filtered |= (1 << HIST_FILTER__PARALLELISM);

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260609010526.1998472-1-acme@kernel.org?part=3