Re: [PATCH 2/7] perf stat: Introduce perf_env__get_cpu_topology() to guard NULL env->cpu

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] Missing bounds validation for the untrusted `sample->cpu` index when accessing the dynamically allocated `env->cpu` array in `machine__resolve()`.
--

commit 50d04a9b8194c04adf66854548c5855bbd4c23f4
Author: Arnaldo Carvalho de Melo <acme@redhat.com>

perf stat: Introduce perf_env__get_cpu_topology() to guard NULL env->cpu

This commit introduces a safe accessor, perf_env__get_cpu_topology(), to
prevent NULL pointer dereferences when accessing env->cpu on older
perf.data files. It also updates several topology aggregation callbacks
in builtin-stat.c to use this new accessor.

> diff --git a/tools/perf/util/env.h b/tools/perf/util/env.h
> index 7621d1f73b83a..7acca39b42ff3 100644
> --- a/tools/perf/util/env.h
> +++ b/tools/perf/util/env.h

[ ... ]

> @@ -187,6 +187,20 @@ const char *perf_env__pmu_mappings(struct perf_env *env);
>  
>  int perf_env__read_cpu_topology_map(struct perf_env *env);
>  
> +/*
> + * Safe accessor for env->cpu[] topology array.  env->cpu can be NULL when
> + * reading old-format perf.data that predates topology information —
> + * process_cpu_topology() in header.c frees it while nr_cpus_avail remains
> + * set, so callers must not index env->cpu[] without this check.
> + */
> +static inline struct cpu_topology_map *
> +perf_env__get_cpu_topology(struct perf_env *env, struct perf_cpu cpu)
> +{
> +	if (env->cpu && cpu.cpu >= 0 && cpu.cpu < env->nr_cpus_avail)
> +		return &env->cpu[cpu.cpu];
> +	return NULL;
> +}

[Severity: High]
This is a pre-existing issue, but I noticed that tools/perf/util/event.c
accesses the dynamically allocated env->cpu array without validation.

In machine__resolve():

	if (al->cpu >= 0) {
		struct perf_env *env = machine->env;

		if (env && env->cpu)
			al->socket = env->cpu[al->cpu].socket_id;
	}

Since al->cpu is derived from the untrusted perf.data file payload,
can this trigger an out-of-bounds heap read on env->cpu[] if the
file provides an artificially large CPU index?

Would it make sense to update machine__resolve() to use this new
perf_env__get_cpu_topology() helper to ensure proper validation against
env->nr_cpus_avail?

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