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

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

[Arnaldo Carvalho de Melo]

Hi,

Fifteen 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 in the no-libelf path (patches 1-3):
  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, and sysfs__read_build_id() trusts
  fstat() on sysfs pseudo-files that report incorrect sizes.  Replaced
  with a fixed stack buffer and single read(), matching symbol-elf.c.

ELF note iteration (patch 4):
  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 5-6):
  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 7):
  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 8):
  hwmon_pmu__describe_items() tests fd > 0, rejecting the valid fd 0.

Undefined behavior in perf sched (patch 9):
  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 10-12):
  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 13):
  c2c hist entries register format list entries but never unregister
  them on free, leaking the list nodes.

O_NONBLOCK for untrusted paths (patch 14):
  open() calls that process paths from perf.data events can hang on
  FIFOs or device nodes planted by a crafted file.  Add O_NONBLOCK as
  defense-in-depth on these foreign file opens.

CoreSight ETM CPU ID validation (patch 15):
  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 (15):
  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: Use fixed buffer in sysfs__read_build_id() for no-libelf build
  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 symbols: Add O_NONBLOCK to DSO open() calls for untrusted paths
  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      | 14 +++++++++++
 tools/perf/util/cs-etm.c         |  9 ++++++-
 tools/perf/util/dso.c            | 18 +++++++++++---
 tools/perf/util/hwmon_pmu.c      |  2 +-
 tools/perf/util/machine.c        |  2 +-
 tools/perf/util/symbol-elf.c     | 10 +++++---
 tools/perf/util/symbol-minimal.c | 54 ++++++++++++++++++++++++----------------
 tools/perf/util/symbol.c         |  2 +-
 11 files changed, 108 insertions(+), 40 deletions(-)

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

Thanks,

- Arnaldo

[PATCH 01/15] 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/15] 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/15] perf symbols: Use fixed buffer in sysfs__read_build_id() for no-libelf build

[Arnaldo Carvalho de Melo]

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

sysfs__read_build_id() in the no-libelf build path uses fstat() to get
the file size and then allocates an exact-sized buffer.  Sysfs pseudo-
files (/sys/kernel/notes) often report st_size=0 or a page-sized value
(4096) regardless of actual content.  When st_size is 0, malloc(0)
returns a zero-sized allocation and read(fd, buf, 0) returns 0, which
matches the expected (ssize_t)buf_size check, then read_build_id() is
called with a zero-length buffer and never finds any notes.

Replace the fstat + malloc approach with a fixed BUFSIZ stack buffer and
a single read(), matching the strategy used by the libelf-based
sysfs__read_build_id() in symbol-elf.c.  This also eliminates the
malloc/free overhead for what is always a small sysfs file.

Reported-by: sashiko-bot <sashiko-bot@kernel.org>
Fixes: b691f64360ecec49 ("perf symbols: Implement poor man's ELF parser")
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/symbol-minimal.c | 41 +++++++++++++++++++-------------
 1 file changed, 24 insertions(+), 17 deletions(-)

diff --git a/tools/perf/util/symbol-minimal.c b/tools/perf/util/symbol-minimal.c
index 0a71d146395271a6..ea2de3d50d33cf33 100644
--- a/tools/perf/util/symbol-minimal.c
+++ b/tools/perf/util/symbol-minimal.c
@@ -220,29 +220,36 @@ int sysfs__read_build_id(const char *filename, struct build_id *bid)
 {
 	int fd;
 	int ret = -1;
-	struct stat stbuf;
-	size_t buf_size;
-	void *buf;
+	/*
+	 * read_build_id() casts this buffer to a u32 note header pointer,
+	 * so it must be aligned for u32 access.
+	 *
+	 * Accessing a char[] through a u32* is technically type-punning
+	 * under C strict aliasing rules, but perf unconditionally builds
+	 * with -fno-strict-aliasing (Makefile.config), so this is safe.
+	 *
+	 * This file is only compiled when libelf is not available — the
+	 * common case uses the libelf-based path in symbol-elf.c instead.
+	 */
+	char buf[BUFSIZ] __aligned(4);
+	ssize_t len;
 
 	fd = open(filename, O_RDONLY);
 	if (fd < 0)
 		return -1;
 
-	if (fstat(fd, &stbuf) < 0)
-		goto out;
-
-	buf_size = stbuf.st_size;
-	buf = malloc(buf_size);
-	if (buf == NULL)
-		goto out;
-
-	if (read(fd, buf, buf_size) != (ssize_t) buf_size)
-		goto out_free;
+	/*
+	 * Use a fixed buffer and a single read() instead of fstat() + malloc(),
+	 * because sysfs pseudo-files often report st_size=0 or 4096
+	 * regardless of actual content size.
+	 *
+	 * BUFSIZ (8192) is more than sufficient: a sysfs build-id note is
+	 * ~36 bytes (12-byte note header + 4-byte "GNU" name + 20-byte SHA1).
+	 */
+	len = read(fd, buf, sizeof(buf));
+	if (len > 0)
+		ret = read_build_id(buf, len, bid, false);
 
-	ret = read_build_id(buf, buf_size, bid, false);
-out_free:
-	free(buf);
-out:
 	close(fd);
 	return ret;
 }
-- 
2.54.0

[PATCH 04/15] 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 05/15] 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 06/15] 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 07/15] 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 08/15] 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 09/15] 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 10/15] 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 11/15] 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 12/15] 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 | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/tools/perf/util/bpf-utils.c b/tools/perf/util/bpf-utils.c
index d6d2c9c190f7afbf..98cf21a9113428dc 100644
--- a/tools/perf/util/bpf-utils.c
+++ b/tools/perf/util/bpf-utils.c
@@ -264,12 +264,26 @@ 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);
+			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 13/15] 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 14/15] perf symbols: Add O_NONBLOCK to DSO open() calls for untrusted paths

[Arnaldo Carvalho de Melo]

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

DSO paths from perf.data MMAP events could reference FIFOs, device
nodes, or other special files.  Opening such paths without O_NONBLOCK
can hang perf report/script indefinitely waiting for a writer or device.

Add O_NONBLOCK to all open() calls in the DSO symbol resolution paths:
filename__read_build_id(), filename__read_debuglink(), symsrc__init(),
do_open(), filename__decompress(), and __dso__read_symbol().

O_NONBLOCK is a no-op on regular files but prevents blocking on special
files, following the systemd "foreign file" convention.  Also add missing
O_CLOEXEC flags in symbol-minimal.c for consistency.

Fixes: a08cae03f430b971 ("perf tools: Allow to close dso fd in case of open failure")
Reported-by: sashiko-bot <sashiko-bot@kernel.org>
Closes: https://sashiko.dev/finding/6
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            | 6 +++---
 tools/perf/util/symbol-elf.c     | 7 ++++---
 tools/perf/util/symbol-minimal.c | 6 +++---
 3 files changed, 10 insertions(+), 9 deletions(-)

diff --git a/tools/perf/util/dso.c b/tools/perf/util/dso.c
index 1a2fc6d18da74d6c..dc0c8b9c63017239 100644
--- a/tools/perf/util/dso.c
+++ b/tools/perf/util/dso.c
@@ -344,7 +344,7 @@ 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);
+		fd = open(name, O_RDONLY | O_CLOEXEC | O_NONBLOCK);
 		if (fd < 0)
 			*err = errno;
 		if (pathname && len > 0)
@@ -547,7 +547,7 @@ static void close_first_dso(void);
 static int do_open(char *name) EXCLUSIVE_LOCKS_REQUIRED(_dso__data_open_lock)
 {
 	do {
-		int fd = open(name, O_RDONLY|O_CLOEXEC);
+		int fd = open(name, O_RDONLY|O_CLOEXEC|O_NONBLOCK);
 
 		if (fd >= 0)
 			return fd;
@@ -1927,7 +1927,7 @@ static const u8 *__dso__read_symbol(struct dso *dso, const char *symfs_filename,
 	int saved_errno;
 
 	nsinfo__mountns_enter(dso__nsinfo(dso), &nsc);
-	fd = open(symfs_filename, O_RDONLY | O_CLOEXEC);
+	fd = open(symfs_filename, O_RDONLY | O_CLOEXEC | O_NONBLOCK);
 	saved_errno = errno;
 	nsinfo__mountns_exit(&nsc);
 	if (fd < 0) {
diff --git a/tools/perf/util/symbol-elf.c b/tools/perf/util/symbol-elf.c
index dc48e2d2763379b9..01c7b09fc82bcddb 100644
--- a/tools/perf/util/symbol-elf.c
+++ b/tools/perf/util/symbol-elf.c
@@ -886,7 +886,8 @@ static int read_build_id(const char *filename, struct build_id *bid)
 	if (size < BUILD_ID_SIZE)
 		goto out;
 
-	fd = open(filename, O_RDONLY | O_CLOEXEC);
+	/* O_NONBLOCK avoids hangs on FIFOs/devices from crafted perf.data paths */
+	fd = open(filename, O_RDONLY | O_CLOEXEC | O_NONBLOCK);
 	if (fd < 0)
 		goto out;
 
@@ -1022,7 +1023,7 @@ int filename__read_debuglink(const char *filename, char *debuglink,
 	if (err >= 0)
 		goto out;
 
-	fd = open(filename, O_RDONLY | O_CLOEXEC);
+	fd = open(filename, O_RDONLY | O_CLOEXEC | O_NONBLOCK);
 	if (fd < 0)
 		goto out;
 
@@ -1187,7 +1188,7 @@ int symsrc__init(struct symsrc *ss, struct dso *dso, const char *name,
 
 		type = dso__symtab_type(dso);
 	} else {
-		fd = open(name, O_RDONLY | O_CLOEXEC);
+		fd = open(name, O_RDONLY | O_CLOEXEC | O_NONBLOCK);
 		if (fd < 0) {
 			*dso__load_errno(dso) = errno;
 			return -1;
diff --git a/tools/perf/util/symbol-minimal.c b/tools/perf/util/symbol-minimal.c
index ea2de3d50d33cf33..dc4a06b2a03ad3cb 100644
--- a/tools/perf/util/symbol-minimal.c
+++ b/tools/perf/util/symbol-minimal.c
@@ -118,7 +118,7 @@ int filename__read_build_id(const char *filename, struct build_id *bid)
 	if (!is_regular_file(filename))
 		return errno == 0 ? -EWOULDBLOCK : -errno;
 
-	fd = open(filename, O_RDONLY);
+	fd = open(filename, O_RDONLY | O_CLOEXEC | O_NONBLOCK);
 	if (fd < 0)
 		return -1;
 
@@ -234,7 +234,7 @@ int sysfs__read_build_id(const char *filename, struct build_id *bid)
 	char buf[BUFSIZ] __aligned(4);
 	ssize_t len;
 
-	fd = open(filename, O_RDONLY);
+	fd = open(filename, O_RDONLY | O_CLOEXEC | O_NONBLOCK);
 	if (fd < 0)
 		return -1;
 
@@ -257,7 +257,7 @@ int sysfs__read_build_id(const char *filename, struct build_id *bid)
 int symsrc__init(struct symsrc *ss, struct dso *dso, const char *name,
 	         enum dso_binary_type type)
 {
-	int fd = open(name, O_RDONLY);
+	int fd = open(name, O_RDONLY | O_CLOEXEC | O_NONBLOCK);
 	if (fd < 0)
 		goto out_errno;
 
-- 
2.54.0

[PATCH 15/15] 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..d8876b8362056c98 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 be a valid index */
+		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 03/15] perf symbols: Use fixed buffer in sysfs__read_build_id() for no-libelf build

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] Type punning via pointer casting from a declared character array to a struct pointer violates strict aliasing rules required for `tools/` code.
--

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

perf symbols: Use fixed buffer in sysfs__read_build_id() for no-libelf build

This commit replaces an fstat() and malloc() approach with a fixed stack
buffer and a single read() in sysfs__read_build_id(). This ensures proper
handling of sysfs pseudo-files that report a size of 0 or 4096 regardless
of actual content.

> diff --git a/tools/perf/util/symbol-minimal.c b/tools/perf/util/symbol-minimal.c
> index 0a71d14639527..ea2de3d50d33c 100644
> --- a/tools/perf/util/symbol-minimal.c
> +++ b/tools/perf/util/symbol-minimal.c
> @@ -220,29 +220,36 @@ int sysfs__read_build_id(const char *filename, struct build_id *bid)
>  {

[ ... ]

> +	/*
> +	 * read_build_id() casts this buffer to a u32 note header pointer,
> +	 * so it must be aligned for u32 access.
> +	 *
> +	 * Accessing a char[] through a u32* is technically type-punning
> +	 * under C strict aliasing rules, but perf unconditionally builds
> +	 * with -fno-strict-aliasing (Makefile.config), so this is safe.
> +	 *
> +	 * This file is only compiled when libelf is not available — the
> +	 * common case uses the libelf-based path in symbol-elf.c instead.
> +	 */
> +	char buf[BUFSIZ] __aligned(4);

[Severity: Low]
While the comment notes that perf currently builds with -fno-strict-aliasing,
the tools directory guidelines require assuming standard -fstrict-aliasing
for all tools code.

Does casting this character array to an incompatible u32 struct pointer
inside read_build_id() violate the strict aliasing rules expected for this
subsystem?

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

Re: [PATCH 12/15] perf bpf: Bounds-check array offsets in bpil_offs_to_addr()

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] Information leak of heap address due to incomplete offset sanitization.
--

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

perf bpf: Bounds-check array offsets in bpil_offs_to_addr()

This commit adds a bounds check to bpil_offs_to_addr() for offsets coming from
untrusted perf.data files. When an offset exceeds data_len, the offset fields
are zeroed out and the pointer conversion is skipped to prevent out-of-bounds
pointer construction.

> diff --git a/tools/perf/util/bpf-utils.c b/tools/perf/util/bpf-utils.c
> --- a/tools/perf/util/bpf-utils.c
> +++ b/tools/perf/util/bpf-utils.c

[ ... ]

> @@ -264,12 +264,26 @@ 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);
> +			continue;

[Severity: Medium]
Does skipping this conversion without clearing the corresponding array bit
in info_linear->arrays leak the heap address later?

When the bounds check fails, the offset is set to 0, but the active array
bit remains set in the info_linear->arrays bitmask.

Later, if the tool rewrites the data (such as during perf inject or archive),
write_bpf_prog_info() will call bpil_addr_to_offs(), which processes all
arrays that still have their bits set. It subtracts the heap base pointer
(info_linear->data) from the array address.

Since the address was explicitly forced to 0 here, the calculation becomes
0 - ptr_to_u64(info_linear->data). This results in the two's complement of
the heap address, which is then saved into the bpf_prog_info struct and
written back out to the perf.data file.

Would it be safer to also clear the active array bit via
info_linear->arrays &= ~(1UL << i) when zeroing out these offsets?

> +		}
>  		addr = offs + ptr_to_u64(info_linear->data);
>  		bpf_prog_info_set_offset_u64(&info_linear->info,
>  					     desc->array_offset, addr);
>  	}
>  }

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

Re: [PATCH 10/15] perf bpf: Validate func_info_rec_size and sub_id in synthesize_bpf_prog_name()

[sashiko-bot]

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

Pre-existing issues:
- [Medium] Arbitrary memory read due to unvalidated array offsets in `bpil_offs_to_addr()` when array bits are unset
--

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

perf bpf: Validate func_info_rec_size and sub_id in synthesize_bpf_prog_name()

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, so bounds checks are added
before computing the pointer offset.

> diff --git a/tools/perf/util/bpf-event.c b/tools/perf/util/bpf-event.c
> index c4594969d7677..fe6fbca508c51 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);

[Severity: Medium]
This is a pre-existing issue, but does casting info->prog_tags to a pointer
and dereferencing it here allow an arbitrary memory read if a malicious
perf.data is provided?

If an attacker clears the array bit in info_linear->arrays,
bpil_offs_to_addr() will silently skip converting or validating the offset:

tools/perf/util/bpf-utils.c:bpil_offs_to_addr() {
    ...
    if ((info_linear->arrays & (1UL << i)) == 0)
        continue;
    ...
}

This leaves the raw, untrusted 64-bit value from the file intact in
info->prog_tags.

> -	if (btf) {
> +	if (btf &&
> +	    info->func_info_rec_size >= sizeof(*finfo) &&
> +	    sub_id < info->nr_func_info) {

[Severity: Medium]
This is a pre-existing issue, but does info->func_info face the same
arbitrary memory read vulnerability?

The func_infos pointer is assigned from info->func_info earlier in
synthesize_bpf_prog_name(). If its array bit is cleared in
info_linear->arrays, it retains the untrusted value.

Since info->nr_func_info is also fully attacker-controlled in this
scenario, could an attacker bypass this bounds check and read from
an arbitrary address?

Similar unvalidated pointer accesses appear to happen in
__bpf_event__print_bpf_prog_info() when it casts info->jited_func_lens
and info->jited_ksyms to pointers.

>  		finfo = func_infos + sub_id * info->func_info_rec_size;
>  		t = btf__type_by_id(btf, finfo->type_id);
>  		if (t)

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

Re: [PATCH 06/15] perf dso: Set error code when open() fails on uncompressed fallback path

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] Type punning by accessing a char array through an incompatible pointer type in tools/.
--

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

perf dso: Set error code when open() fails on uncompressed fallback path

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, leading to a stale
or zero errno. This causes callers to treat fd 0 as a valid DSO file
descriptor.

> diff --git a/tools/perf/util/symbol-minimal.c b/tools/perf/util/symbol-minimal.c
> index 091071d06416e..ea2de3d50d33c 100644
> --- a/tools/perf/util/symbol-minimal.c
> +++ b/tools/perf/util/symbol-minimal.c

[ ... ]

> @@ -217,29 +220,36 @@ int sysfs__read_build_id(const char *filename, struct build_id *bid)
>  {
>  	int fd;
>  	int ret = -1;
> -	struct stat stbuf;
> -	size_t buf_size;
> -	void *buf;
> +	/*
> +	 * read_build_id() casts this buffer to a u32 note header pointer,
> +	 * so it must be aligned for u32 access.
> +	 *
> +	 * Accessing a char[] through a u32* is technically type-punning
> +	 * under C strict aliasing rules, but perf unconditionally builds
> +	 * with -fno-strict-aliasing (Makefile.config), so this is safe.
> +	 *
> +	 * This file is only compiled when libelf is not available — the
> +	 * common case uses the libelf-based path in symbol-elf.c instead.
> +	 */
> +	char buf[BUFSIZ] __aligned(4);

[Severity: Medium]
The comment mentions that perf unconditionally builds with
-fno-strict-aliasing, making this safe. However, do files under the tools/
directory generally assume standard -fstrict-aliasing?

When this character array is passed to read_build_id() and cast to an
incompatible structure pointer type:

sysfs__read_build_id()->read_build_id() {
	nhdr = ptr;
	if (need_swap) {
		nhdr->n_namesz = bswap_32(nhdr->n_namesz);

Could this type punning lead to undefined behavior or compiler optimization
issues if standard compiler flags are used for tools?

> +	ssize_t len;
>  
>  	fd = open(filename, O_RDONLY);
>  	if (fd < 0)
>  		return -1;
>  
> -	if (fstat(fd, &stbuf) < 0)
> -		goto out;
> +	/*
> +	 * Use a fixed buffer and a single read() instead of fstat() + malloc(),
> +	 * because sysfs pseudo-files often report st_size=0 or 4096
> +	 * regardless of actual content size.
> +	 *
> +	 * BUFSIZ (8192) is more than sufficient: a sysfs build-id note is
> +	 * ~36 bytes (12-byte note header + 4-byte "GNU" name + 20-byte SHA1).
> +	 */
> +	len = read(fd, buf, sizeof(buf));
> +	if (len > 0)
> +		ret = read_build_id(buf, len, bid, false);
>  
> -	buf_size = stbuf.st_size;
> -	buf = malloc(buf_size);
> -	if (buf == NULL)
> -		goto out;
> -
> -	if (read(fd, buf, buf_size) != (ssize_t) buf_size)
> -		goto out_free;
> -
> -	ret = read_build_id(buf, buf_size, bid, false);
> -out_free:
> -	free(buf);
> -out:
>  	close(fd);
>  	return ret;
>  }

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

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

[sashiko-bot]

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

Pre-existing issues:
- [High] Memory leak of inner `hist_entry` objects in `entries_in_array` when releasing outer cacheline entries in `c2c_he_free()`.
- [Medium] Memory leak of dynamically allocated format list entries on the error path in `he__get_c2c_hists()`.
--

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

perf c2c: Free format list entries when releasing c2c hist entries

This commit fixes a memory leak in c2c_he_free() by calling
perf_hpp__reset_output_field() before freeing the hists container. This
properly unregisters and frees all format entries that were allocated by
hpp_list__parse().

Closes: https://sashiko.dev/finding/41

> diff --git a/tools/perf/builtin-c2c.c b/tools/perf/builtin-c2c.c
> index e205f58b2f3d3..07c7e8fb315e6 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);

[Severity: High]
This is a pre-existing issue, but does this adequately clear out the inner
entries?

During the tracking phase, node-level events are inserted into the inner
hists of each cacheline c2c_hist_entry, populating
hists->entries_in_array[0]. When perf c2c resorts the main cachelines, it
may merge entries and delete the redundant ones by invoking c2c_he_free().

At this stage, the inner hists of the redundant entries have not yet been
output-resorted, so their entries tree is empty. Because
hists__delete_entries() only cleans the empty entries tree, could the
subsequent zfree(&c2c_he->hists) leak all inner hist_entry objects in
entries_in_array[0] or entries_collapsed?

> +		perf_hpp__reset_output_field(&c2c_he->hists->list);
>  		zfree(&c2c_he->hists);
>  	}

[Severity: Medium]
This is a pre-existing issue, but is there a similar leak of the format
list entries on the error path in he__get_c2c_hists()?

tools/perf/builtin-c2c.c:he__get_c2c_hists() {
    ...
	ret = c2c_hists__init(hists, sort, nr_header_lines, env);
	if (ret) {
		c2c_he->hists = NULL;
		free(hists);
		return NULL;
	}
    ...
}

If c2c_hists__init() fails partway through after appending some format
entries to hists->list, they don't seem to be cleared before freeing the
hists container. Should perf_hpp__reset_output_field() be called here as
well?

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

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

[sashiko-bot]

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

New issues:
- [Medium] The new bounds check allows `INT_MAX`, resulting in undefined behavior via signed integer overflow when calculating `max_cpu + 1`.

Pre-existing issues:
- [Critical] The calculation `sizeof(*metadata) * num_cpu` wraps around on 32-bit architectures, leading to a zero-sized allocation and massive out-of-bounds heap write.
- [High] Global metadata attributes are accessed from the event payload before verifying that the event buffer is large enough.
- [High] Unvalidated `sample.cpu` index leads to an out-of-bounds array access in `cs_etm__get_queue()` when triggered by `PERF_RECORD_AUX_OUTPUT_HW_ID` events.
--

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

perf cs-etm: Reject CPU IDs that would overflow signed comparison

This commit fixes an out-of-bounds access in
cs_etm__process_auxtrace_info_full() caused by a cast from u64 to int. It
adds a validation check to ensure the CPU ID fits within an int before
performing a signed comparison against max_cpu.

Cc: James Clark <james.clark@arm.com>
Cc: Adrian Hunter <adrian.hunter@intel.com>
Reported-by: sashiko-bot <sashiko-bot@kernel.org>
Closes: https://sashiko.dev/finding/2

> diff --git a/tools/perf/util/cs-etm.c b/tools/perf/util/cs-etm.c
> index 5e92359f51a7c..d8876b8362056 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 be a valid index */
> +		if (metadata[j][CS_ETM_CPU] > INT_MAX) {
> +			err = -EINVAL;
> +			goto err_free_metadata;
> +		}

[Severity: Medium]
Does this upper bound check still allow max_cpu to become INT_MAX?

If the trace provides exactly INT_MAX, max_cpu is set to INT_MAX. Later in
cs_etm__process_auxtrace_info_full(), it calls:

    err = auxtrace_queues__init_nr(&etm->queues, max_cpu + 1);

Will computing INT_MAX + 1 cause a signed integer overflow?

[Severity: Critical]
This is a pre-existing issue, but I noticed a potential calculation overflow
earlier in cs_etm__process_auxtrace_info_full():

    num_cpu = ptr[CS_PMU_TYPE_CPUS] & 0xffffffff;
    metadata = zalloc(sizeof(*metadata) * num_cpu);

On 32-bit platforms, could a crafted num_cpu (e.g., 1073741825) cause the
multiplication to wrap around to 4? If zalloc() succeeds with a tiny buffer,
wouldn't the subsequent loop:

    for (int j = 0; j < num_cpu; j++) {
        ...
        metadata[j] = cs_etm__create_meta_blk(...);

continuously write out-of-bounds into the undersized allocation?

[Severity: High]
This is a pre-existing issue, but is it possible to read past the end of the
event payload before its size is fully validated?

In cs_etm__process_auxtrace_info_full():

    ptr = (u64 *) auxtrace_info->priv;
    num_cpu = ptr[CS_PMU_TYPE_CPUS] & 0xffffffff;

Since the initial perf_record_auxtrace_info size check only guarantees
16 bytes, and priv[] starts at offset 16, accessing ptr[CS_PMU_TYPE_CPUS]
reads from offset 24.

Could this access unmapped memory if the event is truncated?

[Severity: High]
This is a pre-existing issue, but is there an unvalidated index used for
queue array lookups?

When processing PERF_RECORD_AUX_OUTPUT_HW_ID events, sample.cpu is parsed
directly from the payload. It gets passed down the call chain:

cs_etm__process_aux_output_hw_id()
  cs_etm__process_trace_id_v0_1()
    cs_etm__get_queue()

Inside cs_etm__get_queue():

    return etm->queues.queue_array[cpu].priv;

Since cpu comes directly from the trace data, could an arbitrarily large or
negative value cause an out-of-bounds read if it exceeds nr_queues?

> +
> +		if ((int)metadata[j][CS_ETM_CPU] > max_cpu)
>  			max_cpu = metadata[j][CS_ETM_CPU];
>  	}

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

Re: [PATCH 07/15] perf tools: Use snprintf() for root_dir path construction

[sashiko-bot]

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

Pre-existing issues:
- [High] Incomplete fix for stack buffer overflows caused by unbounded `--guestmount` arguments.
--

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

perf tools: Use snprintf() for root_dir path construction

This patch mitigates stack buffer overflows in get_kernel_version() and
dso__load_guest_kernel_sym() by replacing sprintf() with snprintf(). It 
prevents unbounded root_dir inputs (from KVM configurations or 
--guestmount) from overflowing the PATH_MAX stack buffers.

> diff --git a/tools/perf/util/machine.c b/tools/perf/util/machine.c
> index da1ad58758afd..58fa57e3d1a15 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);

[Severity: High]
This isn't a bug introduced by this patch, but does the same overflow
vulnerability remain in other places that use symbol_conf.guestmount?

For example, in tools/perf/util/machine.c:machines__findnew(), the unbounded
path is formatted into a stack-allocated buffer:

	if ((pid != HOST_KERNEL_ID) &&
	    (pid != DEFAULT_GUEST_KERNEL_ID) &&
	    (symbol_conf.guestmount)) {
		sprintf(path, "%s/%d", symbol_conf.guestmount, pid);

And in machines__create_guest_kernel_maps():

			sprintf(path, "%s/%s/proc/kallsyms",
				symbol_conf.guestmount,
				namelist[i]->d_name);

Both format unbounded user input into a stack-allocated path buffer using
sprintf(). Should these also be converted to snprintf() to fully prevent
stack buffer overflows?

>  	file = fopen(version, "r");
>  	if (!file)

[ ... ]

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

Re: [PATCH 14/15] perf symbols: Add O_NONBLOCK to DSO open() calls for untrusted paths

[sashiko-bot]

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

Pre-existing issues:
- [High] The patch fails to comprehensively mitigate indefinite hangs caused by crafted perf.data DSO paths pointing to FIFOs because it misses numerous fallback, compression, and auxiliary code paths.
- [Low] Type punning via pointer casting violates the strict aliasing rule for tools code.
--

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

perf symbols: Add O_NONBLOCK to DSO open() calls for untrusted paths

DSO paths from perf.data MMAP events could reference FIFOs, device
nodes, or other special files.  Opening such paths without O_NONBLOCK
can hang perf report/script indefinitely waiting for a writer or device.

Fixes: a08cae03f430b971 ("perf tools: Allow to close dso fd in case of open failure")
Reported-by: sashiko-bot <sashiko-bot@kernel.org>
Closes: https://sashiko.dev/finding/6
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>

> Add O_NONBLOCK to all open() calls in the DSO symbol resolution paths:
> filename__read_build_id(), filename__read_debuglink(), symsrc__init(),
> do_open(), filename__decompress(), and __dso__read_symbol().

[Severity: High]
This isn't a bug introduced by this patch, but are there several other
DSO resolution paths missing this protection?

For example, decompression checks seem to perform blocking open() calls
before filename__decompress() is reached:

- tools/perf/util/zlib.c:gzip_is_compressed()
- tools/perf/util/lzma.c:lzma_is_compressed()

Additionally, blocking opens appear to still be used in:

- tools/perf/util/symbol-elf.c:sysfs__read_build_id()
- tools/perf/util/capstone.c:symbol__disassemble_capstone_powerpc()
- tools/perf/util/unwind-libunwind.c:read_unwind_spec_debug_frame()

Since crafted perf.data paths ending in .xz or .gz might hit these checks
first, will they still cause an indefinite hang?

> diff --git a/tools/perf/util/symbol-minimal.c b/tools/perf/util/symbol-minimal.c
> index ea2de3d50d33c..dc4a06b2a03ad 100644
> --- a/tools/perf/util/symbol-minimal.c
> +++ b/tools/perf/util/symbol-minimal.c

[ ... ]

> @@ -234,7 +234,7 @@ int sysfs__read_build_id(const char *filename, struct build_id *bid)
>  	char buf[BUFSIZ] __aligned(4);
>  	ssize_t len;
>  
> -	fd = open(filename, O_RDONLY);
> +	fd = open(filename, O_RDONLY | O_CLOEXEC | O_NONBLOCK);
>  	if (fd < 0)
>  		return -1;

[Severity: Low]
This is a pre-existing issue, but is it possible this function violates the
strict aliasing rule for tools code?

Later in this function, the char buffer is passed into read_build_id():

    len = read(fd, buf, sizeof(buf));
    if (len > 0)
        ret = read_build_id(buf, len, bid, false);

If read_build_id() casts this buffer to a u32 struct pointer to read the
ELF note header, this would be type punning. Although perf is built with
-fno-strict-aliasing, tools code assumes standard -fstrict-aliasing per
the subsystem guidelines.

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

Re: [PATCH 14/15] perf symbols: Add O_NONBLOCK to DSO open() calls for untrusted paths

[Ian Rogers]

On Thu, Jun 11, 2026 at 5:35 PM Arnaldo Carvalho de Melo
<acme@kernel.org> wrote:
>
> From: Arnaldo Carvalho de Melo <acme@redhat.com>
>
> DSO paths from perf.data MMAP events could reference FIFOs, device
> nodes, or other special files.  Opening such paths without O_NONBLOCK
> can hang perf report/script indefinitely waiting for a writer or device.
>
> Add O_NONBLOCK to all open() calls in the DSO symbol resolution paths:
> filename__read_build_id(), filename__read_debuglink(), symsrc__init(),
> do_open(), filename__decompress(), and __dso__read_symbol().
>
> O_NONBLOCK is a no-op on regular files but prevents blocking on special
> files, following the systemd "foreign file" convention.  Also add missing
> O_CLOEXEC flags in symbol-minimal.c for consistency.

I think these changes are wrong and based on the bad O_NONBLOCK prompt
I fixed. I did the original addition of O_NONBLOCK and James
fixed/removed it with:
https://lore.kernel.org/linux-perf-users/20251124-james-perf-non-block-v2-1-54cdc2f13a24@linaro.org/
As we don't read from files with TEMP_FAILURE_RETRY the use of
O_NONBLOCK is going to mean slow file systems fail. The
is_regular_file tests are intended to avoid the hangs described in the
commit message.

Thanks,
Ian

> Fixes: a08cae03f430b971 ("perf tools: Allow to close dso fd in case of open failure")
> Reported-by: sashiko-bot <sashiko-bot@kernel.org>
> Closes: https://sashiko.dev/finding/6
> 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            | 6 +++---
>  tools/perf/util/symbol-elf.c     | 7 ++++---
>  tools/perf/util/symbol-minimal.c | 6 +++---
>  3 files changed, 10 insertions(+), 9 deletions(-)
>
> diff --git a/tools/perf/util/dso.c b/tools/perf/util/dso.c
> index 1a2fc6d18da74d6c..dc0c8b9c63017239 100644
> --- a/tools/perf/util/dso.c
> +++ b/tools/perf/util/dso.c
> @@ -344,7 +344,7 @@ 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);
> +               fd = open(name, O_RDONLY | O_CLOEXEC | O_NONBLOCK);
>                 if (fd < 0)
>                         *err = errno;
>                 if (pathname && len > 0)
> @@ -547,7 +547,7 @@ static void close_first_dso(void);
>  static int do_open(char *name) EXCLUSIVE_LOCKS_REQUIRED(_dso__data_open_lock)
>  {
>         do {
> -               int fd = open(name, O_RDONLY|O_CLOEXEC);
> +               int fd = open(name, O_RDONLY|O_CLOEXEC|O_NONBLOCK);
>
>                 if (fd >= 0)
>                         return fd;
> @@ -1927,7 +1927,7 @@ static const u8 *__dso__read_symbol(struct dso *dso, const char *symfs_filename,
>         int saved_errno;
>
>         nsinfo__mountns_enter(dso__nsinfo(dso), &nsc);
> -       fd = open(symfs_filename, O_RDONLY | O_CLOEXEC);
> +       fd = open(symfs_filename, O_RDONLY | O_CLOEXEC | O_NONBLOCK);
>         saved_errno = errno;
>         nsinfo__mountns_exit(&nsc);
>         if (fd < 0) {
> diff --git a/tools/perf/util/symbol-elf.c b/tools/perf/util/symbol-elf.c
> index dc48e2d2763379b9..01c7b09fc82bcddb 100644
> --- a/tools/perf/util/symbol-elf.c
> +++ b/tools/perf/util/symbol-elf.c
> @@ -886,7 +886,8 @@ static int read_build_id(const char *filename, struct build_id *bid)
>         if (size < BUILD_ID_SIZE)
>                 goto out;
>
> -       fd = open(filename, O_RDONLY | O_CLOEXEC);
> +       /* O_NONBLOCK avoids hangs on FIFOs/devices from crafted perf.data paths */
> +       fd = open(filename, O_RDONLY | O_CLOEXEC | O_NONBLOCK);
>         if (fd < 0)
>                 goto out;
>
> @@ -1022,7 +1023,7 @@ int filename__read_debuglink(const char *filename, char *debuglink,
>         if (err >= 0)
>                 goto out;
>
> -       fd = open(filename, O_RDONLY | O_CLOEXEC);
> +       fd = open(filename, O_RDONLY | O_CLOEXEC | O_NONBLOCK);
>         if (fd < 0)
>                 goto out;
>
> @@ -1187,7 +1188,7 @@ int symsrc__init(struct symsrc *ss, struct dso *dso, const char *name,
>
>                 type = dso__symtab_type(dso);
>         } else {
> -               fd = open(name, O_RDONLY | O_CLOEXEC);
> +               fd = open(name, O_RDONLY | O_CLOEXEC | O_NONBLOCK);
>                 if (fd < 0) {
>                         *dso__load_errno(dso) = errno;
>                         return -1;
> diff --git a/tools/perf/util/symbol-minimal.c b/tools/perf/util/symbol-minimal.c
> index ea2de3d50d33cf33..dc4a06b2a03ad3cb 100644
> --- a/tools/perf/util/symbol-minimal.c
> +++ b/tools/perf/util/symbol-minimal.c
> @@ -118,7 +118,7 @@ int filename__read_build_id(const char *filename, struct build_id *bid)
>         if (!is_regular_file(filename))
>                 return errno == 0 ? -EWOULDBLOCK : -errno;
>
> -       fd = open(filename, O_RDONLY);
> +       fd = open(filename, O_RDONLY | O_CLOEXEC | O_NONBLOCK);
>         if (fd < 0)
>                 return -1;
>
> @@ -234,7 +234,7 @@ int sysfs__read_build_id(const char *filename, struct build_id *bid)
>         char buf[BUFSIZ] __aligned(4);
>         ssize_t len;
>
> -       fd = open(filename, O_RDONLY);
> +       fd = open(filename, O_RDONLY | O_CLOEXEC | O_NONBLOCK);
>         if (fd < 0)
>                 return -1;
>
> @@ -257,7 +257,7 @@ int sysfs__read_build_id(const char *filename, struct build_id *bid)
>  int symsrc__init(struct symsrc *ss, struct dso *dso, const char *name,
>                  enum dso_binary_type type)
>  {
> -       int fd = open(name, O_RDONLY);
> +       int fd = open(name, O_RDONLY | O_CLOEXEC | O_NONBLOCK);
>         if (fd < 0)
>                 goto out_errno;
>
> --
> 2.54.0
>