[PATCHES v1 00/23] perf tools: Fix OOB accesses, leaks, and missing bounds checks across pmu/symbols/bpf/hwmon

[PATCHES v1 00/23] perf tools: Fix OOB accesses, leaks, and missing bounds checks across pmu/symbols/bpf/hwmon

[Arnaldo Carvalho de Melo]

Hi,

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

The fixes fall into several recurring patterns:

Empty/short sysfs file reads (patches 1, 2, 3, 8, 9, 14, 17):
  Multiple functions that read sysfs files assume a non-empty result.
  When the file is empty or the read returns zero bytes,
  str[len - 1] = '\0' underwrites the heap (pmu_id), scale[-1]
  accesses out of bounds (parse_scale), atoi/strtoull read
  uninitialized stack bytes (filename__read_int/ull), and
  thread__set_comm_from_proc passes an unterminated heap buffer
  to strlen() via thread__set_comm().

snprintf accumulation bugs (patches 7, 10, 15):
  snprintf returns the would-have-written count on truncation.
  Code that accumulates into a fixed buffer using snprintf return
  values overshoots the buffer size, causing size_t underflow on
  subsequent sizeof(buf) - buf_used calculations.  Switch to
  scnprintf which returns actual bytes written.

ELF/build-id parsing (patches 4, 5, 12, 22, 23):
  sysfs__read_build_id() has signed integer overflow when summing
  namesz + descsz.  filename__read_debuglink() copies section data
  without checking d_size.  The GNU build-id fallback path lacks
  descsz validation.  elf_read_build_id() iterates note sections
  without bounds-checking the note header or name/desc sizes against
  the section data buffer.  The no-libelf build path (symbol-minimal.c)
  has the same note iteration vulnerability.

fd leak prevention (patch 6):
  mkstemp() creates file descriptors without O_CLOEXEC, leaking
  them to child processes.  Replace with mkostemp(., O_CLOEXEC).

Uninitialized pathname on uncompressed fallback (patch 13):
  filename__decompress() left pathname uninitialized when the file
  was not compressed, causing four callers to treat stale stack
  contents as a temp file path and potentially unlink real files.

Buffer overflows (patches 11, 16, 18):
  parse_hwmon_filename() passes sizeof(buf) + 1 to strlcpy.
  dso__read_running_kernel_build_id() uses sprintf without bounds.
  mount_overload() passes name_len instead of sizeof(upper_name)
  to snprintf, and mem_toupper scans past the actual string.

BPF metadata bugs (patches 19, 20, 21):
  synthesize_bpf_prog_name() dereferences btf__type_by_id() without
  NULL check.  bpf_metadata_create() leaks partially built map data
  on allocation failure.  perf_env__add_bpf_info() leaks metadata
  when inserting a duplicate info node.

Most require unusual sysfs contents, crafted ELF files, or specific
allocation failure timing to trigger.  Verified with gcc and clang
builds, checkpatch, and perf test.

Arnaldo Carvalho de Melo (23):
  perf pmu: Fix pmu_id() heap underwrite on empty identifier file
  perf pmu: Fix perf_pmu__parse_scale/unit() OOB access on empty sysfs file
  tools lib api: Fix missing null termination in filename__read_int/ull()
  perf symbols: Fix signed overflow in sysfs__read_build_id() size check
  perf symbols: Bounds-check .gnu_debuglink section data
  perf tools: Use mkostemp() for O_CLOEXEC on temporary files
  perf intel-pt: Fix snprintf size tracking bug in insn decoder
  perf tools: Fix thread__set_comm_from_proc() on empty comm file
  perf hwmon: Fix off-by-one null termination on sysfs reads
  perf hwmon: Use scnprintf() in hwmon_pmu__for_each_event()
  perf hwmon: Fix parse_hwmon_filename() strlcpy buffer overflow
  perf symbols: Bounds-check descsz in sysfs__read_build_id() GNU fallback
  perf tools: Fix uninitialized pathname on uncompressed fallback in filename__decompress()
  perf hwmon: Guard label read against empty or failed reads
  perf pmu: Use scnprintf() in format_alias()
  perf tools: Use snprintf() in dso__read_running_kernel_build_id()
  tools lib api: Fix filename__write_int() writing uninitialized stack data
  tools lib api: Fix mount_overload() snprintf truncation and toupper range
  perf bpf: Add NULL check for btf__type_by_id() in synthesize_bpf_prog_name()
  perf bpf: Fix map data leak in bpf_metadata_create() on alloc failure
  perf bpf: Fix metadata leak in perf_env__add_bpf_info() on duplicate insert
  perf symbols: Add bounds checks to elf_read_build_id() note iteration
  perf symbols: Add bounds checks to read_build_id() note iteration in minimal build

 tools/lib/api/fs/fs.c                              | 19 ++++---
 tools/perf/tests/code-reading.c                    |  7 ++-
 tools/perf/util/bpf-event.c                        |  8 ++-
 tools/perf/util/disasm.c                           |  7 ++-
 tools/perf/util/dso.c                              | 16 ++++--
 tools/perf/util/hwmon_pmu.c                        | 22 ++++----
 .../util/intel-pt-decoder/intel-pt-insn-decoder.c  | 11 ++--
 tools/perf/util/pmu.c                              | 14 ++++--
 tools/perf/util/symbol-elf.c                       | 58 +++++++++++++++++-----
 tools/perf/util/symbol-minimal.c                   | 11 +++-
 tools/perf/util/thread.c                           |  5 ++
 11 files changed, 126 insertions(+), 52 deletions(-)

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

Thanks,

- Arnaldo

[PATCH 01/23] perf pmu: Fix pmu_id() heap underwrite on empty identifier file

[Arnaldo Carvalho de Melo]

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

pmu_id() calls filename__read_str() then strips the trailing newline
via str[len - 1] = 0.  If the PMU identifier file is empty,
filename__read_str() succeeds with len = 0.  len - 1 underflows
size_t to SIZE_MAX, writing a null byte before the heap allocation.

Add a len == 0 check before the newline stripping.

Fixes: 51d548471510843e ("perf pmu: Add pmu_id()")
Reported-by: sashiko-bot <sashiko-bot@kernel.org>
Cc: John Garry <john.g.garry@oracle.com>
Assisted-by: Claude Opus 4.6 <noreply@anthropic.com>
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/util/pmu.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/tools/perf/util/pmu.c b/tools/perf/util/pmu.c
index 9994709ef12be9ee..50f54674430e6206 100644
--- a/tools/perf/util/pmu.c
+++ b/tools/perf/util/pmu.c
@@ -865,6 +865,12 @@ static char *pmu_id(const char *name)
 	if (filename__read_str(path, &str, &len) < 0)
 		return NULL;
 
+	/* empty identifier file — nothing useful */
+	if (len == 0) {
+		free(str);
+		return NULL;
+	}
+
 	str[len - 1] = 0; /* remove line feed */
 
 	return str;
-- 
2.54.0

[PATCH 02/23] perf pmu: Fix perf_pmu__parse_scale/unit() OOB access on empty sysfs file

[Arnaldo Carvalho de Melo]

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

perf_pmu__parse_scale() reads a PMU scale file then accesses
scale[sret - 1] to strip a trailing newline.  Only sret < 0 is
guarded, so an empty file (sret == 0) causes scale[-1] — a stack
buffer underflow that reads and potentially writes out of bounds.

perf_pmu__parse_unit() has the same pattern: alias->unit[sret - 1]
with sret == 0 accesses the byte before the struct member, which
may corrupt the adjacent pmu_name pointer field.

Change both guards from sret < 0 to sret <= 0 so that empty files
are treated as read errors.

Fixes: 410136f5dd96b601 ("tools/perf/stat: Add event unit and scale support")
Reported-by: sashiko-bot <sashiko-bot@kernel.org>
Cc: Stephane Eranian <eranian@google.com>
Assisted-by: Claude Opus 4.6 <noreply@anthropic.com>
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/util/pmu.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/tools/perf/util/pmu.c b/tools/perf/util/pmu.c
index 50f54674430e6206..4ddccc863727d6ca 100644
--- a/tools/perf/util/pmu.c
+++ b/tools/perf/util/pmu.c
@@ -314,7 +314,7 @@ static int perf_pmu__parse_scale(struct perf_pmu *pmu, struct perf_pmu_alias *al
 		goto error;
 
 	sret = read(fd, scale, sizeof(scale)-1);
-	if (sret < 0)
+	if (sret <= 0)
 		goto error;
 
 	if (scale[sret - 1] == '\n')
@@ -346,7 +346,7 @@ static int perf_pmu__parse_unit(struct perf_pmu *pmu, struct perf_pmu_alias *ali
 		return -1;
 
 	sret = read(fd, alias->unit, UNIT_MAX_LEN);
-	if (sret < 0)
+	if (sret <= 0)
 		goto error;
 
 	close(fd);
-- 
2.54.0

[PATCH 03/23] tools lib api: Fix missing null termination in filename__read_int/ull()

[Arnaldo Carvalho de Melo]

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

filename__read_int() passes a stack buffer to read() using the full
sizeof(line) and then hands it to atoi() without null-terminating.
If a sysfs file fills the 64-byte buffer exactly, atoi() reads past
the array into uninitialized stack memory.

filename__read_ull_base() has the same issue with strtoull().

Fix both by reading sizeof(line) - 1 bytes and explicitly
null-terminating after a successful read.

Fixes: 3a351127cbc682c3 ("tools lib fs: Adopt filename__read_int from tools/perf/")
Reported-by: sashiko-bot <sashiko-bot@kernel.org>
Assisted-by: Claude Opus 4.6 <noreply@anthropic.com>
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/lib/api/fs/fs.c | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/tools/lib/api/fs/fs.c b/tools/lib/api/fs/fs.c
index edec23406dbc619f..3cc302d4c47b1669 100644
--- a/tools/lib/api/fs/fs.c
+++ b/tools/lib/api/fs/fs.c
@@ -294,11 +294,14 @@ int filename__read_int(const char *filename, int *value)
 {
 	char line[64];
 	int fd = open(filename, O_RDONLY), err = -1;
+	ssize_t n;
 
 	if (fd < 0)
 		return -errno;
 
-	if (read(fd, line, sizeof(line)) > 0) {
+	n = read(fd, line, sizeof(line) - 1);
+	if (n > 0) {
+		line[n] = '\0';
 		*value = atoi(line);
 		err = 0;
 	}
@@ -312,11 +315,14 @@ static int filename__read_ull_base(const char *filename,
 {
 	char line[64];
 	int fd = open(filename, O_RDONLY), err = -1;
+	ssize_t n;
 
 	if (fd < 0)
 		return -errno;
 
-	if (read(fd, line, sizeof(line)) > 0) {
+	n = read(fd, line, sizeof(line) - 1);
+	if (n > 0) {
+		line[n] = '\0';
 		*value = strtoull(line, NULL, base);
 		if (*value != ULLONG_MAX)
 			err = 0;
-- 
2.54.0

[PATCH 04/23] perf symbols: Fix signed overflow in sysfs__read_build_id() size check

[Arnaldo Carvalho de Melo]

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

sysfs__read_build_id() reads ELF note headers from sysfs files.  The
note's namesz and descsz fields are used to compute the skip size:

    int n = namesz + descsz;
    if (n > (int)sizeof(bf))

Both namesz and descsz are size_t from NOTE_ALIGN() of 32-bit note
header fields.  Their sum can exceed INT_MAX, overflowing the signed
int n to a negative value.  The check n > sizeof(bf) then evaluates
false (negative < positive in signed comparison), and read(fd, bf, n)
reinterprets the negative n as a huge size_t count — the kernel writes
up to MAX_RW_COUNT bytes into the 8192-byte stack buffer.

In practice the overflow is bounded by the sysfs file's actual size,
so a real sysfs notes file won't trigger it organically.  But crafted
input (e.g. via a mounted debugfs/sysfs image) could.

Fix by validating namesz and descsz individually against the buffer
size before summing, and change n to size_t to avoid the signed
overflow entirely.

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

diff --git a/tools/perf/util/symbol-elf.c b/tools/perf/util/symbol-elf.c
index c2bdfd0003df2abe..8fb25a5692b56c53 100644
--- a/tools/perf/util/symbol-elf.c
+++ b/tools/perf/util/symbol-elf.c
@@ -964,14 +964,17 @@ int sysfs__read_build_id(const char *filename, struct build_id *bid)
 			} else if (read(fd, bf, descsz) != (ssize_t)descsz)
 				break;
 		} else {
-			int n = namesz + descsz;
+			size_t n;
 
-			if (n > (int)sizeof(bf)) {
+			/* int sum of namesz+descsz can overflow negative, bypassing size check */
+			if (namesz > sizeof(bf) || descsz > sizeof(bf) - namesz) {
 				n = sizeof(bf);
 				pr_debug("%s: truncating reading of build id in sysfs file %s: n_namesz=%u, n_descsz=%u.\n",
 					 __func__, filename, nhdr.n_namesz, nhdr.n_descsz);
+			} else {
+				n = namesz + descsz;
 			}
-			if (read(fd, bf, n) != n)
+			if (read(fd, bf, n) != (ssize_t)n)
 				break;
 		}
 	}
-- 
2.54.0

[PATCH 05/23] perf symbols: Bounds-check .gnu_debuglink section data

[Arnaldo Carvalho de Melo]

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

filename__read_debuglink() copies .gnu_debuglink section data into a
caller-provided buffer via:

    strncpy(debuglink, data->d_buf, size);

where size is PATH_MAX.  If the ELF section is smaller than size and
lacks a null terminator, strncpy reads past data->d_buf into adjacent
memory.  A malformed ELF file can trigger this, potentially causing a
segfault or leaking heap data.

Additionally, strncpy does not guarantee null termination when the
source fills the buffer.

Replace with an explicit memcpy bounded by both the output buffer
size and the actual section data size (data->d_size), followed by
explicit null termination.

Fixes: e5a1845fc0aeca85 ("perf symbols: Split out util/symbol-elf.c")
Reported-by: sashiko-bot <sashiko-bot@kernel.org>
Cc: Namhyung Kim <namhyung@kernel.org>
Assisted-by: Claude Opus 4.6 <noreply@anthropic.com>
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/util/symbol-elf.c | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/tools/perf/util/symbol-elf.c b/tools/perf/util/symbol-elf.c
index 8fb25a5692b56c53..51e7cfe0f5934875 100644
--- a/tools/perf/util/symbol-elf.c
+++ b/tools/perf/util/symbol-elf.c
@@ -1027,7 +1027,14 @@ int filename__read_debuglink(const char *filename, char *debuglink,
 		goto out_elf_end;
 
 	/* the start of this section is a zero-terminated string */
-	strncpy(debuglink, data->d_buf, size);
+	if (data->d_size > 0) {
+		size_t len = min(size - 1, data->d_size);
+
+		memcpy(debuglink, data->d_buf, len);
+		debuglink[len] = '\0';
+	} else {
+		debuglink[0] = '\0';
+	}
 
 	err = 0;
 
-- 
2.54.0

[PATCH 06/23] perf tools: Use mkostemp() for O_CLOEXEC on temporary files

[Arnaldo Carvalho de Melo]

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

mkstemp() creates file descriptors without the close-on-exec flag.
These fds leak to child processes spawned during symbol resolution
(addr2line, objdump), wasting descriptors and potentially exposing
temporary file contents.

Replace mkstemp() with mkostemp(tmpbuf, O_CLOEXEC) at all three
call sites:

  - filename__decompress() in dso.c
  - read_gnu_debugdata() in symbol-elf.c
  - kcore__init() in symbol-elf.c

Fixes: 42b3fa670825983f ("perf tools: Introduce dso__decompress_kmodule_{fd,path}")
Fixes: b10f74308e130527 ("perf symbol: Support .gnu_debugdata for symbols")
Fixes: afba19d9dc8eba66 ("perf symbols: Workaround objdump difficulties with kcore")
Reported-by: sashiko-bot <sashiko-bot@kernel.org>
Cc: Adrian Hunter <adrian.hunter@intel.com>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Stephen Brennan <stephen.s.brennan@oracle.com>
Assisted-by: Claude Opus 4.6 <noreply@anthropic.com>
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/util/dso.c        | 2 +-
 tools/perf/util/symbol-elf.c | 6 +++---
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/tools/perf/util/dso.c b/tools/perf/util/dso.c
index fb2e78fe2aa8eb94..ee06a252a54d338d 100644
--- a/tools/perf/util/dso.c
+++ b/tools/perf/util/dso.c
@@ -346,7 +346,7 @@ int filename__decompress(const char *name, char *pathname,
 	if (!compressions[comp].is_compressed(name))
 		return open(name, O_RDONLY | O_CLOEXEC);
 
-	fd = mkstemp(tmpbuf);
+	fd = mkostemp(tmpbuf, O_CLOEXEC);
 	if (fd < 0) {
 		*err = errno;
 		return -1;
diff --git a/tools/perf/util/symbol-elf.c b/tools/perf/util/symbol-elf.c
index 51e7cfe0f5934875..36a0304707e13138 100644
--- a/tools/perf/util/symbol-elf.c
+++ b/tools/perf/util/symbol-elf.c
@@ -1119,9 +1119,9 @@ static Elf *read_gnu_debugdata(struct dso *dso, Elf *elf, const char *name, int
 		return NULL;
 	}
 
-	temp_fd = mkstemp(temp_filename);
+	temp_fd = mkostemp(temp_filename, O_CLOEXEC);
 	if (temp_fd < 0) {
-		pr_debug("%s: mkstemp: %m\n", __func__);
+		pr_debug("%s: mkostemp: %m\n", __func__);
 		*dso__load_errno(dso) = -errno;
 		fclose(wrapped);
 		return NULL;
@@ -1993,7 +1993,7 @@ static int kcore__init(struct kcore *kcore, char *filename, int elfclass,
 	kcore->elfclass = elfclass;
 
 	if (temp)
-		kcore->fd = mkstemp(filename);
+		kcore->fd = mkostemp(filename, O_CLOEXEC);
 	else
 		kcore->fd = open(filename, O_WRONLY | O_CREAT | O_EXCL | O_CLOEXEC, 0400);
 	if (kcore->fd == -1)
-- 
2.54.0

[PATCH 07/23] perf intel-pt: Fix snprintf size tracking bug in insn decoder

[Arnaldo Carvalho de Melo]

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

dump_insn() tracks remaining buffer space with a 'left' variable,
but the loop subtracts the cumulative offset 'n' each iteration
instead of just the per-iteration delta:

    n += snprintf(x->out + n, left, "%02x ", inbuf[i]);
    left -= n;  /* BUG: n is cumulative, not the delta */

After two iterations left goes massively negative, wrapping to a
huge value when passed as size_t to snprintf(), disabling all bounds
checking for the rest of the loop.

Switch to scnprintf() accumulation using sizeof(x->out) - n as the
remaining space, which is always correct and eliminates the separate
'left' variable entirely.

Fixes: 48d02a1d5c137d36 ("perf script: Add 'brstackinsn' for branch stacks")
Reported-by: sashiko-bot <sashiko-bot@kernel.org>
Cc: Adrian Hunter <adrian.hunter@intel.com>
Cc: Andi Kleen <ak@linux.intel.com>
Assisted-by: Claude Opus 4.6 <noreply@anthropic.com>
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 .../util/intel-pt-decoder/intel-pt-insn-decoder.c     | 11 +++--------
 1 file changed, 3 insertions(+), 8 deletions(-)

diff --git a/tools/perf/util/intel-pt-decoder/intel-pt-insn-decoder.c b/tools/perf/util/intel-pt-decoder/intel-pt-insn-decoder.c
index 72c7a4e15d617b60..f90fcbc4302df58f 100644
--- a/tools/perf/util/intel-pt-decoder/intel-pt-insn-decoder.c
+++ b/tools/perf/util/intel-pt-decoder/intel-pt-insn-decoder.c
@@ -220,7 +220,6 @@ const char *dump_insn(struct perf_insn *x, uint64_t ip __maybe_unused,
 {
 	struct insn insn;
 	int n, i, ret;
-	int left;
 
 	ret = insn_decode(&insn, inbuf, inlen,
 			  x->is64bit ? INSN_MODE_64 : INSN_MODE_32);
@@ -229,13 +228,9 @@ const char *dump_insn(struct perf_insn *x, uint64_t ip __maybe_unused,
 		return "<bad>";
 	if (lenp)
 		*lenp = insn.length;
-	left = sizeof(x->out);
-	n = snprintf(x->out, left, "insn: ");
-	left -= n;
-	for (i = 0; i < insn.length; i++) {
-		n += snprintf(x->out + n, left, "%02x ", inbuf[i]);
-		left -= n;
-	}
+	n = scnprintf(x->out, sizeof(x->out), "insn: ");
+	for (i = 0; i < insn.length; i++)
+		n += scnprintf(x->out + n, sizeof(x->out) - n, "%02x ", inbuf[i]);
 	return x->out;
 }
 
-- 
2.54.0

[PATCH 08/23] perf tools: Fix thread__set_comm_from_proc() on empty comm file

[Arnaldo Carvalho de Melo]

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

thread__set_comm_from_proc() calls procfs__read_str() then strips
the trailing newline via comm[sz - 1] = '\0'.  procfs__read_str()
allocates the buffer before reading, so on an empty /proc/pid/comm
(reachable during late exit teardown) it returns success with sz = 0
and an unterminated heap buffer.

The sz - 1 underflow was the original sashiko finding: it writes a
null byte before the allocation.  But even with a sz > 0 guard on
the newline strip, the unterminated buffer would still be passed to
thread__set_comm() which calls strlen() — an unbounded heap read.

Fix by treating sz == 0 as failure: free the buffer and return -1.
This is consistent with pmu.c's perf_pmu__parse_scale/unit which
already treat len == 0 from filename__read_str as an error.

Fixes: 2f3027ac28bf6bc3 ("perf thread: Introduce method to set comm from /proc/pid/self")
Reported-by: sashiko-bot <sashiko-bot@kernel.org>
Assisted-by: Claude Opus 4.6 <noreply@anthropic.com>
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/util/thread.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/tools/perf/util/thread.c b/tools/perf/util/thread.c
index ba33c0dfc18fe242..e483ffcb5d937fbc 100644
--- a/tools/perf/util/thread.c
+++ b/tools/perf/util/thread.c
@@ -295,6 +295,11 @@ int thread__set_comm_from_proc(struct thread *thread)
 	if (!(snprintf(path, sizeof(path), "%d/task/%d/comm",
 		       thread__pid(thread), thread__tid(thread)) >= (int)sizeof(path)) &&
 	    procfs__read_str(path, &comm, &sz) == 0) {
+		/* sz==0: read got nothing, e.g. race during exit teardown */
+		if (sz == 0) {
+			free(comm);
+			return -1;
+		}
 		comm[sz - 1] = '\0';
 		err = thread__set_comm(thread, comm, 0);
 	}
-- 
2.54.0

[PATCH 09/23] perf hwmon: Fix off-by-one null termination on sysfs reads

[Arnaldo Carvalho de Melo]

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

Three functions read sysfs files into fixed-size stack buffers using
the full buffer size, then null-terminate at buf[read_len].  If the
read fills the buffer exactly, read_len equals sizeof(buf) and the
null byte writes one past the array, corrupting an adjacent stack
variable.

Fix all three by reading sizeof(buf) - 1 bytes, reserving space for
the null terminator:

  - hwmon_pmu__read_events(): buf[128]
  - hwmon_pmu__describe_items(): buf[64]
  - evsel__hwmon_pmu_read(): buf[32]

Fixes: 53cc0b351ec99278 ("perf hwmon_pmu: Add a tool PMU exposing events from hwmon in sysfs")
Reported-by: sashiko-bot <sashiko-bot@kernel.org>
Cc: Ian Rogers <irogers@google.com>
Assisted-by: Claude Opus 4.6 <noreply@anthropic.com>
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/util/hwmon_pmu.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/tools/perf/util/hwmon_pmu.c b/tools/perf/util/hwmon_pmu.c
index dbf6a71af47f9a42..d895cd74f2a05f9d 100644
--- a/tools/perf/util/hwmon_pmu.c
+++ b/tools/perf/util/hwmon_pmu.c
@@ -289,7 +289,7 @@ static int hwmon_pmu__read_events(struct hwmon_pmu *pmu)
 			if (fd < 0)
 				continue;
 
-			read_len = read(fd, buf, sizeof(buf));
+			read_len = read(fd, buf, sizeof(buf) - 1);
 
 			while (read_len > 0 && buf[read_len - 1] == '\n')
 				read_len--;
@@ -432,7 +432,7 @@ static size_t hwmon_pmu__describe_items(struct hwmon_pmu *hwm, char *out_buf, si
 			is_alarm ? "_alarm" : "");
 		fd = openat(dir, buf, O_RDONLY);
 		if (fd > 0) {
-			ssize_t read_len = read(fd, buf, sizeof(buf));
+			ssize_t read_len = read(fd, buf, sizeof(buf) - 1);
 
 			while (read_len > 0 && buf[read_len - 1] == '\n')
 				read_len--;
@@ -816,7 +816,7 @@ int evsel__hwmon_pmu_read(struct evsel *evsel, int cpu_map_idx, int thread)
 
 	count = perf_counts(evsel->counts, cpu_map_idx, thread);
 	fd = FD(evsel, cpu_map_idx, thread);
-	len = pread(fd, buf, sizeof(buf), 0);
+	len = pread(fd, buf, sizeof(buf) - 1, 0);
 	if (len <= 0) {
 		count->lost++;
 		return -EINVAL;
-- 
2.54.0

[PATCH 10/23] perf hwmon: Use scnprintf() in hwmon_pmu__for_each_event()

[Arnaldo Carvalho de Melo]

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

hwmon_pmu__for_each_event() formats description strings via:

    len = snprintf(desc_buf, sizeof(desc_buf), "%s in unit %s named %s.", ...);
    len += hwmon_pmu__describe_items(hwm, desc_buf + len, sizeof(desc_buf) - len, ...);

If value->label is long enough to cause snprintf() to truncate, it
returns the would-have-been-written count, making len exceed
sizeof(desc_buf).  The subsequent sizeof(desc_buf) - len underflows
to a huge size_t value, disabling bounds checking in
hwmon_pmu__describe_items().

The alias_buf snprintf has the same issue.  Switch both to scnprintf()
which returns actual bytes written.

Fixes: 53cc0b351ec99278 ("perf hwmon_pmu: Add a tool PMU exposing events from hwmon in sysfs")
Reported-by: sashiko-bot <sashiko-bot@kernel.org>
Cc: Ian Rogers <irogers@google.com>
Assisted-by: Claude Opus 4.6 <noreply@anthropic.com>
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/util/hwmon_pmu.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/tools/perf/util/hwmon_pmu.c b/tools/perf/util/hwmon_pmu.c
index d895cd74f2a05f9d..fbfb872ceb1826d8 100644
--- a/tools/perf/util/hwmon_pmu.c
+++ b/tools/perf/util/hwmon_pmu.c
@@ -514,14 +514,14 @@ int hwmon_pmu__for_each_event(struct perf_pmu *pmu, void *state, pmu_event_callb
 		int ret;
 		size_t len;
 
-		len = snprintf(alias_buf, sizeof(alias_buf), "%s%d",
-			       hwmon_type_strs[key.type], key.num);
+		scnprintf(alias_buf, sizeof(alias_buf), "%s%d",
+			  hwmon_type_strs[key.type], key.num);
 		if (!info.name) {
 			info.name = info.alias;
 			info.alias = NULL;
 		}
 
-		len = snprintf(desc_buf, sizeof(desc_buf), "%s in unit %s named %s.",
+		len = scnprintf(desc_buf, sizeof(desc_buf), "%s in unit %s named %s.",
 			hwmon_desc[key.type],
 			pmu->name + 6,
 			value->label ?: info.name);
-- 
2.54.0

[PATCH 11/23] perf hwmon: Fix parse_hwmon_filename() strlcpy buffer overflow

[Arnaldo Carvalho de Melo]

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

parse_hwmon_filename() strips the "_alarm" suffix from event names
by copying into a 24-byte stack buffer:

    strlcpy(fn_type, fn_item, fn_item_len - 5);

The third argument is the source length minus the suffix, not the
destination buffer capacity.  A long event name ending in "_alarm"
can have fn_item_len - 5 > sizeof(fn_type), causing strlcpy() to
write past the 24-byte fn_type[] array.  The assert() only validates
that the longest *valid* hwmon item fits, but does not protect
against crafted input.

Clamp the strlcpy size to min(fn_item_len - 5, sizeof(fn_type)).

Fixes: 4810b761f812da3c ("perf hwmon_pmu: Add hwmon filename parser")
Reported-by: sashiko-bot <sashiko-bot@kernel.org>
Cc: Ian Rogers <irogers@google.com>
Assisted-by: Claude Opus 4.6 <noreply@anthropic.com>
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/util/hwmon_pmu.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/tools/perf/util/hwmon_pmu.c b/tools/perf/util/hwmon_pmu.c
index fbfb872ceb1826d8..bdcbe887579a1f08 100644
--- a/tools/perf/util/hwmon_pmu.c
+++ b/tools/perf/util/hwmon_pmu.c
@@ -202,7 +202,8 @@ bool parse_hwmon_filename(const char *filename,
 	fn_item_len = strlen(fn_item);
 	if (fn_item_len > 6 && !strcmp(&fn_item[fn_item_len - 6], "_alarm")) {
 		assert(strlen(LONGEST_HWMON_ITEM_STR) < sizeof(fn_type));
-		strlcpy(fn_type, fn_item, fn_item_len - 5);
+		/* fn_item_len - 5 strips "_alarm"; clamp to buffer size */
+		strlcpy(fn_type, fn_item, min_t(size_t, fn_item_len - 5, sizeof(fn_type)));
 		fn_item = fn_type;
 		*alarm = true;
 	}
-- 
2.54.0

[PATCH 12/23] perf symbols: Bounds-check descsz in sysfs__read_build_id() GNU fallback

[Arnaldo Carvalho de Melo]

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

When sysfs__read_build_id() matches NT_GNU_BUILD_ID with the right
namesz but the name content is not "GNU", it falls back to reading
descsz bytes into the stack buffer bf[BUFSIZ]:

    } else if (read(fd, bf, descsz) != (ssize_t)descsz)

Unlike the else branch which validates namesz + descsz against
sizeof(bf), this path passes descsz directly to read() without any
bounds check.  A crafted sysfs file with a large n_descsz overflows
the 8192-byte stack buffer.

Add a descsz > sizeof(bf) check before the read, breaking out of
the loop on oversized values.

Fixes: e5a1845fc0aeca85 ("perf symbols: Split out util/symbol-elf.c")
Reported-by: sashiko-bot <sashiko-bot@kernel.org>
Cc: Namhyung Kim <namhyung@kernel.org>
Assisted-by: Claude Opus 4.6 <noreply@anthropic.com>
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/util/symbol-elf.c | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/tools/perf/util/symbol-elf.c b/tools/perf/util/symbol-elf.c
index 36a0304707e13138..06cfb84f86eb2f64 100644
--- a/tools/perf/util/symbol-elf.c
+++ b/tools/perf/util/symbol-elf.c
@@ -961,8 +961,13 @@ int sysfs__read_build_id(const char *filename, struct build_id *bid)
 					err = 0;
 					break;
 				}
-			} else if (read(fd, bf, descsz) != (ssize_t)descsz)
-				break;
+			} else {
+				/* descsz from untrusted file — clamp to buffer */
+				if (descsz > sizeof(bf))
+					break;
+				if (read(fd, bf, descsz) != (ssize_t)descsz)
+					break;
+			}
 		} else {
 			size_t n;
 
-- 
2.54.0

[PATCH 13/23] perf tools: Fix uninitialized pathname on uncompressed fallback in filename__decompress()

[Arnaldo Carvalho de Melo]

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

filename__decompress() has an early return path for files that are not
actually compressed.  This path returns the fd from open() directly but
never writes to the pathname output parameter, leaving the caller with
an uninitialized buffer despite a successful return.

Callers like dso__decompress_kmodule_path() pass pathname to
decompress_kmodule() which uses it to set the decompressed file path.
If pathname is uninitialized, subsequent operations on the path produce
undefined behavior.

Fix by copying the original filename to pathname before the early return,
matching the behavior of the normal decompression path.

Reported-by: sashiko-bot <sashiko-bot@kernel.org>
Fixes: 7ac22b088afe26a4 ("perf tools: Add filename__decompress function")
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/tests/code-reading.c |  7 +++++--
 tools/perf/util/disasm.c        |  7 +++++--
 tools/perf/util/dso.c           | 12 +++++++++---
 tools/perf/util/symbol-elf.c    |  6 ++++--
 4 files changed, 23 insertions(+), 9 deletions(-)

diff --git a/tools/perf/tests/code-reading.c b/tools/perf/tests/code-reading.c
index 47043a3a2fb4f833..e82ecdc9577785e8 100644
--- a/tools/perf/tests/code-reading.c
+++ b/tools/perf/tests/code-reading.c
@@ -471,8 +471,11 @@ static int read_object_code(u64 addr, size_t len, u8 cpumode,
 			goto out;
 		}
 
-		decomp = true;
-		objdump_name = decomp_name;
+		/* empty pathname means file wasn't actually compressed */
+		if (decomp_name[0] != '\0') {
+			decomp = true;
+			objdump_name = decomp_name;
+		}
 	}
 
 	/* Read the object code using objdump */
diff --git a/tools/perf/util/disasm.c b/tools/perf/util/disasm.c
index 59ba88e1f7443c02..0a1a7e9cf3efee3e 100644
--- a/tools/perf/util/disasm.c
+++ b/tools/perf/util/disasm.c
@@ -1577,8 +1577,11 @@ int symbol__disassemble(struct symbol *sym, struct annotate_args *args)
 		if (dso__decompress_kmodule_path(dso, symfs_filename, tmp, sizeof(tmp)) < 0)
 			return -1;
 
-		decomp = true;
-		strcpy(symfs_filename, tmp);
+		/* empty pathname means file wasn't actually compressed */
+		if (tmp[0] != '\0') {
+			decomp = true;
+			strcpy(symfs_filename, tmp);
+		}
 	}
 
 	/*
diff --git a/tools/perf/util/dso.c b/tools/perf/util/dso.c
index ee06a252a54d338d..6a34717c9f31f18d 100644
--- a/tools/perf/util/dso.c
+++ b/tools/perf/util/dso.c
@@ -343,8 +343,11 @@ int filename__decompress(const char *name, char *pathname,
 	 * To keep this transparent, we detect this and return the file
 	 * descriptor to the uncompressed file.
 	 */
-	if (!compressions[comp].is_compressed(name))
+	if (!compressions[comp].is_compressed(name)) {
+		if (pathname && len > 0)
+			pathname[0] = '\0';
 		return open(name, O_RDONLY | O_CLOEXEC);
+	}
 
 	fd = mkostemp(tmpbuf, O_CLOEXEC);
 	if (fd < 0) {
@@ -598,8 +601,11 @@ static char *dso__get_filename(struct dso *dso, const char *root_dir,
 			goto out;
 		}
 
-		*decomp = true;
-		strcpy(name, newpath);
+		/* empty pathname means file wasn't actually compressed */
+		if (newpath[0] != '\0') {
+			*decomp = true;
+			strcpy(name, newpath);
+		}
 	}
 	return name;
 
diff --git a/tools/perf/util/symbol-elf.c b/tools/perf/util/symbol-elf.c
index 06cfb84f86eb2f64..10902a5dc6dbe6cc 100644
--- a/tools/perf/util/symbol-elf.c
+++ b/tools/perf/util/symbol-elf.c
@@ -920,12 +920,14 @@ int filename__read_build_id(const char *filename, struct build_id *bid)
 			return -1;
 		}
 		close(fd);
-		filename = path;
+		/* non-empty path means a temp file was created */
+		if (path[0] != '\0')
+			filename = path;
 	}
 
 	err = read_build_id(filename, bid);
 
-	if (m.comp)
+	if (m.comp && filename == path)
 		unlink(filename);
 	return err;
 }
-- 
2.54.0

Re: [PATCH 13/23] perf tools: Fix uninitialized pathname on uncompressed fallback in filename__decompress()

[sashiko-bot]

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

New issues:
- [Medium] The commit message explicitly describes a logic flow that would cause a severe data-loss bug, while the code implements the opposite (safe) logic.

Pre-existing issues:
- [High] Missing error code propagation and improper negative error handling cause `__open_dso` to mistakenly return valid file descriptors on failure.
--

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

perf tools: Fix uninitialized pathname on uncompressed fallback in filename__decompress()

This commit addresses an issue where filename__decompress() left the
pathname parameter uninitialized on the early return path for uncompressed
files. The code is updated to set an empty string in this case, and
callers are modified to check for this empty string condition.

> Fix by copying the original filename to pathname before the early return,
> matching the behavior of the normal decompression path.

[Severity: Medium]
Does this commit message accurately describe the implementation?

The code below sets pathname[0] = '\0' instead of copying the filename. If
the code had copied the original filename as described here, wouldn't callers
assume the populated pathname was a newly created temporary file and call
unlink() on it, deleting the user's uncompressed kernel module or binary?

[ ... ]

> diff --git a/tools/perf/util/dso.c b/tools/perf/util/dso.c
> index ee06a252a54d3..6a34717c9f31f 100644
> --- a/tools/perf/util/dso.c
> +++ b/tools/perf/util/dso.c
> @@ -343,8 +343,11 @@ int filename__decompress(const char *name, char *pathname,
>  	 * To keep this transparent, we detect this and return the file
>  	 * descriptor to the uncompressed file.
>  	 */
> -	if (!compressions[comp].is_compressed(name))
> +	if (!compressions[comp].is_compressed(name)) {
> +		if (pathname && len > 0)
> +			pathname[0] = '\0';
>  		return open(name, O_RDONLY | O_CLOEXEC);

[Severity: High]
This is a pre-existing issue, but if open() fails here, it returns -1
without populating the *err pointer.

If open() fails, could this sequence occur?

filename__decompress()
   returns -1 without setting *err

dso__get_filename() catches the failure:
   errno = *dso__load_errno(dso); // errno is overwritten with 0

__open_dso() handles the NULL return:
   fd = -errno; // results in fd = 0

Alternatively, if decompression fails later in this function, *err is
populated with DSO_LOAD_ERRNO__DECOMPRESSION_FAILURE (which expands to
-10005).

In that case, would errno become -10005, and fd = -(-10005) = 10005?

Since both 0 and 10005 are non-negative, does this cause callers to treat
the operation as successful and attempt to read from standard input or a
bogus file descriptor?

> +	}
>  
>  	fd = mkostemp(tmpbuf, O_CLOEXEC);

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

Re: [PATCH 13/23] perf tools: Fix uninitialized pathname on uncompressed fallback in filename__decompress()

[Arnaldo Carvalho de Melo]

On Wed, Jun 10, 2026 at 08:08:10PM +0000, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
> 
> New issues:
> - [Medium] The commit message explicitly describes a logic flow that would cause a severe data-loss bug, while the code implements the opposite (safe) logic.
> 
> Pre-existing issues:
> - [High] Missing error code propagation and improper negative error handling cause `__open_dso` to mistakenly return valid file descriptors on failure.
> --
> 
> commit 5ae85aedfdb730a58a0d285fd77526ec0c92c20e
> Author: Arnaldo Carvalho de Melo <acme@redhat.com>
> 
> perf tools: Fix uninitialized pathname on uncompressed fallback in filename__decompress()
> 
> This commit addresses an issue where filename__decompress() left the
> pathname parameter uninitialized on the early return path for uncompressed
> files. The code is updated to set an empty string in this case, and
> callers are modified to check for this empty string condition.
> 
> > Fix by copying the original filename to pathname before the early return,
> > matching the behavior of the normal decompression path.
> 
> [Severity: Medium]
> Does this commit message accurately describe the implementation?
> 
> The code below sets pathname[0] = '\0' instead of copying the filename. If
> the code had copied the original filename as described here, wouldn't callers
> assume the populated pathname was a newly created temporary file and call
> unlink() on it, deleting the user's uncompressed kernel module or binary?

So this was the only non pre-existing issue found in this series, I
rebased it on top of the 28 patches I merged in perf-tools-next while
waiting for this last series to be reviewed and will do a full container
suite test build, then push everything publicly.

- Arnaldo
 
> [ ... ]
> 
> > diff --git a/tools/perf/util/dso.c b/tools/perf/util/dso.c
> > index ee06a252a54d3..6a34717c9f31f 100644
> > --- a/tools/perf/util/dso.c
> > +++ b/tools/perf/util/dso.c
> > @@ -343,8 +343,11 @@ int filename__decompress(const char *name, char *pathname,
> >  	 * To keep this transparent, we detect this and return the file
> >  	 * descriptor to the uncompressed file.
> >  	 */
> > -	if (!compressions[comp].is_compressed(name))
> > +	if (!compressions[comp].is_compressed(name)) {
> > +		if (pathname && len > 0)
> > +			pathname[0] = '\0';
> >  		return open(name, O_RDONLY | O_CLOEXEC);
> 
> [Severity: High]
> This is a pre-existing issue, but if open() fails here, it returns -1
> without populating the *err pointer.
> 
> If open() fails, could this sequence occur?
> 
> filename__decompress()
>    returns -1 without setting *err
> 
> dso__get_filename() catches the failure:
>    errno = *dso__load_errno(dso); // errno is overwritten with 0
> 
> __open_dso() handles the NULL return:
>    fd = -errno; // results in fd = 0
> 
> Alternatively, if decompression fails later in this function, *err is
> populated with DSO_LOAD_ERRNO__DECOMPRESSION_FAILURE (which expands to
> -10005).
> 
> In that case, would errno become -10005, and fd = -(-10005) = 10005?
> 
> Since both 0 and 10005 are non-negative, does this cause callers to treat
> the operation as successful and attempt to read from standard input or a
> bogus file descriptor?
> 
> > +	}
> >  
> >  	fd = mkostemp(tmpbuf, O_CLOEXEC);
> 
> -- 
> Sashiko AI review · https://sashiko.dev/#/patchset/20260610195157.2091137-1-acme@kernel.org?part=13

Re: [PATCH 13/23] perf tools: Fix uninitialized pathname on uncompressed fallback in filename__decompress()

[Arnaldo Carvalho de Melo]

On Wed, Jun 10, 2026 at 06:52:20PM -0300, Arnaldo Carvalho de Melo wrote:
> On Wed, Jun 10, 2026 at 08:08:10PM +0000, sashiko-bot@kernel.org wrote:
> > New issues:
> > - [Medium] The commit message explicitly describes a logic flow that would cause a severe data-loss bug, while the code implements the opposite (safe) logic.
> > 
> > Pre-existing issues:
> > - [High] Missing error code propagation and improper negative error handling cause `__open_dso` to mistakenly return valid file descriptors on failure.
> > --
> > 
> > commit 5ae85aedfdb730a58a0d285fd77526ec0c92c20e
> > Author: Arnaldo Carvalho de Melo <acme@redhat.com>
> > 
> > perf tools: Fix uninitialized pathname on uncompressed fallback in filename__decompress()
> > 
> > This commit addresses an issue where filename__decompress() left the
> > pathname parameter uninitialized on the early return path for uncompressed
> > files. The code is updated to set an empty string in this case, and
> > callers are modified to check for this empty string condition.
> > 
> > > Fix by copying the original filename to pathname before the early return,
> > > matching the behavior of the normal decompression path.
> > 
> > [Severity: Medium]
> > Does this commit message accurately describe the implementation?
> > 
> > The code below sets pathname[0] = '\0' instead of copying the filename. If
> > the code had copied the original filename as described here, wouldn't callers
> > assume the populated pathname was a newly created temporary file and call
> > unlink() on it, deleting the user's uncompressed kernel module or binary?
> 
> So this was the only non pre-existing issue found in this series, I
> rebased it on top of the 28 patches I merged in perf-tools-next while
> waiting for this last series to be reviewed and will do a full container
> suite test build, then push everything publicly.

Ah, there was a patch for replacing snprintf with scnprintf by Athira
that made one of the patches in my series to be dropped, same changes.

- Arnaldo

[PATCH 14/23] perf hwmon: Guard label read against empty or failed reads

[Arnaldo Carvalho de Melo]

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

hwmon_pmu__read_events() reads label files with read() into a stack
buffer, strips trailing newlines, then checks buf[0] == '\0'.  When
read() returns 0 (empty file) or -1 (error), the buffer is never
written, so buf[0] reads uninitialized stack memory.  If the garbage
byte is non-zero, the code falls through to strdup(buf) which copies
arbitrary stack data as the label string.

Fix by checking read_len <= 0 before accessing buf contents, closing
the fd and skipping the entry.

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 | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/tools/perf/util/hwmon_pmu.c b/tools/perf/util/hwmon_pmu.c
index bdcbe887579a1f08..efd3067a2e591050 100644
--- a/tools/perf/util/hwmon_pmu.c
+++ b/tools/perf/util/hwmon_pmu.c
@@ -295,8 +295,11 @@ static int hwmon_pmu__read_events(struct hwmon_pmu *pmu)
 			while (read_len > 0 && buf[read_len - 1] == '\n')
 				read_len--;
 
-			if (read_len > 0)
-				buf[read_len] = '\0';
+			if (read_len <= 0) {
+				close(fd);
+				continue;
+			}
+			buf[read_len] = '\0';
 
 			if (buf[0] == '\0') {
 				pr_debug("hwmon_pmu: empty label file %s %s\n",
-- 
2.54.0

[PATCH 15/23] perf pmu: Use scnprintf() in format_alias()

[Arnaldo Carvalho de Melo]

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

format_alias() accumulates strings into a caller-provided buffer using
snprintf().  On truncation, snprintf() returns the would-have-written
count, which can exceed the buffer size.  When used is larger than len,
buf + used computes a pointer past the buffer end — undefined behavior
even though sub_non_neg() clamps the subsequent write size to zero.

Switch to scnprintf() which returns the actual number of bytes written,
keeping used within bounds.

Reported-by: sashiko-bot <sashiko-bot@kernel.org>
Fixes: aaea36174991ff39 ("perf tools: Extend format_alias() to include event parameters")
Cc: Cody P Schafer <cody@linux.vnet.ibm.com>
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/pmu.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/tools/perf/util/pmu.c b/tools/perf/util/pmu.c
index 4ddccc863727d6ca..2fce38140eb8af2a 100644
--- a/tools/perf/util/pmu.c
+++ b/tools/perf/util/pmu.c
@@ -2155,11 +2155,11 @@ static char *format_alias(char *buf, int len, const struct perf_pmu *pmu,
 		snprintf(buf, len, "%.*s/%s/", (int)pmu_name_len, pmu->name, alias->name);
 		return buf;
 	}
-	used = snprintf(buf, len, "%.*s/%s", (int)pmu_name_len, pmu->name, alias->name);
+	used = scnprintf(buf, len, "%.*s/%s", (int)pmu_name_len, pmu->name, alias->name);
 
 	list_for_each_entry(term, &terms.terms, list) {
 		if (term->type_val == PARSE_EVENTS__TERM_TYPE_STR)
-			used += snprintf(buf + used, sub_non_neg(len, used),
+			used += scnprintf(buf + used, sub_non_neg(len, used),
 					",%s=%s", term->config,
 					term->val.str);
 	}
-- 
2.54.0

Re: [PATCH 15/23] perf pmu: Use scnprintf() in format_alias()

[sashiko-bot]

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

Pre-existing issues:
- [High] Pre-existing stack buffer overflow in `perf_pmu__for_each_event` due to unchecked `snprintf` return values.
--

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

perf pmu: Use scnprintf() in format_alias()

format_alias() accumulates strings into a caller-provided buffer using
snprintf(). On truncation, snprintf() returns the would-have-written
count, which can exceed the buffer size. Switch to scnprintf() which
returns the actual number of bytes written, keeping used within bounds.

> diff --git a/tools/perf/util/pmu.c b/tools/perf/util/pmu.c
> index 4ddccc863727d..2fce38140eb8a 100644
> --- a/tools/perf/util/pmu.c
> +++ b/tools/perf/util/pmu.c
> @@ -2155,11 +2155,11 @@ static char *format_alias(char *buf, int len, const struct perf_pmu *pmu,
[ ... ]
> -	used = snprintf(buf, len, "%.*s/%s", (int)pmu_name_len, pmu->name, alias->name);
> +	used = scnprintf(buf, len, "%.*s/%s", (int)pmu_name_len, pmu->name, alias->name);
>  
>  	list_for_each_entry(term, &terms.terms, list) {
>  		if (term->type_val == PARSE_EVENTS__TERM_TYPE_STR)
> -			used += snprintf(buf + used, sub_non_neg(len, used),
> +			used += scnprintf(buf + used, sub_non_neg(len, used),
>  					",%s=%s", term->config,
>  					term->val.str);
>  	}

[Severity: High]
This isn't a bug introduced by this patch, but I noticed a similar issue in
perf_pmu__for_each_event() in the same file:

tools/perf/util/pmu.c:perf_pmu__for_each_event()
		if (strlen(event->unit) || event->scale != 1.0) {
			info.scale_unit = buf + buf_used;
			buf_used += snprintf(buf + buf_used, sizeof(buf) - buf_used,
					"%G%s", event->scale, event->unit) + 1;
		}
		info.desc = event->desc;
		info.long_desc = event->long_desc;
		info.encoding_desc = buf + buf_used;
		buf_used += snprintf(buf + buf_used, sizeof(buf) - buf_used,
				"%.*s/%s/", (int)pmu_name_len, info.pmu_name, event->terms) + 1;

Does this code also need to use scnprintf() instead of snprintf() for the
event unit, scale, and terms?

Since buf_used accumulates the return values of snprintf(), can it exceed
sizeof(buf) if the event properties are sufficiently long?

If that happens, the unsigned subtraction sizeof(buf) - buf_used would underflow
to a very large value. 

Could this disable bounds checking on subsequent calls and allow snprintf() to
write past the end of the buf[] stack buffer?

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

[PATCH 16/23] perf tools: Use snprintf() in dso__read_running_kernel_build_id()

[Arnaldo Carvalho de Melo]

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

dso__read_running_kernel_build_id() uses sprintf() to format a sysfs
path from machine->root_dir into a PATH_MAX buffer.  If root_dir is
close to PATH_MAX in length, appending "/sys/kernel/notes" (18 bytes)
overflows the stack buffer.

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

Reported-by: sashiko-bot <sashiko-bot@kernel.org>
Fixes: cdd059d731eeb466 ("perf tools: Move dso_* related functions into dso object")
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 | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/perf/util/dso.c b/tools/perf/util/dso.c
index 6a34717c9f31f18d..5d017975873817ec 100644
--- a/tools/perf/util/dso.c
+++ b/tools/perf/util/dso.c
@@ -1779,7 +1779,7 @@ void dso__read_running_kernel_build_id(struct dso *dso, struct machine *machine)
 
 	if (machine__is_default_guest(machine))
 		return;
-	sprintf(path, "%s/sys/kernel/notes", machine->root_dir);
+	snprintf(path, sizeof(path), "%s/sys/kernel/notes", machine->root_dir);
 	sysfs__read_build_id(path, &bid);
 	dso__set_build_id(dso, &bid);
 }
-- 
2.54.0

Re: [PATCH 16/23] perf tools: Use snprintf() in dso__read_running_kernel_build_id()

[sashiko-bot]

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

Pre-existing issues:
- [High] Heap buffer overflow when replacing the DSO path with a decompressed file path in `dso__get_filename`.
- [High] Unmitigated stack buffer overflows in sibling functions formatting `PATH_MAX` paths.
- [Medium] Infinite CPU-spinning loop when reading ELF notes from a zero-filled stream.
--


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

perf tools: Use snprintf() in dso__read_running_kernel_build_id()

This commit replaces sprintf() with snprintf() in 
dso__read_running_kernel_build_id() to prevent a stack buffer overflow 
when appending /sys/kernel/notes to a root_dir that is close to PATH_MAX 
in length.

> diff --git a/tools/perf/util/dso.c b/tools/perf/util/dso.c
> index 6a34717c9f31f..5d01797587381 100644
> --- a/tools/perf/util/dso.c
> +++ b/tools/perf/util/dso.c
> @@ -1779,7 +1779,7 @@ void dso__read_running_kernel_build_id(struct dso *dso, struct machine *machine)
>  
>  	if (machine__is_default_guest(machine))
>  		return;
> -	sprintf(path, "%s/sys/kernel/notes", machine->root_dir);
> +	snprintf(path, sizeof(path), "%s/sys/kernel/notes", machine->root_dir);

[Severity: High]
This is a pre-existing issue, but while we are fixing sprintf() overflows 
in PATH_MAX buffers, are there similar unmitigated buffer overflows in 
sibling functions?

For instance, in tools/perf/util/machine.c:get_kernel_version():

    sprintf(version, "%s/proc/version", root_dir);

And in tools/perf/util/symbol.c:dso__load_guest_kernel_sym():

    sprintf(path, "%s/proc/kallsyms", machine->root_dir);

If the root_dir or symbol_conf.guestmount paths are close to PATH_MAX, 
appending these suffix strings could write past the stack bounds in those 
locations as well.

>  	sysfs__read_build_id(path, &bid);

[Severity: Medium]
This is a pre-existing issue, but does sysfs__read_build_id() contain an
infinite loop if the target stream is filled with zeros?

In tools/perf/util/symbol-elf.c:sysfs__read_build_id():

    } else {
        n = namesz + descsz;
    }
    if (read(fd, bf, n) != (ssize_t)n)
        break;

If read(fd, &nhdr, sizeof(nhdr)) reads zeroed headers from a stream, 
namesz and descsz evaluate to 0, making n = 0. The subsequent read(fd, bf, 0)
would succeed and return 0, bypassing the break condition and potentially
causing a CPU hang.

>  	dso__set_build_id(dso, &bid);
>  }

[Severity: High]
This is a pre-existing issue, but could there be a heap buffer overflow
in dso__get_filename() when dealing with decompressed modules?

In tools/perf/util/dso.c:dso__get_filename():

    if (newpath[0] != '\0') {
        *decomp = true;
        strcpy(name, newpath);
    }

Earlier in the function, dso__filename_with_chroot() might allocate the name
buffer using asprintf(), making it exactly the length of the chrooted
filename.

If the module requires decompression, dso__decompress_kmodule_path()
creates a temporary file like /tmp/perf-kmod-XXXXXX.

If the original dynamically allocated name string was shorter than the
newpath string length, the strcpy() call will overwrite the heap buffer.

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

[PATCH 17/23] tools lib api: Fix filename__write_int() writing uninitialized stack data

[Arnaldo Carvalho de Melo]

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

filename__write_int() formats an integer into a 64-byte buffer with
sprintf() then passes sizeof(buf) (64) as the write length.  This
writes all 64 bytes including uninitialized stack data past the
formatted string.  Most sysfs files reject the oversized write,
making the function always return -1.

Fix by capturing the sprintf() return value and using it as the
write length.

Reported-by: sashiko-bot <sashiko-bot@kernel.org>
Fixes: 3b00ea938653d136 ("tools lib api fs: Add sysfs__write_int function")
Cc: Kan Liang <kan.liang@intel.com>
Assisted-by: Claude Opus 4.6 <noreply@anthropic.com>
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/lib/api/fs/fs.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/tools/lib/api/fs/fs.c b/tools/lib/api/fs/fs.c
index 3cc302d4c47b1669..d16911818d4d3569 100644
--- a/tools/lib/api/fs/fs.c
+++ b/tools/lib/api/fs/fs.c
@@ -376,12 +376,13 @@ int filename__write_int(const char *filename, int value)
 {
 	int fd = open(filename, O_WRONLY), err = -1;
 	char buf[64];
+	int len;
 
 	if (fd < 0)
 		return -errno;
 
-	sprintf(buf, "%d", value);
-	if (write(fd, buf, sizeof(buf)) == sizeof(buf))
+	len = sprintf(buf, "%d", value);
+	if (write(fd, buf, len) == len)
 		err = 0;
 
 	close(fd);
-- 
2.54.0

[PATCH 18/23] tools lib api: Fix mount_overload() snprintf truncation and toupper range

[Arnaldo Carvalho de Melo]

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

mount_overload() builds an environment variable name like
"PERF_SYSFS_ENVIRONMENT" from fs->name.  Two bugs:

1) snprintf() uses name_len as the buffer size instead of sizeof(upper_name).
   For fs->name = "sysfs" (len=5), the output is truncated to "PERF" (4
   chars + null), so getenv() never finds the intended variable.

2) mem_toupper() only uppercases name_len bytes, converting just the "PERF"
   prefix rather than the full string including the filesystem name portion.

Fix by using sizeof(upper_name) for snprintf and strlen(upper_name) for
mem_toupper, so the full "PERF_SYSFS_ENVIRONMENT" string is correctly
formatted and uppercased.

Reported-by: sashiko-bot <sashiko-bot@kernel.org>
Fixes: 73ca85ad364769ff ("tools lib api fs: Add FSTYPE__mount() method")
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/lib/api/fs/fs.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/tools/lib/api/fs/fs.c b/tools/lib/api/fs/fs.c
index d16911818d4d3569..cbd8eab0d1df0c6b 100644
--- a/tools/lib/api/fs/fs.c
+++ b/tools/lib/api/fs/fs.c
@@ -261,8 +261,8 @@ static const char *mount_overload(struct fs *fs)
 	/* "PERF_" + name + "_ENVIRONMENT" + '\0' */
 	char upper_name[5 + name_len + 12 + 1];
 
-	snprintf(upper_name, name_len, "PERF_%s_ENVIRONMENT", fs->name);
-	mem_toupper(upper_name, name_len);
+	snprintf(upper_name, sizeof(upper_name), "PERF_%s_ENVIRONMENT", fs->name);
+	mem_toupper(upper_name, strlen(upper_name));
 
 	return getenv(upper_name) ?: *fs->mounts;
 }
-- 
2.54.0

[PATCH 19/23] perf bpf: Add NULL check for btf__type_by_id() in synthesize_bpf_prog_name()

[Arnaldo Carvalho de Melo]

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

synthesize_bpf_prog_name() calls btf__type_by_id() and immediately
dereferences the result via t->name_off without checking for NULL.
btf__type_by_id() returns NULL when the type_id is invalid or out
of range.  When processing perf.data files, finfo->type_id comes from
untrusted input, so an invalid ID causes a NULL pointer dereference.

Fix by checking t for NULL before dereferencing.

Reported-by: sashiko-bot <sashiko-bot@kernel.org>
Fixes: fc462ac75b36daaa ("perf bpf: Extract logic to create program names from perf_event__synthesize_one_bpf_prog()")
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 | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/tools/perf/util/bpf-event.c b/tools/perf/util/bpf-event.c
index 2c09842469f1f28c..fe01551dc3e6cc29 100644
--- a/tools/perf/util/bpf-event.c
+++ b/tools/perf/util/bpf-event.c
@@ -146,7 +146,8 @@ static int synthesize_bpf_prog_name(char *buf, int size,
 	if (btf) {
 		finfo = func_infos + sub_id * info->func_info_rec_size;
 		t = btf__type_by_id(btf, finfo->type_id);
-		short_name = btf__name_by_offset(btf, t->name_off);
+		if (t)
+			short_name = btf__name_by_offset(btf, t->name_off);
 	} else if (sub_id == 0 && sub_prog_cnt == 1) {
 		/* no subprog */
 		if (info->name[0])
-- 
2.54.0

Re: [PATCH 19/23] perf bpf: Add NULL check for btf__type_by_id() in synthesize_bpf_prog_name()

[sashiko-bot]

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

Pre-existing issues:
- [Critical] Unvalidated array offsets and skipped pointer translations in perf.data BPF payload parsing lead to arbitrary memory read vulnerabilities.
- [High] Missing bounds and array existence checks for func_infos allow out-of-bounds reads and segmentation faults when parsing crafted BPF program info.
--

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

perf bpf: Add NULL check for btf__type_by_id() in synthesize_bpf_prog_name()

synthesize_bpf_prog_name() calls btf__type_by_id() and immediately
dereferences the result via t->name_off without checking for NULL.
btf__type_by_id() returns NULL when the type_id is invalid or out
of range, so an invalid ID from untrusted input causes a NULL pointer
dereference. This patch adds a NULL check before dereferencing t.

> diff --git a/tools/perf/util/bpf-event.c b/tools/perf/util/bpf-event.c
> index 2c09842469f1f..fe01551dc3e6c 100644
> --- a/tools/perf/util/bpf-event.c
> +++ b/tools/perf/util/bpf-event.c
> @@ -146,7 +146,8 @@ static int synthesize_bpf_prog_name(char *buf, int size,
>  	if (btf) {
>  		finfo = func_infos + sub_id * info->func_info_rec_size;

[Severity: High]
This is a pre-existing issue, but is there a missing bounds check on sub_id
before we calculate the finfo pointer?

In __bpf_event__print_bpf_prog_info(), this function is called in a loop
bounded by info->nr_jited_ksyms, which is passed as sub_id.  However,
func_infos is sized according to info->nr_func_info.

Since these lengths and func_info_rec_size can be attacker-controlled from an
untrusted perf.data file, couldn't a large nr_jited_ksyms and a small
nr_func_info cause the pointer arithmetic here to point out of bounds,
leading to a segmentation fault when accessing finfo->type_id below?

>  		t = btf__type_by_id(btf, finfo->type_id);

[Severity: Critical]
This isn't a bug introduced by this patch, but does the broader handling of
perf.data BPF payloads allow for arbitrary memory reads?

When parsing HEADER_BPF_PROG_INFO from an untrusted perf.data file,
process_bpf_prog_info() reads attacker-controlled lengths and fields into
info_linear->info.

The helper bpil_offs_to_addr() translates these offsets into memory pointers,
but if the corresponding bit in the arrays bitmask is unset, the translation
is skipped, leaving the raw attacker-controlled 64-bit value intact:

tools/perf/util/bpf-utils.c:bpil_offs_to_addr() {
	...
	if ((info_linear->arrays & (1UL << i)) == 0)
		continue;
	offs = bpf_prog_info_read_offset_u64(&info_linear->info,
					     desc->array_offset);
	addr = offs + ptr_to_u64(info_linear->data);
	...
}

Furthermore, even if the bit is set, it appears there is no verification that
the calculated offset falls within info_linear->data_len.

When functions like synthesize_bpf_prog_name() or
__bpf_event__print_bpf_prog_info() dereference these unvalidated pointers
(like info->func_info or info->prog_tags) and print their contents, could this
provide an arbitrary memory read primitive if a privileged user runs
perf report -D on an untrusted file?

> -		short_name = btf__name_by_offset(btf, t->name_off);
> +		if (t)
> +			short_name = btf__name_by_offset(btf, t->name_off);
>  	} else if (sub_id == 0 && sub_prog_cnt == 1) {
[ ... ]

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

[PATCH 20/23] perf bpf: Fix map data leak in bpf_metadata_create() on alloc failure

[Arnaldo Carvalho de Melo]

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

bpf_metadata_create() calls bpf_metadata_read_map_data() which
allocates map.btf and map.rodata.  If the subsequent
bpf_metadata_alloc() fails, the code does 'continue' which skips
bpf_metadata_free_map_data(), permanently leaking both allocations.

Fix by calling bpf_metadata_free_map_data() before continue.

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 | 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 fe01551dc3e6cc29..b65ad28cd950fc64 100644
--- a/tools/perf/util/bpf-event.c
+++ b/tools/perf/util/bpf-event.c
@@ -395,8 +395,10 @@ static struct bpf_metadata *bpf_metadata_create(struct bpf_prog_info *info)
 			continue;
 
 		metadata = bpf_metadata_alloc(info->nr_prog_tags, map.num_vars);
-		if (!metadata)
+		if (!metadata) {
+			bpf_metadata_free_map_data(&map);
 			continue;
+		}
 
 		bpf_metadata_fill_event(&map, &metadata->event->bpf_metadata);
 
-- 
2.54.0

Re: [PATCH 20/23] perf bpf: Fix map data leak in bpf_metadata_create() on alloc failure

[sashiko-bot]

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

Pre-existing issues:
- [High] Integer truncation of `event_size` causes a heap buffer overflow in `synthesize_perf_record_bpf_metadata()`.
--

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

perf bpf: Fix map data leak in bpf_metadata_create() on alloc failure

This patch fixes a memory leak in bpf_metadata_create() where map.btf
and map.rodata were leaked. It addresses this by calling
bpf_metadata_free_map_data() before continuing if bpf_metadata_alloc()
fails.

> diff --git a/tools/perf/util/bpf-event.c b/tools/perf/util/bpf-event.c
> index fe01551dc3e6c..b65ad28cd950f 100644
> --- a/tools/perf/util/bpf-event.c
> +++ b/tools/perf/util/bpf-event.c
> @@ -395,8 +395,10 @@ static struct bpf_metadata *bpf_metadata_create(struct bpf_prog_info *info)
>  			continue;
>  
>  		metadata = bpf_metadata_alloc(info->nr_prog_tags, map.num_vars);
[Severity: High]
This is a pre-existing issue, but this allocation can lead to a heap buffer
overflow due to integer truncation.

In bpf_metadata_alloc(), the calculated event_size is stored into a __u16:

    event_size = sizeof(metadata->event->bpf_metadata) +
        nr_variables * sizeof(metadata->event->bpf_metadata.entries[0]);
    ...
    metadata->event->bpf_metadata = (struct perf_record_bpf_metadata) {
        .header = {
            .type = PERF_RECORD_BPF_METADATA,
            .size = event_size,
        },

If a BPF program contains 204 or more .rodata variables prefixed with
"bpf_metadata_", event_size exceeds 65535 and silently truncates (e.g.,
65808 becomes 272).

Later, synthesize_perf_record_bpf_metadata() allocates a buffer using
this truncated size:

    const size_t event_size = metadata->event->header.size;
    ...
    event = zalloc(event_size + machine->id_hdr_size);

It then copies 512 bytes (BPF_PROG_NAME_LEN), overflowing the truncated
heap allocation:

    memcpy(event->bpf_metadata.prog_name,
           metadata->prog_names[index], BPF_PROG_NAME_LEN);

Could we add bounds checking for map.num_vars or validate event_size before
truncation?

> -		if (!metadata)
> +		if (!metadata) {
> +			bpf_metadata_free_map_data(&map);
>  			continue;
> +		}
>  
>  		bpf_metadata_fill_event(&map, &metadata->event->bpf_metadata);
>

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

[PATCH 21/23] perf bpf: Fix metadata leak in perf_env__add_bpf_info() on duplicate insert

[Arnaldo Carvalho de Melo]

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

When perf_env__insert_bpf_prog_info() returns false (duplicate
program), the error path frees info_linear and info_node but not
info_node->metadata.  If bpf_metadata_create() had succeeded, the
metadata allocation is permanently leaked.

Fix by calling bpf_metadata_free() on info_node->metadata before
freeing info_node.  bpf_metadata_free() handles NULL, so this is
safe even when bpf_metadata_create() returned NULL.

Reported-by: sashiko-bot <sashiko-bot@kernel.org>
Fixes: fdc3441f2d317b40 ("perf record: collect BPF metadata from new 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 | 1 +
 1 file changed, 1 insertion(+)

diff --git a/tools/perf/util/bpf-event.c b/tools/perf/util/bpf-event.c
index b65ad28cd950fc64..c4594969d7677238 100644
--- a/tools/perf/util/bpf-event.c
+++ b/tools/perf/util/bpf-event.c
@@ -873,6 +873,7 @@ static int perf_env__add_bpf_info(struct perf_env *env, u32 id)
 		if (!perf_env__insert_bpf_prog_info(env, info_node)) {
 			pr_debug("%s: duplicate add bpf info request for id %u\n",
 				 __func__, btf_id);
+			bpf_metadata_free(info_node->metadata);
 			free(info_linear);
 			free(info_node);
 			goto out;
-- 
2.54.0

[PATCH 22/23] perf symbols: Add bounds checks to elf_read_build_id() note iteration

[Arnaldo Carvalho de Melo]

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

elf_read_build_id() iterates ELF notes using pointer arithmetic
driven by n_namesz and n_descsz from the note headers.  Neither
the note header read nor the subsequent name/desc advances are
checked against the section boundary.  A malformed ELF file with
oversized note sizes causes out-of-bounds reads past the section
data buffer.

Add two bounds checks: verify the note header fits within the
remaining section data, and verify that namesz + descsz (after
alignment) fits before advancing the pointer.

Fixes: fd7a346ea292074e ("perf symbols: Filename__read_build_id should look at .notes section too")
Reported-by: sashiko-bot <sashiko-bot@kernel.org>
Cc: Namhyung Kim <namhyung@kernel.org>
Assisted-by: Claude Opus 4.6 <noreply@anthropic.com>
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/util/symbol-elf.c | 18 ++++++++++++++++--
 1 file changed, 16 insertions(+), 2 deletions(-)

diff --git a/tools/perf/util/symbol-elf.c b/tools/perf/util/symbol-elf.c
index 10902a5dc6dbe6cc..d84e2e031d430cf5 100644
--- a/tools/perf/util/symbol-elf.c
+++ b/tools/perf/util/symbol-elf.c
@@ -835,10 +835,24 @@ static int elf_read_build_id(Elf *elf, void *bf, size_t size)
 	ptr = data->d_buf;
 	while (ptr < (data->d_buf + data->d_size)) {
 		GElf_Nhdr *nhdr = ptr;
-		size_t namesz = NOTE_ALIGN(nhdr->n_namesz),
-		       descsz = NOTE_ALIGN(nhdr->n_descsz);
+		size_t namesz, descsz, remaining;
 		const char *name;
 
+		/* ensure the note header fits within the section */
+		if (ptr + sizeof(*nhdr) > data->d_buf + data->d_size)
+			break;
+
+		namesz = NOTE_ALIGN(nhdr->n_namesz);
+		descsz = NOTE_ALIGN(nhdr->n_descsz);
+
+		/* validate individually to avoid size_t overflow on 32-bit */
+		remaining = data->d_buf + data->d_size - ptr - sizeof(*nhdr);
+		if (namesz > remaining || descsz > remaining - namesz) {
+			pr_warning("%s: oversized note: n_namesz=%u, n_descsz=%u\n",
+				   __func__, nhdr->n_namesz, nhdr->n_descsz);
+			break;
+		}
+
 		ptr += sizeof(*nhdr);
 		name = ptr;
 		ptr += namesz;
-- 
2.54.0

[PATCH 23/23] perf symbols: Add bounds checks to read_build_id() note iteration in minimal build

[Arnaldo Carvalho de Melo]

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

symbol-minimal.c's read_build_id() iterates ELF notes with the same
pattern as symbol-elf.c's elf_read_build_id(): pointer arithmetic
driven by n_namesz and n_descsz from 32-bit note header fields,
without validating that the name and desc fit within the note section
data.  A malformed ELF file with oversized note sizes causes
out-of-bounds reads past the section data buffer.

Add the same bounds check as the libelf path: validate namesz and
descsz individually against remaining data before advancing the
pointer, avoiding size_t overflow on 32-bit.

Fixes: b691f64360ecec49 ("perf symbols: Implement poor man's ELF parser")
Reported-by: sashiko-bot <sashiko-bot@kernel.org>
Cc: Namhyung Kim <namhyung@kernel.org>
Assisted-by: Claude Opus 4.6 <noreply@anthropic.com>
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/util/symbol-minimal.c | 11 ++++++++++-
 1 file changed, 10 insertions(+), 1 deletion(-)

diff --git a/tools/perf/util/symbol-minimal.c b/tools/perf/util/symbol-minimal.c
index 8221dc9868f7c9f8..091071d06416e290 100644
--- a/tools/perf/util/symbol-minimal.c
+++ b/tools/perf/util/symbol-minimal.c
@@ -1,3 +1,4 @@
+#include "debug.h"
 #include "dso.h"
 #include "symbol.h"
 #include "symsrc.h"
@@ -44,7 +45,7 @@ static int read_build_id(void *note_data, size_t note_len, struct build_id *bid,
 	ptr = note_data;
 	while ((ptr + sizeof(*nhdr)) < (note_data + note_len)) {
 		const char *name;
-		size_t namesz, descsz;
+		size_t namesz, descsz, remaining;
 
 		nhdr = ptr;
 		if (need_swap) {
@@ -56,6 +57,14 @@ static int read_build_id(void *note_data, size_t note_len, struct build_id *bid,
 		namesz = NOTE_ALIGN(nhdr->n_namesz);
 		descsz = NOTE_ALIGN(nhdr->n_descsz);
 
+		/* validate individually to avoid size_t overflow on 32-bit */
+		remaining = note_data + note_len - ptr - sizeof(*nhdr);
+		if (namesz > remaining || descsz > remaining - namesz) {
+			pr_warning("%s: oversized note: n_namesz=%u, n_descsz=%u\n",
+				   __func__, nhdr->n_namesz, nhdr->n_descsz);
+			break;
+		}
+
 		ptr += sizeof(*nhdr);
 		name = ptr;
 		ptr += namesz;
-- 
2.54.0

Re: [PATCH 23/23] perf symbols: Add bounds checks to read_build_id() note iteration in minimal build

[sashiko-bot]

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

Pre-existing issues:
- [High] Malicious ELF files with a crafted `p_filesz` of -1 bypass size checks and lead to a null pointer dereference or severe out-of-bounds memory access in `read_build_id()`.
- [High] Copy-paste typo clobbers the `p_filesz` field when byte-swapping 32-bit ELF program headers.
- [High] `sysfs__read_build_id()` fails to correctly read sysfs files due to reliance on exact `fstat()` sizes.
--

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

perf symbols: Add bounds checks to read_build_id() note iteration in minimal build

This commit adds bounds checking to read_build_id() in the minimal ELF parser.
It validates namesz and descsz individually against the remaining data before
advancing the pointer to prevent out-of-bounds reads from malformed ELF files.

> diff --git a/tools/perf/util/symbol-minimal.c b/tools/perf/util/symbol-minimal.c
> index 8221dc9868f7c..091071d06416e 100644
> --- a/tools/perf/util/symbol-minimal.c
> +++ b/tools/perf/util/symbol-minimal.c
[ ... ]
> @@ -44,7 +45,7 @@ static int read_build_id(void *note_data, size_t note_len, struct build_id *bid,
>  	ptr = note_data;
>  	while ((ptr + sizeof(*nhdr)) < (note_data + note_len)) {
>  		const char *name;
> -		size_t namesz, descsz;
> +		size_t namesz, descsz, remaining;
>  
>  		nhdr = ptr;
>  		if (need_swap) {

[Severity: High]
This is a pre-existing issue and was not introduced by the patch under review,
but there is a severe out-of-bounds memory access or null pointer dereference
possible here.

If note_data is NULL and note_len is -1 (which casts to the maximum size), ptr
will be NULL and this code will deference a NULL pointer when reading
nhdr->n_namesz.

This state can be reached from filename__read_build_id() if a malicious ELF
file crafts p_filesz to be -1:

tools/perf/util/symbol-minimal.c:filename__read_build_id() {
    ...
    p_filesz = elf32 ? hdrs.phdr32[i].p_filesz : hdrs.phdr64[i].p_filesz;
    if (p_filesz > buf_size) {
        ...
        tmp = realloc(buf, buf_size);
        ...
    }
    ...
    if (read(fd, buf, p_filesz) != p_filesz)
        goto out_free;

    ret = read_build_id(buf, p_filesz, bid, need_swap);
    ...
}

Because p_filesz is evaluated as a signed ssize_t, -1 is less than buf_size
(which is 0 initially), bypassing the memory allocation and leaving buf as NULL.

The error check read(fd, buf, p_filesz) != p_filesz is also bypassed because
read(fd, NULL, -1) returns -1, which matches p_filesz.

This passes note_data = NULL and note_len = -1 into read_build_id().

Could we add stricter bounds checking on p_filesz in filename__read_build_id()
to prevent this?

[ ... ]

[Severity: High]
This isn't a bug introduced by this patch, but I noticed a copy-paste typo in
filename__read_build_id() when byte-swapping 32-bit ELF program headers.

tools/perf/util/symbol-minimal.c:filename__read_build_id() {
    ...
    if (need_swap) {
        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);
        ...
    }
    ...
}

The code correctly swaps p_offset, but then assigns the swapped p_offset value
to p_filesz, instead of swapping the actual p_filesz field. This reverts the
byte swap and clobbers p_filesz with the original un-swapped p_offset.

Could this be corrected to swap hdrs.phdr32[i].p_filesz?


[Severity: High]
This is another pre-existing issue not introduced by this patch, but
sysfs__read_build_id() appears to rely on exact fstat() sizes.

tools/perf/util/symbol-minimal.c:sysfs__read_build_id() {
    ...
    if (fstat(fd, &stbuf) < 0)
        goto out;

    buf_size = stbuf.st_size;
    buf = malloc(buf_size);
    ...
    if (read(fd, buf, buf_size) != (ssize_t) buf_size)
        goto out_free;
    ...
}

Sysfs pseudo-files often report inaccurate lengths via st_size (like 0 or 4096),
regardless of their actual content size. 

By requiring the bytes read to strictly match buf_size, won't this
systematically fail for sysfs pseudo-files and incorrectly discard valid build
IDs?

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