mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH v1 0/7] perf tools: Fix NULL derefs, OOB reads, use-after-free, and memory leaks
@ 2026-06-06 20:05 Arnaldo Carvalho de Melo
  2026-06-06 20:05 ` [PATCH 1/7] perf mmap: Fix NULL deref in aio cleanup on alloc failure Arnaldo Carvalho de Melo
                   ` (6 more replies)
  0 siblings, 7 replies; 8+ messages in thread
From: Arnaldo Carvalho de Melo @ 2026-06-06 20:05 UTC (permalink / raw)
  To: Namhyung Kim
  Cc: Ingo Molnar, Thomas Gleixner, James Clark, Jiri Olsa, Ian Rogers,
	Adrian Hunter, Clark Williams, linux-kernel, linux-perf-users,
	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

^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH 1/7] perf mmap: Fix NULL deref in aio cleanup on alloc failure
  2026-06-06 20:05 [PATCH v1 0/7] perf tools: Fix NULL derefs, OOB reads, use-after-free, and memory leaks Arnaldo Carvalho de Melo
@ 2026-06-06 20:05 ` Arnaldo Carvalho de Melo
  2026-06-06 20:05 ` [PATCH 2/7] perf stat: Introduce perf_env__get_cpu_topology() to guard NULL env->cpu Arnaldo Carvalho de Melo
                   ` (5 subsequent siblings)
  6 siblings, 0 replies; 8+ messages in thread
From: Arnaldo Carvalho de Melo @ 2026-06-06 20:05 UTC (permalink / raw)
  To: Namhyung Kim
  Cc: Ingo Molnar, Thomas Gleixner, James Clark, Jiri Olsa, Ian Rogers,
	Adrian Hunter, Clark Williams, linux-kernel, linux-perf-users,
	Arnaldo Carvalho de Melo, sashiko-bot, Alexey Budankov,
	Claude Opus 4.6

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


^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH 2/7] perf stat: Introduce perf_env__get_cpu_topology() to guard NULL env->cpu
  2026-06-06 20:05 [PATCH v1 0/7] perf tools: Fix NULL derefs, OOB reads, use-after-free, and memory leaks Arnaldo Carvalho de Melo
  2026-06-06 20:05 ` [PATCH 1/7] perf mmap: Fix NULL deref in aio cleanup on alloc failure Arnaldo Carvalho de Melo
@ 2026-06-06 20:05 ` Arnaldo Carvalho de Melo
  2026-06-06 20:05 ` [PATCH 3/7] perf c2c: Fix use-after-free in he__get_c2c_hists() error path Arnaldo Carvalho de Melo
                   ` (4 subsequent siblings)
  6 siblings, 0 replies; 8+ messages in thread
From: Arnaldo Carvalho de Melo @ 2026-06-06 20:05 UTC (permalink / raw)
  To: Namhyung Kim
  Cc: Ingo Molnar, Thomas Gleixner, James Clark, Jiri Olsa, Ian Rogers,
	Adrian Hunter, Clark Williams, linux-kernel, linux-perf-users,
	Arnaldo Carvalho de Melo, sashiko-bot, Claude Opus 4.6

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


^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH 3/7] perf c2c: Fix use-after-free in he__get_c2c_hists() error path
  2026-06-06 20:05 [PATCH v1 0/7] perf tools: Fix NULL derefs, OOB reads, use-after-free, and memory leaks Arnaldo Carvalho de Melo
  2026-06-06 20:05 ` [PATCH 1/7] perf mmap: Fix NULL deref in aio cleanup on alloc failure Arnaldo Carvalho de Melo
  2026-06-06 20:05 ` [PATCH 2/7] perf stat: Introduce perf_env__get_cpu_topology() to guard NULL env->cpu Arnaldo Carvalho de Melo
@ 2026-06-06 20:05 ` Arnaldo Carvalho de Melo
  2026-06-06 20:05 ` [PATCH 4/7] perf timechart: Fix cpu2y() OOB read on untrusted CPU index Arnaldo Carvalho de Melo
                   ` (3 subsequent siblings)
  6 siblings, 0 replies; 8+ messages in thread
From: Arnaldo Carvalho de Melo @ 2026-06-06 20:05 UTC (permalink / raw)
  To: Namhyung Kim
  Cc: Ingo Molnar, Thomas Gleixner, James Clark, Jiri Olsa, Ian Rogers,
	Adrian Hunter, Clark Williams, linux-kernel, linux-perf-users,
	Arnaldo Carvalho de Melo, sashiko-bot, Claude Opus 4.6

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


^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH 4/7] perf timechart: Fix cpu2y() OOB read on untrusted CPU index
  2026-06-06 20:05 [PATCH v1 0/7] perf tools: Fix NULL derefs, OOB reads, use-after-free, and memory leaks Arnaldo Carvalho de Melo
                   ` (2 preceding siblings ...)
  2026-06-06 20:05 ` [PATCH 3/7] perf c2c: Fix use-after-free in he__get_c2c_hists() error path Arnaldo Carvalho de Melo
@ 2026-06-06 20:05 ` Arnaldo Carvalho de Melo
  2026-06-06 20:05 ` [PATCH 5/7] perf tools: Fix int16_t truncation of max_cpu_num in set_max_cpu_num() Arnaldo Carvalho de Melo
                   ` (2 subsequent siblings)
  6 siblings, 0 replies; 8+ messages in thread
From: Arnaldo Carvalho de Melo @ 2026-06-06 20:05 UTC (permalink / raw)
  To: Namhyung Kim
  Cc: Ingo Molnar, Thomas Gleixner, James Clark, Jiri Olsa, Ian Rogers,
	Adrian Hunter, Clark Williams, linux-kernel, linux-perf-users,
	Arnaldo Carvalho de Melo, sashiko-bot, Stanislav Fomichev,
	Claude Opus 4.6

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


^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH 5/7] perf tools: Fix int16_t truncation of max_cpu_num in set_max_cpu_num()
  2026-06-06 20:05 [PATCH v1 0/7] perf tools: Fix NULL derefs, OOB reads, use-after-free, and memory leaks Arnaldo Carvalho de Melo
                   ` (3 preceding siblings ...)
  2026-06-06 20:05 ` [PATCH 4/7] perf timechart: Fix cpu2y() OOB read on untrusted CPU index Arnaldo Carvalho de Melo
@ 2026-06-06 20:05 ` Arnaldo Carvalho de Melo
  2026-06-06 20:05 ` [PATCH 6/7] perf sched: Free callchain nodes in idle thread cleanup Arnaldo Carvalho de Melo
  2026-06-06 20:05 ` [PATCH 7/7] libperf: Document struct perf_cpu int16_t ABI limitation Arnaldo Carvalho de Melo
  6 siblings, 0 replies; 8+ messages in thread
From: Arnaldo Carvalho de Melo @ 2026-06-06 20:05 UTC (permalink / raw)
  To: Namhyung Kim
  Cc: Ingo Molnar, Thomas Gleixner, James Clark, Jiri Olsa, Ian Rogers,
	Adrian Hunter, Clark Williams, linux-kernel, linux-perf-users,
	Arnaldo Carvalho de Melo, sashiko-bot, Claude Opus 4.6

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


^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH 6/7] perf sched: Free callchain nodes in idle thread cleanup
  2026-06-06 20:05 [PATCH v1 0/7] perf tools: Fix NULL derefs, OOB reads, use-after-free, and memory leaks Arnaldo Carvalho de Melo
                   ` (4 preceding siblings ...)
  2026-06-06 20:05 ` [PATCH 5/7] perf tools: Fix int16_t truncation of max_cpu_num in set_max_cpu_num() Arnaldo Carvalho de Melo
@ 2026-06-06 20:05 ` Arnaldo Carvalho de Melo
  2026-06-06 20:05 ` [PATCH 7/7] libperf: Document struct perf_cpu int16_t ABI limitation Arnaldo Carvalho de Melo
  6 siblings, 0 replies; 8+ messages in thread
From: Arnaldo Carvalho de Melo @ 2026-06-06 20:05 UTC (permalink / raw)
  To: Namhyung Kim
  Cc: Ingo Molnar, Thomas Gleixner, James Clark, Jiri Olsa, Ian Rogers,
	Adrian Hunter, Clark Williams, linux-kernel, linux-perf-users,
	Arnaldo Carvalho de Melo, sashiko-bot, Claude Opus 4.6

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


^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH 7/7] libperf: Document struct perf_cpu int16_t ABI limitation
  2026-06-06 20:05 [PATCH v1 0/7] perf tools: Fix NULL derefs, OOB reads, use-after-free, and memory leaks Arnaldo Carvalho de Melo
                   ` (5 preceding siblings ...)
  2026-06-06 20:05 ` [PATCH 6/7] perf sched: Free callchain nodes in idle thread cleanup Arnaldo Carvalho de Melo
@ 2026-06-06 20:05 ` Arnaldo Carvalho de Melo
  6 siblings, 0 replies; 8+ messages in thread
From: Arnaldo Carvalho de Melo @ 2026-06-06 20:05 UTC (permalink / raw)
  To: Namhyung Kim
  Cc: Ingo Molnar, Thomas Gleixner, James Clark, Jiri Olsa, Ian Rogers,
	Adrian Hunter, Clark Williams, linux-kernel, linux-perf-users,
	Arnaldo Carvalho de Melo, Claude Opus 4.6

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


^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2026-06-06 20:06 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-06-06 20:05 [PATCH v1 0/7] perf tools: Fix NULL derefs, OOB reads, use-after-free, and memory leaks Arnaldo Carvalho de Melo
2026-06-06 20:05 ` [PATCH 1/7] perf mmap: Fix NULL deref in aio cleanup on alloc failure Arnaldo Carvalho de Melo
2026-06-06 20:05 ` [PATCH 2/7] perf stat: Introduce perf_env__get_cpu_topology() to guard NULL env->cpu Arnaldo Carvalho de Melo
2026-06-06 20:05 ` [PATCH 3/7] perf c2c: Fix use-after-free in he__get_c2c_hists() error path Arnaldo Carvalho de Melo
2026-06-06 20:05 ` [PATCH 4/7] perf timechart: Fix cpu2y() OOB read on untrusted CPU index Arnaldo Carvalho de Melo
2026-06-06 20:05 ` [PATCH 5/7] perf tools: Fix int16_t truncation of max_cpu_num in set_max_cpu_num() Arnaldo Carvalho de Melo
2026-06-06 20:05 ` [PATCH 6/7] perf sched: Free callchain nodes in idle thread cleanup Arnaldo Carvalho de Melo
2026-06-06 20:05 ` [PATCH 7/7] libperf: Document struct perf_cpu int16_t ABI limitation Arnaldo Carvalho de Melo

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®