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

process_cpu_topology() in header.c frees env->cpu on old-format
perf.data files that predate topology information, but leaves
nr_cpus_avail set.  The six perf_env__get_*_aggr_by_cpu() functions
in builtin-stat.c pass the bounds check but dereference a NULL
env->cpu pointer, crashing on old recordings.

Introduce perf_env__get_cpu_topology() as a safe accessor that
validates env->cpu, cpu.cpu >= 0, and cpu.cpu < nr_cpus_avail in
one place, returning a struct cpu_topology_map pointer or NULL.
Convert all six topology aggregation callbacks to use it.

Fixes: 88031a0de7d68d13 ("perf stat: Switch to cpu version of cpu_map__get()")
Reported-by: sashiko-bot <sashiko-bot@kernel.org>
Cc: Ian Rogers <irogers@google.com>
Assisted-by: Claude:claude-opus-4.6
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>

diff --git a/tools/perf/builtin-stat.c b/tools/perf/builtin-stat.c
index 9a045811c4197ccdbff2596d3cb87a9700b8b331..a04466ea3b0a0657fafc0ba4ed7b517ed74da837 100644 (file)
--- a/tools/perf/builtin-stat.c
+++ b/tools/perf/builtin-stat.c
@@ -1637,10 +1637,10 @@ static struct aggr_cpu_id perf_env__get_socket_aggr_by_cpu(struct perf_cpu cpu,
 {
        struct perf_env *env = data;
        struct aggr_cpu_id id = aggr_cpu_id__empty();
+       struct cpu_topology_map *topo = perf_env__get_cpu_topology(env, cpu);
 
-       /* env->cpu[] has env->nr_cpus_avail entries; reject untrusted indices */
-       if (cpu.cpu >= 0 && cpu.cpu < env->nr_cpus_avail)
-               id.socket = env->cpu[cpu.cpu].socket_id;
+       if (topo)
+               id.socket = topo->socket_id;
 
        return id;
 }
@@ -1649,15 +1649,16 @@ static struct aggr_cpu_id perf_env__get_die_aggr_by_cpu(struct perf_cpu cpu, voi
 {
        struct perf_env *env = data;
        struct aggr_cpu_id id = aggr_cpu_id__empty();
+       struct cpu_topology_map *topo = perf_env__get_cpu_topology(env, cpu);
 
-       if (cpu.cpu >= 0 && cpu.cpu < env->nr_cpus_avail) {
+       if (topo) {
                /*
                 * die_id is relative to socket, so start
                 * with the socket ID and then add die to
                 * make a unique ID.
                 */
-               id.socket = env->cpu[cpu.cpu].socket_id;
-               id.die = env->cpu[cpu.cpu].die_id;
+               id.socket = topo->socket_id;
+               id.die = topo->die_id;
        }
 
        return id;
@@ -1705,12 +1706,13 @@ static struct aggr_cpu_id perf_env__get_cache_aggr_by_cpu(struct perf_cpu cpu,
 {
        struct perf_env *env = data;
        struct aggr_cpu_id id = aggr_cpu_id__empty();
+       struct cpu_topology_map *topo = perf_env__get_cpu_topology(env, cpu);
 
-       if (cpu.cpu >= 0 && cpu.cpu < env->nr_cpus_avail) {
+       if (topo) {
                u32 cache_level = (perf_stat.aggr_level) ?: stat_config.aggr_level;
 
-               id.socket = env->cpu[cpu.cpu].socket_id;
-               id.die = env->cpu[cpu.cpu].die_id;
+               id.socket = topo->socket_id;
+               id.die = topo->die_id;
                perf_env__get_cache_id_for_cpu(cpu, env, cache_level, &id);
        }
 
@@ -1722,11 +1724,12 @@ static struct aggr_cpu_id perf_env__get_cluster_aggr_by_cpu(struct perf_cpu cpu,
 {
        struct perf_env *env = data;
        struct aggr_cpu_id id = aggr_cpu_id__empty();
+       struct cpu_topology_map *topo = perf_env__get_cpu_topology(env, cpu);
 
-       if (cpu.cpu >= 0 && cpu.cpu < env->nr_cpus_avail) {
-               id.socket = env->cpu[cpu.cpu].socket_id;
-               id.die = env->cpu[cpu.cpu].die_id;
-               id.cluster = env->cpu[cpu.cpu].cluster_id;
+       if (topo) {
+               id.socket = topo->socket_id;
+               id.die = topo->die_id;
+               id.cluster = topo->cluster_id;
        }
 
        return id;
@@ -1736,16 +1739,17 @@ static struct aggr_cpu_id perf_env__get_core_aggr_by_cpu(struct perf_cpu cpu, vo
 {
        struct perf_env *env = data;
        struct aggr_cpu_id id = aggr_cpu_id__empty();
+       struct cpu_topology_map *topo = perf_env__get_cpu_topology(env, cpu);
 
-       if (cpu.cpu >= 0 && cpu.cpu < env->nr_cpus_avail) {
+       if (topo) {
                /*
                 * core_id is relative to socket, die and cluster, we need a
                 * global id. So we set socket, die id, cluster id and core id.
                 */
-               id.socket = env->cpu[cpu.cpu].socket_id;
-               id.die = env->cpu[cpu.cpu].die_id;
-               id.cluster = env->cpu[cpu.cpu].cluster_id;
-               id.core = env->cpu[cpu.cpu].core_id;
+               id.socket = topo->socket_id;
+               id.die = topo->die_id;
+               id.cluster = topo->cluster_id;
+               id.core = topo->core_id;
        }
 
        return id;
@@ -1755,18 +1759,19 @@ static struct aggr_cpu_id perf_env__get_cpu_aggr_by_cpu(struct perf_cpu cpu, voi
 {
        struct perf_env *env = data;
        struct aggr_cpu_id id = aggr_cpu_id__empty();
+       struct cpu_topology_map *topo = perf_env__get_cpu_topology(env, cpu);
 
-       if (cpu.cpu >= 0 && cpu.cpu < env->nr_cpus_avail) {
+       if (topo) {
                /*
                 * core_id is relative to socket and die,
                 * we need a global id. So we set
                 * socket, die id and core id
                 */
-               id.socket = env->cpu[cpu.cpu].socket_id;
-               id.die = env->cpu[cpu.cpu].die_id;
-               id.core = env->cpu[cpu.cpu].core_id;
-               id.cpu = cpu;
+               id.socket = topo->socket_id;
+               id.die = topo->die_id;
+               id.core = topo->core_id;
        }
+       id.cpu = cpu;
 
        return id;
 }

diff --git a/tools/perf/util/env.h b/tools/perf/util/env.h
index 7621d1f73b83a3415d18e970d77069b42101899e..7acca39b42ff3531071f62af7fffdfcfb4726b0e 100644 (file)
--- 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;
+}
+
 void cpu_cache_level__free(struct cpu_cache_level *cache);
 
 uint16_t perf_env__e_machine_nocache(struct perf_env *env, uint32_t *e_flags);