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

[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 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 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 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 to negative numbers (two's complement), bypassing
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 | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/tools/lib/perf/TODO b/tools/lib/perf/TODO
index 486dd95dc57208a8..e179728697d8c7c0 100644
--- a/tools/lib/perf/TODO
+++ b/tools/lib/perf/TODO
@@ -11,6 +11,14 @@ 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 negative
+     numbers (two's complement), 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

[PATCHES v5 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).

Patch 11 expands the libperf ABI TODO with the code simplification
argument for widening struct perf_cpu.cpu — the int16_t forces
truncation checks at every boundary where wider CPU indices are
narrowed.

Arnaldo Carvalho de Melo (11):
  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()
  libperf: Document code simplification case for widening struct perf_cpu

 tools/lib/perf/TODO          |  8 ++++++++
 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   |  7 +++++--
 tools/perf/util/cpumap.c     | 24 +++++++++++++++---------
 tools/perf/util/dso.c        |  4 ++--
 tools/perf/util/event.c      | 15 +++++++++++++--
 tools/perf/util/hist.c       |  7 ++++---
 tools/perf/util/hwmon_pmu.c  | 12 ++++++------
 tools/perf/util/mmap.c       |  2 ++
 tools/perf/util/symbol-elf.c | 20 ++++++++++----------
 12 files changed, 77 insertions(+), 41 deletions(-)

Changes since v4:
  - Patch 4 (bitmap_free): also reset nbits to 0 in mmap__munmap()
    — without this, a reused struct mmap retains the old positive
    nbits while bits is NULL, and bitmap_scnprintf() dereferences
    the NULL pointer when perf runs with -vv.
    (Found by sashiko-bot lore review)
  - Patch 10 (build_id__snprintf): null-terminate bf before the loop
    — when bf_size == 1, the offs + 1 < bf_size guard skips the loop
    entirely, leaving bf[0] uninitialized.
    (Found by sashiko-bot lore review)
  - Collected Reviewed-by tags from Ian Rogers' v3 reviews
    (patches 1, 2, 4, 5, 7, 8, 9).
  - Reordered Reviewed-by tags before Cc lines in commit trailers.

Changes since v3:
  - Patch 3 (machine__resolve): expanded comment explaining why the
    outer al->cpu < nr_cpus_avail check is needed — the int16_t cast
    to struct perf_cpu silently truncates e.g. 65536 to 0, bypassing
    the accessor's internal bounds check.
    (Ian Rogers review)
  - Patch 10 (build_id__snprintf): fixed loop termination — after
    switching to scnprintf(), offs never reaches bf_size, so the
    loop spun doing zero-byte writes.  Changed condition to
    offs + 1 < bf_size.
    (Found by sashiko-bot, confirmed by Ian Rogers)
  - Patch 11 (TODO wording): fixed "wrap to small positive numbers"
    to "wrap to negative numbers (two's complement)".
    (Found by sashiko-bot, confirmed by Ian Rogers)

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.
  - Split libperf ABI TODO hunk out of prio patch into standalone
    patch 11.
  - 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>
Reviewed-by: Ian Rogers <irogers@google.com>
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>
Reviewed-by: Ian Rogers <irogers@google.com>
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.
Also bounds-check al->cpu before the cast to struct perf_cpu (int16_t):
without this, values like 65536 silently truncate to 0, bypassing the
accessor's internal check and returning CPU 0's topology.

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

diff --git a/tools/perf/util/event.c b/tools/perf/util/event.c
index 66f4843bb235df53..ea75816d126a14be 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,18 @@ 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 al->cpu (s32) before casting to struct perf_cpu
+		 * (int16_t): without this, e.g. 65536 truncates to 0 and silently
+		 * returns CPU 0's topology.  Can go once perf_cpu.cpu is widened.
+		 */
+		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>
Reviewed-by: Ian Rogers <irogers@google.com>
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      | 2 ++
 2 files changed, 3 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..358e70c4f3edd077 100644
--- a/tools/perf/util/mmap.c
+++ b/tools/perf/util/mmap.c
@@ -238,6 +238,8 @@ 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;
+	map->affinity_mask.nbits = 0;
 
 	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>
Reviewed-by: Ian Rogers <irogers@google.com>
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>
Acked-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/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>
Reviewed-by: Ian Rogers <irogers@google.com>
Cc: Jiri Olsa <jolsa@kernel.org>
Cc: Namhyung Kim <namhyung@kernel.org>
Assisted-by: Claude Opus 4.6 <noreply@anthropic.com>
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/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>
Reviewed-by: Ian Rogers <irogers@google.com>
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>
Reviewed-by: Ian Rogers <irogers@google.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/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.

In build_id__snprintf(), also tighten the loop guard from
'offs < bf_size' to 'offs + 1 < bf_size': since scnprintf() returns
at most size-1, offs never reaches bf_size, and the original condition
would spin doing zero-byte writes once the buffer fills.

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  |  7 +++++--
 tools/perf/util/hwmon_pmu.c | 12 ++++++------
 2 files changed, 11 insertions(+), 8 deletions(-)

diff --git a/tools/perf/util/build-id.c b/tools/perf/util/build-id.c
index 8c0a9ae932aa5798..eb95ab90f9741d5f 100644
--- a/tools/perf/util/build-id.c
+++ b/tools/perf/util/build-id.c
@@ -93,8 +93,11 @@ int build_id__snprintf(const struct build_id *build_id, char *bf, size_t bf_size
 		return 0;
 	}
 
-	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]);
+	if (bf_size > 0)
+		bf[0] = '\0';
+
+	for (size_t i = 0; i < build_id->size && offs + 1 < bf_size; ++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 to negative numbers (two's complement), bypassing
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 | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/tools/lib/perf/TODO b/tools/lib/perf/TODO
index 486dd95dc57208a8..e179728697d8c7c0 100644
--- a/tools/lib/perf/TODO
+++ b/tools/lib/perf/TODO
@@ -11,6 +11,14 @@ 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 negative
+     numbers (two's complement), 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 05/11] perf sched: Bounds-check prio before test_bit() in timehist

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] The patch introduces a logic error that causes invalid negative priorities (<-1) to bypass the `--prio` filter.
--

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

perf sched: Bounds-check prio before test_bit() in timehist

This commit adds a bounds check to timehist_skip_sample() to ensure that
priority values from untrusted tracepoint data do not exceed MAX_PRIO before
calling test_bit(). It also updates the sentinel check from != -1 to >= 0.

> diff --git a/tools/perf/builtin-sched.c b/tools/perf/builtin-sched.c
> index 1ff01f03d2ad1..ded511d851880 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))) {

[Severity: Low]
Does this logic inadvertently allow invalid negative priorities (less than -1)
to bypass the --prio filter?

By explicitly replacing the prio != -1 sentinel check with prio >= 0, the new
logic evaluates to false for any priority less than -1. This causes the block
to be bypassed, meaning rc remains false and the invalid negative priority is
not skipped by the filter.

Should the check be restructured to ensure that invalid negative priorities
from crafted or corrupted perf.data files are also properly skipped?

>  			rc = true;
>  			sched->skipped_samples++;
>  		}

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

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 1 potential issue(s) to consider:

Pre-existing issues:
- [High] size_t underflow in pmu_id() causing an out-of-bounds write when reading an empty sysfs identifier file.
--

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

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

This commit fixes a size_t underflow in get_max_num() that occurs when
reading an empty sysfs file. By adding an early return when the read
length is zero, it prevents the backward scanning loop from reading
out-of-bounds across the heap.

> 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 it appears there is another
instance of this exact same empty sysfs file underflow issue in pmu_id():

tools/perf/util/pmu.c:pmu_id() {
        ...
        if (filename__read_str(path, &str, &len) < 0)
                return NULL;

        str[len - 1] = 0; /* remove line feed */
        ...
}

Does this cause an out-of-bounds write? If the sysfs identifier file is
empty, filename__read_str() returns len = 0. The str[len - 1] = 0
assignment then underflows, writing a null byte to str[-1]. Could this
corrupt the malloc chunk metadata located immediately before the allocated
buffer?

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

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

[Ian Rogers]

On Wed, Jun 10, 2026 at 9:52 AM 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.
> Also bounds-check al->cpu before the cast to struct perf_cpu (int16_t):
> without this, values like 65536 silently truncate to 0, bypassing the
> accessor's internal check and returning CPU 0's topology.

Thanks for clarifying this. Perhaps we should have a general
int-to-CPU helper, but that's out of scope here.

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

Thanks,
Ian

> 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 | 15 +++++++++++++--
>  1 file changed, 13 insertions(+), 2 deletions(-)
>
> diff --git a/tools/perf/util/event.c b/tools/perf/util/event.c
> index 66f4843bb235df53..ea75816d126a14be 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,18 @@ 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 al->cpu (s32) before casting to struct perf_cpu
> +                * (int16_t): without this, e.g. 65536 truncates to 0 and silently
> +                * returns CPU 0's topology.  Can go once perf_cpu.cpu is widened.
> +                */
> +               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
>

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

[Ian Rogers]

On Wed, Jun 10, 2026 at 9:53 AM Arnaldo Carvalho de Melo
<acme@kernel.org> wrote:
>
> 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.
>
> In build_id__snprintf(), also tighten the loop guard from
> 'offs < bf_size' to 'offs + 1 < bf_size': since scnprintf() returns
> at most size-1, offs never reaches bf_size, and the original condition
> would spin doing zero-byte writes once the buffer fills.

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

Thanks,
Ian

> 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  |  7 +++++--
>  tools/perf/util/hwmon_pmu.c | 12 ++++++------
>  2 files changed, 11 insertions(+), 8 deletions(-)
>
> diff --git a/tools/perf/util/build-id.c b/tools/perf/util/build-id.c
> index 8c0a9ae932aa5798..eb95ab90f9741d5f 100644
> --- a/tools/perf/util/build-id.c
> +++ b/tools/perf/util/build-id.c
> @@ -93,8 +93,11 @@ int build_id__snprintf(const struct build_id *build_id, char *bf, size_t bf_size
>                 return 0;
>         }
>
> -       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]);
> +       if (bf_size > 0)
> +               bf[0] = '\0';
> +
> +       for (size_t i = 0; i < build_id->size && offs + 1 < bf_size; ++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
>

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:

Pre-existing issues:
- [Medium] Incomplete fix for file descriptor leaks to child processes (O_CLOEXEC missed for mkstemp).
--

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

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

This commit adds the O_CLOEXEC flag to open() calls in dso.c and symbol-elf.c.
This aims to prevent file descriptors from leaking to child processes spawned
during symbol resolution, which can exhaust the file descriptor limit.

> diff --git a/tools/perf/util/dso.c b/tools/perf/util/dso.c
> --- 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]
This isn't a bug introduced by this patch, but since the goal is to prevent
file descriptor leaks, doesn't mkstemp() also leave the file descriptor
without the O_CLOEXEC flag?

If the file is compressed, the returned decompressed file descriptor might
still leak to child processes. Could mkstemp(tmpbuf) be changed to
mkostemp(tmpbuf, O_CLOEXEC) to fully close the leak?

> diff --git a/tools/perf/util/symbol-elf.c b/tools/perf/util/symbol-elf.c
> --- 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);

[Severity: Medium]
This is a pre-existing issue, but updating only the open() call creates an
inconsistency between the temp and non-temp paths. Will temporary kcore file
descriptors still leak to child processes when temp is true?

Additionally, looking at read_gnu_debugdata() in this same file, there is
another pre-existing mkstemp() call used for temporary files. Would it make
sense to convert all of these mkstemp() calls to mkostemp(..., O_CLOEXEC) in
this series to completely prevent file descriptor exhaustion?

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

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

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] The bounds check on `al->cpu` fails to prevent 16-bit integer truncation when `env->nr_cpus_avail` exceeds 32767, breaking topology resolution and bypassing the patch's stated fix.
--

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

perf tools: Use perf_env__get_cpu_topology() in machine__resolve()

This commit fixes an out-of-bounds heap read in machine__resolve() when a
perf.data sample contains a large CPU index. It replaces the unsafe array
access with perf_env__get_cpu_topology(), which performs bounds and NULL
validation. A bounds check is also added to attempt to prevent 16-bit integer
truncation during the cast to struct perf_cpu.

> diff --git a/tools/perf/util/event.c b/tools/perf/util/event.c
> index 66f4843bb235d..ea75816d126a1 100644
> --- a/tools/perf/util/event.c
> +++ b/tools/perf/util/event.c
[ ... ]
> @@ -836,8 +837,18 @@ 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 al->cpu (s32) before casting to struct perf_cpu
> +		 * (int16_t): without this, e.g. 65536 truncates to 0 and silently
> +		 * returns CPU 0's topology.  Can go once perf_cpu.cpu is widened.
> +		 */
> +		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 });

[Severity: Medium]
Does this bounds check prevent truncation if env->nr_cpus_avail exceeds 32767?

If a perf.data file specifies a large nr_cpus_avail (for instance, 
process_nrcpus() bounds it to 1,048,576), could a malicious al->cpu of 65536 
still pass this check? If so, wouldn't it cast to int16_t as 0 and silently
return CPU 0's topology, bypassing the fix?

Additionally, does this break valid topology resolution for CPUs >= 32768? A
valid al->cpu of 32768 would truncate to -32768, causing 
perf_env__get_cpu_topology() to return NULL since it verifies cpu >= 0.

> +			if (topo)
> +				al->socket = topo->socket_id;
> +		}
>  	}
>

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

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

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] The bounds check in `machine__resolve` fails to prevent `s32 al->cpu` from truncating to `int16_t` for large CPU values, leading to silent topology resolution errors for systems with >= 32768 CPUs.
--

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

perf tools: Use perf_env__get_cpu_topology() in machine__resolve()

This commit modifies machine__resolve() to retrieve CPU socket IDs using
perf_env__get_cpu_topology() instead of direct array access. It adds a
bounds check against env->nr_cpus_avail to prevent out-of-bounds heap
reads before casting the CPU index to a struct perf_cpu.

> diff --git a/tools/perf/util/event.c b/tools/perf/util/event.c
> index 66f4843bb235d..ea75816d126a1 100644
> --- a/tools/perf/util/event.c
> +++ b/tools/perf/util/event.c
[ ... ]
> @@ -836,8 +837,18 @@ 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 al->cpu (s32) before casting to struct perf_cpu
> +		 * (int16_t): without this, e.g. 65536 truncates to 0 and silently
> +		 * returns CPU 0's topology.  Can go once perf_cpu.cpu is widened.
> +		 */
> +		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 });

[Severity: Medium]
Will this still truncate to int16_t if env->nr_cpus_avail is larger than 32767?

Since env->nr_cpus_avail can legally be up to 1,048,576, the check
al->cpu < env->nr_cpus_avail evaluates to true for CPU values like 65536.
When cast to struct perf_cpu, which wraps an int16_t, 65536 wraps to 0 and
silently fetches the topology for CPU 0 instead of the requested CPU.

For CPUs between 32768 and 65535, the cast results in a negative number,
causing perf_env__get_cpu_topology() to return NULL.

Prior to this patch, env->cpu[al->cpu] successfully handled large CPU indices.
Does this code introduce a functional regression on high-core-count systems
or when parsing perf.data files with a large nr_cpus_avail?

> +			if (topo)
> +				al->socket = topo->socket_id;
> +		}
>  	}

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

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:

Pre-existing issues:
- [High] Off-by-one stack buffer overflow when reading sysfs files without reserving space for the null terminator.
--

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

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

build_id__snprintf() and hwmon_pmu__read_events() accumulate formatted
output via snprintf(), which returns the would-have-been-written count
on truncation.

> diff --git a/tools/perf/util/hwmon_pmu.c b/tools/perf/util/hwmon_pmu.c
> index fb3ffa8d32ad2..dbf6a71af47f9 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';

[Severity: High]
This isn't a bug introduced by this patch, but could this lead to an
off-by-one stack buffer overflow?

Looking at hwmon_pmu__describe_items() in tools/perf/util/hwmon_pmu.c, we
read up to sizeof(buf) bytes:

    ssize_t read_len = read(fd, buf, sizeof(buf));

If the sysfs file contains exactly the capacity of the buffer (64 bytes) and
does not end with a newline, read_len will be 64.

When terminating the string here:

    buf[read_len] = '\0';

Does this write a null byte exactly one byte past the end of the buf array?
It appears similar patterns exist in hwmon_pmu__read_events() and
evsel__hwmon_pmu_read() as well, where reading the full buffer size
leads to an out-of-bounds null termination.

Should we reserve space for the null terminator by reading sizeof(buf) - 1
instead?

>  				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);
>  		}

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

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 6 potential issue(s) to consider:

Pre-existing issues:
- [High] Missing NULL check for the result of `btf__type_by_id()` in `synthesize_bpf_prog_name` leading to a potential NULL pointer dereference.
- [High] The `synthesize_bpf_prog_name` function dereferences `func_infos` unconditionally if a BTF object is present, even if the BPF program does not have function info.
- [High] Unsafe use of snprintf in length accumulation sequences leads to size_t underflow and stack buffer overflows in `perf_pmu__for_each_event`.
- [High] The `bpil_offs_to_addr` function translates offsets to addresses without bounds checking, allowing out-of-bounds reads from malicious perf.data files.
- [Medium] Leaked `info_node->metadata` on the error path where `perf_env__insert_bpf_prog_info` fails.
- [Medium] Leaked BTF and rodata objects in `bpf_metadata_create` when `bpf_metadata_alloc` fails.
--

commit 7fa567954fb3de5f92865c25d5e803bee0ef956a
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 snprintf. This patch switches
them to scnprintf() to prevent buffer size underflows and write overruns
when accumulating the output length.

> diff --git a/tools/perf/util/bpf-event.c b/tools/perf/util/bpf-event.c
> index a27945c279efb..2c09842469f1f 100644
> --- 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);
>  	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;
>  }

[Severity: High]
This isn't a bug introduced by this patch, but could the surrounding code in
synthesize_bpf_prog_name() segfault if a BPF program does not have func_info?

In synthesize_bpf_prog_name(), if the program lacks function info, func_infos
becomes NULL. However, bpf_metadata_create() can pass the .rodata MAP's BTF,
satisfying the if (btf) check:

tools/perf/util/bpf-event.c:synthesize_bpf_prog_name() {
    ...
    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);
    ...
}

Is it possible that finfo evaluates to an offset from NULL, leading to a
segfault when dereferencing finfo->type_id?

[Severity: High]
This is a pre-existing issue, but does btf__type_by_id() guarantee returning
a non-NULL pointer here?

If the provided type_id is out of bounds, t becomes NULL. Would the
immediate dereference of t->name_off then cause a segfault?

While reviewing these formatting string fixes, I noticed a few other
pre-existing regressions in the perf bpf and pmu utilities.

[Severity: High]
This isn't a bug introduced by this patch, but does the length accumulation
in perf_pmu__for_each_event() risk a stack buffer overflow?

tools/perf/util/pmu.c:perf_pmu__for_each_event() {
    ...
    buf_used += snprintf(buf + buf_used, sizeof(buf) - buf_used,
                         "%G%s", event->scale, event->unit) + 1;
    ...
}

If the snprintf output is truncated, buf_used exceeds the buffer size. The
subsequent call uses the remaining size, which would underflow to a large
positive value. Can this cause the next snprintf call to write out of
bounds?

[Severity: Medium]
This is a pre-existing issue, but does the error path in
perf_env__add_bpf_info() leak the metadata object?

tools/perf/util/bpf-event.c:perf_env__add_bpf_info() {
    ...
    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);
        free(info_linear);
        free(info_node);
        goto out;
    }
    ...
}

If perf_env__insert_bpf_prog_info() fails, info_linear and info_node are
freed, but there is no call to bpf_metadata_free(info_node->metadata).
Should the allocated metadata be freed before jumping to the out label?

[Severity: Medium]
This is a pre-existing issue, but does the failure path for
bpf_metadata_alloc() in bpf_metadata_create() leak the map resources?

tools/perf/util/bpf-event.c:bpf_metadata_create() {
    ...
    metadata = bpf_metadata_alloc(info->nr_prog_tags, map.num_vars);
    if (!metadata)
        continue;
    ...
}

If bpf_metadata_read_map_data() succeeds earlier in the loop, map.btf and
map.rodata are allocated. If bpf_metadata_alloc() then fails, the loop
continues without calling bpf_metadata_free_map_data(). Should these
resources be cleaned up before continuing the loop?

[Severity: High]
This is a pre-existing issue, but does bpil_offs_to_addr() translate offsets
to addresses without bounds checking?

tools/perf/util/bpf-utils.c: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);
    ...
}

The untrusted offset from info_linear->info is added to
ptr_to_u64(info_linear->data) to compute addr. If a crafted perf.data file
provides an offset exceeding data_len, could this allow arbitrary
out-of-bounds reads when the perf tool dereferences it?

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

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

[Ian Rogers]

On Wed, Jun 10, 2026 at 9:53 AM Arnaldo Carvalho de Melo
<acme@kernel.org> wrote:
>
> 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 to negative numbers (two's complement), bypassing
> 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>

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

The size motivation was for cases like having 1000 CPUs and a struct
perf_cpu_map with a CPU for each entry. In the data file we compress
this by tracking such cases as a range of CPUs. I wonder if the struct
perf_cpu_map abstraction should do something similar. It would save
memory, potentially be faster (iterating a range rather than loading
incrementally increasing values from an array) and then we wouldn't
care so much whether the struct perf_cpu was an int16_t or an int32_t.

Looking at lscpu, which does a similar job:
https://github.com/util-linux/util-linux/blob/master/sys-utils/lscpu.c
They use cpu_set_t but that seems to be a bitmap with fixed support
for around 1024 CPUs. So I prefer perf_cpu_map.

Thanks,
Ian

> ---
>  tools/lib/perf/TODO | 8 ++++++++
>  1 file changed, 8 insertions(+)
>
> diff --git a/tools/lib/perf/TODO b/tools/lib/perf/TODO
> index 486dd95dc57208a8..e179728697d8c7c0 100644
> --- a/tools/lib/perf/TODO
> +++ b/tools/lib/perf/TODO
> @@ -11,6 +11,14 @@ 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 negative
> +     numbers (two's complement), 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 10/11] perf tools: Use scnprintf() in build_id__snprintf() and hwmon read_events()

[Arnaldo Carvalho de Melo]

On Wed, Jun 10, 2026 at 05:17:26PM +0000, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> 
> Pre-existing issues:
> - [High] Off-by-one stack buffer overflow when reading sysfs files without reserving space for the null terminator.
> --
> 
> commit e6063aeb686d4d547e9272a51fc950e7a84f13a8
> Author: Arnaldo Carvalho de Melo <acme@redhat.com>
> 
> perf tools: Use scnprintf() in build_id__snprintf() and hwmon read_events()
> 
> build_id__snprintf() and hwmon_pmu__read_events() accumulate formatted
> output via snprintf(), which returns the would-have-been-written count
> on truncation.
> 
> > diff --git a/tools/perf/util/hwmon_pmu.c b/tools/perf/util/hwmon_pmu.c
> > index fb3ffa8d32ad2..dbf6a71af47f9 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';
> 
> [Severity: High]
> This isn't a bug introduced by this patch, but could this lead to an
> off-by-one stack buffer overflow?
> 
> Looking at hwmon_pmu__describe_items() in tools/perf/util/hwmon_pmu.c, we
> read up to sizeof(buf) bytes:
> 
>     ssize_t read_len = read(fd, buf, sizeof(buf));
> 
> If the sysfs file contains exactly the capacity of the buffer (64 bytes) and
> does not end with a newline, read_len will be 64.
> 
> When terminating the string here:
> 
>     buf[read_len] = '\0';

This is fixed in the following series, acting on your comments.

- Arnaldo
 
> Does this write a null byte exactly one byte past the end of the buf array?
> It appears similar patterns exist in hwmon_pmu__read_events() and
> evsel__hwmon_pmu_read() as well, where reading the full buffer size
> leads to an out-of-bounds null termination.
> 
> Should we reserve space for the null terminator by reading sizeof(buf) - 1
> instead?
> 
> >  				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);
> >  		}
> 
> -- 
> Sashiko AI review · https://sashiko.dev/#/patchset/20260610165207.2077258-1-acme@kernel.org?part=10

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

[Arnaldo Carvalho de Melo]

On Wed, Jun 10, 2026 at 05:04:00PM +0000, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [Low] The patch introduces a logic error that causes invalid negative priorities (<-1) to bypass the `--prio` filter.
> --
> 
> commit 3165982942697d89513b8b104d95d745a4412391
> Author: Arnaldo Carvalho de Melo <acme@redhat.com>
> 
> perf sched: Bounds-check prio before test_bit() in timehist
> 
> This commit adds a bounds check to timehist_skip_sample() to ensure that
> priority values from untrusted tracepoint data do not exceed MAX_PRIO before
> calling test_bit(). It also updates the sentinel check from != -1 to >= 0.
> 
> > diff --git a/tools/perf/builtin-sched.c b/tools/perf/builtin-sched.c
> > index 1ff01f03d2ad1..ded511d851880 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 */

Updated the comment to:

-               /* prio comes from untrusted tracepoint data — bounds-check before test_bit */
+               /* negative prio means no info; out-of-range prio can't match the filter */

To address the comment below.

- Arnaldo

> > +		if (prio >= 0 &&
> > +		    (prio >= MAX_PRIO || !test_bit(prio, sched->prio_bitmap))) {
> 
> [Severity: Low]
> Does this logic inadvertently allow invalid negative priorities (less than -1)
> to bypass the --prio filter?
> 
> By explicitly replacing the prio != -1 sentinel check with prio >= 0, the new
> logic evaluates to false for any priority less than -1. This causes the block
> to be bypassed, meaning rc remains false and the invalid negative priority is
> not skipped by the filter.
> 
> Should the check be restructured to ensure that invalid negative priorities
> from crafted or corrupted perf.data files are also properly skipped?
> 
> >  			rc = true;
> >  			sched->skipped_samples++;
> >  		}
> 
> -- 
> Sashiko AI review · https://sashiko.dev/#/patchset/20260610165207.2077258-1-acme@kernel.org?part=5