[PATCHES v2 00/13] perf tools: Fix pre-existing bugs in symbols, dso, bpf, sched, c2c, hwmon, and cs-etm

[PATCHES v2 00/13] perf tools: Fix pre-existing bugs in symbols, dso, bpf, sched, c2c, hwmon, and cs-etm

[Arnaldo Carvalho de Melo]

Hi,

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

The fixes are grouped by subsystem:

ELF/build-id parsing (patches 1-2):
  symbol-minimal.c carries a copy-paste typo that byte-swaps p_offset
  instead of p_filesz for 32-bit ELF.  The ssize_t p_filesz value is
  used without checking for negative.

ELF note iteration (patch 3):
  sysfs__read_build_id() in the libelf path can loop forever when a
  note section contains zero-filled entries (namesz + descsz == 0).
  Break when no progress can be made.

DSO decompression and open (patches 4-5):
  dso__get_filename() copies a decompressed path with strcpy() into a
  potentially shorter heap buffer.  filename__decompress() fails to set
  the error code on the uncompressed fallback path, leaving callers
  with a stale errno.

Buffer overflow in root_dir path construction (patch 6):
  machine.c and symbol.c use sprintf() to build paths with root_dir,
  which can overflow the fixed-size buffer.  Switch to snprintf().

hwmon fd check (patch 7):
  hwmon_pmu__describe_items() tests fd > 0, rejecting the valid fd 0.

Undefined behavior in perf sched (patch 8):
  map__findnew_thread() uses (void*)1 as a sentinel for colored threads.
  This value gets dereferenced as a struct pointer and passed to free()
  on cleanup.  Replace with a proper allocation and a boolean color flag.

BPF metadata validation (patches 9-11):
  synthesize_bpf_prog_name() trusts func_info_rec_size and sub_id from
  perf.data without validation.  bpf_metadata_alloc() stores the event
  size in a __u16 without overflow checking.  bpil_offs_to_addr()
  converts untrusted offsets to heap pointers without bounds checking.

Memory leak in c2c (patch 12):
  c2c hist entries register format list entries but never unregister
  them on free, leaking the list nodes.

CoreSight ETM CPU ID validation (patch 13):
  cs_etm__process_auxtrace_info_full() compares an unsigned CPU ID
  from perf.data metadata against a signed int without range checking.
  A large unsigned value wraps negative, bypassing the bounds check.

Build-tested with gcc and clang.  Passes perf test on x86_64.

Arnaldo Carvalho de Melo (13):
  perf symbols: Fix bswap copy-paste error for 32-bit ELF p_filesz
  perf symbols: Validate p_filesz before use in filename__read_build_id()
  perf symbols: Break infinite loop on zero-filled notes in sysfs__read_build_id()
  perf dso: Fix heap overflow in dso__get_filename() on decompressed path
  perf dso: Set error code when open() fails on uncompressed fallback path
  perf tools: Use snprintf() for root_dir path construction
  perf hwmon: Fix fd check to accept fd 0 in hwmon_pmu__describe_items()
  perf sched: Replace (void*)1 sentinel with proper runtime allocation
  perf bpf: Validate func_info_rec_size and sub_id in synthesize_bpf_prog_name()
  perf bpf: Reject oversized BPF metadata events that truncate header.size
  perf bpf: Bounds-check array offsets in bpil_offs_to_addr()
  perf c2c: Free format list entries when releasing c2c hist entries
  perf cs-etm: Reject CPU IDs that would overflow signed comparison

 tools/perf/builtin-c2c.c         |  1 +
 tools/perf/builtin-sched.c       | 23 +++++++++++++++++------
 tools/perf/util/bpf-event.c      | 13 ++++++++++++-
 tools/perf/util/bpf-utils.c      | 16 ++++++++++++++++
 tools/perf/util/cs-etm.c         |  9 ++++++++-
 tools/perf/util/dso.c            | 14 ++++++++++++--
 tools/perf/util/hwmon_pmu.c      |  2 +-
 tools/perf/util/machine.c        |  2 +-
 tools/perf/util/symbol-elf.c     |  3 +++
 tools/perf/util/symbol-minimal.c |  5 ++++-
 tools/perf/util/symbol.c         |  2 +-
 11 files changed, 76 insertions(+), 14 deletions(-)

Changes since v1:
- Dropped O_NONBLOCK patch per Ian Rogers' review: without
  TEMP_FAILURE_RETRY, O_NONBLOCK causes slow file systems to fail; the
  is_regular_file() checks are the correct mitigation.
- Dropped fixed-buffer rewrite of sysfs__read_build_id() for the
  no-libelf path (type-punning fix); needs more consideration.
- Patch 11 (bpil bounds check): clear the array bit when zeroing invalid
  offsets, so bpil_addr_to_offs() won't leak the heap address into
  output perf.data.
- Patch 13 (cs-etm): change > INT_MAX to >= INT_MAX, preventing
  max_cpu + 1 signed integer overflow in auxtrace_queues__init_nr().

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

[PATCH 01/13] perf symbols: Fix bswap copy-paste error for 32-bit ELF p_filesz

[Arnaldo Carvalho de Melo]

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

filename__read_build_id() byte-swaps 32-bit ELF program headers on
cross-endian files, but line 178 passes p_offset to bswap_32() instead
of p_filesz:

  hdrs.phdr32[i].p_filesz = bswap_32(hdrs.phdr32[i].p_offset);

This clobbers p_filesz with the already-swapped p_offset value.  The
64-bit path on line 182 is correct and swaps p_filesz from p_filesz.

The consequence is that the PT_NOTE segment read uses the wrong size,
which can cause either a short read (missing the build-id) or an
oversized read (reading past the segment into adjacent data).

Fix by swapping the correct field.

Reported-by: sashiko-bot <sashiko-bot@kernel.org>
Fixes: fef8f648bb47726d ("perf symbol: Fix use-after-free in filename__read_build_id")
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/symbol-minimal.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/perf/util/symbol-minimal.c b/tools/perf/util/symbol-minimal.c
index 091071d06416e290..f4b0a711a62cf3de 100644
--- a/tools/perf/util/symbol-minimal.c
+++ b/tools/perf/util/symbol-minimal.c
@@ -175,7 +175,7 @@ int filename__read_build_id(const char *filename, struct build_id *bid)
 			if (elf32) {
 				hdrs.phdr32[i].p_type = bswap_32(hdrs.phdr32[i].p_type);
 				hdrs.phdr32[i].p_offset = bswap_32(hdrs.phdr32[i].p_offset);
-				hdrs.phdr32[i].p_filesz = bswap_32(hdrs.phdr32[i].p_offset);
+				hdrs.phdr32[i].p_filesz = bswap_32(hdrs.phdr32[i].p_filesz);
 			} else {
 				hdrs.phdr64[i].p_type = bswap_32(hdrs.phdr64[i].p_type);
 				hdrs.phdr64[i].p_offset = bswap_64(hdrs.phdr64[i].p_offset);
-- 
2.54.0

[PATCH 02/13] perf symbols: Validate p_filesz before use in filename__read_build_id()

[Arnaldo Carvalho de Melo]

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

filename__read_build_id() stores ELF p_filesz in a ssize_t variable.
A crafted 32-bit ELF with p_filesz = 0xFFFFFFFF produces ssize_t value
-1.  The comparison `p_filesz > buf_size` evaluates false because signed
-1 is less than any non-negative buf_size, so the realloc is skipped and
buf remains NULL.

The subsequent read(fd, NULL, -1) returns -1, which equals p_filesz,
passing the error check.  read_build_id() then dereferences the NULL
buffer.

Add an explicit check for p_filesz <= 0 before using the value,
catching both zero-length and sign-wrapped negative sizes from crafted
ELF files.

Reported-by: sashiko-bot <sashiko-bot@kernel.org>
Fixes: ba0b7081f7a521d7 ("perf symbol-minimal: Fix ehdr reading in filename__read_build_id")
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/symbol-minimal.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/tools/perf/util/symbol-minimal.c b/tools/perf/util/symbol-minimal.c
index f4b0a711a62cf3de..0a71d146395271a6 100644
--- a/tools/perf/util/symbol-minimal.c
+++ b/tools/perf/util/symbol-minimal.c
@@ -186,6 +186,9 @@ int filename__read_build_id(const char *filename, struct build_id *bid)
 			continue;
 
 		p_filesz = elf32 ? hdrs.phdr32[i].p_filesz : hdrs.phdr64[i].p_filesz;
+		/* ssize_t can go negative with crafted ELF p_filesz values */
+		if (p_filesz <= 0)
+			continue;
 		if (p_filesz > buf_size) {
 			void *tmp;
 
-- 
2.54.0

[PATCH 03/13] perf symbols: Break infinite loop on zero-filled notes in sysfs__read_build_id()

[Arnaldo Carvalho de Melo]

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

sysfs__read_build_id() iterates ELF note headers from sysfs files in a
while(1) loop.  If the file contains a zero-filled note header (both
n_namesz and n_descsz are 0), the code computes n = namesz + descsz = 0
and calls read(fd, bf, 0).  read() with count 0 returns 0, which
matches the expected (ssize_t)n value, so the error check passes and
the loop repeats — reading the same zero bytes and spinning forever.

This can happen with corrupted or zero-padded sysfs pseudo-files.

Add a check for n == 0 before the read, since no valid ELF note has
both name and description of zero length.

Reported-by: sashiko-bot <sashiko-bot@kernel.org>
Fixes: f1617b40596cb341 ("perf symbols: Record the build_ids of kernel modules too")
Assisted-by: Claude Opus 4.6 <noreply@anthropic.com>
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/util/symbol-elf.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/tools/perf/util/symbol-elf.c b/tools/perf/util/symbol-elf.c
index d84e2e031d430cf5..dc48e2d2763379b9 100644
--- a/tools/perf/util/symbol-elf.c
+++ b/tools/perf/util/symbol-elf.c
@@ -995,6 +995,9 @@ int sysfs__read_build_id(const char *filename, struct build_id *bid)
 			} else {
 				n = namesz + descsz;
 			}
+			/* no valid note has both namesz and descsz zero */
+			if (n == 0)
+				break;
 			if (read(fd, bf, n) != (ssize_t)n)
 				break;
 		}
-- 
2.54.0

[PATCH 04/13] perf dso: Fix heap overflow in dso__get_filename() on decompressed path

[Arnaldo Carvalho de Melo]

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

dso__get_filename() allocates name with malloc(PATH_MAX), but the
dso__filename_with_chroot() path replaces name with an asprintf'd
exact-size string (e.g. 8 bytes for "/a/b.ko").  When the DSO needs
decompression, dso__decompress_kmodule_path() writes the temp path
("/tmp/perf-kmod-XXXXXX", 22 bytes) into newpath, and strcpy(name,
newpath) overflows the smaller allocation.

Replace the strcpy with strdup(newpath) + free(name) so the buffer
is always correctly sized for its content.

Reported-by: sashiko-bot <sashiko-bot@kernel.org>
Fixes: 1d6b3c9ba756a513 ("perf tools: Decompress kernel module when reading DSO data")
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/dso.c | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/tools/perf/util/dso.c b/tools/perf/util/dso.c
index 5d017975873817ec..511921bd901d8145 100644
--- a/tools/perf/util/dso.c
+++ b/tools/perf/util/dso.c
@@ -603,8 +603,15 @@ static char *dso__get_filename(struct dso *dso, const char *root_dir,
 
 		/* empty pathname means file wasn't actually compressed */
 		if (newpath[0] != '\0') {
+			char *tmp = strdup(newpath);
+
+			if (!tmp) {
+				unlink(newpath);
+				goto out;
+			}
+			free(name);
+			name = tmp;
 			*decomp = true;
-			strcpy(name, newpath);
 		}
 	}
 	return name;
-- 
2.54.0

[PATCH 05/13] perf dso: Set error code when open() fails on uncompressed fallback path

[Arnaldo Carvalho de Melo]

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

filename__decompress() has an early return for files that are not
actually compressed, where it calls open() directly.  When open()
fails, the function returns -1 but never sets *err.  The caller chain
(decompress_kmodule → dso__decompress_kmodule_path → dso__get_filename)
then reads *dso__load_errno(dso) to set errno, but that field was never
populated, so errno gets a stale or zero value.

With errno=0, __open_dso() computes fd = -errno = 0, which is non-
negative, so callers treat fd 0 (stdin) as a valid DSO file descriptor.

Set *err = errno when open() fails on the uncompressed path, matching
the error handling on the compressed path at line 354.

Reported-by: sashiko-bot <sashiko-bot@kernel.org>
Fixes: 8b42b7e5e8b5692b ("perf tools: Add is_compressed callback to compressions array")
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/util/dso.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/tools/perf/util/dso.c b/tools/perf/util/dso.c
index 511921bd901d8145..1a2fc6d18da74d6c 100644
--- a/tools/perf/util/dso.c
+++ b/tools/perf/util/dso.c
@@ -344,9 +344,12 @@ int filename__decompress(const char *name, char *pathname,
 	 * descriptor to the uncompressed file.
 	 */
 	if (!compressions[comp].is_compressed(name)) {
+		fd = open(name, O_RDONLY | O_CLOEXEC);
+		if (fd < 0)
+			*err = errno;
 		if (pathname && len > 0)
 			pathname[0] = '\0';
-		return open(name, O_RDONLY | O_CLOEXEC);
+		return fd;
 	}
 
 	fd = mkostemp(tmpbuf, O_CLOEXEC);
-- 
2.54.0

[PATCH 06/13] perf tools: Use snprintf() for root_dir path construction

[Arnaldo Carvalho de Melo]

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

get_kernel_version() in machine.c and dso__load_guest_kernel_sym() in
symbol.c use sprintf() to construct paths by prepending root_dir to
"/proc/version" and "/proc/kallsyms" respectively.  Both write into
PATH_MAX stack buffers, but root_dir comes from --guestmount or KVM
configuration and is not length-checked.  A root_dir at or near
PATH_MAX causes a stack buffer overflow.

Switch to snprintf() with sizeof(path) to prevent overflow.

Reported-by: sashiko-bot <sashiko-bot@kernel.org>
Fixes: a1645ce12adb6c9c ("perf: 'perf kvm' tool for monitoring guest performance from host")
Cc: Zhang Yanmin <yanmin_zhang@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/machine.c | 2 +-
 tools/perf/util/symbol.c  | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/tools/perf/util/machine.c b/tools/perf/util/machine.c
index da1ad58758afd9d9..58fa57e3d1a15f37 100644
--- a/tools/perf/util/machine.c
+++ b/tools/perf/util/machine.c
@@ -1336,7 +1336,7 @@ static char *get_kernel_version(const char *root_dir)
 	char *name, *tmp;
 	const char *prefix = "Linux version ";
 
-	sprintf(version, "%s/proc/version", root_dir);
+	snprintf(version, sizeof(version), "%s/proc/version", root_dir);
 	file = fopen(version, "r");
 	if (!file)
 		return NULL;
diff --git a/tools/perf/util/symbol.c b/tools/perf/util/symbol.c
index 0c46b24ee0986059..c2328c9cec1565fc 100644
--- a/tools/perf/util/symbol.c
+++ b/tools/perf/util/symbol.c
@@ -2275,7 +2275,7 @@ static int dso__load_guest_kernel_sym(struct dso *dso, struct map *map)
 		if (!kallsyms_filename)
 			return -1;
 	} else {
-		sprintf(path, "%s/proc/kallsyms", machine->root_dir);
+		snprintf(path, sizeof(path), "%s/proc/kallsyms", machine->root_dir);
 		kallsyms_filename = path;
 	}
 
-- 
2.54.0

[PATCH 07/13] perf hwmon: Fix fd check to accept fd 0 in hwmon_pmu__describe_items()

[Arnaldo Carvalho de Melo]

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

hwmon_pmu__describe_items() checks 'if (fd > 0)' after openat(), which
incorrectly rejects fd 0.  While fd 0 is normally stdin, if stdin has
been closed (common in daemon/service contexts), the kernel reuses fd 0
for the next open.  With fd > 0, the sysfs file is not read and the fd
is leaked.

Change to 'if (fd >= 0)' to match the standard openat() error check.

Reported-by: sashiko-bot <sashiko-bot@kernel.org>
Fixes: 53cc0b351ec99278 ("perf hwmon_pmu: Add a tool PMU exposing events from hwmon in sysfs")
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/hwmon_pmu.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/perf/util/hwmon_pmu.c b/tools/perf/util/hwmon_pmu.c
index efd3067a2e591050..ed544dca70c380b2 100644
--- a/tools/perf/util/hwmon_pmu.c
+++ b/tools/perf/util/hwmon_pmu.c
@@ -435,7 +435,7 @@ static size_t hwmon_pmu__describe_items(struct hwmon_pmu *hwm, char *out_buf, si
 			hwmon_item_strs[bit],
 			is_alarm ? "_alarm" : "");
 		fd = openat(dir, buf, O_RDONLY);
-		if (fd > 0) {
+		if (fd >= 0) {
 			ssize_t read_len = read(fd, buf, sizeof(buf) - 1);
 
 			while (read_len > 0 && buf[read_len - 1] == '\n')
-- 
2.54.0

[PATCH 08/13] perf sched: Replace (void*)1 sentinel with proper runtime allocation

[Arnaldo Carvalho de Melo]

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

map__findnew_thread() marks color-pid threads by storing (void*)1 as
the thread private data via thread__set_priv().  This sentinel value
causes two problems:

1. thread__get_runtime() returns (void*)1 as a struct thread_runtime
   pointer.  Any field access (e.g. tr->shortname) dereferences address
   1, which is an unmapped page — immediate segfault.

2. cmd_sched() registers free() as the thread priv destructor, so thread
   cleanup calls free((void*)1) — undefined behavior that corrupts the
   heap on many allocators.

Fix by adding a 'color' flag to struct thread_runtime and allocating a
real runtime struct for color-pid threads.  thread__has_color() now
checks the flag instead of relying on priv being non-NULL.

Reported-by: sashiko-bot <sashiko-bot@kernel.org>
Fixes: 58a606149c60d5da ("perf sched: Avoid union type punning undefined behavior")
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-sched.c | 23 +++++++++++++++++------
 1 file changed, 17 insertions(+), 6 deletions(-)

diff --git a/tools/perf/builtin-sched.c b/tools/perf/builtin-sched.c
index 5f16caebd9645da0..7fd63a9db4574fbf 100644
--- a/tools/perf/builtin-sched.c
+++ b/tools/perf/builtin-sched.c
@@ -274,6 +274,7 @@ struct thread_runtime {
 	u64 migrations;
 
 	int prio;
+	bool color;
 };
 
 /* per event run time data */
@@ -1589,22 +1590,32 @@ static int process_sched_wakeup_ignore(const struct perf_tool *tool __maybe_unus
 
 static bool thread__has_color(struct thread *thread)
 {
-	return thread__priv(thread) != NULL;
+	struct thread_runtime *tr = thread__priv(thread);
+
+	return tr != NULL && tr->color;
 }
 
 static struct thread*
 map__findnew_thread(struct perf_sched *sched, struct machine *machine, pid_t pid, pid_t tid)
 {
 	struct thread *thread = machine__findnew_thread(machine, pid, tid);
-	bool color = false;
 
-	if (!sched->map.color_pids || !thread || thread__priv(thread))
+	if (!sched->map.color_pids || !thread)
 		return thread;
 
-	if (thread_map__has(sched->map.color_pids, tid))
-		color = true;
+	/*
+	 * Always check the color-pids map, even if thread__priv() is
+	 * already set.  COMM events processed before the first sched_switch
+	 * allocate a thread_runtime via thread__get_runtime(), so priv is
+	 * non-NULL before we ever get here.  Skipping the check on non-NULL
+	 * priv would prevent those threads from being colored.
+	 */
+	if (thread_map__has(sched->map.color_pids, tid)) {
+		struct thread_runtime *tr = thread__get_runtime(thread);
 
-	thread__set_priv(thread, color ? ((void*)1) : NULL);
+		if (tr)
+			tr->color = true;
+	}
 	return thread;
 }
 
-- 
2.54.0

[PATCH 09/13] perf bpf: Validate func_info_rec_size and sub_id in synthesize_bpf_prog_name()

[Arnaldo Carvalho de Melo]

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

synthesize_bpf_prog_name() computes a pointer into the func_info array
using sub_id * info->func_info_rec_size without validating either value.
Both come from perf.data and are untrusted:

- A func_info_rec_size smaller than sizeof(struct bpf_func_info) means
  the finfo pointer would reference a truncated entry, reading past it
  into adjacent data.

- A sub_id >= nr_func_info computes an offset past the func_info buffer,
  causing an out-of-bounds read.

Add bounds checks for both values before computing the pointer offset.
When validation fails, fall through to the non-BTF name path instead
of reading garbage.

Reported-by: sashiko-bot <sashiko-bot@kernel.org>
Fixes: 7b612e291a5affb1 ("perf tools: Synthesize PERF_RECORD_* for loaded BPF programs")
Cc: Song Liu <songliubraving@fb.com>
Assisted-by: Claude Opus 4.6 <noreply@anthropic.com>
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/util/bpf-event.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/tools/perf/util/bpf-event.c b/tools/perf/util/bpf-event.c
index c4594969d7677238..fe6fbca508c5135c 100644
--- a/tools/perf/util/bpf-event.c
+++ b/tools/perf/util/bpf-event.c
@@ -143,7 +143,9 @@ static int synthesize_bpf_prog_name(char *buf, int size,
 	name_len = scnprintf(buf, size, "bpf_prog_");
 	name_len += snprintf_hex(buf + name_len, size - name_len,
 				 prog_tags[sub_id], BPF_TAG_SIZE);
-	if (btf) {
+	if (btf &&
+	    info->func_info_rec_size >= sizeof(*finfo) &&
+	    sub_id < info->nr_func_info) {
 		finfo = func_infos + sub_id * info->func_info_rec_size;
 		t = btf__type_by_id(btf, finfo->type_id);
 		if (t)
-- 
2.54.0

[PATCH 10/13] perf bpf: Reject oversized BPF metadata events that truncate header.size

[Arnaldo Carvalho de Melo]

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

bpf_metadata_alloc() computes event_size from the number of BPF metadata
variables and stores it in header.size, which is __u16.  With 204 or
more .rodata variables prefixed "bpf_metadata_", event_size exceeds
65535 and silently truncates.

The truncated header.size causes synthesize_perf_record_bpf_metadata()
to allocate a buffer sized by the truncated value, then memcpy the full
event data into it — a heap buffer overflow.

Add a check that event_size fits in __u16 before proceeding.  BPF
programs with that many metadata variables are exotic enough that
silently dropping the metadata is acceptable.

Reported-by: sashiko-bot <sashiko-bot@kernel.org>
Fixes: ab38e84ba9a80581 ("perf record: collect BPF metadata from existing BPF programs")
Cc: Blake Jones <blakejones@google.com>
Assisted-by: Claude Opus 4.6 <noreply@anthropic.com>
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/util/bpf-event.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/tools/perf/util/bpf-event.c b/tools/perf/util/bpf-event.c
index fe6fbca508c5135c..57d53ba848359e12 100644
--- a/tools/perf/util/bpf-event.c
+++ b/tools/perf/util/bpf-event.c
@@ -369,6 +369,15 @@ static struct bpf_metadata *bpf_metadata_alloc(__u32 nr_prog_tags,
 
 	event_size = sizeof(metadata->event->bpf_metadata) +
 	    nr_variables * sizeof(metadata->event->bpf_metadata.entries[0]);
+	/*
+	 * header.size is __u16.  synthesize_perf_record_bpf_metadata()
+	 * adds machine->id_hdr_size (up to ~64 bytes) after this, so
+	 * leave headroom to prevent the final size from wrapping.
+	 */
+	if (event_size > UINT16_MAX - 256) {
+		bpf_metadata_free(metadata);
+		return NULL;
+	}
 	metadata->event = zalloc(event_size);
 	if (!metadata->event) {
 		bpf_metadata_free(metadata);
-- 
2.54.0

[PATCH 11/13] perf bpf: Bounds-check array offsets in bpil_offs_to_addr()

[Arnaldo Carvalho de Melo]

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

bpil_offs_to_addr() converts offsets stored in perf.data's
bpf_prog_info_linear structure into heap pointers by adding the offset
to the data allocation base.  The offsets come from untrusted file input
and are not validated against data_len.

If an offset exceeds data_len, the computed address points outside the
allocated data buffer.  Callers like synthesize_bpf_prog_name() then
dereference prog_tags[sub_id] or func_info pointers, reading arbitrary
heap memory.

Add a bounds check: when an offset exceeds data_len, zero the field
and skip the conversion.  This prevents out-of-bounds pointer
construction from crafted perf.data files.

Reported-by: sashiko-bot <sashiko-bot@kernel.org>
Fixes: 6ac22d036f86c4e2 ("perf bpf: Pull in bpf_program__get_prog_info_linear()")
Cc: Dave Marchevsky <davemarchevsky@fb.com>
Assisted-by: Claude Opus 4.6 <noreply@anthropic.com>
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/util/bpf-utils.c | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

diff --git a/tools/perf/util/bpf-utils.c b/tools/perf/util/bpf-utils.c
index d6d2c9c190f7afbf..69148197b1ef9e8b 100644
--- a/tools/perf/util/bpf-utils.c
+++ b/tools/perf/util/bpf-utils.c
@@ -264,12 +264,28 @@ void bpil_offs_to_addr(struct perf_bpil *info_linear)
 	for (i = PERF_BPIL_FIRST_ARRAY; i < PERF_BPIL_LAST_ARRAY; ++i) {
 		const struct bpil_array_desc *desc = &bpil_array_desc[i];
 		__u64 addr, offs;
+		__u32 count, size;
 
 		if ((info_linear->arrays & (1UL << i)) == 0)
 			continue;
 
 		offs = bpf_prog_info_read_offset_u64(&info_linear->info,
 						     desc->array_offset);
+		count = bpf_prog_info_read_offset_u32(&info_linear->info,
+						      desc->count_offset);
+		size = bpf_prog_info_read_offset_u32(&info_linear->info,
+						     desc->size_offset);
+		/* offset and extent from perf.data are untrusted — keep within data[] */
+		if (offs >= info_linear->data_len ||
+		    (u64)count * size > info_linear->data_len - offs) {
+			bpf_prog_info_set_offset_u64(&info_linear->info,
+						     desc->array_offset, 0);
+			bpf_prog_info_set_offset_u32(&info_linear->info,
+						     desc->count_offset, 0);
+			/* clear the bit so bpil_addr_to_offs() won't reverse a zeroed address */
+			info_linear->arrays &= ~(1UL << i);
+			continue;
+		}
 		addr = offs + ptr_to_u64(info_linear->data);
 		bpf_prog_info_set_offset_u64(&info_linear->info,
 					     desc->array_offset, addr);
-- 
2.54.0

[PATCH 12/13] perf c2c: Free format list entries when releasing c2c hist entries

[Arnaldo Carvalho de Melo]

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

c2c_hists__init() calls hpp_list__parse() which allocates and registers
format entries on hists->list.  When c2c_he_free() destroys a c2c hist
entry, it deletes the histogram entries and frees the hists container but
never unregisters the format list entries, leaking them.

Call perf_hpp__reset_output_field() before freeing the hists to properly
unregister and free all format entries.

Fixes: f485e33c4543ac31 ("perf c2c report: Add cacheline hists processing")
Reported-by: sashiko-bot <sashiko-bot@kernel.org>
Closes: https://sashiko.dev/finding/41
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 e205f58b2f3d3786..07c7e8fb315e6cf3 100644
--- a/tools/perf/builtin-c2c.c
+++ b/tools/perf/builtin-c2c.c
@@ -185,6 +185,7 @@ static void c2c_he_free(void *he)
 	c2c_he = container_of(he, struct c2c_hist_entry, he);
 	if (c2c_he->hists) {
 		hists__delete_entries(&c2c_he->hists->hists);
+		perf_hpp__reset_output_field(&c2c_he->hists->list);
 		zfree(&c2c_he->hists);
 	}
 
-- 
2.54.0

[PATCH 13/13] perf cs-etm: Reject CPU IDs that would overflow signed comparison

[Arnaldo Carvalho de Melo]

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

metadata[j][CS_ETM_CPU] is a u64 from perf.data, but the comparison
with max_cpu casts it to (int).  A crafted value like 0xFFFFFFFF becomes
-1 after the cast, which compares less than max_cpu (0), so the queue
array is never sized to accommodate it.  When the value is later passed
to cs_etm__get_queue(), it indexes queue_array with the original large
value, causing an out-of-bounds access.

Validate that CS_ETM_CPU fits in an int before using it in the signed
comparison.

Fixes: 57880a7966be510c ("perf: cs-etm: Allocate queues for all CPUs")
Reported-by: sashiko-bot <sashiko-bot@kernel.org>
Closes: https://sashiko.dev/finding/2
Cc: James Clark <james.clark@arm.com>
Cc: Adrian Hunter <adrian.hunter@intel.com>
Assisted-by: Claude Opus 4.6 <noreply@anthropic.com>
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/util/cs-etm.c | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/tools/perf/util/cs-etm.c b/tools/perf/util/cs-etm.c
index 5e92359f51a7cb87..0927b0b9c06b1504 100644
--- a/tools/perf/util/cs-etm.c
+++ b/tools/perf/util/cs-etm.c
@@ -6,6 +6,7 @@
  * Author: Mathieu Poirier <mathieu.poirier@linaro.org>
  */
 
+#include <limits.h>
 #include <linux/bitfield.h>
 #include <linux/bitops.h>
 #include <linux/coresight-pmu.h>
@@ -3468,7 +3469,13 @@ int cs_etm__process_auxtrace_info_full(union perf_event *event,
 			goto err_free_metadata;
 		}
 
-		if ((int) metadata[j][CS_ETM_CPU] > max_cpu)
+		/* CPU id comes from perf.data and must fit max_cpu + 1 without overflow */
+		if (metadata[j][CS_ETM_CPU] >= INT_MAX) {
+			err = -EINVAL;
+			goto err_free_metadata;
+		}
+
+		if ((int)metadata[j][CS_ETM_CPU] > max_cpu)
 			max_cpu = metadata[j][CS_ETM_CPU];
 	}
 
-- 
2.54.0

Re: [PATCH 01/13] perf symbols: Fix bswap copy-paste error for 32-bit ELF p_filesz

[Ian Rogers]

On Fri, Jun 12, 2026 at 3:24 PM Arnaldo Carvalho de Melo
<acme@kernel.org> wrote:
>
> From: Arnaldo Carvalho de Melo <acme@redhat.com>
>
> filename__read_build_id() byte-swaps 32-bit ELF program headers on
> cross-endian files, but line 178 passes p_offset to bswap_32() instead
> of p_filesz:
>
>   hdrs.phdr32[i].p_filesz = bswap_32(hdrs.phdr32[i].p_offset);
>
> This clobbers p_filesz with the already-swapped p_offset value.  The
> 64-bit path on line 182 is correct and swaps p_filesz from p_filesz.
>
> The consequence is that the PT_NOTE segment read uses the wrong size,
> which can cause either a short read (missing the build-id) or an
> oversized read (reading past the segment into adjacent data).
>
> Fix by swapping the correct field.
>
> Reported-by: sashiko-bot <sashiko-bot@kernel.org>
> Fixes: fef8f648bb47726d ("perf symbol: Fix use-after-free in filename__read_build_id")
> 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>

Reviewed-by: Ian Rogers <irogers@google.com>

Thanks,
Ian

> ---
>  tools/perf/util/symbol-minimal.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/tools/perf/util/symbol-minimal.c b/tools/perf/util/symbol-minimal.c
> index 091071d06416e290..f4b0a711a62cf3de 100644
> --- a/tools/perf/util/symbol-minimal.c
> +++ b/tools/perf/util/symbol-minimal.c
> @@ -175,7 +175,7 @@ int filename__read_build_id(const char *filename, struct build_id *bid)
>                         if (elf32) {
>                                 hdrs.phdr32[i].p_type = bswap_32(hdrs.phdr32[i].p_type);
>                                 hdrs.phdr32[i].p_offset = bswap_32(hdrs.phdr32[i].p_offset);
> -                               hdrs.phdr32[i].p_filesz = bswap_32(hdrs.phdr32[i].p_offset);
> +                               hdrs.phdr32[i].p_filesz = bswap_32(hdrs.phdr32[i].p_filesz);
>                         } else {
>                                 hdrs.phdr64[i].p_type = bswap_32(hdrs.phdr64[i].p_type);
>                                 hdrs.phdr64[i].p_offset = bswap_64(hdrs.phdr64[i].p_offset);
> --
> 2.54.0
>

Re: [PATCH 02/13] perf symbols: Validate p_filesz before use in filename__read_build_id()

[Ian Rogers]

On Fri, Jun 12, 2026 at 3:24 PM Arnaldo Carvalho de Melo
<acme@kernel.org> wrote:
>
> From: Arnaldo Carvalho de Melo <acme@redhat.com>
>
> filename__read_build_id() stores ELF p_filesz in a ssize_t variable.
> A crafted 32-bit ELF with p_filesz = 0xFFFFFFFF produces ssize_t value
> -1.  The comparison `p_filesz > buf_size` evaluates false because signed
> -1 is less than any non-negative buf_size, so the realloc is skipped and
> buf remains NULL.
>
> The subsequent read(fd, NULL, -1) returns -1, which equals p_filesz,
> passing the error check.  read_build_id() then dereferences the NULL
> buffer.
>
> Add an explicit check for p_filesz <= 0 before using the value,
> catching both zero-length and sign-wrapped negative sizes from crafted
> ELF files.
>
> Reported-by: sashiko-bot <sashiko-bot@kernel.org>
> Fixes: ba0b7081f7a521d7 ("perf symbol-minimal: Fix ehdr reading in filename__read_build_id")
> 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>

Reviewed-by: Ian Rogers <irogers@google.com>

Thanks,
Ian

> ---
>  tools/perf/util/symbol-minimal.c | 3 +++
>  1 file changed, 3 insertions(+)
>
> diff --git a/tools/perf/util/symbol-minimal.c b/tools/perf/util/symbol-minimal.c
> index f4b0a711a62cf3de..0a71d146395271a6 100644
> --- a/tools/perf/util/symbol-minimal.c
> +++ b/tools/perf/util/symbol-minimal.c
> @@ -186,6 +186,9 @@ int filename__read_build_id(const char *filename, struct build_id *bid)
>                         continue;
>
>                 p_filesz = elf32 ? hdrs.phdr32[i].p_filesz : hdrs.phdr64[i].p_filesz;
> +               /* ssize_t can go negative with crafted ELF p_filesz values */
> +               if (p_filesz <= 0)
> +                       continue;
>                 if (p_filesz > buf_size) {
>                         void *tmp;
>
> --
> 2.54.0
>

Re: [PATCH 03/13] perf symbols: Break infinite loop on zero-filled notes in sysfs__read_build_id()

[Ian Rogers]

On Fri, Jun 12, 2026 at 3:24 PM Arnaldo Carvalho de Melo
<acme@kernel.org> wrote:
>
> From: Arnaldo Carvalho de Melo <acme@redhat.com>
>
> sysfs__read_build_id() iterates ELF note headers from sysfs files in a
> while(1) loop.  If the file contains a zero-filled note header (both
> n_namesz and n_descsz are 0), the code computes n = namesz + descsz = 0
> and calls read(fd, bf, 0).  read() with count 0 returns 0, which
> matches the expected (ssize_t)n value, so the error check passes and
> the loop repeats — reading the same zero bytes and spinning forever.
>
> This can happen with corrupted or zero-padded sysfs pseudo-files.
>
> Add a check for n == 0 before the read, since no valid ELF note has
> both name and description of zero length.
>
> Reported-by: sashiko-bot <sashiko-bot@kernel.org>
> Fixes: f1617b40596cb341 ("perf symbols: Record the build_ids of kernel modules too")
> Assisted-by: Claude Opus 4.6 <noreply@anthropic.com>
> Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>

Reviewed-by: Ian Rogers <irogers@google.com>

Thanks,
Ian

> ---
>  tools/perf/util/symbol-elf.c | 3 +++
>  1 file changed, 3 insertions(+)
>
> diff --git a/tools/perf/util/symbol-elf.c b/tools/perf/util/symbol-elf.c
> index d84e2e031d430cf5..dc48e2d2763379b9 100644
> --- a/tools/perf/util/symbol-elf.c
> +++ b/tools/perf/util/symbol-elf.c
> @@ -995,6 +995,9 @@ int sysfs__read_build_id(const char *filename, struct build_id *bid)
>                         } else {
>                                 n = namesz + descsz;
>                         }
> +                       /* no valid note has both namesz and descsz zero */
> +                       if (n == 0)
> +                               break;
>                         if (read(fd, bf, n) != (ssize_t)n)
>                                 break;
>                 }
> --
> 2.54.0
>

Re: [PATCH 04/13] perf dso: Fix heap overflow in dso__get_filename() on decompressed path

[Ian Rogers]

On Fri, Jun 12, 2026 at 3:24 PM Arnaldo Carvalho de Melo
<acme@kernel.org> wrote:
>
> From: Arnaldo Carvalho de Melo <acme@redhat.com>
>
> dso__get_filename() allocates name with malloc(PATH_MAX), but the
> dso__filename_with_chroot() path replaces name with an asprintf'd
> exact-size string (e.g. 8 bytes for "/a/b.ko").  When the DSO needs
> decompression, dso__decompress_kmodule_path() writes the temp path
> ("/tmp/perf-kmod-XXXXXX", 22 bytes) into newpath, and strcpy(name,
> newpath) overflows the smaller allocation.
>
> Replace the strcpy with strdup(newpath) + free(name) so the buffer
> is always correctly sized for its content.
>
> Reported-by: sashiko-bot <sashiko-bot@kernel.org>
> Fixes: 1d6b3c9ba756a513 ("perf tools: Decompress kernel module when reading DSO data")
> 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>

Reviewed-by: Ian Rogers <irogers@google.com>

Thanks,
Ian

> ---
>  tools/perf/util/dso.c | 9 ++++++++-
>  1 file changed, 8 insertions(+), 1 deletion(-)
>
> diff --git a/tools/perf/util/dso.c b/tools/perf/util/dso.c
> index 5d017975873817ec..511921bd901d8145 100644
> --- a/tools/perf/util/dso.c
> +++ b/tools/perf/util/dso.c
> @@ -603,8 +603,15 @@ static char *dso__get_filename(struct dso *dso, const char *root_dir,
>
>                 /* empty pathname means file wasn't actually compressed */
>                 if (newpath[0] != '\0') {
> +                       char *tmp = strdup(newpath);
> +
> +                       if (!tmp) {
> +                               unlink(newpath);
> +                               goto out;
> +                       }
> +                       free(name);
> +                       name = tmp;
>                         *decomp = true;
> -                       strcpy(name, newpath);
>                 }
>         }
>         return name;
> --
> 2.54.0
>

Re: [PATCH 07/13] perf hwmon: Fix fd check to accept fd 0 in hwmon_pmu__describe_items()

[Ian Rogers]

On Fri, Jun 12, 2026 at 3:24 PM Arnaldo Carvalho de Melo
<acme@kernel.org> wrote:
>
> From: Arnaldo Carvalho de Melo <acme@redhat.com>
>
> hwmon_pmu__describe_items() checks 'if (fd > 0)' after openat(), which
> incorrectly rejects fd 0.  While fd 0 is normally stdin, if stdin has
> been closed (common in daemon/service contexts), the kernel reuses fd 0
> for the next open.  With fd > 0, the sysfs file is not read and the fd
> is leaked.
>
> Change to 'if (fd >= 0)' to match the standard openat() error check.
>
> Reported-by: sashiko-bot <sashiko-bot@kernel.org>
> Fixes: 53cc0b351ec99278 ("perf hwmon_pmu: Add a tool PMU exposing events from hwmon in sysfs")
> 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>

Reviewed-by: Ian Rogers <irogers@google.com>

Thanks,
Ian

> ---
>  tools/perf/util/hwmon_pmu.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/tools/perf/util/hwmon_pmu.c b/tools/perf/util/hwmon_pmu.c
> index efd3067a2e591050..ed544dca70c380b2 100644
> --- a/tools/perf/util/hwmon_pmu.c
> +++ b/tools/perf/util/hwmon_pmu.c
> @@ -435,7 +435,7 @@ static size_t hwmon_pmu__describe_items(struct hwmon_pmu *hwm, char *out_buf, si
>                         hwmon_item_strs[bit],
>                         is_alarm ? "_alarm" : "");
>                 fd = openat(dir, buf, O_RDONLY);
> -               if (fd > 0) {
> +               if (fd >= 0) {
>                         ssize_t read_len = read(fd, buf, sizeof(buf) - 1);
>
>                         while (read_len > 0 && buf[read_len - 1] == '\n')
> --
> 2.54.0
>

Re: [PATCH 08/13] perf sched: Replace (void*)1 sentinel with proper runtime allocation

[Ian Rogers]

On Fri, Jun 12, 2026 at 3:24 PM Arnaldo Carvalho de Melo
<acme@kernel.org> wrote:
>
> From: Arnaldo Carvalho de Melo <acme@redhat.com>
>
> map__findnew_thread() marks color-pid threads by storing (void*)1 as
> the thread private data via thread__set_priv().  This sentinel value
> causes two problems:
>
> 1. thread__get_runtime() returns (void*)1 as a struct thread_runtime
>    pointer.  Any field access (e.g. tr->shortname) dereferences address
>    1, which is an unmapped page — immediate segfault.
>
> 2. cmd_sched() registers free() as the thread priv destructor, so thread
>    cleanup calls free((void*)1) — undefined behavior that corrupts the
>    heap on many allocators.
>
> Fix by adding a 'color' flag to struct thread_runtime and allocating a
> real runtime struct for color-pid threads.  thread__has_color() now
> checks the flag instead of relying on priv being non-NULL.
>
> Reported-by: sashiko-bot <sashiko-bot@kernel.org>
> Fixes: 58a606149c60d5da ("perf sched: Avoid union type punning undefined behavior")
> 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>

Reviewed-by: Ian Rogers <irogers@google.com>

Thanks,
Ian

> ---
>  tools/perf/builtin-sched.c | 23 +++++++++++++++++------
>  1 file changed, 17 insertions(+), 6 deletions(-)
>
> diff --git a/tools/perf/builtin-sched.c b/tools/perf/builtin-sched.c
> index 5f16caebd9645da0..7fd63a9db4574fbf 100644
> --- a/tools/perf/builtin-sched.c
> +++ b/tools/perf/builtin-sched.c
> @@ -274,6 +274,7 @@ struct thread_runtime {
>         u64 migrations;
>
>         int prio;
> +       bool color;
>  };
>
>  /* per event run time data */
> @@ -1589,22 +1590,32 @@ static int process_sched_wakeup_ignore(const struct perf_tool *tool __maybe_unus
>
>  static bool thread__has_color(struct thread *thread)
>  {
> -       return thread__priv(thread) != NULL;
> +       struct thread_runtime *tr = thread__priv(thread);
> +
> +       return tr != NULL && tr->color;
>  }
>
>  static struct thread*
>  map__findnew_thread(struct perf_sched *sched, struct machine *machine, pid_t pid, pid_t tid)
>  {
>         struct thread *thread = machine__findnew_thread(machine, pid, tid);
> -       bool color = false;
>
> -       if (!sched->map.color_pids || !thread || thread__priv(thread))
> +       if (!sched->map.color_pids || !thread)
>                 return thread;
>
> -       if (thread_map__has(sched->map.color_pids, tid))
> -               color = true;
> +       /*
> +        * Always check the color-pids map, even if thread__priv() is
> +        * already set.  COMM events processed before the first sched_switch
> +        * allocate a thread_runtime via thread__get_runtime(), so priv is
> +        * non-NULL before we ever get here.  Skipping the check on non-NULL
> +        * priv would prevent those threads from being colored.
> +        */
> +       if (thread_map__has(sched->map.color_pids, tid)) {
> +               struct thread_runtime *tr = thread__get_runtime(thread);
>
> -       thread__set_priv(thread, color ? ((void*)1) : NULL);
> +               if (tr)
> +                       tr->color = true;
> +       }
>         return thread;
>  }
>
> --
> 2.54.0
>

Re: [PATCH 10/13] perf bpf: Reject oversized BPF metadata events that truncate header.size

[Ian Rogers]

On Fri, Jun 12, 2026 at 3:24 PM Arnaldo Carvalho de Melo
<acme@kernel.org> wrote:
>
> From: Arnaldo Carvalho de Melo <acme@redhat.com>
>
> bpf_metadata_alloc() computes event_size from the number of BPF metadata
> variables and stores it in header.size, which is __u16.  With 204 or
> more .rodata variables prefixed "bpf_metadata_", event_size exceeds
> 65535 and silently truncates.
>
> The truncated header.size causes synthesize_perf_record_bpf_metadata()
> to allocate a buffer sized by the truncated value, then memcpy the full
> event data into it — a heap buffer overflow.
>
> Add a check that event_size fits in __u16 before proceeding.  BPF
> programs with that many metadata variables are exotic enough that
> silently dropping the metadata is acceptable.
>
> Reported-by: sashiko-bot <sashiko-bot@kernel.org>
> Fixes: ab38e84ba9a80581 ("perf record: collect BPF metadata from existing BPF programs")
> Cc: Blake Jones <blakejones@google.com>
> Assisted-by: Claude Opus 4.6 <noreply@anthropic.com>
> Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>

Reviewed-by: Ian Rogers <irogers@google.com>

Thanks,
Ian

> ---
>  tools/perf/util/bpf-event.c | 9 +++++++++
>  1 file changed, 9 insertions(+)
>
> diff --git a/tools/perf/util/bpf-event.c b/tools/perf/util/bpf-event.c
> index fe6fbca508c5135c..57d53ba848359e12 100644
> --- a/tools/perf/util/bpf-event.c
> +++ b/tools/perf/util/bpf-event.c
> @@ -369,6 +369,15 @@ static struct bpf_metadata *bpf_metadata_alloc(__u32 nr_prog_tags,
>
>         event_size = sizeof(metadata->event->bpf_metadata) +
>             nr_variables * sizeof(metadata->event->bpf_metadata.entries[0]);
> +       /*
> +        * header.size is __u16.  synthesize_perf_record_bpf_metadata()
> +        * adds machine->id_hdr_size (up to ~64 bytes) after this, so
> +        * leave headroom to prevent the final size from wrapping.
> +        */
> +       if (event_size > UINT16_MAX - 256) {
> +               bpf_metadata_free(metadata);
> +               return NULL;
> +       }
>         metadata->event = zalloc(event_size);
>         if (!metadata->event) {
>                 bpf_metadata_free(metadata);
> --
> 2.54.0
>

Re: [PATCH 12/13] perf c2c: Free format list entries when releasing c2c hist entries

[Ian Rogers]

On Fri, Jun 12, 2026 at 3:25 PM Arnaldo Carvalho de Melo
<acme@kernel.org> wrote:
>
> From: Arnaldo Carvalho de Melo <acme@redhat.com>
>
> c2c_hists__init() calls hpp_list__parse() which allocates and registers
> format entries on hists->list.  When c2c_he_free() destroys a c2c hist
> entry, it deletes the histogram entries and frees the hists container but
> never unregisters the format list entries, leaking them.
>
> Call perf_hpp__reset_output_field() before freeing the hists to properly
> unregister and free all format entries.
>
> Fixes: f485e33c4543ac31 ("perf c2c report: Add cacheline hists processing")
> Reported-by: sashiko-bot <sashiko-bot@kernel.org>
> Closes: https://sashiko.dev/finding/41

I don't think these are public "Closes:" links and should probably be stripped.

Thanks,
Ian

> 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 e205f58b2f3d3786..07c7e8fb315e6cf3 100644
> --- a/tools/perf/builtin-c2c.c
> +++ b/tools/perf/builtin-c2c.c
> @@ -185,6 +185,7 @@ static void c2c_he_free(void *he)
>         c2c_he = container_of(he, struct c2c_hist_entry, he);
>         if (c2c_he->hists) {
>                 hists__delete_entries(&c2c_he->hists->hists);
> +               perf_hpp__reset_output_field(&c2c_he->hists->list);
>                 zfree(&c2c_he->hists);
>         }
>
> --
> 2.54.0
>

Re: [PATCH 12/13] perf c2c: Free format list entries when releasing c2c hist entries

[Arnaldo Carvalho de Melo]

On Mon, Jun 15, 2026 at 10:23:45AM -0700, Ian Rogers wrote:
> On Fri, Jun 12, 2026 at 3:25 PM Arnaldo Carvalho de Melo
> <acme@kernel.org> wrote:
> >
> > From: Arnaldo Carvalho de Melo <acme@redhat.com>
> >
> > c2c_hists__init() calls hpp_list__parse() which allocates and registers
> > format entries on hists->list.  When c2c_he_free() destroys a c2c hist
> > entry, it deletes the histogram entries and frees the hists container but
> > never unregisters the format list entries, leaking them.
> >
> > Call perf_hpp__reset_output_field() before freeing the hists to properly
> > unregister and free all format entries.
> >
> > Fixes: f485e33c4543ac31 ("perf c2c report: Add cacheline hists processing")
> > Reported-by: sashiko-bot <sashiko-bot@kernel.org>
> > Closes: https://sashiko.dev/finding/41
> 
> I don't think these are public "Closes:" links and should probably be stripped.

Yeap, will remove and add a memory for this not to be added again,
better to make sure that whatever is added _is_ accessible, this one
isn't :-\

- Arnaldo
 
> Thanks,
> Ian
> 
> > 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 e205f58b2f3d3786..07c7e8fb315e6cf3 100644
> > --- a/tools/perf/builtin-c2c.c
> > +++ b/tools/perf/builtin-c2c.c
> > @@ -185,6 +185,7 @@ static void c2c_he_free(void *he)
> >         c2c_he = container_of(he, struct c2c_hist_entry, he);
> >         if (c2c_he->hists) {
> >                 hists__delete_entries(&c2c_he->hists->hists);
> > +               perf_hpp__reset_output_field(&c2c_he->hists->list);
> >                 zfree(&c2c_he->hists);
> >         }
> >
> > --
> > 2.54.0
> >