[PATCHES v1 0/8] perf tools: Fix OOB writes, refcount bugs, and BUG_ON in mmap/stat/c2c/sched

[PATCHES v1 0/8] perf tools: Fix OOB writes, refcount bugs, and BUG_ON in mmap/stat/c2c/sched

[Arnaldo Carvalho de Melo]

Hi,

Eight more 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_bind() passes cpu__get_node() return value to an
     unsigned long.  When -1 is returned, sign-extension to ULONG_MAX
     causes bitmap_zalloc(0) and a massive OOB __set_bit.

  2. Six perf_env__get_*_aggr_by_cpu() topology aggregation callbacks
     in builtin-stat.c access env->cpu[cpu.cpu] after only checking
     cpu.cpu != -1.  A CPU index from untrusted perf.data exceeding
     env->nr_cpus_avail causes OOB heap reads.

  3. perf c2c: __set_bit on cpuset/nodeset bitmaps without bounds
     checking sample->cpu and node IDs against their allocation sizes.
     Also, cpu2node[] array accessed without upper bound check.

  4. perf c2c: setup_nodes() iterates CPU maps from perf.data topology
     and uses cpu.cpu directly as index into cpu2node[] and __set_bit
     without validating against nr_cpus_avail.

  5. get_idle_thread() leaves a partially initialized thread in
     idle_threads[] when init_idle_thread() fails, causing subsequent
     calls to return a thread with no priv data -- later cast to a
     larger struct causes OOB writes.

  6. timehist_sched_change_event() sets itr->last_thread to NULL
     without calling thread__put() first, leaking a thread reference
     on every idle context switch with --idle-hist.

  7. free_idle_threads() calls thread__delete() directly instead of
     thread__put(), bypassing the refcount lifecycle.

  8. get_new_event() dereferences unchecked zalloc() result and uses
     BUG_ON on realloc failure.  add_sched_event_wakeup() passes
     unchecked zalloc() to sem_init().  All crash on OOM with
     untrusted input.

All require crafted or unusual perf.data inputs to trigger.
Verified with gcc and clang builds, checkpatch, and perf test.

Arnaldo Carvalho de Melo (8):
  perf mmap: Guard cpu__get_node() return in aio_bind()
  perf stat: Bounds-check CPU index in topology aggregation callbacks
  perf c2c: Bounds-check CPU and node IDs before bitmap and array access
  perf c2c: Bounds-check CPU IDs in setup_nodes() topology loop
  perf sched: Clean up idle_threads entry on init failure
  perf sched: Fix thread reference leak in idle hist processing
  perf sched: Use thread__put() in free_idle_threads()
  perf sched: Replace BUG_ON and add NULL checks in replay event helpers

 tools/perf/builtin-c2c.c   | 23 +++++++++++++++++++++--
 tools/perf/builtin-sched.c | 37 +++++++++++++++++++++++++++++++------
 tools/perf/builtin-stat.c  | 13 +++++++------
 tools/perf/util/mmap.c     |  8 +++++++-
 4 files changed, 66 insertions(+), 15 deletions(-)

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

Thanks,

- Arnaldo

[PATCH 1/8] perf mmap: Guard cpu__get_node() return in aio_bind()

[Arnaldo Carvalho de Melo]

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

perf_mmap__aio_bind() passes the cpu__get_node() return value directly
to an unsigned long variable (node_index).  When cpu__get_node() returns
-1 for an unknown CPU, the implicit int-to-unsigned-long conversion
sign-extends it to ULONG_MAX.

This causes bitmap_zalloc(ULONG_MAX + 1) which wraps to
bitmap_zalloc(0), returning a zero-sized allocation.  The subsequent
__set_bit(ULONG_MAX, node_mask) then writes massively out of bounds.

Check the return value in a signed temporary before assigning to
node_index, and skip the NUMA binding when the node is unknown.

Fixes: c44a8b44ca9f ("perf record: Bind the AIO user space buffers to nodes")
Reported-by: sashiko-bot <sashiko-bot@kernel.org>
Cc: Alexey Budankov <alexey.budankov@linux.intel.com>
Cc: Jiri Olsa <jolsa@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/util/mmap.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/tools/perf/util/mmap.c b/tools/perf/util/mmap.c
index b69f926d314b148b..4404a99eee45f9c3 100644
--- a/tools/perf/util/mmap.c
+++ b/tools/perf/util/mmap.c
@@ -104,9 +104,15 @@ static int perf_mmap__aio_bind(struct mmap *map, int idx, struct perf_cpu cpu, i
 	int err = 0;
 
 	if (affinity != PERF_AFFINITY_SYS && cpu__max_node() > 1) {
+		int node;
+
 		data = map->aio.data[idx];
 		mmap_len = mmap__mmap_len(map);
-		node_index = cpu__get_node(cpu);
+		node = cpu__get_node(cpu);
+		/* -1 sign-extends to ULONG_MAX, wrapping bitmap_zalloc(0) and OOB __set_bit */
+		if (node < 0)
+			return 0;
+		node_index = node;
 		node_mask = bitmap_zalloc(node_index + 1);
 		if (!node_mask) {
 			pr_err("Failed to allocate node mask for mbind: error %m\n");
-- 
2.54.0

[PATCH 2/8] perf stat: Bounds-check CPU index in topology aggregation callbacks

[Arnaldo Carvalho de Melo]

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

Six perf_env__get_*_aggr_by_cpu() functions access env->cpu[cpu.cpu]
after only checking cpu.cpu != -1.  env->cpu[] is allocated with
env->nr_cpus_avail entries, so a CPU index from an untrusted perf.data
file that exceeds that count causes an out-of-bounds heap read.

Replace the != -1 guard with >= 0 && < env->nr_cpus_avail in all six
functions.  The >= 0 check also catches -1 and any other negative values
that could bypass the old check.

Affected functions:
  - perf_env__get_socket_aggr_by_cpu()
  - perf_env__get_die_aggr_by_cpu()
  - perf_env__get_cache_aggr_by_cpu()
  - perf_env__get_cluster_aggr_by_cpu()
  - perf_env__get_core_aggr_by_cpu()
  - perf_env__get_cpu_aggr_by_cpu()

Fixes: 68d702f7a120 ("perf stat report: Add support to initialize aggr_map from file")
Reported-by: sashiko-bot <sashiko-bot@kernel.org>
Cc: Ian Rogers <irogers@google.com>
Cc: Jiri Olsa <jolsa@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-stat.c | 13 +++++++------
 1 file changed, 7 insertions(+), 6 deletions(-)

diff --git a/tools/perf/builtin-stat.c b/tools/perf/builtin-stat.c
index 99d7db372b480800..9a045811c4197ccd 100644
--- a/tools/perf/builtin-stat.c
+++ b/tools/perf/builtin-stat.c
@@ -1638,7 +1638,8 @@ 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();
 
-	if (cpu.cpu != -1)
+	/* 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;
 
 	return id;
@@ -1649,7 +1650,7 @@ 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();
 
-	if (cpu.cpu != -1) {
+	if (cpu.cpu >= 0 && cpu.cpu < env->nr_cpus_avail) {
 		/*
 		 * die_id is relative to socket, so start
 		 * with the socket ID and then add die to
@@ -1705,7 +1706,7 @@ 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();
 
-	if (cpu.cpu != -1) {
+	if (cpu.cpu >= 0 && cpu.cpu < env->nr_cpus_avail) {
 		u32 cache_level = (perf_stat.aggr_level) ?: stat_config.aggr_level;
 
 		id.socket = env->cpu[cpu.cpu].socket_id;
@@ -1722,7 +1723,7 @@ 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();
 
-	if (cpu.cpu != -1) {
+	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;
@@ -1736,7 +1737,7 @@ 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();
 
-	if (cpu.cpu != -1) {
+	if (cpu.cpu >= 0 && cpu.cpu < env->nr_cpus_avail) {
 		/*
 		 * 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.
@@ -1755,7 +1756,7 @@ 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();
 
-	if (cpu.cpu != -1) {
+	if (cpu.cpu >= 0 && cpu.cpu < env->nr_cpus_avail) {
 		/*
 		 * core_id is relative to socket and die,
 		 * we need a global id. So we set
-- 
2.54.0

[PATCH 3/8] perf c2c: Bounds-check CPU and node IDs before bitmap and array access

[Arnaldo Carvalho de Melo]

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

c2c_he__set_cpu() passes sample->cpu directly to __set_bit(cpu, cpuset)
after only checking for the (u32)-1 sentinel.  The cpuset bitmap is
allocated with c2c.cpus_cnt bits (from env->nr_cpus_avail), so a crafted
perf.data with CPU IDs exceeding that count causes out-of-bounds heap
writes.

c2c_he__set_node() similarly passes the node ID from mem2node__node()
to __set_bit(node, nodeset) after only checking for negative values.
The nodeset bitmap is sized to c2c.nodes_cnt (from env->nr_numa_nodes),
so a node ID exceeding that causes OOB writes.

process_sample_event() indexes c2c.cpu2node[cpu] and
c2c_he->node_stats[node] without bounds checking.  Both arrays are
sized to c2c.cpus_cnt and c2c.nodes_cnt respectively.

Add bounds checks in all three paths:
  - c2c_he__set_cpu(): return if sample->cpu >= c2c.cpus_cnt
  - c2c_he__set_node(): return if node >= c2c.nodes_cnt
  - process_sample_event(): clamp cpu to 0 if >= cpus_cnt,
    guard node_stats access with bounds check

Fixes: 1e181b92a2da ("perf c2c report: Add 'node' sort key")
Reported-by: sashiko-bot <sashiko-bot@kernel.org>
Cc: Jiri Olsa <jolsa@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-c2c.c | 19 +++++++++++++++++--
 1 file changed, 17 insertions(+), 2 deletions(-)

diff --git a/tools/perf/builtin-c2c.c b/tools/perf/builtin-c2c.c
index 3dd45a550fdb772a..b78217d98988a75c 100644
--- a/tools/perf/builtin-c2c.c
+++ b/tools/perf/builtin-c2c.c
@@ -245,6 +245,10 @@ static void c2c_he__set_cpu(struct c2c_hist_entry *c2c_he,
 		      "WARNING: no sample cpu value"))
 		return;
 
+	/* cpuset bitmap has c2c.cpus_cnt bits from env->nr_cpus_avail */
+	if (sample->cpu >= (unsigned int)c2c.cpus_cnt)
+		return;
+
 	__set_bit(sample->cpu, c2c_he->cpuset);
 }
 
@@ -262,6 +266,10 @@ static void c2c_he__set_node(struct c2c_hist_entry *c2c_he,
 	if (WARN_ONCE(node < 0, "WARNING: failed to find node\n"))
 		return;
 
+	/* nodeset bitmap has c2c.nodes_cnt bits from env->nr_numa_nodes */
+	if (node >= c2c.nodes_cnt)
+		return;
+
 	__set_bit(node, c2c_he->nodeset);
 
 	if (c2c_he->paddr != sample->phys_addr) {
@@ -391,7 +399,12 @@ static int process_sample_event(const struct perf_tool *tool __maybe_unused,
 		 * Doing node stats only for single callchain data.
 		 */
 		int cpu = sample->cpu == (unsigned int) -1 ? 0 : sample->cpu;
-		int node = c2c.cpu2node[cpu];
+		int node;
+
+		/* cpu2node[] has c2c.cpus_cnt entries from env->nr_cpus_avail */
+		if (cpu >= c2c.cpus_cnt)
+			cpu = 0;
+		node = c2c.cpu2node[cpu];
 
 		c2c_hists = he__get_c2c_hists(he, c2c.cl_sort, 2, machine->env);
 		if (!c2c_hists) {
@@ -410,7 +423,9 @@ static int process_sample_event(const struct perf_tool *tool __maybe_unused,
 		c2c_he = container_of(he, struct c2c_hist_entry, he);
 		c2c_add_stats(&c2c_he->stats, &stats);
 		c2c_add_stats(&c2c_hists->stats, &stats);
-		c2c_add_stats(&c2c_he->node_stats[node], &stats);
+		/* node_stats[] has c2c.nodes_cnt entries */
+		if (node >= 0 && node < c2c.nodes_cnt)
+			c2c_add_stats(&c2c_he->node_stats[node], &stats);
 
 		compute_stats(c2c_he, &stats, sample->weight);
 
-- 
2.54.0

[PATCH 4/8] perf c2c: Bounds-check CPU IDs in setup_nodes() topology loop

[Arnaldo Carvalho de Melo]

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

setup_nodes() iterates CPU maps from the perf.data topology header and
uses cpu.cpu directly as an array index into cpu2node[] (allocated with
c2c.cpus_cnt = env->nr_cpus_avail entries) and __set_bit(cpu.cpu, set)
(bitmap also sized to c2c.cpus_cnt).

A crafted perf.data with topology CPU IDs exceeding nr_cpus_avail causes
out-of-bounds heap writes into both the cpu2node array and the per-node
bitmap.

Add a bounds check to skip CPU IDs that fall outside the valid range.

Fixes: 1e181b92a2da ("perf c2c report: Add 'node' sort key")
Reported-by: sashiko-bot <sashiko-bot@kernel.org>
Cc: Jiri Olsa <jolsa@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-c2c.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/tools/perf/builtin-c2c.c b/tools/perf/builtin-c2c.c
index b78217d98988a75c..cbe0e993053d403c 100644
--- a/tools/perf/builtin-c2c.c
+++ b/tools/perf/builtin-c2c.c
@@ -2371,6 +2371,10 @@ static int setup_nodes(struct perf_session *session)
 		nodes[node] = set;
 
 		perf_cpu_map__for_each_cpu_skip_any(cpu, idx, map) {
+			/* topology CPU IDs from perf.data may exceed nr_cpus_avail */
+			if (cpu.cpu < 0 || cpu.cpu >= c2c.cpus_cnt)
+				continue;
+
 			__set_bit(cpu.cpu, set);
 
 			if (WARN_ONCE(cpu2node[cpu.cpu] != -1, "node/cpu topology bug"))
-- 
2.54.0

[PATCH 5/8] perf sched: Clean up idle_threads entry on init failure

[Arnaldo Carvalho de Melo]

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

get_idle_thread() allocates a thread via thread__new() and stores it in
idle_threads[cpu], then calls init_idle_thread() to set up the private
data.  If init_idle_thread() fails (e.g. OOM for the idle_thread_runtime
struct), the function returns NULL but leaves the partially initialized
thread in idle_threads[cpu].

On subsequent calls for the same CPU, get_idle_thread() finds a non-NULL
idle_threads[cpu], skips allocation, and returns thread__get() on a
thread that has no priv data.  Callers then get a thread whose
thread__priv() returns NULL, leading to unexpected behavior.

Release the thread and reset the slot to NULL on init failure so the
entry doesn't persist in a corrupted state.

Fixes: 49394a2a24c7 ("perf sched timehist: Introduce timehist command")
Reported-by: sashiko-bot <sashiko-bot@kernel.org>
Cc: David Ahern <dsahern@gmail.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/perf/builtin-sched.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/tools/perf/builtin-sched.c b/tools/perf/builtin-sched.c
index 21fb820b625b43e1..e4378cc9ab3ed48b 100644
--- a/tools/perf/builtin-sched.c
+++ b/tools/perf/builtin-sched.c
@@ -2514,8 +2514,11 @@ static struct thread *get_idle_thread(int cpu)
 		idle_threads[cpu] = thread__new(0, 0);
 
 		if (idle_threads[cpu]) {
-			if (init_idle_thread(idle_threads[cpu]) < 0)
+			if (init_idle_thread(idle_threads[cpu]) < 0) {
+				/* clean up so next call doesn't find a half-initialized thread */
+				thread__zput(idle_threads[cpu]);
 				return NULL;
+			}
 		}
 	}
 
-- 
2.54.0

[PATCH 6/8] perf sched: Fix thread reference leak in idle hist processing

[Arnaldo Carvalho de Melo]

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

timehist_sched_change_event() sets itr->last_thread to NULL at the end
of idle hist processing without calling thread__put() first.  The
thread reference was acquired via thread__get() in timehist_get_thread()
(line 2581), so every idle context switch leaks a thread reference when
--idle-hist is active.

Use thread__zput() to properly release the reference before clearing
the pointer.

Fixes: 5d8f17fb5822 ("perf sched timehist: Add -I/--idle-hist option")
Reported-by: sashiko-bot <sashiko-bot@kernel.org>
Cc: David Ahern <dsahern@gmail.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/perf/builtin-sched.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/perf/builtin-sched.c b/tools/perf/builtin-sched.c
index e4378cc9ab3ed48b..af6322ffaaa6eb1d 100644
--- a/tools/perf/builtin-sched.c
+++ b/tools/perf/builtin-sched.c
@@ -2934,7 +2934,7 @@ static int timehist_sched_change_event(const struct perf_tool *tool,
 			if (itr->cursor.nr)
 				callchain_append(&itr->callchain, &itr->cursor, t - tprev);
 
-			itr->last_thread = NULL;
+			thread__zput(itr->last_thread);
 		}
 
 		if (!sched->summary_only)
-- 
2.54.0

[PATCH 7/8] perf sched: Use thread__put() in free_idle_threads()

[Arnaldo Carvalho de Melo]

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

free_idle_threads() calls thread__delete() directly instead of
thread__put(), bypassing the reference counting lifecycle.  Under
REFCNT_CHECKING builds, this leaks the pointer handle since
thread__delete() frees the object without going through the refcount
wrapper.

The idle threads are created via thread__new() (refcount=1) in
get_idle_thread().  Callers get additional references via thread__get()
which they release with thread__put().  free_idle_threads() drops the
base reference — thread__put() is the correct call, matching the
thread__new() acquisition.

Fixes: 49394a2a24c7 ("perf sched timehist: Introduce timehist command")
Reported-by: sashiko-bot <sashiko-bot@kernel.org>
Cc: David Ahern <dsahern@gmail.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/perf/builtin-sched.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/perf/builtin-sched.c b/tools/perf/builtin-sched.c
index af6322ffaaa6eb1d..3668048ca96cb71a 100644
--- a/tools/perf/builtin-sched.c
+++ b/tools/perf/builtin-sched.c
@@ -2481,7 +2481,7 @@ static void free_idle_threads(void)
 			if (itr)
 				thread__put(itr->last_thread);
 
-			thread__delete(idle);
+			thread__put(idle);
 		}
 	}
 
-- 
2.54.0

[PATCH 8/8] perf sched: Replace BUG_ON and add NULL checks in replay event helpers

[Arnaldo Carvalho de Melo]

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

get_new_event() has three issues:

1. The zalloc() result is dereferenced without a NULL check, crashing
   on allocation failure.

2. BUG_ON(!task->atoms) kills the process when realloc() fails.
   Since perf.data is untrusted input, this should be a graceful error.

3. The realloc pattern assigns directly to task->atoms, losing the old
   pointer on failure.  task->nr_events is also incremented before the
   realloc, leaving corrupted state on failure.

Fix get_new_event() to:
  - Check the zalloc() result before dereferencing
  - Use a temporary for realloc() to avoid losing the old pointer
  - Increment nr_events only after successful realloc
  - Return NULL instead of calling BUG_ON on failure

Also fix add_sched_event_wakeup() where zalloc() for wait_sem is
passed to sem_init() without a NULL check.

Update all callers (add_sched_event_run, add_sched_event_wakeup,
add_sched_event_sleep) to handle NULL returns by returning early.
The replay may produce incomplete output on OOM but will not crash.

Fixes: ec156764d424 ("perf sched: Import schedbench.c")
Reported-by: sashiko-bot <sashiko-bot@kernel.org>
Cc: Ingo Molnar <mingo@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 | 28 +++++++++++++++++++++++++---
 1 file changed, 25 insertions(+), 3 deletions(-)

diff --git a/tools/perf/builtin-sched.c b/tools/perf/builtin-sched.c
index 3668048ca96cb71a..e7e55925532ff6f1 100644
--- a/tools/perf/builtin-sched.c
+++ b/tools/perf/builtin-sched.c
@@ -365,14 +365,25 @@ get_new_event(struct task_desc *task, u64 timestamp)
 	struct sched_atom *event = zalloc(sizeof(*event));
 	unsigned long idx = task->nr_events;
 	size_t size;
+	struct sched_atom **atoms_p;
+
+	if (event == NULL) {
+		pr_err("ERROR: sched: failed to allocate event\n");
+		return NULL;
+	}
 
 	event->timestamp = timestamp;
 	event->nr = idx;
 
+	size = sizeof(struct sched_atom *) * (task->nr_events + 1);
+	atoms_p = realloc(task->atoms, size);
+	if (!atoms_p) {
+		pr_err("ERROR: sched: failed to grow atoms array\n");
+		free(event);
+		return NULL;
+	}
+	task->atoms = atoms_p;
 	task->nr_events++;
-	size = sizeof(struct sched_atom *) * task->nr_events;
-	task->atoms = realloc(task->atoms, size);
-	BUG_ON(!task->atoms);
 
 	task->atoms[idx] = event;
 
@@ -403,6 +414,8 @@ static void add_sched_event_run(struct perf_sched *sched, struct task_desc *task
 	}
 
 	event = get_new_event(task, timestamp);
+	if (event == NULL)
+		return;
 
 	event->type = SCHED_EVENT_RUN;
 	event->duration = duration;
@@ -416,6 +429,8 @@ static void add_sched_event_wakeup(struct perf_sched *sched, struct task_desc *t
 	struct sched_atom *event, *wakee_event;
 
 	event = get_new_event(task, timestamp);
+	if (event == NULL)
+		return;
 	event->type = SCHED_EVENT_WAKEUP;
 	event->wakee = wakee;
 
@@ -430,6 +445,10 @@ static void add_sched_event_wakeup(struct perf_sched *sched, struct task_desc *t
 	}
 
 	wakee_event->wait_sem = zalloc(sizeof(*wakee_event->wait_sem));
+	if (!wakee_event->wait_sem) {
+		pr_err("ERROR: sched: failed to allocate semaphore\n");
+		return;
+	}
 	sem_init(wakee_event->wait_sem, 0, 0);
 	event->wait_sem = wakee_event->wait_sem;
 
@@ -441,6 +460,9 @@ static void add_sched_event_sleep(struct perf_sched *sched, struct task_desc *ta
 {
 	struct sched_atom *event = get_new_event(task, timestamp);
 
+	if (event == NULL)
+		return;
+
 	event->type = SCHED_EVENT_SLEEP;
 
 	sched->nr_sleep_events++;
-- 
2.54.0

Re: [PATCH 8/8] perf sched: Replace BUG_ON and add NULL checks in replay event helpers

[sashiko-bot]

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

Pre-existing issues:
- [Critical] Heap buffer overflow in register_pid() due to unbounded strcpy() from untrusted input.
--

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

perf sched: Replace BUG_ON and add NULL checks in replay event helpers

This commit replaces BUG_ON() calls with graceful error handling and adds
missing NULL checks for memory allocations in the perf sched replay helpers.
It ensures the tool returns early instead of crashing when processing
untrusted perf.data input that triggers memory exhaustion.

>     get_new_event() has three issues:
>     
>     1. The zalloc() result is dereferenced without a NULL check, crashing
>        on allocation failure.
>     
>     2. BUG_ON(!task->atoms) kills the process when realloc() fails.
>        Since perf.data is untrusted input, this should be a graceful error.

[Severity: Critical]
This is a pre-existing issue, but since perf.data is treated as untrusted
input, does register_pid() have a heap buffer overflow?

When register_pid() allocates a new task_desc, it blindly copies the comm
parameter into a fixed-size buffer:

tools/perf/builtin-sched.c:register_pid() {
	...
	task = zalloc(sizeof(*task));
	task->pid = pid;
	task->nr = sched->nr_tasks;
	strcpy(task->comm, comm);
	...
}

If a crafted perf.data file supplies a trace event containing a comm
string significantly longer than the task->comm array (typically 16 bytes),
could this unbounded strcpy() overflow the buffer and overwrite subsequent
struct fields like nr_events or the atoms array pointer?

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

Re: [PATCH 3/8] perf c2c: Bounds-check CPU and node IDs before bitmap and array access

[sashiko-bot]

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

Pre-existing issues:
- [High] The bounds check for `cpu` uses signed comparison, allowing negative indices to bypass the check and cause an out-of-bounds read on `c2c.cpu2node`.
- [High] Dangling pointer in `he__get_c2c_hists()` error path leads to Use-After-Free and Double Free.
--

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

perf c2c: Bounds-check CPU and node IDs before bitmap and array access

This commit adds bounds checks for CPU and node IDs before accessing bitmaps
and arrays in builtin-c2c.c. These checks help ensure that a crafted
perf.data file with exceedingly large IDs cannot cause out-of-bounds heap
writes.

> diff --git a/tools/perf/builtin-c2c.c b/tools/perf/builtin-c2c.c
> --- a/tools/perf/builtin-c2c.c
> +++ b/tools/perf/builtin-c2c.c
[ ... ]
> @@ -386,7 +394,12 @@ static int process_sample_event(const struct perf_tool *tool __maybe_unused,
>  		 * Doing node stats only for single callchain data.
>  		 */
>  		int cpu = sample->cpu == (unsigned int) -1 ? 0 : sample->cpu;
> -		int node = c2c.cpu2node[cpu];
> +		int node;
> +
> +		/* cpu2node[] has c2c.cpus_cnt entries from env->nr_cpus_avail */
> +		if (cpu >= c2c.cpus_cnt)
> +			cpu = 0;
> +		node = c2c.cpu2node[cpu];

[Severity: High]
This isn't a bug introduced by this patch, but does this bounds check
allow negative indices to bypass it?

Because cpu is declared as a signed int, if sample->cpu is a large value
like 0xFFFFFFFE, it converts to a negative integer (e.g., -2). The bounds
check uses signed comparison, so the negative value bypasses the check,
resulting in an out-of-bounds read at c2c.cpu2node[-2].

>  		c2c_hists = he__get_c2c_hists(he, c2c.cl_sort, 2, machine->env);

[Severity: High]
This is a pre-existing issue, but can he__get_c2c_hists() create a
dangling pointer on its error path?

Inside he__get_c2c_hists():

hists = c2c_he->hists = zalloc(sizeof(*hists));
if (!hists)
        return NULL;

ret = c2c_hists__init(hists, sort, nr_header_lines, env);
if (ret) {
        free(hists);
        return NULL;
}

If c2c_hists__init() fails, it frees hists but leaves the dangling pointer
in c2c_he->hists. Because c2c_he outlives the function execution, later
during shutdown, c2c_he_free() will access the dangling pointer and trigger
a use-after-free and double free.

>  		if (!c2c_hists) {
>  			ret = -ENOMEM;
>  			goto out;
>  		}

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

Re: [PATCH 6/8] perf sched: Fix thread reference leak in idle hist processing

[sashiko-bot]

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

Pre-existing issues:
- [High] `timehist_get_thread()` leaks thread references on both success and error paths when `--idle-hist` is active.
- [High] The callchain root tree within `struct idle_thread_runtime` is not freed when idle threads are destroyed, leaking dynamically allocated callchain nodes.
--

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

perf sched: Fix thread reference leak in idle hist processing

timehist_sched_change_event() sets itr->last_thread to NULL at the end
of idle hist processing without calling thread__put() first.  The
thread reference was acquired via thread__get() in timehist_get_thread()
(line 2581), so every idle context switch leaks a thread reference when
--idle-hist is active.

Use thread__zput() to properly release the reference before clearing
the pointer.

> diff --git a/tools/perf/builtin-sched.c b/tools/perf/builtin-sched.c
> index e47db1c5c6020..e2df69a624765 100644
> --- a/tools/perf/builtin-sched.c
> +++ b/tools/perf/builtin-sched.c
> @@ -2890,7 +2890,7 @@ static int timehist_sched_change_event(const struct perf_tool *tool,
>  			if (itr->cursor.nr)
>  				callchain_append(&itr->callchain, &itr->cursor, t - tprev);
>  
> -			itr->last_thread = NULL;
> +			thread__zput(itr->last_thread);

[Severity: High]
This isn't a bug introduced by this patch, but while we are fixing leaks
around idle thread handling, does timehist_get_thread() also leak thread
references when --idle-hist is active?

tools/perf/builtin-sched.c:timehist_get_thread() {
    ...
		if (sched->idle_hist) {
			struct thread *idle;
			struct idle_thread_runtime *itr;

			idle = get_idle_thread(sample->cpu);
			if (idle == NULL) {
				pr_err("Failed to get idle thread for cpu %d.\n", sample->cpu);
				return NULL;
			}
    ...
}

It appears get_idle_thread() returns a new reference via thread__get(). On
success, this local idle thread reference is never released. Also, on the
error paths returning NULL, it seems to bypass the caller's cleanup code and
permanently leak the task thread reference acquired earlier via
machine__findnew_thread().


[Severity: High]
This is a pre-existing issue, but does free_idle_threads() leak dynamically
allocated callchain nodes?

tools/perf/builtin-sched.c:free_idle_threads() {
    ...
		thread__put(idle);
    ...
}

The thread's private data destructor is set to free() via
thread__set_priv_destructor(free). While free() successfully frees the struct
idle_thread_runtime container, it doesn't appear to recursively free the
nodes embedded in the itr->callchain tree built by callchain_append(). Can
this lead to unbounded memory growth proportional to the trace size?

>  		}
>  
>  		if (!sched->summary_only)

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

Re: [PATCH 1/8] perf mmap: Guard cpu__get_node() return in aio_bind()

[sashiko-bot]

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

Pre-existing issues:
- [Medium] A failure during AIO initialization causes a NULL pointer dereference during cleanup in `perf_mmap__aio_munmap`.
--

commit 288781c48aca8cf278f6a1c8e4af510fc3cb0e4c
Author: Arnaldo Carvalho de Melo <acme@redhat.com>

    perf mmap: Guard cpu__get_node() return in aio_bind()
    
    perf_mmap__aio_bind() passes the cpu__get_node() return value directly
    to an unsigned long variable (node_index).  When cpu__get_node() returns
    -1 for an unknown CPU, the implicit int-to-unsigned-long conversion
    sign-extends it to ULONG_MAX. Check the return value in a signed
    temporary before assigning to node_index, and skip the NUMA binding
    when the node is unknown.

> diff --git a/tools/perf/util/mmap.c b/tools/perf/util/mmap.c
> index b69f926d314b1..4404a99eee45f 100644
> --- a/tools/perf/util/mmap.c
> +++ b/tools/perf/util/mmap.c
> @@ -104,9 +104,15 @@ static int perf_mmap__aio_bind(struct mmap *map, int idx, struct perf_cpu cpu, i
>  	int err = 0;
>  
>  	if (affinity != PERF_AFFINITY_SYS && cpu__max_node() > 1) {
> +		int node;
> +
>  		data = map->aio.data[idx];
>  		mmap_len = mmap__mmap_len(map);
> -		node_index = cpu__get_node(cpu);
> +		node = cpu__get_node(cpu);
> +		/* -1 sign-extends to ULONG_MAX, wrapping bitmap_zalloc(0) and OOB __set_bit */
> +		if (node < 0)
> +			return 0;
> +		node_index = node;
>  		node_mask = bitmap_zalloc(node_index + 1);
>  		if (!node_mask) {
>  			pr_err("Failed to allocate node mask for mbind: error %m\n");

[Severity: Medium]
This isn't a bug introduced by this patch, but I noticed a potential NULL
pointer dereference during error cleanup if AIO initialization fails.

If memory allocation fails early in perf_mmap__aio_mmap():

tools/perf/util/mmap.c:perf_mmap__aio_mmap() {
    ...
	map->aio.nr_cblocks = mp->nr_cblocks;
	if (map->aio.nr_cblocks) {
		map->aio.aiocb = calloc(map->aio.nr_cblocks, sizeof(struct aiocb *));
		if (!map->aio.aiocb) {
			pr_debug2("failed to allocate aiocb for data buffer, error %m\n");
			return -1;
		}
    ...
}

The function returns an error, but leaves map->aio.nr_cblocks set to the
requested number of blocks while map->aio.data remains NULL.

This error bubbles up and triggers error recovery, which eventually calls
perf_mmap__aio_munmap():

tools/perf/util/mmap.c:perf_mmap__aio_munmap() {
    ...
	for (i = 0; i < map->aio.nr_cblocks; ++i)
		perf_mmap__aio_free(map, i);
    ...
}

Which loops based on nr_cblocks and passes the map to perf_mmap__aio_free():

tools/perf/util/mmap.c:perf_mmap__aio_free() {
    ...
	if (map->aio.data[idx]) {
    ...
}

Is it possible for map->aio.data to be NULL here? It seems evaluating this
array index (or calling zfree(&(map->aio.data[idx])) when libnuma is not
supported) could cause an immediate NULL pointer dereference.

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

Re: [PATCH 7/8] perf sched: Use thread__put() in free_idle_threads()

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Replacing thread__delete() with thread__put() fails to free idle threads when --idle-hist is enabled.
--

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

perf sched: Use thread__put() in free_idle_threads()

This commit changes the cleanup of idle threads from thread__delete() to
thread__put() to correctly balance reference counts. It aims to avoid
leaking pointer handles in REFCNT_CHECKING builds by routing the object
destruction through the reference count wrapper.

> diff --git a/tools/perf/builtin-sched.c b/tools/perf/builtin-sched.c
> index e2df69a624765..b5a7a944f5c8c 100644
> --- a/tools/perf/builtin-sched.c
> +++ b/tools/perf/builtin-sched.c
> @@ -2442,7 +2442,7 @@ static void free_idle_threads(void)
>  			if (itr)
>  				thread__put(itr->last_thread);
>  
> -			thread__delete(idle);
> +			thread__put(idle);

[Severity: High]
Does this replace an intentional forceful deletion with a reference decrement
that causes a leak when the idle_hist option is enabled?

In timehist_get_thread(), an additional reference to the idle thread is
acquired if sched->idle_hist is enabled:

tools/perf/builtin-sched.c:timehist_get_thread() {
    ...
		if (sched->idle_hist) {
			struct thread *idle;
			struct idle_thread_runtime *itr;

			idle = get_idle_thread(sample->cpu);
			if (idle == NULL) {
    ...
			thread__put(itr->last_thread);
			itr->last_thread = thread__get(thread);
    ...
}

The reference acquired by get_idle_thread() is never released with a matching
thread__put() before the function returns.

Previously, thread__delete() forcefully destroyed the thread object
regardless of this inflated reference count.

Now that the cleanup relies on thread__put() and the reference count reaching
zero, the leaked reference from timehist_get_thread() prevents the thread
object from being freed.

Does this result in permanently leaking the idle thread objects, their
private idle_thread_runtime data, and accumulated callchains?

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

Re: [PATCH 5/8] perf sched: Clean up idle_threads entry on init failure

[sashiko-bot]

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

Pre-existing issues:
- [High] Negative array index out-of-bounds read and write in `get_idle_thread()` due to unchecked `sample->cpu`.
- [Medium] Missing `thread__put()` causes reference count and handle leaks for both `idle` and `thread` variables in `timehist_get_thread()`.
- [Medium] A NULL pointer dereference occurs in `latency_runtime_event()` if thread allocation fails.
- [Medium] Leaked `callchain_cursor_node` structs inside `idle_thread_runtime` when the thread is destroyed.
--

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

    perf sched: Clean up idle_threads entry on init failure
    
    get_idle_thread() allocates a thread via thread__new() and stores it in
    idle_threads[cpu], then calls init_idle_thread() to set up the private
    data. If init_idle_thread() fails, the partially initialized thread is
    left in idle_threads[cpu], causing subsequent calls to return a thread
    without private data. This patch resets the slot on init failure.

> diff --git a/tools/perf/builtin-sched.c b/tools/perf/builtin-sched.c
> index 555247568e7a6..e47db1c5c6020 100644
> --- a/tools/perf/builtin-sched.c
> +++ b/tools/perf/builtin-sched.c
> @@ -2475,8 +2475,11 @@ static struct thread *get_idle_thread(int cpu)

[Severity: High]
This is a pre-existing issue, but does this function properly validate the
cpu index? The sample->cpu field is read from the untrusted perf.data file.
If a sample lacks a CPU, it typically defaults to (u32)-1. When passed to
get_idle_thread() as an int, cpu becomes -1.

The upper-bounds check will evaluate to false:

        if ((cpu >= idle_max_cpu) || (idle_threads == NULL)) {

And then a negative index is used to read and write memory out of bounds:

        if (idle_threads[cpu] == NULL) {
                idle_threads[cpu] = thread__new(0, 0);

Could this unchecked value lead to heap metadata corruption?

>  		idle_threads[cpu] = thread__new(0, 0);
>  
>  		if (idle_threads[cpu]) {
> -			if (init_idle_thread(idle_threads[cpu]) < 0)
> +			if (init_idle_thread(idle_threads[cpu]) < 0) {
> +				/* clean up so next call doesn't find a half-initialized thread */
> +				thread__zput(idle_threads[cpu]);
>  				return NULL;
> +			}
>  		}
>  	}

[Severity: Medium]
This isn't a bug introduced by this patch, but doesn't the
timehist_get_thread() function leak the thread and idle references on error
paths?

tools/perf/builtin-sched.c:timehist_get_thread() {
        ...
        idle = get_idle_thread(sample->cpu);
        if (idle == NULL) {
            ...
        }

        itr = thread__priv(idle);
        if (itr == NULL)
            return NULL;
        ...
}

It appears get_idle_thread() returns a reference to the idle thread that is
never released with thread__put(). Also, returning NULL when itr is NULL
leaks both the newly acquired idle thread and the primary thread reference.
Could this cause a memory leak over time during large traces?

[Severity: Medium]
This is a pre-existing issue, but does latency_runtime_event() dereference
a potentially NULL thread pointer before checking it?

tools/perf/builtin-sched.c:latency_runtime_event() {
        ...
        struct thread *thread = machine__findnew_thread(machine, -1, pid);
        struct work_atoms *atoms = thread_atoms_search(&sched->atom_root,
                                                       thread, &sched->cmp_pid);
        ...
        if (thread == NULL)
            return -1;
        ...
}

If thread allocation fails, machine__findnew_thread() returns NULL, which is
then passed to thread_atoms_search() and dereferenced before the check
if (thread == NULL) is reached. Is it possible to move the NULL check above
the call to thread_atoms_search()?

[Severity: Medium]
This isn't a bug introduced by this patch, but does the idle_thread_runtime
structure permanently leak its callchain_cursor nodes?

When init_idle_thread() sets up the idle thread:

tools/perf/builtin-sched.c:init_idle_thread() {
        ...
        callchain_cursor_reset(&itr->cursor);
        thread__set_priv(thread, itr);
        ...
}

During execution, save_idle_callchain() allocates nodes dynamically via
callchain_cursor__copy(). However, the destructor for thread private data is
typically just free(), meaning the dynamically allocated callchain_cursor_node
elements within itr->cursor are never deep-freed when the idle thread is
destroyed. Should a proper destructor be registered to free these nodes?

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