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: 88031a0de7 ("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>
This commit is contained in:
Arnaldo Carvalho de Melo
2026-06-06 11:17:45 -03:00
parent 25627346b1
commit afa4363a91
2 changed files with 42 additions and 23 deletions

View File

@@ -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;
}

View File

@@ -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);