[PATCHES v3 00/11] perf tools: Assorted fixes

[PATCHES v3 00/11] perf tools: Assorted fixes

[Arnaldo Carvalho de Melo]

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

Hi,

Sixth batch of pre-existing bug fixes found by sashiko-bot AI review
during the perf-data-validation hardening series.  All bugs are latent
in existing code — none were introduced by the hardening patches.

Three broad categories:

1. snprintf() accumulation overflows (patches 2, 8, 9, 10):

   Several functions accumulate formatted output via ret += snprintf().
   snprintf() returns the would-have-been-written count, so on truncation
   ret overshoots the buffer size and the next 'size - ret' underflows
   to a huge unsigned value, disabling bounds checking.  Switched to
   scnprintf() which returns actual bytes written.

   Affected: cpu_map__snprint(), snprintf_hex(),
   synthesize_bpf_prog_name(), hists__scnprintf_title(),
   build_id__snprintf(), hwmon_pmu__describe_items().

2. Missing safety checks on untrusted data (patches 1, 3, 5, 6):

   - get_max_num(): size_t underflow on empty sysfs file causes heap
     over-read.
   - machine__resolve(): unguarded env->cpu[] access with untrusted
     CPU index — switched to perf_env__get_cpu_topology() accessor,
     added bounds check before int16_t truncation.
   - timehist: test_bit(prio, ...) without bounds check on untrusted
     tracepoint priority.
   - idle-hist: rb_first_cached() on a tree populated with plain
     rb_insert_color() — rb_leftmost never set, callchains silently
     dropped.

3. Resource hygiene (patches 4, 7):

   - bitmap_free() without NULLing the pointer (2 call sites).
   - O_CLOEXEC missing from open() calls in DSO and ELF code
     (12 call sites across 2 files).

Also expanded the libperf ABI TODO (tools/lib/perf/TODO) to emphasize
the code simplification argument for widening struct perf_cpu.cpu from
int16_t to int — the narrow type forces defensive truncation checks at
every boundary where wider CPU indices are narrowed.

Arnaldo Carvalho de Melo (10):
  perf tools: Fix get_max_num() size_t underflow on empty sysfs file
  perf tools: Use scnprintf() in cpu_map__snprint() to prevent overflow
  perf tools: Use perf_env__get_cpu_topology() in machine__resolve()
  perf tools: NULL bitmap pointers after bitmap_free()
  perf sched: Bounds-check prio before test_bit() in timehist
  perf sched: Fix idle-hist callchain display using wrong rb_first variant
  perf tools: Add O_CLOEXEC to open() calls in DSO and ELF code
  perf bpf: Use scnprintf() in snprintf_hex() and synthesize_bpf_prog_name()
  perf hists: Fix snprintf() in hists__scnprintf_title() UID filter path
  perf tools: Use scnprintf() in build_id__snprintf() and hwmon read_events()

 tools/lib/perf/TODO          |  7 +++++++
 tools/perf/builtin-record.c  |  1 +
 tools/perf/builtin-sched.c   |  7 +++++--
 tools/perf/util/bpf-event.c  | 11 ++++++-----
 tools/perf/util/build-id.c   |  2 +-
 tools/perf/util/cpumap.c     | 24 +++++++++++++++---------
 tools/perf/util/dso.c        |  4 ++--
 tools/perf/util/event.c      | 11 +++++++++--
 tools/perf/util/hist.c       |  7 ++++---
 tools/perf/util/hwmon_pmu.c  | 12 ++++++------
 tools/perf/util/mmap.c       |  1 +
 tools/perf/util/symbol-elf.c | 20 ++++++++++----------
 12 files changed, 67 insertions(+), 40 deletions(-)

Changes since v2:
  - Dropped mbind patch (was v2 patch 4): the original code was
    correct — get_nodes() does --maxnode before computing
    BITS_TO_LONGS, so bitmap_zalloc(node_index + 1) with
    maxnode = node_index + 2 already match.  The commit message
    misstated the kernel-side semantics.
  - Patch 3 (machine__resolve): bounds-check al->cpu against
    env->nr_cpus_avail before truncating to int16_t struct perf_cpu.
    (Found by sashiko-bot lore review)
  - Patch 4 (was v2 patch 5, bitmap_free): reworded from "Three
    call sites" to "Two call sites" — removed dead store from
    memory_node__delete_nodes() where NULLing a pointer right
    before freeing the containing struct was useless.
    (Found by sashiko-bot lore review)

Changes since v1:
  - Patch 5 (was v1 patch 6): fix prio bounds-check logic — the
    v1 condition (prio < 0 || prio >= MAX_PRIO || !test_bit(...))
    incorrectly skipped events with unknown priority (prio == -1).
    Changed to (prio >= 0 && (prio >= MAX_PRIO || !test_bit(...)))
    to preserve the original pass-through for events without
    priority info.
    (Found by sashiko-bot lore review)

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

Thanks,

- Arnaldo

[PATCH 01/11] perf tools: Fix get_max_num() size_t underflow on empty sysfs file

[Arnaldo Carvalho de Melo]

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

get_max_num() reads a sysfs file (cpu/possible, cpu/present, or
node/possible) and scans backward from the end to find the last
number.  If the file is empty, filename__read_str() returns num == 0.
The loop `while (--num)` decrements the size_t from 0 to SIZE_MAX,
reading backward across the heap until a comma or hyphen is found
or unmapped memory is hit.

Add an early return for empty files before the backward scan.

Fixes: 7780c25bae59fd04 ("perf tools: Allow ability to map cpus to nodes easily")
Reported-by: sashiko-bot <sashiko-bot@kernel.org>
Cc: Don Zickus <dzickus@redhat.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/cpumap.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/tools/perf/util/cpumap.c b/tools/perf/util/cpumap.c
index 21fa781b03cc7409..1fab00ec4a59a0c7 100644
--- a/tools/perf/util/cpumap.c
+++ b/tools/perf/util/cpumap.c
@@ -448,6 +448,12 @@ static int get_max_num(char *path, int *max)
 
 	buf[num] = '\0';
 
+	/* empty file — nothing to parse */
+	if (num == 0) {
+		err = -1;
+		goto out;
+	}
+
 	/* start on the right, to find highest node num */
 	while (--num) {
 		if ((buf[num] == ',') || (buf[num] == '-')) {
-- 
2.54.0

[PATCH 02/11] perf tools: Use scnprintf() in cpu_map__snprint() to prevent overflow

[Arnaldo Carvalho de Melo]

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

cpu_map__snprint() accumulates snprintf() return values in ret.
snprintf() returns the number of characters that *would have been
written* on truncation, not the actual count.  When a fragmented CPU
list exceeds the buffer, ret grows past size, causing `size - ret` to
underflow (both are size_t), and subsequent snprintf() calls write
past the end of the caller's stack buffer.

Switch to scnprintf() which returns the actual number of characters
written, making ret accumulation safe by construction.

Fixes: a24020e6b7cf6eb8 ("perf tools: Change cpu_map__fprintf output")
Reported-by: sashiko-bot <sashiko-bot@kernel.org>
Cc: Jiri Olsa <jolsa@kernel.org>
Cc: Ian Rogers <irogers@google.com>
Assisted-by: Claude Opus 4.6 <noreply@anthropic.com>
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/util/cpumap.c | 18 +++++++++---------
 1 file changed, 9 insertions(+), 9 deletions(-)

diff --git a/tools/perf/util/cpumap.c b/tools/perf/util/cpumap.c
index 1fab00ec4a59a0c7..23ebe9b97f8e58af 100644
--- a/tools/perf/util/cpumap.c
+++ b/tools/perf/util/cpumap.c
@@ -692,21 +692,21 @@ size_t cpu_map__snprint(struct perf_cpu_map *map, char *buf, size_t size)
 		if (start == -1) {
 			start = i;
 			if (last) {
-				ret += snprintf(buf + ret, size - ret,
-						"%s%d", COMMA,
-						perf_cpu_map__cpu(map, i).cpu);
+				ret += scnprintf(buf + ret, size - ret,
+						 "%s%d", COMMA,
+						 perf_cpu_map__cpu(map, i).cpu);
 			}
 		} else if (((i - start) != (cpu.cpu - perf_cpu_map__cpu(map, start).cpu)) || last) {
 			int end = i - 1;
 
 			if (start == end) {
-				ret += snprintf(buf + ret, size - ret,
-						"%s%d", COMMA,
-						perf_cpu_map__cpu(map, start).cpu);
+				ret += scnprintf(buf + ret, size - ret,
+						 "%s%d", COMMA,
+						 perf_cpu_map__cpu(map, start).cpu);
 			} else {
-				ret += snprintf(buf + ret, size - ret,
-						"%s%d-%d", COMMA,
-						perf_cpu_map__cpu(map, start).cpu, perf_cpu_map__cpu(map, end).cpu);
+				ret += scnprintf(buf + ret, size - ret,
+						 "%s%d-%d", COMMA,
+						 perf_cpu_map__cpu(map, start).cpu, perf_cpu_map__cpu(map, end).cpu);
 			}
 			first = false;
 			start = i;
-- 
2.54.0

[PATCH 03/11] perf tools: Use perf_env__get_cpu_topology() in machine__resolve()

[Arnaldo Carvalho de Melo]

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

machine__resolve() accesses env->cpu[al->cpu].socket_id after checking
al->cpu >= 0 and env->cpu != NULL, but without validating al->cpu
against env->nr_cpus_avail.  Since al->cpu comes from the untrusted
perf.data sample, a crafted file with a large CPU index causes an
out-of-bounds heap read.

Use perf_env__get_cpu_topology() which validates both NULL and bounds.

Fixes: 0c4c4debb0adda4c ("perf tools: Add processor socket info to hist_entry and addr_location")
Reported-by: sashiko-bot <sashiko-bot@kernel.org>
Cc: Kan Liang <kan.liang@intel.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/event.c | 11 +++++++++--
 1 file changed, 9 insertions(+), 2 deletions(-)

diff --git a/tools/perf/util/event.c b/tools/perf/util/event.c
index 66f4843bb235df53..001db00be1073ad4 100644
--- a/tools/perf/util/event.c
+++ b/tools/perf/util/event.c
@@ -14,6 +14,7 @@
 #include <linux/perf_event.h>
 #include "cpumap.h"
 #include "dso.h"
+#include "env.h"
 #include "event.h"
 #include "debug.h"
 #include "hist.h"
@@ -836,8 +837,14 @@ int machine__resolve(struct machine *machine, struct addr_location *al,
 	if (al->cpu >= 0) {
 		struct perf_env *env = machine->env;
 
-		if (env && env->cpu)
-			al->socket = env->cpu[al->cpu].socket_id;
+		/* bounds-check before truncating to struct perf_cpu (int16_t) */
+		if (env && al->cpu < env->nr_cpus_avail) {
+			struct cpu_topology_map *topo;
+
+			topo = perf_env__get_cpu_topology(env, (struct perf_cpu){ al->cpu });
+			if (topo)
+				al->socket = topo->socket_id;
+		}
 	}
 
 	/* Account for possible out-of-order switch events. */
-- 
2.54.0

[PATCH 04/11] perf tools: NULL bitmap pointers after bitmap_free()

[Arnaldo Carvalho de Melo]

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

Two call sites free bitmaps without NULLing the pointer, risking
double-free if the structure is reused or cleanup is called twice:

 - mmap__munmap():                 map->affinity_mask.bits
 - record__mmap_cpu_mask_free():   mask->bits

Set each pointer to NULL after bitmap_free().

Fixes: 8384a2600c7ddfc8 ("perf record: Adapt affinity to machines with #CPUs > 1K")
Fixes: f466e5ed6c356d1d ("perf record: Extend --threads command line option")
Reported-by: sashiko-bot <sashiko-bot@kernel.org>
Cc: Alexey Budankov <alexey.budankov@linux.intel.com>
Cc: Alexey Bayduraev <alexey.v.bayduraev@linux.intel.com>
Cc: Arnaldo Carvalho de Melo <acme@redhat.com>
Assisted-by: Claude Opus 4.6 <noreply@anthropic.com>
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/builtin-record.c | 1 +
 tools/perf/util/mmap.c      | 1 +
 2 files changed, 2 insertions(+)

diff --git a/tools/perf/builtin-record.c b/tools/perf/builtin-record.c
index a33c78f030d91012..e915390556752b9e 100644
--- a/tools/perf/builtin-record.c
+++ b/tools/perf/builtin-record.c
@@ -3084,6 +3084,7 @@ static int record__mmap_cpu_mask_alloc(struct mmap_cpu_mask *mask, int nr_bits)
 static void record__mmap_cpu_mask_free(struct mmap_cpu_mask *mask)
 {
 	bitmap_free(mask->bits);
+	mask->bits = NULL;
 	mask->nbits = 0;
 }
 
diff --git a/tools/perf/util/mmap.c b/tools/perf/util/mmap.c
index d64aec6c7c843e81..c6bd4c37d50ee57e 100644
--- a/tools/perf/util/mmap.c
+++ b/tools/perf/util/mmap.c
@@ -238,6 +238,7 @@ static void perf_mmap__aio_munmap(struct mmap *map __maybe_unused)
 void mmap__munmap(struct mmap *map)
 {
 	bitmap_free(map->affinity_mask.bits);
+	map->affinity_mask.bits = NULL;
 
 	zstd_fini(&map->zstd_data);
 
-- 
2.54.0

[PATCH 05/11] perf sched: Bounds-check prio before test_bit() in timehist

[Arnaldo Carvalho de Melo]

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

timehist_skip_sample() reads prio from untrusted tracepoint data via
perf_sample__intval(sample, "prev_prio") without bounds validation.
A crafted perf.data with prev_prio >= MAX_PRIO (140) causes test_bit()
to read past the end of the prio_bitmap, which is only MAX_PRIO bits.

Add a prio >= 0 && prio < MAX_PRIO check before the test_bit() call.
This also makes the != -1 sentinel check explicit as >= 0.

Fixes: 9b3a48bbe20d9692 ("perf sched timehist: Add --prio option")
Reported-by: sashiko-bot <sashiko-bot@kernel.org>
Cc: Yang Jihong <yangjihong@bytedance.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 | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/tools/perf/builtin-sched.c b/tools/perf/builtin-sched.c
index 1ff01f03d2ad1ad3..ded511d8518803a0 100644
--- a/tools/perf/builtin-sched.c
+++ b/tools/perf/builtin-sched.c
@@ -2645,7 +2645,9 @@ static bool timehist_skip_sample(struct perf_sched *sched,
 		else if (evsel__name_is(sample->evsel, "sched:sched_switch"))
 			prio = perf_sample__intval(sample, "prev_prio");
 
-		if (prio != -1 && !test_bit(prio, sched->prio_bitmap)) {
+		/* prio comes from untrusted tracepoint data — bounds-check before test_bit */
+		if (prio >= 0 &&
+		    (prio >= MAX_PRIO || !test_bit(prio, sched->prio_bitmap))) {
 			rc = true;
 			sched->skipped_samples++;
 		}
-- 
2.54.0

[PATCH 06/11] perf sched: Fix idle-hist callchain display using wrong rb_first variant

[Arnaldo Carvalho de Melo]

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

timehist_print_idlehist_callchain() calls rb_first_cached() on
sorted_root, but the sort function (callchain_param.sort) populates it
via rb_insert_color() on the plain rb_root member — not the cached
variant.  This means rb_leftmost is never set, so rb_first_cached()
always returns NULL and the entire callchain summary is silently
dropped from --idle-hist output.

The original code in ba957ebb54893aca ("perf sched timehist: Show
callchains for idle stat") was correct — it used struct rb_root and
rb_first().  The bug was introduced when sorted_root was converted to
rb_root_cached without converting the sort insertion path to use
rb_insert_color_cached().

Use rb_first(&root->rb_root) to match how the tree was populated.

Fixes: cb4c13a5137766c3 ("perf sched: Use cached rbtrees")
Reported-by: sashiko-bot <sashiko-bot@kernel.org>
Cc: Davidlohr Bueso <dave@stgolabs.net>
Cc: Namhyung Kim <namhyung@kernel.org>
Assisted-by: Claude Opus 4.6 <noreply@anthropic.com>
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/builtin-sched.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/tools/perf/builtin-sched.c b/tools/perf/builtin-sched.c
index ded511d8518803a0..85f11d388392d316 100644
--- a/tools/perf/builtin-sched.c
+++ b/tools/perf/builtin-sched.c
@@ -3130,7 +3130,8 @@ static size_t timehist_print_idlehist_callchain(struct rb_root_cached *root)
 	size_t ret = 0;
 	FILE *fp = stdout;
 	struct callchain_node *chain;
-	struct rb_node *rb_node = rb_first_cached(root);
+	/* sort() uses rb_insert_color() on rb_root, not rb_root_cached */
+	struct rb_node *rb_node = rb_first(&root->rb_root);
 
 	printf("  %16s  %8s  %s\n", "Idle time (msec)", "Count", "Callchains");
 	printf("  %.16s  %.8s  %.50s\n", graph_dotted_line, graph_dotted_line,
-- 
2.54.0

[PATCH 07/11] perf tools: Add O_CLOEXEC to open() calls in DSO and ELF code

[Arnaldo Carvalho de Melo]

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

open() calls in dso.c and symbol-elf.c omit O_CLOEXEC, which leaks
file descriptors to child processes spawned during symbol resolution
(e.g., addr2line, objdump).  This can exhaust the fd limit during
long profiling sessions or when processing many DSOs.

Add O_CLOEXEC to all open() calls in both files (12 call sites).

Fixes: cdd059d731eeb466 ("perf tools: Move dso_* related functions into dso object")
Fixes: e5a1845fc0aeca85 ("perf symbols: Split out util/symbol-elf.c")
Reported-by: sashiko-bot <sashiko-bot@kernel.org>
Cc: Jiri Olsa <jolsa@kernel.org>
Cc: Namhyung Kim <namhyung@kernel.org>
Assisted-by: Claude Opus 4.6 <noreply@anthropic.com>
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/util/dso.c        |  4 ++--
 tools/perf/util/symbol-elf.c | 20 ++++++++++----------
 2 files changed, 12 insertions(+), 12 deletions(-)

diff --git a/tools/perf/util/dso.c b/tools/perf/util/dso.c
index 7dced896c64eafd7..fb2e78fe2aa8eb94 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))
-		return open(name, O_RDONLY);
+		return open(name, O_RDONLY | O_CLOEXEC);
 
 	fd = mkstemp(tmpbuf);
 	if (fd < 0) {
@@ -1911,7 +1911,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);
+	fd = open(symfs_filename, O_RDONLY | O_CLOEXEC);
 	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 186e6d92ac3d7742..c2bdfd0003df2abe 100644
--- a/tools/perf/util/symbol-elf.c
+++ b/tools/perf/util/symbol-elf.c
@@ -217,7 +217,7 @@ bool filename__has_section(const char *filename, const char *sec)
 	GElf_Shdr shdr;
 	bool found = false;
 
-	fd = open(filename, O_RDONLY);
+	fd = open(filename, O_RDONLY | O_CLOEXEC);
 	if (fd < 0)
 		return false;
 
@@ -872,7 +872,7 @@ static int read_build_id(const char *filename, struct build_id *bid)
 	if (size < BUILD_ID_SIZE)
 		goto out;
 
-	fd = open(filename, O_RDONLY);
+	fd = open(filename, O_RDONLY | O_CLOEXEC);
 	if (fd < 0)
 		goto out;
 
@@ -935,7 +935,7 @@ int sysfs__read_build_id(const char *filename, struct build_id *bid)
 	size_t size = sizeof(bid->data);
 	int fd, err = -1;
 
-	fd = open(filename, O_RDONLY);
+	fd = open(filename, O_RDONLY | O_CLOEXEC);
 	if (fd < 0)
 		goto out;
 
@@ -995,7 +995,7 @@ int filename__read_debuglink(const char *filename, char *debuglink,
 	if (err >= 0)
 		goto out;
 
-	fd = open(filename, O_RDONLY);
+	fd = open(filename, O_RDONLY | O_CLOEXEC);
 	if (fd < 0)
 		goto out;
 
@@ -1153,7 +1153,7 @@ int symsrc__init(struct symsrc *ss, struct dso *dso, const char *name,
 
 		type = dso__symtab_type(dso);
 	} else {
-		fd = open(name, O_RDONLY);
+		fd = open(name, O_RDONLY | O_CLOEXEC);
 		if (fd < 0) {
 			*dso__load_errno(dso) = errno;
 			return -1;
@@ -1952,7 +1952,7 @@ static int kcore__open(struct kcore *kcore, const char *filename)
 {
 	GElf_Ehdr *ehdr;
 
-	kcore->fd = open(filename, O_RDONLY);
+	kcore->fd = open(filename, O_RDONLY | O_CLOEXEC);
 	if (kcore->fd == -1)
 		return -1;
 
@@ -1985,7 +1985,7 @@ static int kcore__init(struct kcore *kcore, char *filename, int elfclass,
 	if (temp)
 		kcore->fd = mkstemp(filename);
 	else
-		kcore->fd = open(filename, O_WRONLY | O_CREAT | O_EXCL, 0400);
+		kcore->fd = open(filename, O_WRONLY | O_CREAT | O_EXCL | O_CLOEXEC, 0400);
 	if (kcore->fd == -1)
 		return -1;
 
@@ -2461,11 +2461,11 @@ static int kcore_copy__compare_files(const char *from_filename,
 {
 	int from, to, err = -1;
 
-	from = open(from_filename, O_RDONLY);
+	from = open(from_filename, O_RDONLY | O_CLOEXEC);
 	if (from < 0)
 		return -1;
 
-	to = open(to_filename, O_RDONLY);
+	to = open(to_filename, O_RDONLY | O_CLOEXEC);
 	if (to < 0)
 		goto out_close_from;
 
@@ -2883,7 +2883,7 @@ int get_sdt_note_list(struct list_head *head, const char *target)
 	Elf *elf;
 	int fd, ret;
 
-	fd = open(target, O_RDONLY);
+	fd = open(target, O_RDONLY | O_CLOEXEC);
 	if (fd < 0)
 		return -EBADF;
 
-- 
2.54.0

[PATCH 08/11] perf bpf: Use scnprintf() in snprintf_hex() and synthesize_bpf_prog_name()

[Arnaldo Carvalho de Melo]

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

Both functions accumulate formatted output via ret += snprintf(buf + ret,
size - ret, ...).  If the buffer is too small and snprintf() returns more
than the remaining space, ret exceeds size and the next 'size - ret'
underflows, causing snprintf() to write past the buffer end.

Switch to scnprintf() which returns the actual number of bytes written,
making the accumulation safe.

Fixes: 7b612e291a5affb1 ("perf tools: Synthesize PERF_RECORD_* for loaded BPF programs")
Reported-by: sashiko-bot <sashiko-bot@kernel.org>
Cc: Song Liu <song@kernel.org>
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 | 11 ++++++-----
 1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/tools/perf/util/bpf-event.c b/tools/perf/util/bpf-event.c
index a27945c279efb779..2c09842469f1f28c 100644
--- a/tools/perf/util/bpf-event.c
+++ b/tools/perf/util/bpf-event.c
@@ -36,7 +36,7 @@ static int snprintf_hex(char *buf, size_t size, unsigned char *data, size_t len)
 	size_t i;
 
 	for (i = 0; i < len; i++)
-		ret += snprintf(buf + ret, size - ret, "%02x", data[i]);
+		ret += scnprintf(buf + ret, size - ret, "%02x", data[i]);
 	return ret;
 }
 
@@ -140,7 +140,7 @@ static int synthesize_bpf_prog_name(char *buf, int size,
 	const struct btf_type *t;
 	int name_len;
 
-	name_len = snprintf(buf, size, "bpf_prog_");
+	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) {
@@ -153,9 +153,10 @@ static int synthesize_bpf_prog_name(char *buf, int size,
 			short_name = info->name;
 	} else
 		short_name = "F";
-	if (short_name)
-		name_len += snprintf(buf + name_len, size - name_len,
-				     "_%s", short_name);
+	if (short_name) {
+		name_len += scnprintf(buf + name_len, size - name_len,
+				      "_%s", short_name);
+	}
 	return name_len;
 }
 
-- 
2.54.0

[PATCH 09/11] perf hists: Fix snprintf() in hists__scnprintf_title() UID filter path

[Arnaldo Carvalho de Melo]

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

hists__scnprintf_title() accumulates formatted output into a buffer
using scnprintf() for all filter clauses except the UID filter, which
uses snprintf().  If the buffer fills up and snprintf() returns more
than the remaining space, printed exceeds size and the next 'size -
printed' underflows, causing later scnprintf() calls to write past
the buffer.

Switch the UID filter clause to scnprintf() to match the rest of the
function.

Fixes: 25c312dbf88ca402 ("perf hists: Move hists__scnprintf_title() away from the TUI code")
Reported-by: sashiko-bot <sashiko-bot@kernel.org>
Cc: Arnaldo Carvalho de Melo <acme@redhat.com>
Assisted-by: Claude Opus 4.6 <noreply@anthropic.com>
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/util/hist.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/tools/perf/util/hist.c b/tools/perf/util/hist.c
index 811d68fa6770c5b7..df978c996b6c2262 100644
--- a/tools/perf/util/hist.c
+++ b/tools/perf/util/hist.c
@@ -2963,9 +2963,10 @@ int __hists__scnprintf_title(struct hists *hists, char *bf, size_t size, bool sh
 			   ev_name, sample_freq_str, enable_ref ? ref : " ", nr_events);
 
 
-	if (hists->uid_filter_str)
-		printed += snprintf(bf + printed, size - printed,
-				    ", UID: %s", hists->uid_filter_str);
+	if (hists->uid_filter_str) {
+		printed += scnprintf(bf + printed, size - printed,
+				     ", UID: %s", hists->uid_filter_str);
+	}
 	if (thread) {
 		if (hists__has(hists, thread)) {
 			printed += scnprintf(bf + printed, size - printed,
-- 
2.54.0

[PATCH 10/11] perf tools: Use scnprintf() in build_id__snprintf() and hwmon read_events()

[Arnaldo Carvalho de Melo]

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

build_id__snprintf() and hwmon_pmu__read_events() accumulate formatted
output via snprintf(), which returns the would-have-been-written count
on truncation.  In build_id__snprintf(), this inflates the return
value beyond the buffer size.  In hwmon_pmu__read_events(), len
overshoots out_buf_len and the next 'out_buf_len - len' underflows.

Switch both to scnprintf() which returns actual bytes written.

Fixes: fccaaf6fbbc59910 ("perf build-id: Change sprintf functions to snprintf")
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/build-id.c  |  2 +-
 tools/perf/util/hwmon_pmu.c | 12 ++++++------
 2 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/tools/perf/util/build-id.c b/tools/perf/util/build-id.c
index 8c0a9ae932aa5798..a3b92108f96263c6 100644
--- a/tools/perf/util/build-id.c
+++ b/tools/perf/util/build-id.c
@@ -94,7 +94,7 @@ int build_id__snprintf(const struct build_id *build_id, char *bf, size_t bf_size
 	}
 
 	for (size_t i = 0; i < build_id->size && offs < bf_size; ++i)
-		offs += snprintf(bf + offs, bf_size - offs, "%02x", build_id->data[i]);
+		offs += scnprintf(bf + offs, bf_size - offs, "%02x", build_id->data[i]);
 
 	return offs;
 }
diff --git a/tools/perf/util/hwmon_pmu.c b/tools/perf/util/hwmon_pmu.c
index fb3ffa8d32ad2a93..dbf6a71af47f9a42 100644
--- a/tools/perf/util/hwmon_pmu.c
+++ b/tools/perf/util/hwmon_pmu.c
@@ -442,12 +442,12 @@ static size_t hwmon_pmu__describe_items(struct hwmon_pmu *hwm, char *out_buf, si
 
 				buf[read_len] = '\0';
 				val = strtoll(buf, /*endptr=*/NULL, 10);
-				len += snprintf(out_buf + len, out_buf_len - len, "%s%s%s=%g%s",
-						len == 0 ? " " : ", ",
-						hwmon_item_strs[bit],
-						is_alarm ? "_alarm" : "",
-						(double)val / 1000.0,
-						hwmon_units[key.type]);
+				len += scnprintf(out_buf + len, out_buf_len - len, "%s%s%s=%g%s",
+						 len == 0 ? " " : ", ",
+						 hwmon_item_strs[bit],
+						 is_alarm ? "_alarm" : "",
+						 (double)val / 1000.0,
+						 hwmon_units[key.type]);
 			}
 			close(fd);
 		}
-- 
2.54.0

[PATCH 11/11] libperf: Document code simplification case for widening struct perf_cpu

[Arnaldo Carvalho de Melo]

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

Add a bullet point to the libperf ABI TODO explaining the code
simplification benefit of widening struct perf_cpu.cpu from int16_t
to int: the narrow type forces defensive truncation checks at every
boundary where wider CPU indices are narrowed, and values > 32767
silently wrap past bounds validation without them.

Cc: Ian Rogers <irogers@google.com>
Cc: Namhyung Kim <namhyung@kernel.org>
Assisted-by: Claude Opus 4.6 <noreply@anthropic.com>
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/lib/perf/TODO | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/tools/lib/perf/TODO b/tools/lib/perf/TODO
index 486dd95dc57208a8..1a3644aa1f38dde4 100644
--- a/tools/lib/perf/TODO
+++ b/tools/lib/perf/TODO
@@ -11,6 +11,13 @@ together.
      (x86_64 max is 8192, arm64 is 4096), but NR_CPUS limits keep
      growing.  perf clamps to INT16_MAX in set_max_cpu_num() as a
      safety net.
+   - Code simplification: the int16_t forces defensive truncation
+     checks at every boundary where a wider CPU index (int from
+     sample->cpu, al->cpu, etc.) is narrowed into struct perf_cpu.
+     Without these checks, values > 32767 silently wrap to small
+     positive numbers, bypassing bounds validation.  Widening to int
+     eliminates this entire class of silent truncation bugs and
+     removes the need for the INT16_MAX clamp in set_max_cpu_num().
    - Scope: struct perf_cpu is embedded everywhere — perf_cpu_map__cpu(),
      perf_cpu_map__min(), perf_cpu_map__max(), perf_cpu_map__has(), the
      for_each_cpu macros, and all internal callers.  The perf_cpu_map
-- 
2.54.0

Re: [PATCH 01/11] perf tools: Fix get_max_num() size_t underflow on empty sysfs file

[sashiko-bot]

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

Pre-existing issues:
- [High] Buffer underflow and out-of-bounds write when parsing empty PMU sysfs files
- [High] Missing null termination in `filename__read_int` and `filename__read_ull_base` leads to uninitialized stack memory read
--

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

perf tools: Fix get_max_num() size_t underflow on empty sysfs file

get_max_num() reads a sysfs file (cpu/possible, cpu/present, or
node/possible) and scans backward from the end to find the last
number.  If the file is empty, filename__read_str() returns num == 0.
The loop `while (--num)` decrements the size_t from 0 to SIZE_MAX,
reading backward across the heap until a comma or hyphen is found
or unmapped memory is hit.

Add an early return for empty files before the backward scan.

> diff --git a/tools/perf/util/cpumap.c b/tools/perf/util/cpumap.c
> index 21fa781b03cc7..1fab00ec4a59a 100644
> --- a/tools/perf/util/cpumap.c
> +++ b/tools/perf/util/cpumap.c
> @@ -448,6 +448,12 @@ static int get_max_num(char *path, int *max)
>  
>  	buf[num] = '\0';
>  
> +	/* empty file — nothing to parse */
> +	if (num == 0) {
> +		err = -1;
> +		goto out;
> +	}
> +
>  	/* start on the right, to find highest node num */
>  	while (--num) {
>  		if ((buf[num] == ',') || (buf[num] == '-')) {

[Severity: High]
This isn't a bug introduced by this patch, but do similar underflows occur in
other files when parsing empty PMU sysfs files?

In tools/perf/util/pmu.c:pmu_id():
	str[len - 1] = 0; /* remove line feed */

If len is 0, this writes a null byte at str[-1].

Also in tools/perf/util/pmu.c:perf_pmu__parse_scale():
	if (scale[sret - 1] == '\n')
		scale[sret - 1] = '\0';
	else
		scale[sret] = '\0';

When sret is 0, this reads and potentially writes to scale[-1].

And in tools/perf/util/pmu.c:perf_pmu__parse_unit():
	if (alias->unit[sret - 1] == '\n')
		alias->unit[sret - 1] = '\0';
	else
		alias->unit[sret] = '\0';

When sret is 0, this accesses index -1 of the struct array field.

Could this corrupt heap metadata or memory when perf runs against empty sysfs
files?


[Severity: High]
This is a pre-existing issue, but I noticed another potential read issue.
Does filename__read_int() read uninitialized memory?

tools/lib/api/fs/fs.c:filename__read_int() {
	...
	if (read(fd, line, sizeof(line)) > 0) {
		*value = atoi(line);
	...
}

Since read() doesn't append a null terminator, if the file doesn't end with
a non-digit character or if the contents reach 64 bytes, could atoi() read
past the valid data into uninitialized stack memory?

Similarly, in tools/lib/api/fs/fs.c:filename__read_ull_base():
	if (read(fd, line, sizeof(line)) > 0) {
		*value = strtoull(line, NULL, base);

Could strtoull() also read out of bounds here for the same reason?

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

Re: [PATCH 07/11] perf tools: Add O_CLOEXEC to open() calls in DSO and ELF code

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] Incomplete fix: The patch introduces an inconsistency where uncompressed DSOs are protected from FD leaks via O_CLOEXEC, but compressed DSOs and temporary files using mkstemp() still leak.
--

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

perf tools: Add O_CLOEXEC to open() calls in DSO and ELF code

This commit adds O_CLOEXEC to open() calls in dso.c and symbol-elf.c to prevent
file descriptors from leaking to child processes during symbol resolution.

> diff --git a/tools/perf/util/dso.c b/tools/perf/util/dso.c
> index 7dced896c64ea..fb2e78fe2aa8e 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))
> -		return open(name, O_RDONLY);
> +		return open(name, O_RDONLY | O_CLOEXEC);
>  
>  	fd = mkstemp(tmpbuf);

[Severity: Medium]
Is this fix incomplete for compressed DSOs? The open() call for uncompressed
files now has O_CLOEXEC, but the mkstemp() call directly below it does not set
the close-on-exec flag. Should mkostemp() with O_CLOEXEC be used here to
prevent leaking the compressed DSO file descriptors to child processes?

> diff --git a/tools/perf/util/symbol-elf.c b/tools/perf/util/symbol-elf.c
> index 186e6d92ac3d7..c2bdfd0003df2 100644
> --- a/tools/perf/util/symbol-elf.c
> +++ b/tools/perf/util/symbol-elf.c
[ ... ]
> @@ -1985,7 +1985,7 @@ static int kcore__init(struct kcore *kcore, char *filename, int elfclass,
>  	if (temp)
>  		kcore->fd = mkstemp(filename);
>  	else
> -		kcore->fd = open(filename, O_WRONLY | O_CREAT | O_EXCL, 0400);
> +		kcore->fd = open(filename, O_WRONLY | O_CREAT | O_EXCL | O_CLOEXEC, 0400);
>  	if (kcore->fd == -1)

[Severity: Medium]
Does the temporary file branch also need similar protection? The else branch
now correctly uses O_CLOEXEC, but mkstemp() in the if (temp) branch will still
leak its file descriptor. 

Additionally, there appears to be another mkstemp() call without O_CLOEXEC in
read_gnu_debugdata() in the same file. Do all instances of mkstemp() creating
temporary files for debugging need to be updated to mkostemp() to fully 
address the leak?

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

Re: [PATCH 11/11] libperf: Document code simplification case for widening struct perf_cpu

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] The documentation added to `tools/lib/perf/TODO` incorrectly states that narrowing values > 32767 to `int16_t` wraps them to 'small positive numbers'. In two's complement arithmetic, values from 32768 to 65535 wrap to negative numbers (-32768 to -1) when truncated to a signed 16-bit integer.
--

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

libperf: Document code simplification case for widening struct perf_cpu

Add a bullet point to the libperf ABI TODO explaining the code
simplification benefit of widening struct perf_cpu.cpu from int16_t
to int.

> diff --git a/tools/lib/perf/TODO b/tools/lib/perf/TODO
> index 486dd95dc5720..1a3644aa1f38d 100644
> --- a/tools/lib/perf/TODO
> +++ b/tools/lib/perf/TODO
> @@ -11,6 +11,13 @@ together.
>       (x86_64 max is 8192, arm64 is 4096), but NR_CPUS limits keep
>       growing.  perf clamps to INT16_MAX in set_max_cpu_num() as a
>       safety net.
> +   - Code simplification: the int16_t forces defensive truncation
> +     checks at every boundary where a wider CPU index (int from
> +     sample->cpu, al->cpu, etc.) is narrowed into struct perf_cpu.
> +     Without these checks, values > 32767 silently wrap to small
> +     positive numbers, bypassing bounds validation.  Widening to int
       ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

[Severity: Low]
This isn't a bug, but is the arithmetic description accurate here?

In two's complement arithmetic, truncating values between 32768 and 65535
into a signed 16-bit integer (int16_t) wraps them to negative numbers
(-32768 to -1), rather than small positive numbers.

The overall conclusion remains correct since negative values trivially
bypass bounds checks, but the description of the wrapping behavior appears
mathematically inaccurate.

> +     eliminates this entire class of silent truncation bugs and
> +     removes the need for the INT16_MAX clamp in set_max_cpu_num().
>     - Scope: struct perf_cpu is embedded everywhere — perf_cpu_map__cpu(),

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

Re: [PATCH 08/11] perf bpf: Use scnprintf() in snprintf_hex() and synthesize_bpf_prog_name()

[sashiko-bot]

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

Pre-existing issues:
- [Critical] Missing bounds and presence validation for `prog_tags` and `func_info` array offsets allows arbitrary out-of-bounds reads when parsing untrusted `perf.data` files.
--

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

perf bpf: Use scnprintf() in snprintf_hex() and synthesize_bpf_prog_name()
    
Both functions accumulate formatted output via ret += snprintf(buf + ret,
size - ret, ...). Switch to scnprintf() which returns the actual number of
bytes written, making the accumulation safe.

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

[ ... ]

> @@ -140,7 +140,7 @@ static int synthesize_bpf_prog_name(char *buf, int size,
>  	const struct btf_type *t;
>  	int name_len;
>  
> -	name_len = snprintf(buf, size, "bpf_prog_");
> +	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: Critical]
This isn't a bug introduced by this patch, but is it possible for the
unconditionally dereferenced prog_tags pointer to trigger an out-of-bounds
read when analyzing untrusted perf.data files?

When reading perf.data files, bpil_offs_to_addr() in
tools/perf/util/bpf-utils.c adds an offset read from the file directly to
the heap allocation pointer without bounds checking against data_len:

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

If the PERF_BPIL_PROG_TAGS bit is missing from the arrays mask, info->prog_tags
remains untranslated and points to whatever attacker-controlled value was
provided in the file. 

When synthesize_bpf_prog_name() dereferences prog_tags[sub_id], could this 
leak arbitrary memory into the output buffer? Does this require bounds and
presence validation for the prog_tags offset during parsing?

>  	if (btf) {
> @@ -153,9 +153,10 @@ static int synthesize_bpf_prog_name(char *buf, int size,
>  			short_name = info->name;
>  	} else
>  		short_name = "F";
> -	if (short_name)
> -		name_len += snprintf(buf + name_len, size - name_len,
> -				     "_%s", short_name);
> +	if (short_name) {
> +		name_len += scnprintf(buf + name_len, size - name_len,
> +				      "_%s", short_name);
> +	}
>  	return name_len;
>  }

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

Re: [PATCH 10/11] perf tools: Use scnprintf() in build_id__snprintf() and hwmon read_events()

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] The early termination condition `offs < bf_size` is rendered ineffective by the switch to `scnprintf`.
--

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

perf tools: Use scnprintf() in build_id__snprintf() and hwmon read_events()

This commit replaces snprintf() with scnprintf() in build_id__snprintf()
and hwmon_pmu__describe_items() to prevent accumulated lengths from exceeding
the allocated buffer size and underflowing the remaining size.

> diff --git a/tools/perf/util/build-id.c b/tools/perf/util/build-id.c
> index 8c0a9ae932aa5..a3b92108f9626 100644
> --- a/tools/perf/util/build-id.c
> +++ b/tools/perf/util/build-id.c
> @@ -94,7 +94,7 @@ int build_id__snprintf(const struct build_id *build_id, char *bf, size_t bf_size
>  	}
>  
>  	for (size_t i = 0; i < build_id->size && offs < bf_size; ++i)
> -		offs += snprintf(bf + offs, bf_size - offs, "%02x", build_id->data[i]);
> +		offs += scnprintf(bf + offs, bf_size - offs, "%02x", build_id->data[i]);

[Severity: Low]
Does this change neutralize the early termination condition in the loop?

Since scnprintf() returns the number of characters written excluding the null
terminator, `offs` will reach at most `bf_size - 1` and never equals `bf_size`.

Once the buffer is full (`bf_size - offs == 1`), scnprintf() will only write a
null terminator, returning 0. Because `offs` stops increasing, the condition
`offs < bf_size` remains true. This causes the loop to continue writing nothing
for the remaining `build_id->size` iterations instead of exiting early.

>  
>  	return offs;
>  }

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

Re: [PATCH 01/11] perf tools: Fix get_max_num() size_t underflow on empty sysfs file

[Ian Rogers]

On Mon, Jun 8, 2026 at 1:18 PM Arnaldo Carvalho de Melo <acme@kernel.org> wrote:
>
> From: Arnaldo Carvalho de Melo <acme@redhat.com>
>
> get_max_num() reads a sysfs file (cpu/possible, cpu/present, or
> node/possible) and scans backward from the end to find the last
> number.  If the file is empty, filename__read_str() returns num == 0.
> The loop `while (--num)` decrements the size_t from 0 to SIZE_MAX,
> reading backward across the heap until a comma or hyphen is found
> or unmapped memory is hit.
>
> Add an early return for empty files before the backward scan.
>
> Fixes: 7780c25bae59fd04 ("perf tools: Allow ability to map cpus to nodes easily")
> Reported-by: sashiko-bot <sashiko-bot@kernel.org>
> Cc: Don Zickus <dzickus@redhat.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>

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

Thanks,
Ian

> ---
>  tools/perf/util/cpumap.c | 6 ++++++
>  1 file changed, 6 insertions(+)
>
> diff --git a/tools/perf/util/cpumap.c b/tools/perf/util/cpumap.c
> index 21fa781b03cc7409..1fab00ec4a59a0c7 100644
> --- a/tools/perf/util/cpumap.c
> +++ b/tools/perf/util/cpumap.c
> @@ -448,6 +448,12 @@ static int get_max_num(char *path, int *max)
>
>         buf[num] = '\0';
>
> +       /* empty file — nothing to parse */
> +       if (num == 0) {
> +               err = -1;
> +               goto out;
> +       }
> +
>         /* start on the right, to find highest node num */
>         while (--num) {
>                 if ((buf[num] == ',') || (buf[num] == '-')) {
> --
> 2.54.0
>

Re: [PATCH 02/11] perf tools: Use scnprintf() in cpu_map__snprint() to prevent overflow

[Ian Rogers]

On Mon, Jun 8, 2026 at 1:18 PM Arnaldo Carvalho de Melo <acme@kernel.org> wrote:
>
> From: Arnaldo Carvalho de Melo <acme@redhat.com>
>
> cpu_map__snprint() accumulates snprintf() return values in ret.
> snprintf() returns the number of characters that *would have been
> written* on truncation, not the actual count.  When a fragmented CPU
> list exceeds the buffer, ret grows past size, causing `size - ret` to
> underflow (both are size_t), and subsequent snprintf() calls write
> past the end of the caller's stack buffer.
>
> Switch to scnprintf() which returns the actual number of characters
> written, making ret accumulation safe by construction.
>
> Fixes: a24020e6b7cf6eb8 ("perf tools: Change cpu_map__fprintf output")
> Reported-by: sashiko-bot <sashiko-bot@kernel.org>
> Cc: Jiri Olsa <jolsa@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>

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

Thanks,
Ian

> ---
>  tools/perf/util/cpumap.c | 18 +++++++++---------
>  1 file changed, 9 insertions(+), 9 deletions(-)
>
> diff --git a/tools/perf/util/cpumap.c b/tools/perf/util/cpumap.c
> index 1fab00ec4a59a0c7..23ebe9b97f8e58af 100644
> --- a/tools/perf/util/cpumap.c
> +++ b/tools/perf/util/cpumap.c
> @@ -692,21 +692,21 @@ size_t cpu_map__snprint(struct perf_cpu_map *map, char *buf, size_t size)
>                 if (start == -1) {
>                         start = i;
>                         if (last) {
> -                               ret += snprintf(buf + ret, size - ret,
> -                                               "%s%d", COMMA,
> -                                               perf_cpu_map__cpu(map, i).cpu);
> +                               ret += scnprintf(buf + ret, size - ret,
> +                                                "%s%d", COMMA,
> +                                                perf_cpu_map__cpu(map, i).cpu);
>                         }
>                 } else if (((i - start) != (cpu.cpu - perf_cpu_map__cpu(map, start).cpu)) || last) {
>                         int end = i - 1;
>
>                         if (start == end) {
> -                               ret += snprintf(buf + ret, size - ret,
> -                                               "%s%d", COMMA,
> -                                               perf_cpu_map__cpu(map, start).cpu);
> +                               ret += scnprintf(buf + ret, size - ret,
> +                                                "%s%d", COMMA,
> +                                                perf_cpu_map__cpu(map, start).cpu);
>                         } else {
> -                               ret += snprintf(buf + ret, size - ret,
> -                                               "%s%d-%d", COMMA,
> -                                               perf_cpu_map__cpu(map, start).cpu, perf_cpu_map__cpu(map, end).cpu);
> +                               ret += scnprintf(buf + ret, size - ret,
> +                                                "%s%d-%d", COMMA,
> +                                                perf_cpu_map__cpu(map, start).cpu, perf_cpu_map__cpu(map, end).cpu);
>                         }
>                         first = false;
>                         start = i;
> --
> 2.54.0
>

Re: [PATCH 03/11] perf tools: Use perf_env__get_cpu_topology() in machine__resolve()

[Ian Rogers]

On Mon, Jun 8, 2026 at 1:18 PM Arnaldo Carvalho de Melo <acme@kernel.org> wrote:
>
> From: Arnaldo Carvalho de Melo <acme@redhat.com>
>
> machine__resolve() accesses env->cpu[al->cpu].socket_id after checking
> al->cpu >= 0 and env->cpu != NULL, but without validating al->cpu
> against env->nr_cpus_avail.  Since al->cpu comes from the untrusted
> perf.data sample, a crafted file with a large CPU index causes an
> out-of-bounds heap read.
>
> Use perf_env__get_cpu_topology() which validates both NULL and bounds.
>
> Fixes: 0c4c4debb0adda4c ("perf tools: Add processor socket info to hist_entry and addr_location")
> Reported-by: sashiko-bot <sashiko-bot@kernel.org>
> Cc: Kan Liang <kan.liang@intel.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/event.c | 11 +++++++++--
>  1 file changed, 9 insertions(+), 2 deletions(-)
>
> diff --git a/tools/perf/util/event.c b/tools/perf/util/event.c
> index 66f4843bb235df53..001db00be1073ad4 100644
> --- a/tools/perf/util/event.c
> +++ b/tools/perf/util/event.c
> @@ -14,6 +14,7 @@
>  #include <linux/perf_event.h>
>  #include "cpumap.h"
>  #include "dso.h"
> +#include "env.h"
>  #include "event.h"
>  #include "debug.h"
>  #include "hist.h"
> @@ -836,8 +837,14 @@ int machine__resolve(struct machine *machine, struct addr_location *al,
>         if (al->cpu >= 0) {
>                 struct perf_env *env = machine->env;
>
> -               if (env && env->cpu)
> -                       al->socket = env->cpu[al->cpu].socket_id;
> +               /* bounds-check before truncating to struct perf_cpu (int16_t) */
> +               if (env && al->cpu < env->nr_cpus_avail) {

Isn't this already covered by the test in perf_env__get_cpu_topology?

https://web.git.kernel.org/pub/scm/linux/kernel/git/perf/perf-tools-next.git/tree/tools/perf/util/env.h?h=perf-tools-next#n199
```
if (env->cpu && cpu.cpu >= 0 && cpu.cpu < env->nr_cpus_avail)
  return &env->cpu[cpu.cpu];
return NULL;
```

Thanks,
Ian




> +                       struct cpu_topology_map *topo;
> +
> +                       topo = perf_env__get_cpu_topology(env, (struct perf_cpu){ al->cpu });
> +                       if (topo)
> +                               al->socket = topo->socket_id;
> +               }
>         }
>
>         /* Account for possible out-of-order switch events. */
> --
> 2.54.0
>

Re: [PATCH 04/11] perf tools: NULL bitmap pointers after bitmap_free()

[Ian Rogers]

On Mon, Jun 8, 2026 at 1:18 PM Arnaldo Carvalho de Melo <acme@kernel.org> wrote:
>
> From: Arnaldo Carvalho de Melo <acme@redhat.com>
>
> Two call sites free bitmaps without NULLing the pointer, risking
> double-free if the structure is reused or cleanup is called twice:
>
>  - mmap__munmap():                 map->affinity_mask.bits
>  - record__mmap_cpu_mask_free():   mask->bits
>
> Set each pointer to NULL after bitmap_free().
>
> Fixes: 8384a2600c7ddfc8 ("perf record: Adapt affinity to machines with #CPUs > 1K")
> Fixes: f466e5ed6c356d1d ("perf record: Extend --threads command line option")
> Reported-by: sashiko-bot <sashiko-bot@kernel.org>
> Cc: Alexey Budankov <alexey.budankov@linux.intel.com>
> Cc: Alexey Bayduraev <alexey.v.bayduraev@linux.intel.com>
> Cc: Arnaldo Carvalho de Melo <acme@redhat.com>
> Assisted-by: Claude Opus 4.6 <noreply@anthropic.com>
> Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>

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

Perhaps there's a need for bitmap_zfree.

Thanks,
Ian

> ---
>  tools/perf/builtin-record.c | 1 +
>  tools/perf/util/mmap.c      | 1 +
>  2 files changed, 2 insertions(+)
>
> diff --git a/tools/perf/builtin-record.c b/tools/perf/builtin-record.c
> index a33c78f030d91012..e915390556752b9e 100644
> --- a/tools/perf/builtin-record.c
> +++ b/tools/perf/builtin-record.c
> @@ -3084,6 +3084,7 @@ static int record__mmap_cpu_mask_alloc(struct mmap_cpu_mask *mask, int nr_bits)
>  static void record__mmap_cpu_mask_free(struct mmap_cpu_mask *mask)
>  {
>         bitmap_free(mask->bits);
> +       mask->bits = NULL;
>         mask->nbits = 0;
>  }
>
> diff --git a/tools/perf/util/mmap.c b/tools/perf/util/mmap.c
> index d64aec6c7c843e81..c6bd4c37d50ee57e 100644
> --- a/tools/perf/util/mmap.c
> +++ b/tools/perf/util/mmap.c
> @@ -238,6 +238,7 @@ static void perf_mmap__aio_munmap(struct mmap *map __maybe_unused)
>  void mmap__munmap(struct mmap *map)
>  {
>         bitmap_free(map->affinity_mask.bits);
> +       map->affinity_mask.bits = NULL;
>
>         zstd_fini(&map->zstd_data);
>
> --
> 2.54.0
>

Re: [PATCH 05/11] perf sched: Bounds-check prio before test_bit() in timehist

[Ian Rogers]

On Mon, Jun 8, 2026 at 1:18 PM Arnaldo Carvalho de Melo <acme@kernel.org> wrote:
>
> From: Arnaldo Carvalho de Melo <acme@redhat.com>
>
> timehist_skip_sample() reads prio from untrusted tracepoint data via
> perf_sample__intval(sample, "prev_prio") without bounds validation.
> A crafted perf.data with prev_prio >= MAX_PRIO (140) causes test_bit()
> to read past the end of the prio_bitmap, which is only MAX_PRIO bits.
>
> Add a prio >= 0 && prio < MAX_PRIO check before the test_bit() call.
> This also makes the != -1 sentinel check explicit as >= 0.
>
> Fixes: 9b3a48bbe20d9692 ("perf sched timehist: Add --prio option")
> Reported-by: sashiko-bot <sashiko-bot@kernel.org>
> Cc: Yang Jihong <yangjihong@bytedance.com>
> Assisted-by: Claude Opus 4.6 <noreply@anthropic.com>
> Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>

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

Thanks,
Ian

> ---
>  tools/perf/builtin-sched.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/tools/perf/builtin-sched.c b/tools/perf/builtin-sched.c
> index 1ff01f03d2ad1ad3..ded511d8518803a0 100644
> --- a/tools/perf/builtin-sched.c
> +++ b/tools/perf/builtin-sched.c
> @@ -2645,7 +2645,9 @@ static bool timehist_skip_sample(struct perf_sched *sched,
>                 else if (evsel__name_is(sample->evsel, "sched:sched_switch"))
>                         prio = perf_sample__intval(sample, "prev_prio");
>
> -               if (prio != -1 && !test_bit(prio, sched->prio_bitmap)) {
> +               /* prio comes from untrusted tracepoint data — bounds-check before test_bit */
> +               if (prio >= 0 &&
> +                   (prio >= MAX_PRIO || !test_bit(prio, sched->prio_bitmap))) {
>                         rc = true;
>                         sched->skipped_samples++;
>                 }
> --
> 2.54.0
>

Re: [PATCH 06/11] perf sched: Fix idle-hist callchain display using wrong rb_first variant

[Ian Rogers]

On Mon, Jun 8, 2026 at 1:18 PM Arnaldo Carvalho de Melo <acme@kernel.org> wrote:
>
> From: Arnaldo Carvalho de Melo <acme@redhat.com>
>
> timehist_print_idlehist_callchain() calls rb_first_cached() on
> sorted_root, but the sort function (callchain_param.sort) populates it
> via rb_insert_color() on the plain rb_root member — not the cached
> variant.  This means rb_leftmost is never set, so rb_first_cached()
> always returns NULL and the entire callchain summary is silently
> dropped from --idle-hist output.
>
> The original code in ba957ebb54893aca ("perf sched timehist: Show
> callchains for idle stat") was correct — it used struct rb_root and
> rb_first().  The bug was introduced when sorted_root was converted to
> rb_root_cached without converting the sort insertion path to use
> rb_insert_color_cached().
>
> Use rb_first(&root->rb_root) to match how the tree was populated.
>
> Fixes: cb4c13a5137766c3 ("perf sched: Use cached rbtrees")
> Reported-by: sashiko-bot <sashiko-bot@kernel.org>
> Cc: Davidlohr Bueso <dave@stgolabs.net>
> 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>

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

Thanks,
Ian

> ---
>  tools/perf/builtin-sched.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/tools/perf/builtin-sched.c b/tools/perf/builtin-sched.c
> index ded511d8518803a0..85f11d388392d316 100644
> --- a/tools/perf/builtin-sched.c
> +++ b/tools/perf/builtin-sched.c
> @@ -3130,7 +3130,8 @@ static size_t timehist_print_idlehist_callchain(struct rb_root_cached *root)
>         size_t ret = 0;
>         FILE *fp = stdout;
>         struct callchain_node *chain;
> -       struct rb_node *rb_node = rb_first_cached(root);
> +       /* sort() uses rb_insert_color() on rb_root, not rb_root_cached */
> +       struct rb_node *rb_node = rb_first(&root->rb_root);
>
>         printf("  %16s  %8s  %s\n", "Idle time (msec)", "Count", "Callchains");
>         printf("  %.16s  %.8s  %.50s\n", graph_dotted_line, graph_dotted_line,
> --
> 2.54.0
>

Re: [PATCH 07/11] perf tools: Add O_CLOEXEC to open() calls in DSO and ELF code

[Ian Rogers]

On Mon, Jun 8, 2026 at 1:18 PM Arnaldo Carvalho de Melo <acme@kernel.org> wrote:
>
> From: Arnaldo Carvalho de Melo <acme@redhat.com>
>
> open() calls in dso.c and symbol-elf.c omit O_CLOEXEC, which leaks
> file descriptors to child processes spawned during symbol resolution
> (e.g., addr2line, objdump).  This can exhaust the fd limit during
> long profiling sessions or when processing many DSOs.
>
> Add O_CLOEXEC to all open() calls in both files (12 call sites).
>
> Fixes: cdd059d731eeb466 ("perf tools: Move dso_* related functions into dso object")
> Fixes: e5a1845fc0aeca85 ("perf symbols: Split out util/symbol-elf.c")
> Reported-by: sashiko-bot <sashiko-bot@kernel.org>
> Cc: Jiri Olsa <jolsa@kernel.org>
> Cc: Namhyung Kim <namhyung@kernel.org>
> Assisted-by: Claude Opus 4.6 <noreply@anthropic.com>
> Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>

Reviewed-by: Ian Rogers <irogers@google.com>
and Sashiko is asking for more of this goodness.

Thanks,
Ian

> ---
>  tools/perf/util/dso.c        |  4 ++--
>  tools/perf/util/symbol-elf.c | 20 ++++++++++----------
>  2 files changed, 12 insertions(+), 12 deletions(-)
>
> diff --git a/tools/perf/util/dso.c b/tools/perf/util/dso.c
> index 7dced896c64eafd7..fb2e78fe2aa8eb94 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))
> -               return open(name, O_RDONLY);
> +               return open(name, O_RDONLY | O_CLOEXEC);
>
>         fd = mkstemp(tmpbuf);
>         if (fd < 0) {
> @@ -1911,7 +1911,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);
> +       fd = open(symfs_filename, O_RDONLY | O_CLOEXEC);
>         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 186e6d92ac3d7742..c2bdfd0003df2abe 100644
> --- a/tools/perf/util/symbol-elf.c
> +++ b/tools/perf/util/symbol-elf.c
> @@ -217,7 +217,7 @@ bool filename__has_section(const char *filename, const char *sec)
>         GElf_Shdr shdr;
>         bool found = false;
>
> -       fd = open(filename, O_RDONLY);
> +       fd = open(filename, O_RDONLY | O_CLOEXEC);
>         if (fd < 0)
>                 return false;
>
> @@ -872,7 +872,7 @@ static int read_build_id(const char *filename, struct build_id *bid)
>         if (size < BUILD_ID_SIZE)
>                 goto out;
>
> -       fd = open(filename, O_RDONLY);
> +       fd = open(filename, O_RDONLY | O_CLOEXEC);
>         if (fd < 0)
>                 goto out;
>
> @@ -935,7 +935,7 @@ int sysfs__read_build_id(const char *filename, struct build_id *bid)
>         size_t size = sizeof(bid->data);
>         int fd, err = -1;
>
> -       fd = open(filename, O_RDONLY);
> +       fd = open(filename, O_RDONLY | O_CLOEXEC);
>         if (fd < 0)
>                 goto out;
>
> @@ -995,7 +995,7 @@ int filename__read_debuglink(const char *filename, char *debuglink,
>         if (err >= 0)
>                 goto out;
>
> -       fd = open(filename, O_RDONLY);
> +       fd = open(filename, O_RDONLY | O_CLOEXEC);
>         if (fd < 0)
>                 goto out;
>
> @@ -1153,7 +1153,7 @@ int symsrc__init(struct symsrc *ss, struct dso *dso, const char *name,
>
>                 type = dso__symtab_type(dso);
>         } else {
> -               fd = open(name, O_RDONLY);
> +               fd = open(name, O_RDONLY | O_CLOEXEC);
>                 if (fd < 0) {
>                         *dso__load_errno(dso) = errno;
>                         return -1;
> @@ -1952,7 +1952,7 @@ static int kcore__open(struct kcore *kcore, const char *filename)
>  {
>         GElf_Ehdr *ehdr;
>
> -       kcore->fd = open(filename, O_RDONLY);
> +       kcore->fd = open(filename, O_RDONLY | O_CLOEXEC);
>         if (kcore->fd == -1)
>                 return -1;
>
> @@ -1985,7 +1985,7 @@ static int kcore__init(struct kcore *kcore, char *filename, int elfclass,
>         if (temp)
>                 kcore->fd = mkstemp(filename);
>         else
> -               kcore->fd = open(filename, O_WRONLY | O_CREAT | O_EXCL, 0400);
> +               kcore->fd = open(filename, O_WRONLY | O_CREAT | O_EXCL | O_CLOEXEC, 0400);
>         if (kcore->fd == -1)
>                 return -1;
>
> @@ -2461,11 +2461,11 @@ static int kcore_copy__compare_files(const char *from_filename,
>  {
>         int from, to, err = -1;
>
> -       from = open(from_filename, O_RDONLY);
> +       from = open(from_filename, O_RDONLY | O_CLOEXEC);
>         if (from < 0)
>                 return -1;
>
> -       to = open(to_filename, O_RDONLY);
> +       to = open(to_filename, O_RDONLY | O_CLOEXEC);
>         if (to < 0)
>                 goto out_close_from;
>
> @@ -2883,7 +2883,7 @@ int get_sdt_note_list(struct list_head *head, const char *target)
>         Elf *elf;
>         int fd, ret;
>
> -       fd = open(target, O_RDONLY);
> +       fd = open(target, O_RDONLY | O_CLOEXEC);
>         if (fd < 0)
>                 return -EBADF;
>
> --
> 2.54.0
>

Re: [PATCH 08/11] perf bpf: Use scnprintf() in snprintf_hex() and synthesize_bpf_prog_name()

[Ian Rogers]

On Mon, Jun 8, 2026 at 1:18 PM Arnaldo Carvalho de Melo <acme@kernel.org> wrote:
>
> From: Arnaldo Carvalho de Melo <acme@redhat.com>
>
> Both functions accumulate formatted output via ret += snprintf(buf + ret,
> size - ret, ...).  If the buffer is too small and snprintf() returns more
> than the remaining space, ret exceeds size and the next 'size - ret'
> underflows, causing snprintf() to write past the buffer end.
>
> Switch to scnprintf() which returns the actual number of bytes written,
> making the accumulation safe.
>
> Fixes: 7b612e291a5affb1 ("perf tools: Synthesize PERF_RECORD_* for loaded BPF programs")
> Reported-by: sashiko-bot <sashiko-bot@kernel.org>
> Cc: Song Liu <song@kernel.org>
> Assisted-by: Claude Opus 4.6 <noreply@anthropic.com>
> Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>

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

Thanks,
Ian

> ---
>  tools/perf/util/bpf-event.c | 11 ++++++-----
>  1 file changed, 6 insertions(+), 5 deletions(-)
>
> diff --git a/tools/perf/util/bpf-event.c b/tools/perf/util/bpf-event.c
> index a27945c279efb779..2c09842469f1f28c 100644
> --- a/tools/perf/util/bpf-event.c
> +++ b/tools/perf/util/bpf-event.c
> @@ -36,7 +36,7 @@ static int snprintf_hex(char *buf, size_t size, unsigned char *data, size_t len)
>         size_t i;
>
>         for (i = 0; i < len; i++)
> -               ret += snprintf(buf + ret, size - ret, "%02x", data[i]);
> +               ret += scnprintf(buf + ret, size - ret, "%02x", data[i]);
>         return ret;
>  }
>
> @@ -140,7 +140,7 @@ static int synthesize_bpf_prog_name(char *buf, int size,
>         const struct btf_type *t;
>         int name_len;
>
> -       name_len = snprintf(buf, size, "bpf_prog_");
> +       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) {
> @@ -153,9 +153,10 @@ static int synthesize_bpf_prog_name(char *buf, int size,
>                         short_name = info->name;
>         } else
>                 short_name = "F";
> -       if (short_name)
> -               name_len += snprintf(buf + name_len, size - name_len,
> -                                    "_%s", short_name);
> +       if (short_name) {
> +               name_len += scnprintf(buf + name_len, size - name_len,
> +                                     "_%s", short_name);
> +       }
>         return name_len;
>  }
>
> --
> 2.54.0
>

Re: [PATCH 09/11] perf hists: Fix snprintf() in hists__scnprintf_title() UID filter path

[Ian Rogers]

On Mon, Jun 8, 2026 at 1:18 PM Arnaldo Carvalho de Melo <acme@kernel.org> wrote:
>
> From: Arnaldo Carvalho de Melo <acme@redhat.com>
>
> hists__scnprintf_title() accumulates formatted output into a buffer
> using scnprintf() for all filter clauses except the UID filter, which
> uses snprintf().  If the buffer fills up and snprintf() returns more
> than the remaining space, printed exceeds size and the next 'size -
> printed' underflows, causing later scnprintf() calls to write past
> the buffer.
>
> Switch the UID filter clause to scnprintf() to match the rest of the
> function.
>
> Fixes: 25c312dbf88ca402 ("perf hists: Move hists__scnprintf_title() away from the TUI code")
> Reported-by: sashiko-bot <sashiko-bot@kernel.org>
> Cc: Arnaldo Carvalho de Melo <acme@redhat.com>
> Assisted-by: Claude Opus 4.6 <noreply@anthropic.com>
> Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>

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

Thanks,
Ian

> ---
>  tools/perf/util/hist.c | 7 ++++---
>  1 file changed, 4 insertions(+), 3 deletions(-)
>
> diff --git a/tools/perf/util/hist.c b/tools/perf/util/hist.c
> index 811d68fa6770c5b7..df978c996b6c2262 100644
> --- a/tools/perf/util/hist.c
> +++ b/tools/perf/util/hist.c
> @@ -2963,9 +2963,10 @@ int __hists__scnprintf_title(struct hists *hists, char *bf, size_t size, bool sh
>                            ev_name, sample_freq_str, enable_ref ? ref : " ", nr_events);
>
>
> -       if (hists->uid_filter_str)
> -               printed += snprintf(bf + printed, size - printed,
> -                                   ", UID: %s", hists->uid_filter_str);
> +       if (hists->uid_filter_str) {
> +               printed += scnprintf(bf + printed, size - printed,
> +                                    ", UID: %s", hists->uid_filter_str);
> +       }
>         if (thread) {
>                 if (hists__has(hists, thread)) {
>                         printed += scnprintf(bf + printed, size - printed,
> --
> 2.54.0
>

Re: [PATCH 10/11] perf tools: Use scnprintf() in build_id__snprintf() and hwmon read_events()

[Ian Rogers]

On Mon, Jun 8, 2026 at 1:51 PM <sashiko-bot@kernel.org> wrote:
>
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [Low] The early termination condition `offs < bf_size` is rendered ineffective by the switch to `scnprintf`.
> --
>
> commit 0dbaa7b9065f01d9d8a6117248bffa97b3021ddf
> Author: Arnaldo Carvalho de Melo <acme@redhat.com>
>
> perf tools: Use scnprintf() in build_id__snprintf() and hwmon read_events()
>
> This commit replaces snprintf() with scnprintf() in build_id__snprintf()
> and hwmon_pmu__describe_items() to prevent accumulated lengths from exceeding
> the allocated buffer size and underflowing the remaining size.
>
> > diff --git a/tools/perf/util/build-id.c b/tools/perf/util/build-id.c
> > index 8c0a9ae932aa5..a3b92108f9626 100644
> > --- a/tools/perf/util/build-id.c
> > +++ b/tools/perf/util/build-id.c
> > @@ -94,7 +94,7 @@ int build_id__snprintf(const struct build_id *build_id, char *bf, size_t bf_size
> >       }
> >
> >       for (size_t i = 0; i < build_id->size && offs < bf_size; ++i)
> > -             offs += snprintf(bf + offs, bf_size - offs, "%02x", build_id->data[i]);
> > +             offs += scnprintf(bf + offs, bf_size - offs, "%02x", build_id->data[i]);
>
> [Severity: Low]
> Does this change neutralize the early termination condition in the loop?
>
> Since scnprintf() returns the number of characters written excluding the null
> terminator, `offs` will reach at most `bf_size - 1` and never equals `bf_size`.
>
> Once the buffer is full (`bf_size - offs == 1`), scnprintf() will only write a
> null terminator, returning 0. Because `offs` stops increasing, the condition
> `offs < bf_size` remains true. This causes the loop to continue writing nothing
> for the remaining `build_id->size` iterations instead of exiting early.

Looks like a "-1" is needed to address this.

Thanks,
Ian

> >
> >       return offs;
> >  }
>
> --
> Sashiko AI review · https://sashiko.dev/#/patchset/20260608201753.1979464-1-acme@kernel.org?part=10
>

Re: [PATCH 11/11] libperf: Document code simplification case for widening struct perf_cpu

[Ian Rogers]

On Mon, Jun 8, 2026 at 1:50 PM <sashiko-bot@kernel.org> wrote:
>
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [Low] The documentation added to `tools/lib/perf/TODO` incorrectly states that narrowing values > 32767 to `int16_t` wraps them to 'small positive numbers'. In two's complement arithmetic, values from 32768 to 65535 wrap to negative numbers (-32768 to -1) when truncated to a signed 16-bit integer.
> --
>
> commit e8f5b26d672396c3173a77e6aaeee613f6d309bd
> Author: Arnaldo Carvalho de Melo <acme@redhat.com>
>
> libperf: Document code simplification case for widening struct perf_cpu
>
> Add a bullet point to the libperf ABI TODO explaining the code
> simplification benefit of widening struct perf_cpu.cpu from int16_t
> to int.
>
> > diff --git a/tools/lib/perf/TODO b/tools/lib/perf/TODO
> > index 486dd95dc5720..1a3644aa1f38d 100644
> > --- a/tools/lib/perf/TODO
> > +++ b/tools/lib/perf/TODO
> > @@ -11,6 +11,13 @@ together.
> >       (x86_64 max is 8192, arm64 is 4096), but NR_CPUS limits keep
> >       growing.  perf clamps to INT16_MAX in set_max_cpu_num() as a
> >       safety net.
> > +   - Code simplification: the int16_t forces defensive truncation

I think Sashiko just wants this to be uint16_t :-)

Thanks,
Ian

> > +     checks at every boundary where a wider CPU index (int from
> > +     sample->cpu, al->cpu, etc.) is narrowed into struct perf_cpu.
> > +     Without these checks, values > 32767 silently wrap to small
> > +     positive numbers, bypassing bounds validation.  Widening to int
>        ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
>
> [Severity: Low]
> This isn't a bug, but is the arithmetic description accurate here?
>
> In two's complement arithmetic, truncating values between 32768 and 65535
> into a signed 16-bit integer (int16_t) wraps them to negative numbers
> (-32768 to -1), rather than small positive numbers.
>
> The overall conclusion remains correct since negative values trivially
> bypass bounds checks, but the description of the wrapping behavior appears
> mathematically inaccurate.
>
> > +     eliminates this entire class of silent truncation bugs and
> > +     removes the need for the INT16_MAX clamp in set_max_cpu_num().
> >     - Scope: struct perf_cpu is embedded everywhere — perf_cpu_map__cpu(),
>
> --
> Sashiko AI review · https://sashiko.dev/#/patchset/20260608201753.1979464-1-acme@kernel.org?part=11
>

Re: [PATCH 11/11] libperf: Document code simplification case for widening struct perf_cpu

[Arnaldo Carvalho de Melo]

On Mon, Jun 08, 2026 at 03:04:40PM -0700, Ian Rogers wrote:
> On Mon, Jun 8, 2026 at 1:50 PM <sashiko-bot@kernel.org> wrote:
> >
> > Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> > - [Low] The documentation added to `tools/lib/perf/TODO` incorrectly states that narrowing values > 32767 to `int16_t` wraps them to 'small positive numbers'. In two's complement arithmetic, values from 32768 to 65535 wrap to negative numbers (-32768 to -1) when truncated to a signed 16-bit integer.
> > --
> >
> > commit e8f5b26d672396c3173a77e6aaeee613f6d309bd
> > Author: Arnaldo Carvalho de Melo <acme@redhat.com>
> >
> > libperf: Document code simplification case for widening struct perf_cpu
> >
> > Add a bullet point to the libperf ABI TODO explaining the code
> > simplification benefit of widening struct perf_cpu.cpu from int16_t
> > to int.
> >
> > > diff --git a/tools/lib/perf/TODO b/tools/lib/perf/TODO
> > > index 486dd95dc5720..1a3644aa1f38d 100644
> > > --- a/tools/lib/perf/TODO
> > > +++ b/tools/lib/perf/TODO
> > > @@ -11,6 +11,13 @@ together.
> > >       (x86_64 max is 8192, arm64 is 4096), but NR_CPUS limits keep
> > >       growing.  perf clamps to INT16_MAX in set_max_cpu_num() as a
> > >       safety net.
> > > +   - Code simplification: the int16_t forces defensive truncation
> 
> I think Sashiko just wants this to be uint16_t :-)

That would buy us some more decades, maybe. :-)

- Arnaldo
 
> Thanks,
> Ian
> 
> > > +     checks at every boundary where a wider CPU index (int from
> > > +     sample->cpu, al->cpu, etc.) is narrowed into struct perf_cpu.
> > > +     Without these checks, values > 32767 silently wrap to small
> > > +     positive numbers, bypassing bounds validation.  Widening to int
> >        ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
> >
> > [Severity: Low]
> > This isn't a bug, but is the arithmetic description accurate here?
> >
> > In two's complement arithmetic, truncating values between 32768 and 65535
> > into a signed 16-bit integer (int16_t) wraps them to negative numbers
> > (-32768 to -1), rather than small positive numbers.
> >
> > The overall conclusion remains correct since negative values trivially
> > bypass bounds checks, but the description of the wrapping behavior appears
> > mathematically inaccurate.
> >
> > > +     eliminates this entire class of silent truncation bugs and
> > > +     removes the need for the INT16_MAX clamp in set_max_cpu_num().
> > >     - Scope: struct perf_cpu is embedded everywhere — perf_cpu_map__cpu(),
> >
> > --
> > Sashiko AI review · https://sashiko.dev/#/patchset/20260608201753.1979464-1-acme@kernel.org?part=11
> >

[PATCH 07/11] perf tools: Add O_CLOEXEC to open() calls in DSO and ELF code

[Arnaldo Carvalho de Melo]

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

open() calls in dso.c and symbol-elf.c omit O_CLOEXEC, which leaks
file descriptors to child processes spawned during symbol resolution
(e.g., addr2line, objdump).  This can exhaust the fd limit during
long profiling sessions or when processing many DSOs.

Add O_CLOEXEC to all open() calls in both files (12 call sites).

Fixes: cdd059d731eeb466 ("perf tools: Move dso_* related functions into dso object")
Fixes: e5a1845fc0aeca85 ("perf symbols: Split out util/symbol-elf.c")
Reported-by: sashiko-bot <sashiko-bot@kernel.org>
Cc: Jiri Olsa <jolsa@kernel.org>
Cc: Namhyung Kim <namhyung@kernel.org>
Reviewed-by: 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/dso.c        |  4 ++--
 tools/perf/util/symbol-elf.c | 20 ++++++++++----------
 2 files changed, 12 insertions(+), 12 deletions(-)

diff --git a/tools/perf/util/dso.c b/tools/perf/util/dso.c
index 7dced896c64eafd7..fb2e78fe2aa8eb94 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))
-		return open(name, O_RDONLY);
+		return open(name, O_RDONLY | O_CLOEXEC);
 
 	fd = mkstemp(tmpbuf);
 	if (fd < 0) {
@@ -1911,7 +1911,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);
+	fd = open(symfs_filename, O_RDONLY | O_CLOEXEC);
 	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 186e6d92ac3d7742..c2bdfd0003df2abe 100644
--- a/tools/perf/util/symbol-elf.c
+++ b/tools/perf/util/symbol-elf.c
@@ -217,7 +217,7 @@ bool filename__has_section(const char *filename, const char *sec)
 	GElf_Shdr shdr;
 	bool found = false;
 
-	fd = open(filename, O_RDONLY);
+	fd = open(filename, O_RDONLY | O_CLOEXEC);
 	if (fd < 0)
 		return false;
 
@@ -872,7 +872,7 @@ static int read_build_id(const char *filename, struct build_id *bid)
 	if (size < BUILD_ID_SIZE)
 		goto out;
 
-	fd = open(filename, O_RDONLY);
+	fd = open(filename, O_RDONLY | O_CLOEXEC);
 	if (fd < 0)
 		goto out;
 
@@ -935,7 +935,7 @@ int sysfs__read_build_id(const char *filename, struct build_id *bid)
 	size_t size = sizeof(bid->data);
 	int fd, err = -1;
 
-	fd = open(filename, O_RDONLY);
+	fd = open(filename, O_RDONLY | O_CLOEXEC);
 	if (fd < 0)
 		goto out;
 
@@ -995,7 +995,7 @@ int filename__read_debuglink(const char *filename, char *debuglink,
 	if (err >= 0)
 		goto out;
 
-	fd = open(filename, O_RDONLY);
+	fd = open(filename, O_RDONLY | O_CLOEXEC);
 	if (fd < 0)
 		goto out;
 
@@ -1153,7 +1153,7 @@ int symsrc__init(struct symsrc *ss, struct dso *dso, const char *name,
 
 		type = dso__symtab_type(dso);
 	} else {
-		fd = open(name, O_RDONLY);
+		fd = open(name, O_RDONLY | O_CLOEXEC);
 		if (fd < 0) {
 			*dso__load_errno(dso) = errno;
 			return -1;
@@ -1952,7 +1952,7 @@ static int kcore__open(struct kcore *kcore, const char *filename)
 {
 	GElf_Ehdr *ehdr;
 
-	kcore->fd = open(filename, O_RDONLY);
+	kcore->fd = open(filename, O_RDONLY | O_CLOEXEC);
 	if (kcore->fd == -1)
 		return -1;
 
@@ -1985,7 +1985,7 @@ static int kcore__init(struct kcore *kcore, char *filename, int elfclass,
 	if (temp)
 		kcore->fd = mkstemp(filename);
 	else
-		kcore->fd = open(filename, O_WRONLY | O_CREAT | O_EXCL, 0400);
+		kcore->fd = open(filename, O_WRONLY | O_CREAT | O_EXCL | O_CLOEXEC, 0400);
 	if (kcore->fd == -1)
 		return -1;
 
@@ -2461,11 +2461,11 @@ static int kcore_copy__compare_files(const char *from_filename,
 {
 	int from, to, err = -1;
 
-	from = open(from_filename, O_RDONLY);
+	from = open(from_filename, O_RDONLY | O_CLOEXEC);
 	if (from < 0)
 		return -1;
 
-	to = open(to_filename, O_RDONLY);
+	to = open(to_filename, O_RDONLY | O_CLOEXEC);
 	if (to < 0)
 		goto out_close_from;
 
@@ -2883,7 +2883,7 @@ int get_sdt_note_list(struct list_head *head, const char *target)
 	Elf *elf;
 	int fd, ret;
 
-	fd = open(target, O_RDONLY);
+	fd = open(target, O_RDONLY | O_CLOEXEC);
 	if (fd < 0)
 		return -EBADF;
 
-- 
2.54.0

Re: [PATCH 07/11] perf tools: Add O_CLOEXEC to open() calls in DSO and ELF code

[sashiko-bot]

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

New issues:
- [High] Incomplete fix for file descriptor leaks in DSO and ELF code.

Pre-existing issues:
- [Critical] Stack buffer overflow in sysfs__read_build_id when skipping non-matching GNU notes.
--

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

perf tools: Add O_CLOEXEC to open() calls in DSO and ELF code

The commit adds O_CLOEXEC to open() calls in dso.c and symbol-elf.c to
prevent leaking file descriptors to child processes during symbol
resolution.

> diff --git a/tools/perf/util/dso.c b/tools/perf/util/dso.c
> index 7dced896c64ea..fb2e78fe2aa8e 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))
> -		return open(name, O_RDONLY);
> +		return open(name, O_RDONLY | O_CLOEXEC);
>  
>  	fd = mkstemp(tmpbuf);

[Severity: High]
Since the goal of this patch is to prevent leaking file descriptors to child
processes, should we also address the mkstemp() calls in these files?

mkstemp() creates file descriptors without O_CLOEXEC. For instance, here in
filename__decompress(), the temporary file descriptor will still be leaked.
Should mkostemp(tmpbuf, O_CLOEXEC) be used instead?

[ ... ]
> diff --git a/tools/perf/util/symbol-elf.c b/tools/perf/util/symbol-elf.c
> index 186e6d92ac3d7..c2bdfd0003df2 100644
> --- a/tools/perf/util/symbol-elf.c
> +++ b/tools/perf/util/symbol-elf.c
[ ... ]
> @@ -935,7 +935,7 @@ int sysfs__read_build_id(const char *filename, struct build_id *bid)
>  	size_t size = sizeof(bid->data);
>  	int fd, err = -1;
>  
> -	fd = open(filename, O_RDONLY);
> +	fd = open(filename, O_RDONLY | O_CLOEXEC);
>  	if (fd < 0)
>  		goto out;

[Severity: Critical]
This is a pre-existing issue, but while reviewing this function, could
an unbounded read() happen later in sysfs__read_build_id() when skipping
non-matching GNU notes?

If a crafted sysfs note has a maliciously large nhdr.n_descsz, the code does:

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

Since bf is a fixed-size stack array (char bf[BUFSIZ]), could this read past
the end of bf and overwrite the stack frame if descsz exceeds BUFSIZ?

[ ... ]
> @@ -1985,7 +1985,7 @@ static int kcore__init(struct kcore *kcore, char *filename, int elfclass,
>  	if (temp)
>  		kcore->fd = mkstemp(filename);
>  	else
> -		kcore->fd = open(filename, O_WRONLY | O_CREAT | O_EXCL, 0400);
> +		kcore->fd = open(filename, O_WRONLY | O_CREAT | O_EXCL | O_CLOEXEC, 0400);
>  	if (kcore->fd == -1)
>  		return -1;

[Severity: High]
Similarly to the issue in filename__decompress(), if temp is true here,
mkstemp() is called without O_CLOEXEC.

There is also a similar mkstemp() call in read_gnu_debugdata() in this file
that extracts debug data without O_CLOEXEC. Should all of these be updated
to prevent leaking file descriptors?

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