[PATCH v1 0/7] perf tools: Fix NULL derefs, OOB reads, use-after-free, and memory leaks

[PATCH v1 0/7] perf tools: Fix NULL derefs, OOB reads, use-after-free, and memory leaks

[Arnaldo Carvalho de Melo]

Hi,

Fifth batch of pre-existing bugs found by sashiko-bot during AI-assisted
review of the perf-data-validation hardening series.  All are independent
of that series -- they are latent bugs in surrounding code exposed during
review.

  1. perf_mmap__aio_mmap() sets nr_cblocks before allocating arrays.
     If calloc() fails, cleanup dereferences NULL map->aio.data.

  2. env->cpu can be NULL when reading old-format perf.data that
     predates topology info (process_cpu_topology() frees it while
     nr_cpus_avail remains set).  The six topology aggregation
     callbacks in builtin-stat.c dereference NULL.  Introduces
     perf_env__get_cpu_topology() accessor with the NULL/bounds guard.

  3. he__get_c2c_hists() error path frees hists without clearing
     c2c_he->hists.  Teardown finds the dangling pointer and calls
     hists__delete_entries() on freed memory.

  4. cpu2y() indexes topology_map[] without bounds check.  Also fixes
     str_to_bitmap() where perf_cpu_map__new("") returns cpu.cpu == -1,
     bypassing the signed >= nr_cpus check and calling __set_bit(-1, ...).

  5. set_max_cpu_num() assigns sysfs CPU count to int16_t max_cpu_num
     without clamping.  Systems with >32767 possible CPUs silently
     truncate to negative, causing undersized allocations.  Also makes
     max_present_cpu_num clamp consistently instead of erroring.

  6. free_idle_threads() calls free() on the thread priv without
     deep-freeing callchain cursor nodes or callchain root entries
     allocated during --idle-hist processing.  Introduces
     callchain_cursor_cleanup() for the cursor node linked list.

  7. Documents the struct perf_cpu int16_t limitation as a libperf
     ABI constraint, and creates tools/lib/perf/TODO to collect
     items that require a future ABI bump.

All require crafted or unusual perf.data inputs to trigger (except
patch 6 which is a memory leak on normal --idle-hist runs, and patch 7
which is documentation only).

Verified with gcc and clang builds, checkpatch, and perf test.

Arnaldo Carvalho de Melo (7):
  perf mmap: Fix NULL deref in aio cleanup on alloc failure
  perf stat: Introduce perf_env__get_cpu_topology() to guard NULL env->cpu
  perf c2c: Fix use-after-free in he__get_c2c_hists() error path
  perf timechart: Fix cpu2y() OOB read on untrusted CPU index
  perf tools: Fix int16_t truncation of max_cpu_num in set_max_cpu_num()
  perf sched: Free callchain nodes in idle thread cleanup
  libperf: Document struct perf_cpu int16_t ABI limitation

 tools/lib/perf/TODO                  | 22 ++++++++++++++++
 tools/lib/perf/include/perf/cpumap.h |  8 +++++-
 tools/perf/builtin-c2c.c             |  1 +
 tools/perf/builtin-sched.c           |  5 +++-
 tools/perf/builtin-stat.c            | 51 ++++++++++++++++++++----------------
 tools/perf/util/callchain.c          | 15 +++++++++++
 tools/perf/util/callchain.h          |  1 +
 tools/perf/util/cpumap.c             | 21 ++++++++++++---
 tools/perf/util/env.h                | 14 ++++++++++
 tools/perf/util/mmap.c               | 10 ++++---
 tools/perf/util/svghelper.c          | 10 ++++---
 11 files changed, 121 insertions(+), 37 deletions(-)
 create mode 100644 tools/lib/perf/TODO

Developed with AI assistance (Claude/sashiko), tagged in commits.

Thanks,

- Arnaldo

[PATCH 1/7] perf mmap: Fix NULL deref in aio cleanup on alloc failure

[Arnaldo Carvalho de Melo]

From: Arnaldo Carvalho de Melo <acme@redhat.com>

perf_mmap__aio_mmap() sets map->aio.nr_cblocks before allocating the
data array.  If calloc() for aiocb or cblocks fails before the data
array is allocated, the return -1 path leads to perf_mmap__aio_munmap()
which loops nr_cblocks times calling perf_mmap__aio_free().  Both
versions of perf_mmap__aio_free() (NUMA and non-NUMA) dereference
map->aio.data[idx] without checking if data is NULL, causing a NULL
pointer dereference.

Add NULL checks for map->aio.data at the top of both
perf_mmap__aio_free() variants so the cleanup path is safe when
allocation fails partway through perf_mmap__aio_mmap().

Fixes: d3d1af6f011a553a ("perf record: Enable asynchronous trace writing")
Reported-by: sashiko-bot <sashiko-bot@kernel.org>
Cc: Alexey Budankov <alexey.budankov@linux.intel.com>
Assisted-by: Claude Opus 4.6 <noreply@anthropic.com>
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/util/mmap.c | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/tools/perf/util/mmap.c b/tools/perf/util/mmap.c
index 4404a99eee45f9c3..d64aec6c7c843e81 100644
--- a/tools/perf/util/mmap.c
+++ b/tools/perf/util/mmap.c
@@ -89,10 +89,10 @@ static int perf_mmap__aio_alloc(struct mmap *map, int idx)
 
 static void perf_mmap__aio_free(struct mmap *map, int idx)
 {
-	if (map->aio.data[idx]) {
-		munmap(map->aio.data[idx], mmap__mmap_len(map));
-		map->aio.data[idx] = NULL;
-	}
+	if (!map->aio.data || !map->aio.data[idx])
+		return;
+	munmap(map->aio.data[idx], mmap__mmap_len(map));
+	map->aio.data[idx] = NULL;
 }
 
 static int perf_mmap__aio_bind(struct mmap *map, int idx, struct perf_cpu cpu, int affinity)
@@ -141,6 +141,8 @@ static int perf_mmap__aio_alloc(struct mmap *map, int idx)
 
 static void perf_mmap__aio_free(struct mmap *map, int idx)
 {
+	if (!map->aio.data)
+		return;
 	zfree(&(map->aio.data[idx]));
 }
 
-- 
2.54.0

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

[Arnaldo Carvalho de Melo]

From: Arnaldo Carvalho de Melo <acme@redhat.com>

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 Opus 4.6 <noreply@anthropic.com>
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/builtin-stat.c | 51 +++++++++++++++++++++------------------
 tools/perf/util/env.h     | 14 +++++++++++
 2 files changed, 42 insertions(+), 23 deletions(-)

diff --git a/tools/perf/builtin-stat.c b/tools/perf/builtin-stat.c
index 9a045811c4197ccd..a04466ea3b0a0657 100644
--- 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 7621d1f73b83a341..7acca39b42ff3531 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;
+}
+
 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);
-- 
2.54.0

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

[sashiko-bot]

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

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

[Arnaldo Melo]

On June 6, 2026 5:23:35 PM GMT-03:00, sashiko-bot@kernel.org wrote:
>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()`.



One more series that has small patches and no issues from local and public dashiki, only detecting (more) preexisting issues, all noted and being quickly addressed in follow up series.

Waiting a bit more for possible human reviews to them merge it and submit the next, that's already in the works :-)

Best regards, 

- Arnaldo 

>--
>
>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?
>

- Arnaldo

[PATCH 3/7] perf c2c: Fix use-after-free in he__get_c2c_hists() error path

[Arnaldo Carvalho de Melo]

From: Arnaldo Carvalho de Melo <acme@redhat.com>

he__get_c2c_hists() assigns c2c_he->hists before calling
c2c_hists__init().  If init fails, the error path calls free(hists)
but leaves c2c_he->hists pointing to freed memory.  On teardown,
c2c_he_free() finds the non-NULL pointer and calls
hists__delete_entries() on it, causing a use-after-free.

Set c2c_he->hists to NULL before freeing so teardown skips the
already-freed allocation.

Fixes: b2252ae67b687d2b ("perf c2c report: Decode c2c_stats for hist entries")
Reported-by: sashiko-bot <sashiko-bot@kernel.org>
Cc: Jiri Olsa <jolsa@kernel.org>
Assisted-by: Claude Opus 4.6 <noreply@anthropic.com>
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/builtin-c2c.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/tools/perf/builtin-c2c.c b/tools/perf/builtin-c2c.c
index cfc1ebe8c0af74dc..e205f58b2f3d3786 100644
--- a/tools/perf/builtin-c2c.c
+++ b/tools/perf/builtin-c2c.c
@@ -225,6 +225,7 @@ he__get_c2c_hists(struct hist_entry *he,
 
 	ret = c2c_hists__init(hists, sort, nr_header_lines, env);
 	if (ret) {
+		c2c_he->hists = NULL;
 		free(hists);
 		return NULL;
 	}
-- 
2.54.0

[PATCH 4/7] perf timechart: Fix cpu2y() OOB read on untrusted CPU index

[Arnaldo Carvalho de Melo]

From: Arnaldo Carvalho de Melo <acme@redhat.com>

cpu2y() indexes topology_map[cpu] without bounds checking.  The array
is allocated with nr_cpus entries (from env->nr_cpus_online), but
callers pass sample CPU values from perf.data which can exceed that
size with cross-machine recordings.

Track the topology_map allocation size and bounds-check the CPU
argument in cpu2y() before indexing.  Out-of-bounds CPUs fall back
to the identity mapping (cpu2slot(cpu)), which is the same behavior
as when no topology is available.

Fixes: c507999790438cde ("perf timechart: Add support for topology")
Reported-by: sashiko-bot <sashiko-bot@kernel.org>
Cc: Stanislav Fomichev <stfomichev@yandex-team.ru>
Assisted-by: Claude Opus 4.6 <noreply@anthropic.com>
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/util/svghelper.c | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/tools/perf/util/svghelper.c b/tools/perf/util/svghelper.c
index e360e7736c7ba65b..826bd2577344b20f 100644
--- a/tools/perf/util/svghelper.c
+++ b/tools/perf/util/svghelper.c
@@ -47,13 +47,13 @@ static double cpu2slot(int cpu)
 }
 
 static int *topology_map;
+static int topology_map_size;
 
 static double cpu2y(int cpu)
 {
-	if (topology_map)
+	if (topology_map && cpu >= 0 && cpu < topology_map_size)
 		return cpu2slot(topology_map[cpu]) * SLOT_MULT;
-	else
-		return cpu2slot(cpu) * SLOT_MULT;
+	return cpu2slot(cpu) * SLOT_MULT;
 }
 
 static double time2pixels(u64 __time)
@@ -736,7 +736,8 @@ static int str_to_bitmap(char *s, cpumask_t *b, int nr_cpus)
 		return -1;
 
 	perf_cpu_map__for_each_cpu(cpu, idx, map) {
-		if (cpu.cpu >= nr_cpus) {
+		/* perf_cpu_map__new("") returns cpu.cpu == -1 */
+		if (cpu.cpu < 0 || cpu.cpu >= nr_cpus) {
 			ret = -1;
 			break;
 		}
@@ -794,6 +795,7 @@ int svg_build_topology_map(struct perf_env *env)
 		fprintf(stderr, "topology: no memory\n");
 		goto exit;
 	}
+	topology_map_size = nr_cpus;
 
 	for (i = 0; i < nr_cpus; i++)
 		topology_map[i] = -1;
-- 
2.54.0

[PATCH 5/7] perf tools: Fix int16_t truncation of max_cpu_num in set_max_cpu_num()

[Arnaldo Carvalho de Melo]

From: Arnaldo Carvalho de Melo <acme@redhat.com>

set_max_cpu_num() assigns the sysfs "possible" CPU count to
max_cpu_num.cpu which is int16_t (struct perf_cpu).  On systems
with >32767 possible CPUs the value silently truncates, potentially
wrapping negative.  This causes cpunode_map to be underallocated
and subsequent cpu__get_node() calls to read out of bounds.

The matching check for max_present_cpu_num was added by commit
c760174401f6 ("perf cpumap: Reduce cpu size from int to int16_t")
but max_cpu_num was missed.  Add the same INT16_MAX guard.

Fixes: c760174401f605cf ("perf cpumap: Reduce cpu size from int to int16_t")
Reported-by: sashiko-bot <sashiko-bot@kernel.org>
Cc: Ian Rogers <irogers@google.com>
Assisted-by: Claude Opus 4.6 <noreply@anthropic.com>
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/util/cpumap.c | 21 +++++++++++++++++----
 1 file changed, 17 insertions(+), 4 deletions(-)

diff --git a/tools/perf/util/cpumap.c b/tools/perf/util/cpumap.c
index d3432622b2adc994..21fa781b03cc7409 100644
--- a/tools/perf/util/cpumap.c
+++ b/tools/perf/util/cpumap.c
@@ -494,6 +494,16 @@ static void set_max_cpu_num(void)
 	if (ret)
 		goto out;
 
+	/*
+	 * struct perf_cpu.cpu is int16_t (libperf ABI) — clamp to avoid
+	 * truncation to negative.  See tools/lib/perf/TODO for the ABI
+	 * widening plan.
+	 */
+	if (max > INT16_MAX) {
+		pr_warning("WARNING: max possible cpus %d exceeds int16_t, clamping to %d\n",
+			   max, INT16_MAX);
+		max = INT16_MAX;
+	}
 	max_cpu_num.cpu = max;
 
 	/* get the highest present cpu number for a sparse allocation */
@@ -506,11 +516,12 @@ static void set_max_cpu_num(void)
 	ret = get_max_num(path, &max);
 
 	if (!ret && max > INT16_MAX) {
-		pr_err("Read out of bounds max cpus of %d\n", max);
-		ret = -1;
+		pr_warning("WARNING: max present cpus %d exceeds int16_t, clamping to %d\n",
+			   max, INT16_MAX);
+		max = INT16_MAX;
 	}
 	if (!ret)
-		max_present_cpu_num.cpu = (int16_t)max;
+		max_present_cpu_num.cpu = max;
 out:
 	if (ret)
 		pr_err("Failed to read max cpus, using default of %d\n", max_cpu_num.cpu);
@@ -647,7 +658,9 @@ int cpu__setup_cpunode_map(void)
 		while ((dent2 = readdir(dir2)) != NULL) {
 			if (dent2->d_type != DT_LNK || sscanf(dent2->d_name, "cpu%u", &cpu) < 1)
 				continue;
-			cpunode_map[cpu] = mem;
+			/* cpunode_map allocated for max_cpu_num entries */
+			if (cpu < (unsigned int)max_cpu_num.cpu)
+				cpunode_map[cpu] = mem;
 		}
 		closedir(dir2);
 	}
-- 
2.54.0

Re: [PATCH 5/7] perf tools: Fix int16_t truncation of max_cpu_num in set_max_cpu_num()

[sashiko-bot]

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

Pre-existing issues:
- [Critical] Stack buffer overflow in `cpu_map__snprint` due to unsafe accumulation of `snprintf` return values.
- [Medium] Integer underflow in `get_max_num()` leads to out-of-bounds heap read when parsing an empty sysfs file.
--

commit 191c419f9d3237462ce91c226e3e3cccc5097d51
Author: Arnaldo Carvalho de Melo <acme@redhat.com>

perf tools: Fix int16_t truncation of max_cpu_num in set_max_cpu_num()

set_max_cpu_num() assigns the sysfs "possible" CPU count to
max_cpu_num.cpu which is int16_t (struct perf_cpu).  On systems
with >32767 possible CPUs the value silently truncates, potentially
wrapping negative.  This causes cpunode_map to be underallocated
and subsequent cpu__get_node() calls to read out of bounds.

> diff --git a/tools/perf/util/cpumap.c b/tools/perf/util/cpumap.c
> index d3432622b2adc..21fa781b03cc7 100644
> --- a/tools/perf/util/cpumap.c
> +++ b/tools/perf/util/cpumap.c
> @@ -494,6 +494,16 @@ static void set_max_cpu_num(void)
>  	if (ret)
>  		goto out;
>  

[Severity: Medium]
This isn't a bug introduced by this patch, but while reviewing
get_max_num() which is called just before this line, I noticed a
pre-existing issue where an integer underflow can occur.

If filename__read_str() reads an empty sysfs file (for example from a
mocked sysfs directory or malformed kernel state), it returns 0 and sets
num to 0.

tools/perf/util/cpumap.c:get_max_num() {
...
    buf[num] = '\0';

    /* start on the right, to find highest node num */
    while (--num) {
        if ((buf[num] == ',') || (buf[num] == '-')) {
...
}

When num is 0, the pre-decrement --num on a size_t underflows to SIZE_MAX.
This causes an immediate out-of-bounds memory read on the dynamically
allocated buf array. Can we add a check to ensure num is greater than 0
before entering the loop?

> +	/*
> +	 * struct perf_cpu.cpu is int16_t (libperf ABI) — clamp to avoid
> +	 * truncation to negative.  See tools/lib/perf/TODO for the ABI
> +	 * widening plan.
> +	 */

[ ... ]

While reviewing cpumap.c, I also noticed another pre-existing issue in
cpu_map__snprint():

tools/perf/util/cpumap.c:cpu_map__snprint() {
...
    if (start == end) {
        ret += snprintf(buf + ret, size - ret,
                        "%s%d", COMMA,
                        perf_cpu_map__cpu(map, start).cpu);
    } else {
...
}

[Severity: Critical]
This is a pre-existing issue, but does this loop unsafely accumulate
snprintf return values?

The standard snprintf function returns the number of characters that would
have been written if the buffer was large enough, not the actual bytes
written. If the constructed string length exceeds the provided buffer size
(such as the 1024-byte stack buffer passed by cpu_map__fprintf()), ret
will become strictly greater than size.

On the next iteration, the unsigned arithmetic size - ret will underflow
to a huge value, and buf + ret will point past the end of the stack buffer.

Subsequent snprintf calls could write entirely out of bounds on the stack,
leading to a stack buffer overflow.

Could this happen when processing an untrusted perf.data file with a
highly fragmented or large CPU map?

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

[PATCH 6/7] perf sched: Free callchain nodes in idle thread cleanup

[Arnaldo Carvalho de Melo]

From: Arnaldo Carvalho de Melo <acme@redhat.com>

free_idle_threads() relies on the thread priv destructor (free()) to
clean up idle_thread_runtime structs.  But free() doesn't walk the
callchain_cursor linked list or the callchain_root tree allocated
by callchain_cursor__copy() and callchain_append() during --idle-hist
processing.  Every idle thread with callchain data leaks these nodes.

Introduce callchain_cursor_cleanup() to free the cursor's linked list
of callchain_cursor_node entries, and call it together with
free_callchain() in free_idle_threads() before thread__put().

Fixes: 225b24f569980ac9 ("perf sched timehist: Save callchain when entering idle")
Reported-by: sashiko-bot <sashiko-bot@kernel.org>
Cc: Namhyung Kim <namhyung@kernel.org>
Assisted-by: Claude Opus 4.6 <noreply@anthropic.com>
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/builtin-sched.c  |  5 ++++-
 tools/perf/util/callchain.c | 15 +++++++++++++++
 tools/perf/util/callchain.h |  1 +
 3 files changed, 20 insertions(+), 1 deletion(-)

diff --git a/tools/perf/builtin-sched.c b/tools/perf/builtin-sched.c
index b7ccdc6a985d1c7b..1ff01f03d2ad1ad3 100644
--- a/tools/perf/builtin-sched.c
+++ b/tools/perf/builtin-sched.c
@@ -2500,8 +2500,11 @@ static void free_idle_threads(void)
 			struct idle_thread_runtime *itr;
 
 			itr = thread__priv(idle);
-			if (itr)
+			if (itr) {
 				thread__put(itr->last_thread);
+				free_callchain(&itr->callchain);
+				callchain_cursor_cleanup(&itr->cursor);
+			}
 
 			thread__put(idle);
 		}
diff --git a/tools/perf/util/callchain.c b/tools/perf/util/callchain.c
index 8981ae879ebb887c..31c675cbab63187b 100644
--- a/tools/perf/util/callchain.c
+++ b/tools/perf/util/callchain.c
@@ -1578,6 +1578,21 @@ void free_callchain(struct callchain_root *root)
 	free_callchain_node(&root->node);
 }
 
+void callchain_cursor_cleanup(struct callchain_cursor *cursor)
+{
+	struct callchain_cursor_node *node, *next;
+
+	callchain_cursor_reset(cursor);
+
+	for (node = cursor->first; node; node = next) {
+		next = node->next;
+		free(node);
+	}
+	cursor->first = NULL;
+	cursor->last = &cursor->first;
+	cursor->curr = NULL;
+}
+
 static u64 decay_callchain_node(struct callchain_node *node)
 {
 	struct callchain_node *child;
diff --git a/tools/perf/util/callchain.h b/tools/perf/util/callchain.h
index 0eb5d7a1a41d81e1..8b1e405d1cdf4660 100644
--- a/tools/perf/util/callchain.h
+++ b/tools/perf/util/callchain.h
@@ -286,6 +286,7 @@ int callchain_list_counts__printf_value(struct callchain_list *clist,
 					FILE *fp, char *bf, int bfsize);
 
 void free_callchain(struct callchain_root *root);
+void callchain_cursor_cleanup(struct callchain_cursor *cursor);
 void decay_callchain(struct callchain_root *root);
 int callchain_node__make_parent_list(struct callchain_node *node);
 
-- 
2.54.0

[PATCH 7/7] libperf: Document struct perf_cpu int16_t ABI limitation

[Arnaldo Carvalho de Melo]

From: Arnaldo Carvalho de Melo <acme@redhat.com>

struct perf_cpu.cpu is int16_t, limiting perf to 32767 CPUs.  This is
part of the libperf ABI (returned by value from perf_cpu_map__cpu() and
friends), so widening it requires an ABI bump.

Add a comment on the struct definition noting this, and create a TODO
file to collect future ABI changes so they can be batched into a single
version bump.

Cc: Ian Rogers <irogers@google.com>
Cc: Namhyung Kim <namhyung@kernel.org>
Assisted-by: Claude Opus 4.6 <noreply@anthropic.com>
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/lib/perf/TODO                  | 22 ++++++++++++++++++++++
 tools/lib/perf/include/perf/cpumap.h |  8 +++++++-
 2 files changed, 29 insertions(+), 1 deletion(-)
 create mode 100644 tools/lib/perf/TODO

diff --git a/tools/lib/perf/TODO b/tools/lib/perf/TODO
new file mode 100644
index 0000000000000000..486dd95dc57208a8
--- /dev/null
+++ b/tools/lib/perf/TODO
@@ -0,0 +1,22 @@
+Future ABI changes
+==================
+
+This file collects items that require a libperf ABI bump.  Each entry
+should describe the current limitation, the desired end state, and the
+scope of the change so that a future ABI revision can batch them
+together.
+
+1. Widen struct perf_cpu.cpu from int16_t to int
+   - Current limit: 32767 CPUs.  No architecture exceeds this today
+     (x86_64 max is 8192, arm64 is 4096), but NR_CPUS limits keep
+     growing.  perf clamps to INT16_MAX in set_max_cpu_num() as a
+     safety net.
+   - Scope: struct perf_cpu is embedded everywhere — perf_cpu_map__cpu(),
+     perf_cpu_map__min(), perf_cpu_map__max(), perf_cpu_map__has(), the
+     for_each_cpu macros, and all internal callers.  The perf_cpu_map
+     internal array (RC_CHK_ACCESS(map)->map[]) stores struct perf_cpu
+     directly.  Widening changes the struct layout and every function
+     that returns or accepts struct perf_cpu by value.
+   - Migration: bump LIBPERF version in libperf.map, audit all
+     sizeof(struct perf_cpu) assumptions, update perf.data
+     serialization if needed.
diff --git a/tools/lib/perf/include/perf/cpumap.h b/tools/lib/perf/include/perf/cpumap.h
index a1dd25db65b62f6b..e1a0b0d272109ecb 100644
--- a/tools/lib/perf/include/perf/cpumap.h
+++ b/tools/lib/perf/include/perf/cpumap.h
@@ -6,7 +6,13 @@
 #include <stdbool.h>
 #include <stdint.h>
 
-/** A wrapper around a CPU to avoid confusion with the perf_cpu_map's map's indices. */
+/**
+ * struct perf_cpu - wrapper around a CPU number.
+ * @cpu: CPU number, -1 for the "any CPU"/dummy value.
+ *
+ * int16_t limits this to 32767 CPUs.  Widening to int requires a libperf
+ * ABI bump — see tools/lib/perf/TODO for the full scope.
+ */
 struct perf_cpu {
 	int16_t cpu;
 };
-- 
2.54.0