[PATCH 6/9] perf c2c: Free format list entries when c2c_hists__init() fails

[PATCH 6/9] perf c2c: Free format list entries when c2c_hists__init() fails

[Arnaldo Carvalho de Melo]

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

When c2c_hists__init() fails partway through hpp_list__parse(),
dynamically allocated format structures that were already added to
hists->list are leaked because he__get_c2c_hists() frees the hists
container without first unregistering the format entries.

Call perf_hpp__reset_output_field() before freeing the hists container
on the error path, matching what c2c_he_free() already does on the
normal destruction path.

Fixes: 17a7c5946d79a12c ("perf c2c report: Decode c2c_stats for hist entries")
Reported-by: sashiko-bot <sashiko-bot@kernel.org>
Cc: Jiri Olsa <jolsa@kernel.org>
Assisted-by: Claude <noreply@anthropic.com>
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/builtin-c2c.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/tools/perf/builtin-c2c.c b/tools/perf/builtin-c2c.c
index 07c7e8fb315e6cf3..eabb922ef295ef86 100644
--- a/tools/perf/builtin-c2c.c
+++ b/tools/perf/builtin-c2c.c
@@ -226,6 +226,7 @@ he__get_c2c_hists(struct hist_entry *he,
 
 	ret = c2c_hists__init(hists, sort, nr_header_lines, env);
 	if (ret) {
+		perf_hpp__reset_output_field(&hists->list);
 		c2c_he->hists = NULL;
 		free(hists);
 		return NULL;
-- 
2.54.0

Re: [PATCH 6/9] perf c2c: Free format list entries when c2c_hists__init() fails

[sashiko-bot]

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

Pre-existing issues:
- [High] `perf_hpp__setup_output_field()` mistakenly appends format elements to the global `perf_hpp_list.fields` instead of the local `list->fields`, leading to an unbounded accumulation of `c2c_fmt` structures.
- [Medium] `hpp_list__parse()` and the `PARSE_LIST` macro silently overwrite and drop error return codes during format parsing.
--

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

perf c2c: Free format list entries when c2c_hists__init() fails

This commit fixes a memory leak in the error path of he__get_c2c_hists().
It calls perf_hpp__reset_output_field() before freeing the hists container
when c2c_hists__init() fails, ensuring dynamically allocated format entries
are properly unregistered.

> diff --git a/tools/perf/builtin-c2c.c b/tools/perf/builtin-c2c.c
> --- a/tools/perf/builtin-c2c.c
> +++ b/tools/perf/builtin-c2c.c
> @@ -226,6 +226,7 @@ he__get_c2c_hists(struct hist_entry *he,
>  
>  	ret = c2c_hists__init(hists, sort, nr_header_lines, env);

[Severity: Medium]
This isn't a bug introduced by this patch, but does hpp_list__parse()
(called within c2c_hists__init()) silently drop error return codes?

Looking at the PARSE_LIST macro in tools/perf/builtin-c2c.c, it initializes
ret = 0 internally on every invocation:

#define PARSE_LIST(_list, _fn) \
    do { \
        char *tmp, *tok; \
        ret = 0;

In hpp_list__parse(), sequential macro invocations mean the first error
code can be discarded:

    PARSE_LIST(output, c2c_hists__init_output);
    PARSE_LIST(sort,   c2c_hists__init_sort);

If the output parsing encounters an error, it breaks out and correctly sets
ret, but the subsequent sort parsing invocation immediately resets ret = 0.
Additionally, if strdup fails, it breaks but leaves ret = 0.

Could this cause allocation or parsing failures during c2c_hists__init()
to be silently ignored, returning success despite missing formatting?

[Severity: High]
This is a pre-existing issue, but is there an unbounded accumulation of
c2c_fmt structures occurring when he__get_c2c_hists() initializes
per-cacheline history lists?

When c2c_hists__init() sets up the local list, it eventually calls
perf_hpp__setup_output_field(). This function takes a local list parameter
but uses an inline wrapper that hardcodes the global list:

tools/perf/util/hist.h:perf_hpp__column_register() {
    perf_hpp_list__column_register(&perf_hpp_list, format);
}

Because perf_hpp__setup_output_field() in tools/perf/ui/hist.c uses this
global wrapper (perf_hpp__column_register(fmt)) instead of the local
variant, the format elements intended for each cacheline's local history
get appended to the global perf_hpp_list.fields.

Since perf c2c can process millions of cachelines, could this cause the
global list to endlessly accumulate dynamically allocated format elements?

>  	if (ret) {
> +		perf_hpp__reset_output_field(&hists->list);
>  		c2c_he->hists = NULL;
>  		free(hists);
>  		return NULL;

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

[PATCH 6/9] perf c2c: Free format list entries when c2c_hists__init() fails

[Arnaldo Carvalho de Melo]

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

When c2c_hists__init() fails partway through hpp_list__parse(),
dynamically allocated format structures that were already added to
hists->list are leaked because he__get_c2c_hists() frees the hists
container without first unregistering the format entries.

Call perf_hpp__reset_output_field() before freeing the hists container
on the error path, matching what c2c_he_free() already does on the
normal destruction path.

Fixes: 17a7c5946d79a12c ("perf c2c report: Decode c2c_stats for hist entries")
Reported-by: sashiko-bot <sashiko-bot@kernel.org>
Cc: Jiri Olsa <jolsa@kernel.org>
Assisted-by: Claude <noreply@anthropic.com>
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/builtin-c2c.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/tools/perf/builtin-c2c.c b/tools/perf/builtin-c2c.c
index 07c7e8fb315e6cf3..eabb922ef295ef86 100644
--- a/tools/perf/builtin-c2c.c
+++ b/tools/perf/builtin-c2c.c
@@ -226,6 +226,7 @@ he__get_c2c_hists(struct hist_entry *he,
 
 	ret = c2c_hists__init(hists, sort, nr_header_lines, env);
 	if (ret) {
+		perf_hpp__reset_output_field(&hists->list);
 		c2c_he->hists = NULL;
 		free(hists);
 		return NULL;
-- 
2.54.0

[PATCH 6/9] perf c2c: Free format list entries when c2c_hists__init() fails

[Arnaldo Carvalho de Melo]

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

When c2c_hists__init() fails partway through hpp_list__parse(),
dynamically allocated format structures that were already added to
hists->list are leaked because he__get_c2c_hists() frees the hists
container without first unregistering the format entries.

Call perf_hpp__reset_output_field() before freeing the hists container
on the error path, matching what c2c_he_free() already does on the
normal destruction path.

Fixes: 17a7c5946d79a12c ("perf c2c report: Decode c2c_stats for hist entries")
Reported-by: sashiko-bot <sashiko-bot@kernel.org>
Cc: Jiri Olsa <jolsa@kernel.org>
Assisted-by: Claude <noreply@anthropic.com>
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/builtin-c2c.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/tools/perf/builtin-c2c.c b/tools/perf/builtin-c2c.c
index 07c7e8fb315e6cf3..eabb922ef295ef86 100644
--- a/tools/perf/builtin-c2c.c
+++ b/tools/perf/builtin-c2c.c
@@ -226,6 +226,7 @@ he__get_c2c_hists(struct hist_entry *he,
 
 	ret = c2c_hists__init(hists, sort, nr_header_lines, env);
 	if (ret) {
+		perf_hpp__reset_output_field(&hists->list);
 		c2c_he->hists = NULL;
 		free(hists);
 		return NULL;
-- 
2.54.0

Re: [PATCH 6/9] perf c2c: Free format list entries when c2c_hists__init() fails

[sashiko-bot]

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

Pre-existing issues:
- [High] perf_hpp__setup_output_field() unconditionally appends missing sort keys to the global perf_hpp_list instead of the provided local list.
- [Medium] Error return from PARSE_LIST(output, ...) is silently overwritten by PARSE_LIST(sort, ...) in hpp_list__parse().
- [Medium] Out-of-memory failures during string duplication are silently masked as empty inputs in hpp_list__parse() and kvm_hpp_list__parse().
--

commit 82a61a9eafaa1d6dd28dc01f46106f2ec1acc81e
Author: Arnaldo Carvalho de Melo <acme@redhat.com>
perf c2c: Free format list entries when c2c_hists__init() fails

This commit fixes a memory leak in the error path of he__get_c2c_hists(). It
ensures that dynamically allocated format structures are correctly cleaned up by
calling perf_hpp__reset_output_field() before freeing the hists container.

> diff --git a/tools/perf/builtin-c2c.c b/tools/perf/builtin-c2c.c
> index 07c7e8fb315e6..eabb922ef295e 100644
> --- a/tools/perf/builtin-c2c.c
> +++ b/tools/perf/builtin-c2c.c
> @@ -226,6 +226,7 @@ he__get_c2c_hists(struct hist_entry *he,
>  
>  	ret = c2c_hists__init(hists, sort, nr_header_lines, env);

[Severity: Medium]
This is a pre-existing issue, but I noticed something when looking at
c2c_hists__init() -> hpp_list__parse().

When an invalid --fields key is provided, the first PARSE_LIST(output, ...)
macro correctly sets ret to an error code and breaks its internal loop.

However, the subsequent PARSE_LIST(sort, ...) macro invocation immediately
resets ret = 0:

tools/perf/builtin-c2c.c:hpp_list__parse() {
    ...
    PARSE_LIST(output, c2c_hists__init_output);
    PARSE_LIST(sort,   c2c_hists__init_sort);
    ...
}

Does this silently swallow the error from the output parsing, returning
success and allowing execution to proceed with an invalid configuration?

[Severity: Medium]
This isn't a bug introduced by this patch, but while reviewing the parsing
logic in hpp_list__parse() and kvm_hpp_list__parse(), I noticed how strdup()
failures are handled.

If strdup() fails due to memory exhaustion, the resulting variables become NULL:

tools/perf/builtin-c2c.c:hpp_list__parse() {
    char *output = output_ ? strdup(output_) : NULL;
    char *sort   = sort_   ? strdup(sort_) : NULL;
    ...
    PARSE_LIST(output, c2c_hists__init_output);
}

Since PARSE_LIST short-circuits a NULL list to success (0), does this mask
out-of-memory failures as empty inputs, skipping configuration entirely
instead of properly aborting?

[Severity: High]
This is a pre-existing issue, but looking deeper into the initialization
path, perf_hpp__setup_output_field() seems to append missing sort keys
to the wrong list.

The function iterates over the local list->sorts, but then calls
perf_hpp__column_register(fmt):

tools/perf/ui/hist.c:perf_hpp__setup_output_field() {
    perf_hpp_list__for_each_sort_list(list, fmt) {
        ...
        perf_hpp__column_register(fmt);
next:
        continue;
    }
}

This inline wrapper explicitly targets the global perf_hpp_list:

tools/perf/util/hist.h:perf_hpp__column_register() {
    perf_hpp_list__column_register(&perf_hpp_list, format);
}

Could this pollute the global list with pointers to dynamically allocated
format structures owned by the local c2c_hists or kvm_hists?

If so, does this lead to use-after-free vulnerabilities for concurrent
global list accessors when the local list is destroyed?

>  	if (ret) {
> +		perf_hpp__reset_output_field(&hists->list);
>  		c2c_he->hists = NULL;
>  		free(hists);
>  		return NULL;

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

[PATCH 6/9] perf c2c: Free format list entries when c2c_hists__init() fails

[Arnaldo Carvalho de Melo]

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

When c2c_hists__init() fails partway through hpp_list__parse(),
dynamically allocated format structures that were already added to
hists->list are leaked because he__get_c2c_hists() frees the hists
container without first unregistering the format entries.

Call perf_hpp__reset_output_field() before freeing the hists container
on the error path, matching what c2c_he_free() already does on the
normal destruction path.

Fixes: 17a7c5946d79a12c ("perf c2c report: Decode c2c_stats for hist entries")
Reported-by: sashiko-bot <sashiko-bot@kernel.org>
Cc: Jiri Olsa <jolsa@kernel.org>
Assisted-by: Claude <noreply@anthropic.com>
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/builtin-c2c.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/tools/perf/builtin-c2c.c b/tools/perf/builtin-c2c.c
index 07c7e8fb315e6cf3..eabb922ef295ef86 100644
--- a/tools/perf/builtin-c2c.c
+++ b/tools/perf/builtin-c2c.c
@@ -226,6 +226,7 @@ he__get_c2c_hists(struct hist_entry *he,
 
 	ret = c2c_hists__init(hists, sort, nr_header_lines, env);
 	if (ret) {
+		perf_hpp__reset_output_field(&hists->list);
 		c2c_he->hists = NULL;
 		free(hists);
 		return NULL;
-- 
2.54.0

[PATCHES v5 0/9] perf tools: Fix pre-existing bugs in machine, cs-etm, c2c, bpf, and dso

[Arnaldo Carvalho de Melo]

Hi,

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

The fixes are grouped by subsystem:

machine__init() error propagation (patches 1-2):
  machine__init() always returns 0 on allocation failure because the
  error code is never propagated through the return statement.  Callers
  (including machines__init() and __machine__new_host()) proceed with a
  partially initialized machine struct.  The error cleanup also uses
  zfree() on refcounted kmaps instead of maps__zput().  Additionally,
  machines__findnew() and machines__create_guest_kernel_maps() use
  sprintf() with unsanitized guestmount paths that can overflow
  PATH_MAX stack buffers.

CoreSight ETM metadata validation (patches 3-5):
  cs_etm__process_auxtrace_info_full() reads num_cpu from untrusted
  perf.data and uses it directly in a multiplication that can overflow
  to zero on 32-bit, producing a zero-sized allocation followed by OOB
  writes.  The minimum size check in cs_etm__process_auxtrace_info()
  doesn't cover the global header fields actually accessed.
  cs_etm__get_queue() indexes queue_array[] without bounds checking
  the CPU value from untrusted trace payload, and several queue
  iteration loops dereference .priv without NULL checks after array
  growth zero-initializes new entries.

c2c hist entry leaks (patches 6-7):
  When c2c_hists__init() fails, dynamically allocated format structures
  are leaked because the error path frees the container without
  unregistering them.  During resort merges, c2c_he_free() only walks
  the output-sorted tree (empty before resort), leaking all inner
  hist_entry objects from entries_in_array[] and entries_collapsed.

BPF prog info pointer validation (patch 8):
  Several functions cast bpf_prog_info u64 fields to pointers without
  checking whether bpil_offs_to_addr() actually converted the file
  offsets.  A crafted perf.data with PERF_BPIL_* bits unset but non-zero
  counts causes raw file offsets to be dereferenced as pointers.

DSO decompression errno (patch 9):
  dso__get_filename() sets errno to a negative custom DSO_LOAD_ERRNO
  value on decompression failure.  __open_dso() computes fd = -errno,
  producing a large positive value that looks like a valid fd, causing
  close_data_fd() to close an unrelated file descriptor.

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

Changes in v5 (patch 1 only):
  - Check machine__init() return value in test__kallsyms_split() and
    test__vmlinux_matches_kallsyms() — two test callers missed in v1
    (sashiko-bot).

Changes in v4 (patch 2 only):
  - Remove incorrect get_kernel_version() reference from commit
    message — that function already uses snprintf() in the baseline
    (sashiko-bot).

Changes in v3 (patch 1 only):
  - Move perf_env__init() before machines__init() in
    __perf_session__new() so the goto out_delete error path doesn't
    call perf_env__exit() on uninitialized mutexes/rwlocks
    (sashiko-bot).

Changes in v2 (patch 1 only):
  - Move dsos__init()/threads__init() before maps__new() so that
    machine__exit() is safe to call when machine__init() fails at the
    first allocation (sashiko-bot).
  - Propagate machines__init() error in aslr_tool__init(), which was
    added by the ASLR patches after v1 was written (sashiko-bot).

Arnaldo Carvalho de Melo (9):
  perf machine: Propagate machine__init() error to callers
  perf machine: Use snprintf() for guestmount path construction
  perf cs-etm: Validate num_cpu before metadata allocation
  perf cs-etm: Require full global header in auxtrace_info size check
  perf cs-etm: Bounds-check CPU in cs_etm__get_queue()
  perf c2c: Free format list entries when c2c_hists__init() fails
  perf c2c: Fix hist entry and format list leaks in c2c_he_free()
  perf bpf: Validate array presence before casting BPF prog info pointers
  perf dso: Set standard errno on decompression failure

 tools/perf/builtin-c2c.c             |  3 ++-
 tools/perf/tests/hists_cumulate.c    |  3 ++-
 tools/perf/tests/hists_filter.c      |  3 ++-
 tools/perf/tests/hists_link.c        |  3 ++-
 tools/perf/tests/hists_output.c      |  3 ++-
 tools/perf/tests/kallsyms-split.c    |  5 ++++-
 tools/perf/tests/thread-maps-share.c |  2 +-
 tools/perf/tests/vmlinux-kallsyms.c  |  6 ++++--
 tools/perf/util/aslr.c               | 12 +++++++++---
 tools/perf/util/bpf-event.c          | 20 ++++++++++++++++---
 tools/perf/util/bpf-event.h          |  4 ++--
 tools/perf/util/cs-etm-base.c        |  4 +++-
 tools/perf/util/cs-etm.c             | 37 ++++++++++++++++++++++++++++++++++--
 tools/perf/util/dso.c                | 18 +++++++++++++++++-
 tools/perf/util/header.c             |  3 +--
 tools/perf/util/hist.c               |  2 +-
 tools/perf/util/hist.h               |  1 +
 tools/perf/util/machine.c            | 32 +++++++++++++++++--------------
 tools/perf/util/machine.h            |  2 +-
 tools/perf/util/session.c            |  7 ++++---
 20 files changed, 128 insertions(+), 42 deletions(-)

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

Thanks,

- Arnaldo

[PATCH 1/9] perf machine: Propagate machine__init() error to callers

[Arnaldo Carvalho de Melo]

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

machine__init() always returns 0 even when memory allocation fails,
because commit 81f981d7ec43ed93 ("perf machine: Free root_dir in
machine__init() error path") introduced 'int err = -ENOMEM' and an
error cleanup path but left the final 'return 0' instead of
'return err'.

Fix by returning err, check the return value in __machine__new_host()
which was ignoring it, and change machines__init() from void to int so
it too can propagate the error to perf_session__new(), aslr_tool__init()
and test callers.

The error cleanup also used zfree(&machine->kmaps), but kmaps is a
refcounted maps structure — use maps__zput() to properly drop the
reference, matching machine__exit().

Move dsos__init() and threads__init() before the first fallible
allocation (maps__new) so that machine__exit() is safe to call on
any machine struct that machine__init() touched, even on early failure.

Fixes: 81f981d7ec43ed93 ("perf machine: Free root_dir in machine__init() error path")
Reported-by: sashiko-bot <sashiko-bot@kernel.org>
Cc: Ian Rogers <irogers@google.com>
Cc: Jiri Olsa <jolsa@kernel.org>
Assisted-by: Claude <noreply@anthropic.com>
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/tests/hists_cumulate.c    |  3 ++-
 tools/perf/tests/hists_filter.c      |  3 ++-
 tools/perf/tests/hists_link.c        |  3 ++-
 tools/perf/tests/hists_output.c      |  3 ++-
 tools/perf/tests/kallsyms-split.c    |  5 ++++-
 tools/perf/tests/thread-maps-share.c |  2 +-
 tools/perf/tests/vmlinux-kallsyms.c  |  6 ++++--
 tools/perf/util/aslr.c               | 12 +++++++++---
 tools/perf/util/machine.c            | 24 ++++++++++++++----------
 tools/perf/util/machine.h            |  2 +-
 tools/perf/util/session.c            |  7 ++++---
 11 files changed, 45 insertions(+), 25 deletions(-)

diff --git a/tools/perf/tests/hists_cumulate.c b/tools/perf/tests/hists_cumulate.c
index 267cbc24691acd77..9f4d4e281e11edaa 100644
--- a/tools/perf/tests/hists_cumulate.c
+++ b/tools/perf/tests/hists_cumulate.c
@@ -723,7 +723,8 @@ static int test__hists_cumulate(struct test_suite *test __maybe_unused, int subt
 		goto out;
 	err = TEST_FAIL;
 
-	machines__init(&machines);
+	if (machines__init(&machines))
+		goto out;
 
 	/* setup threads/dso/map/symbols also */
 	machine = setup_fake_machine(&machines);
diff --git a/tools/perf/tests/hists_filter.c b/tools/perf/tests/hists_filter.c
index 002e3a4c1ca59b9d..fa47e995de82d427 100644
--- a/tools/perf/tests/hists_filter.c
+++ b/tools/perf/tests/hists_filter.c
@@ -131,7 +131,8 @@ static int test__hists_filter(struct test_suite *test __maybe_unused, int subtes
 		goto out;
 	err = TEST_FAIL;
 
-	machines__init(&machines);
+	if (machines__init(&machines))
+		goto out;
 
 	/* setup threads/dso/map/symbols also */
 	machine = setup_fake_machine(&machines);
diff --git a/tools/perf/tests/hists_link.c b/tools/perf/tests/hists_link.c
index 996f5f0b3bd17fe5..05078dac93c4ec6b 100644
--- a/tools/perf/tests/hists_link.c
+++ b/tools/perf/tests/hists_link.c
@@ -303,7 +303,8 @@ static int test__hists_link(struct test_suite *test __maybe_unused, int subtest
 		goto out;
 
 	err = TEST_FAIL;
-	machines__init(&machines);
+	if (machines__init(&machines))
+		goto out;
 
 	/* setup threads/dso/map/symbols also */
 	machine = setup_fake_machine(&machines);
diff --git a/tools/perf/tests/hists_output.c b/tools/perf/tests/hists_output.c
index fa683fd7b1e5ebb2..1ec51c15b1046eb4 100644
--- a/tools/perf/tests/hists_output.c
+++ b/tools/perf/tests/hists_output.c
@@ -610,7 +610,8 @@ static int test__hists_output(struct test_suite *test __maybe_unused, int subtes
 		goto out;
 	err = TEST_FAIL;
 
-	machines__init(&machines);
+	if (machines__init(&machines))
+		goto out;
 
 	/* setup threads/dso/map/symbols also */
 	machine = setup_fake_machine(&machines);
diff --git a/tools/perf/tests/kallsyms-split.c b/tools/perf/tests/kallsyms-split.c
index 117ed3b70f630a97..6de04414a356ca9b 100644
--- a/tools/perf/tests/kallsyms-split.c
+++ b/tools/perf/tests/kallsyms-split.c
@@ -113,7 +113,10 @@ static int test__kallsyms_split(struct test_suite *test __maybe_unused,
 	signal(SIGTERM, remove_proc_dir);
 
 	pr_debug("create kernel maps from the fake root directory\n");
-	machine__init(&m, root_dir, HOST_KERNEL_ID);
+	if (machine__init(&m, root_dir, HOST_KERNEL_ID)) {
+		pr_debug("FAIL: failed to init machine\n");
+		goto out;
+	}
 	if (machine__create_kernel_maps(&m) < 0) {
 		pr_debug("FAIL: failed to create kernel maps\n");
 		goto out;
diff --git a/tools/perf/tests/thread-maps-share.c b/tools/perf/tests/thread-maps-share.c
index e9ecd30a5c058076..0431bff31b3a18c3 100644
--- a/tools/perf/tests/thread-maps-share.c
+++ b/tools/perf/tests/thread-maps-share.c
@@ -27,7 +27,7 @@ static int test__thread_maps_share(struct test_suite *test __maybe_unused, int s
 	 * other  group (pid: 4, tids: 4, 5)
 	*/
 
-	machines__init(&machines);
+	TEST_ASSERT_VAL("failed to init machines", machines__init(&machines) == 0);
 	machine = &machines.host;
 
 	/* create process with 4 threads */
diff --git a/tools/perf/tests/vmlinux-kallsyms.c b/tools/perf/tests/vmlinux-kallsyms.c
index 7409abe4aa3692ea..e88e7b2a82c5c6a8 100644
--- a/tools/perf/tests/vmlinux-kallsyms.c
+++ b/tools/perf/tests/vmlinux-kallsyms.c
@@ -203,8 +203,10 @@ static int test__vmlinux_matches_kallsyms(struct test_suite *test __maybe_unused
 	 * Init the machines that will hold kernel, modules obtained from
 	 * both vmlinux + .ko files and from /proc/kallsyms split by modules.
 	 */
-	machine__init(&args.kallsyms, "", HOST_KERNEL_ID);
-	machine__init(&vmlinux, "", HOST_KERNEL_ID);
+	if (machine__init(&args.kallsyms, "", HOST_KERNEL_ID))
+		goto out;
+	if (machine__init(&vmlinux, "", HOST_KERNEL_ID))
+		goto out;
 
 	maps = machine__kernel_maps(&vmlinux);
 
diff --git a/tools/perf/util/aslr.c b/tools/perf/util/aslr.c
index a946fff2ac4dd4b4..6a7542e7db827d1b 100644
--- a/tools/perf/util/aslr.c
+++ b/tools/perf/util/aslr.c
@@ -1237,12 +1237,13 @@ void aslr_tool__strip_attr_event(union perf_event *event, struct evlist *evlist)
 	}
 }
 
-static void aslr_tool__init(struct aslr_tool *aslr, struct perf_tool *delegate)
+static int aslr_tool__init(struct aslr_tool *aslr, struct perf_tool *delegate)
 {
 	delegate_tool__init(&aslr->tool, delegate);
 	aslr->tool.tool.ordered_events = true;
 
-	machines__init(&aslr->machines);
+	if (machines__init(&aslr->machines))
+		return -ENOMEM;
 
 	hashmap__init(&aslr->remap_addresses,
 		      remap_addresses__hash, remap_addresses__equal,
@@ -1276,6 +1277,8 @@ static void aslr_tool__init(struct aslr_tool *aslr, struct perf_tool *delegate)
 	aslr->tool.tool.auxtrace = aslr_tool__process_auxtrace;
 	aslr->tool.tool.auxtrace_info = aslr_tool__process_auxtrace_info;
 	aslr->tool.tool.auxtrace_error = aslr_tool__process_auxtrace_error;
+
+	return 0;
 }
 
 struct perf_tool *aslr_tool__new(struct perf_tool *delegate)
@@ -1285,7 +1288,10 @@ struct perf_tool *aslr_tool__new(struct perf_tool *delegate)
 	if (!aslr)
 		return NULL;
 
-	aslr_tool__init(aslr, delegate);
+	if (aslr_tool__init(aslr, delegate)) {
+		free(aslr);
+		return NULL;
+	}
 	return &aslr->tool.tool;
 }
 
diff --git a/tools/perf/util/machine.c b/tools/perf/util/machine.c
index 31715366e29ff704..9329d319bd033699 100644
--- a/tools/perf/util/machine.c
+++ b/tools/perf/util/machine.c
@@ -79,15 +79,14 @@ int machine__init(struct machine *machine, const char *root_dir, pid_t pid)
 	int err = -ENOMEM;
 
 	memset(machine, 0, sizeof(*machine));
-	machine->kmaps = maps__new(machine);
-	if (machine->kmaps == NULL)
-		return -ENOMEM;
-
 	RB_CLEAR_NODE(&machine->rb_node);
 	dsos__init(&machine->dsos);
-
 	threads__init(&machine->threads);
 
+	machine->kmaps = maps__new(machine);
+	if (machine->kmaps == NULL)
+		goto out;
+
 	machine->vdso_info = NULL;
 	machine->env = NULL;
 
@@ -124,11 +123,11 @@ int machine__init(struct machine *machine, const char *root_dir, pid_t pid)
 
 out:
 	if (err) {
-		zfree(&machine->kmaps);
+		maps__zput(machine->kmaps);
 		zfree(&machine->root_dir);
 		zfree(&machine->mmap_name);
 	}
-	return 0;
+	return err;
 }
 
 static struct machine *__machine__new_host(struct perf_env *host_env, bool kernel_maps)
@@ -138,7 +137,10 @@ static struct machine *__machine__new_host(struct perf_env *host_env, bool kerne
 	if (!machine)
 		return NULL;
 
-	machine__init(machine, "", HOST_KERNEL_ID);
+	if (machine__init(machine, "", HOST_KERNEL_ID) != 0) {
+		free(machine);
+		return NULL;
+	}
 
 	if (kernel_maps && machine__create_kernel_maps(machine) < 0) {
 		free(machine);
@@ -231,10 +233,12 @@ void machine__delete(struct machine *machine)
 	}
 }
 
-void machines__init(struct machines *machines)
+int machines__init(struct machines *machines)
 {
-	machine__init(&machines->host, "", HOST_KERNEL_ID);
+	int err = machine__init(&machines->host, "", HOST_KERNEL_ID);
+
 	machines->guests = RB_ROOT_CACHED;
+	return err;
 }
 
 void machines__exit(struct machines *machines)
diff --git a/tools/perf/util/machine.h b/tools/perf/util/machine.h
index aaddfb70ea665452..26f9827062f5eb5b 100644
--- a/tools/perf/util/machine.h
+++ b/tools/perf/util/machine.h
@@ -152,7 +152,7 @@ struct machines {
 	struct rb_root_cached guests;
 };
 
-void machines__init(struct machines *machines);
+int machines__init(struct machines *machines);
 void machines__exit(struct machines *machines);
 
 void machines__process_guests(struct machines *machines,
diff --git a/tools/perf/util/session.c b/tools/perf/util/session.c
index 1a9a008ddda35120..f391a822480db001 100644
--- a/tools/perf/util/session.c
+++ b/tools/perf/util/session.c
@@ -160,11 +160,12 @@ struct perf_session *__perf_session__new(struct perf_data *data,
 	session->decomp_data.zstd_decomp = &session->zstd_data;
 	session->active_decomp = &session->decomp_data;
 	INIT_LIST_HEAD(&session->auxtrace_index);
-	machines__init(&session->machines);
+	perf_env__init(&session->header.env);
+	if (machines__init(&session->machines))
+		goto out_delete;
+
 	ordered_events__init(&session->ordered_events,
 			     ordered_events__deliver_event, NULL);
-
-	perf_env__init(&session->header.env);
 	if (data) {
 		ret = perf_data__open(data);
 		if (ret < 0)
-- 
2.54.0

[PATCH 2/9] perf machine: Use snprintf() for guestmount path construction

[Arnaldo Carvalho de Melo]

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

machines__findnew() and machines__create_guest_kernel_maps() use
sprintf() to build paths by prepending symbol_conf.guestmount.
Both write into PATH_MAX stack buffers, but guestmount comes from
user configuration and is not length-checked.  A guestmount path
at or near PATH_MAX causes a stack buffer overflow.

Switch to snprintf() with sizeof() to prevent overflow.  The
subsequent access()/fopen() calls will fail on a truncated path.

Fixes: a1645ce12adb6c9c ("perf: 'perf kvm' tool for monitoring guest performance from host")
Reported-by: sashiko-bot <sashiko-bot@kernel.org>
Cc: Zhang, Yanmin <yanmin_zhang@linux.intel.com>
Assisted-by: Claude <noreply@anthropic.com>
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/util/machine.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/tools/perf/util/machine.c b/tools/perf/util/machine.c
index 9329d319bd033699..0d2ebf6a84bcf880 100644
--- a/tools/perf/util/machine.c
+++ b/tools/perf/util/machine.c
@@ -333,7 +333,7 @@ struct machine *machines__findnew(struct machines *machines, pid_t pid)
 	if ((pid != HOST_KERNEL_ID) &&
 	    (pid != DEFAULT_GUEST_KERNEL_ID) &&
 	    (symbol_conf.guestmount)) {
-		sprintf(path, "%s/%d", symbol_conf.guestmount, pid);
+		snprintf(path, sizeof(path), "%s/%d", symbol_conf.guestmount, pid);
 		if (access(path, R_OK)) {
 			static struct strlist *seen;
 
@@ -1260,9 +1260,9 @@ int machines__create_guest_kernel_maps(struct machines *machines)
 					 namelist[i]->d_name);
 				continue;
 			}
-			sprintf(path, "%s/%s/proc/kallsyms",
-				symbol_conf.guestmount,
-				namelist[i]->d_name);
+			snprintf(path, sizeof(path), "%s/%s/proc/kallsyms",
+				 symbol_conf.guestmount,
+				 namelist[i]->d_name);
 			ret = access(path, R_OK);
 			if (ret) {
 				pr_debug("Can't access file %s\n", path);
-- 
2.54.0

[PATCH 3/9] perf cs-etm: Validate num_cpu before metadata allocation

[Arnaldo Carvalho de Melo]

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

cs_etm__process_auxtrace_info_full() reads num_cpu from untrusted
perf.data and uses it to allocate the metadata pointer array:

  metadata = zalloc(sizeof(*metadata) * num_cpu);

On 32-bit, sizeof(*metadata) is 4, so num_cpu = 0x40000000 overflows
the multiplication to 0, causing zalloc(0) to return a valid zero-sized
allocation followed by out-of-bounds writes in the population loop.

Fix by computing priv_size early and using it to bound num_cpu: each
CPU needs at least one u64 metadata entry, so num_cpu cannot exceed
the total number of u64 entries in the event's private data area.

Fixes: cd8bfd8c973eaff8 ("perf tools: Add processing of coresight metadata")
Reported-by: sashiko-bot <sashiko-bot@kernel.org>
Cc: Adrian Hunter <adrian.hunter@intel.com>
Cc: James Clark <james.clark@arm.com>
Cc: Leo Yan <leo.yan@linaro.org>
Cc: Tor Jeremiassen <tor@ti.com>
Assisted-by: Claude <noreply@anthropic.com>
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/util/cs-etm.c | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/tools/perf/util/cs-etm.c b/tools/perf/util/cs-etm.c
index 0927b0b9c06b1504..d121c8f22028d5ba 100644
--- a/tools/perf/util/cs-etm.c
+++ b/tools/perf/util/cs-etm.c
@@ -3431,6 +3431,18 @@ int cs_etm__process_auxtrace_info_full(union perf_event *event,
 	/* First the global part */
 	ptr = (u64 *) auxtrace_info->priv;
 	num_cpu = ptr[CS_PMU_TYPE_CPUS] & 0xffffffff;
+
+	/*
+	 * Bound num_cpu by the event size: the global header consumes
+	 * CS_ETM_HEADER_SIZE bytes, and each CPU needs at least one u64
+	 * metadata entry after that.
+	 */
+	priv_size = total_size - event_header_size - INFO_HEADER_SIZE -
+		    CS_ETM_HEADER_SIZE;
+	if (num_cpu <= 0 || priv_size <= 0 ||
+	    num_cpu > priv_size / (int)sizeof(u64))
+		return -EINVAL;
+
 	metadata = zalloc(sizeof(*metadata) * num_cpu);
 	if (!metadata)
 		return -ENOMEM;
-- 
2.54.0

[PATCH 4/9] perf cs-etm: Require full global header in auxtrace_info size check

[Arnaldo Carvalho de Melo]

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

cs_etm__process_auxtrace_info() checks that header.size covers
event_header_size + INFO_HEADER_SIZE (16 bytes total), but then
accesses ptr[CS_PMU_TYPE_CPUS] at offset 24 from the start of the
event.  A crafted 16-byte auxtrace_info event passes the size check
but reads out-of-bounds.

Include CS_ETM_HEADER_SIZE in the minimum size check so that the
global header entries (version, pmu_type_cpus, snapshot) are
guaranteed to fit within the event.

Fixes: 55c1de9973d66516 ("perf cs-etm: Print auxtrace info even if OpenCSD isn't linked")
Reported-by: sashiko-bot <sashiko-bot@kernel.org>
Cc: Adrian Hunter <adrian.hunter@intel.com>
Cc: James Clark <james.clark@arm.com>
Cc: Leo Yan <leo.yan@linaro.org>
Assisted-by: Claude <noreply@anthropic.com>
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/util/cs-etm-base.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/tools/perf/util/cs-etm-base.c b/tools/perf/util/cs-etm-base.c
index 4abe416e3febd2cc..aebef71d3a0a1d7b 100644
--- a/tools/perf/util/cs-etm-base.c
+++ b/tools/perf/util/cs-etm-base.c
@@ -170,7 +170,9 @@ int cs_etm__process_auxtrace_info(union perf_event *event,
 	u64 *ptr = NULL;
 	u64 hdr_version;
 
-	if (auxtrace_info->header.size < (event_header_size + INFO_HEADER_SIZE))
+	/* Ensure priv[] is large enough for the global header entries */
+	if (auxtrace_info->header.size < (event_header_size + INFO_HEADER_SIZE +
+					  CS_ETM_HEADER_SIZE))
 		return -EINVAL;
 
 	/* First the global part */
-- 
2.54.0

[PATCH 5/9] perf cs-etm: Bounds-check CPU in cs_etm__get_queue()

[Arnaldo Carvalho de Melo]

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

cs_etm__get_queue() indexes etm->queues.queue_array[cpu] without
validating that cpu is within nr_queues.  When processing
AUX_OUTPUT_HW_ID events, the cpu value comes from untrusted perf.data
trace payload and flows through cs_etm__process_trace_id_v0_1() and
cs_etm__queue_aux_fragment() without bounds checking, allowing an
out-of-bounds read with a crafted file.

Add a bounds check in cs_etm__get_queue() and NULL checks in all
callers.

Also add NULL checks for queue_array[i].priv in the queue iteration
loops in cs_etm__map_trace_id_v0() and cs_etm__process_trace_id_v0_1()
— after auxtrace_queues__grow() new entries are zero-initialized so
.priv can be NULL.  Add a get_cpu_data() NULL check in
cs_etm__process_trace_id_v0_1(), matching the existing check in
cs_etm__process_trace_id_v0().

Fixes: 77c123f53e97ad4b ("perf: cs-etm: Move traceid_list to each queue")
Reported-by: sashiko-bot <sashiko-bot@kernel.org>
Cc: Adrian Hunter <adrian.hunter@intel.com>
Cc: James Clark <james.clark@arm.com>
Cc: Leo Yan <leo.yan@linaro.org>
Assisted-by: Claude <noreply@anthropic.com>
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/util/cs-etm.c | 25 +++++++++++++++++++++++--
 1 file changed, 23 insertions(+), 2 deletions(-)

diff --git a/tools/perf/util/cs-etm.c b/tools/perf/util/cs-etm.c
index d121c8f22028d5ba..5d0664ff73b79122 100644
--- a/tools/perf/util/cs-etm.c
+++ b/tools/perf/util/cs-etm.c
@@ -292,8 +292,11 @@ static struct cs_etm_queue *cs_etm__get_queue(struct cs_etm_auxtrace *etm, int c
 {
 	if (etm->per_thread_decoding)
 		return etm->queues.queue_array[0].priv;
-	else
-		return etm->queues.queue_array[cpu].priv;
+
+	if (cpu < 0 || cpu >= (int)etm->queues.nr_queues)
+		return NULL;
+
+	return etm->queues.queue_array[cpu].priv;
 }
 
 static int cs_etm__map_trace_id_v0(struct cs_etm_auxtrace *etm, u8 trace_chan_id,
@@ -306,6 +309,9 @@ static int cs_etm__map_trace_id_v0(struct cs_etm_auxtrace *etm, u8 trace_chan_id
 	 * queue associated with that CPU so only one decoder is made.
 	 */
 	etmq = cs_etm__get_queue(etm, cpu_metadata[CS_ETM_CPU]);
+	if (!etmq)
+		return -EINVAL;
+
 	if (etmq->format == UNFORMATTED)
 		return cs_etm__insert_trace_id_node(etmq, trace_chan_id,
 						    cpu_metadata);
@@ -318,6 +324,9 @@ static int cs_etm__map_trace_id_v0(struct cs_etm_auxtrace *etm, u8 trace_chan_id
 		int ret;
 
 		etmq = etm->queues.queue_array[i].priv;
+		if (!etmq)
+			continue;
+
 		ret = cs_etm__insert_trace_id_node(etmq, trace_chan_id,
 						   cpu_metadata);
 		if (ret)
@@ -358,6 +367,9 @@ static int cs_etm__process_trace_id_v0_1(struct cs_etm_auxtrace *etm, int cpu,
 	u32 sink_id = FIELD_GET(CS_AUX_HW_ID_SINK_ID_MASK, hw_id);
 	u8 trace_id = FIELD_GET(CS_AUX_HW_ID_TRACE_ID_MASK, hw_id);
 
+	if (!etmq)
+		return -EINVAL;
+
 	/*
 	 * Check sink id hasn't changed in per-cpu mode. In per-thread mode,
 	 * let it pass for now until an actual overlapping trace ID is hit. In
@@ -375,6 +387,9 @@ static int cs_etm__process_trace_id_v0_1(struct cs_etm_auxtrace *etm, int cpu,
 	for (unsigned int i = 0; i < etm->queues.nr_queues; ++i) {
 		struct cs_etm_queue *other_etmq = etm->queues.queue_array[i].priv;
 
+		if (!other_etmq)
+			continue;
+
 		/* Different sinks, skip */
 		if (other_etmq->sink_id != etmq->sink_id)
 			continue;
@@ -396,6 +411,9 @@ static int cs_etm__process_trace_id_v0_1(struct cs_etm_auxtrace *etm, int cpu,
 	}
 
 	cpu_data = get_cpu_data(etm, cpu);
+	if (!cpu_data)
+		return -EINVAL;
+
 	ret = cs_etm__insert_trace_id_node(etmq, trace_id, cpu_data);
 	if (ret)
 		return ret;
@@ -3144,6 +3162,9 @@ static int cs_etm__queue_aux_fragment(struct perf_session *session, off_t file_o
 	    aux_offset + aux_size <= auxtrace_event->offset + auxtrace_event->size) {
 		struct cs_etm_queue *etmq = cs_etm__get_queue(etm, auxtrace_event->cpu);
 
+		if (!etmq)
+			return -EINVAL;
+
 		/*
 		 * If this AUX event was inside this buffer somewhere, create a new auxtrace event
 		 * based on the sizes of the aux event, and queue that fragment.
-- 
2.54.0

[PATCH 6/9] perf c2c: Free format list entries when c2c_hists__init() fails

[Arnaldo Carvalho de Melo]

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

When c2c_hists__init() fails partway through hpp_list__parse(),
dynamically allocated format structures that were already added to
hists->list are leaked because he__get_c2c_hists() frees the hists
container without first unregistering the format entries.

Call perf_hpp__reset_output_field() before freeing the hists container
on the error path, matching what c2c_he_free() already does on the
normal destruction path.

Fixes: 17a7c5946d79a12c ("perf c2c report: Decode c2c_stats for hist entries")
Reported-by: sashiko-bot <sashiko-bot@kernel.org>
Cc: Jiri Olsa <jolsa@kernel.org>
Assisted-by: Claude <noreply@anthropic.com>
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/builtin-c2c.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/tools/perf/builtin-c2c.c b/tools/perf/builtin-c2c.c
index 07c7e8fb315e6cf3..eabb922ef295ef86 100644
--- a/tools/perf/builtin-c2c.c
+++ b/tools/perf/builtin-c2c.c
@@ -226,6 +226,7 @@ he__get_c2c_hists(struct hist_entry *he,
 
 	ret = c2c_hists__init(hists, sort, nr_header_lines, env);
 	if (ret) {
+		perf_hpp__reset_output_field(&hists->list);
 		c2c_he->hists = NULL;
 		free(hists);
 		return NULL;
-- 
2.54.0

[PATCH 7/9] perf c2c: Fix hist entry and format list leaks in c2c_he_free()

[Arnaldo Carvalho de Melo]

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

c2c_he_free() calls hists__delete_entries() which only walks the
output-sorted entries tree.  During c2c resort, when cacheline entries
are merged and the redundant entry is freed, the inner hists have not
been output-resorted yet, so hists->entries is empty.  The actual inner
hist_entry objects live in entries_in_array[] and entries_collapsed,
which are never walked, leaking all inner hist_entry objects for every
merged cacheline.

Additionally, the dynamically allocated format entries on hists->list
are never unregistered or freed.

Fix both issues by switching to hists__delete_all_entries() which walks
all rb_root trees, and calling perf_hpp__reset_output_field() to clean
up format entries.

Fixes: bf0e0d407ea09ce5 ("perf c2c report: Add sample processing")
Reported-by: sashiko-bot <sashiko-bot@kernel.org>
Cc: Jiri Olsa <jolsa@kernel.org>
Assisted-by: Claude <noreply@anthropic.com>
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/builtin-c2c.c | 2 +-
 tools/perf/util/hist.c   | 2 +-
 tools/perf/util/hist.h   | 1 +
 3 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/tools/perf/builtin-c2c.c b/tools/perf/builtin-c2c.c
index eabb922ef295ef86..c9584dbedf77afe8 100644
--- a/tools/perf/builtin-c2c.c
+++ b/tools/perf/builtin-c2c.c
@@ -184,7 +184,7 @@ static void c2c_he_free(void *he)
 
 	c2c_he = container_of(he, struct c2c_hist_entry, he);
 	if (c2c_he->hists) {
-		hists__delete_entries(&c2c_he->hists->hists);
+		hists__delete_all_entries(&c2c_he->hists->hists);
 		perf_hpp__reset_output_field(&c2c_he->hists->list);
 		zfree(&c2c_he->hists);
 	}
diff --git a/tools/perf/util/hist.c b/tools/perf/util/hist.c
index df978c996b6c2262..c93915625ee75de1 100644
--- a/tools/perf/util/hist.c
+++ b/tools/perf/util/hist.c
@@ -3041,7 +3041,7 @@ static void hists__delete_remaining_entries(struct rb_root_cached *root)
 	}
 }
 
-static void hists__delete_all_entries(struct hists *hists)
+void hists__delete_all_entries(struct hists *hists)
 {
 	hists__delete_entries(hists);
 	hists__delete_remaining_entries(&hists->entries_in_array[0]);
diff --git a/tools/perf/util/hist.h b/tools/perf/util/hist.h
index 8fb89d81ef069d95..b830cbe7f95bf597 100644
--- a/tools/perf/util/hist.h
+++ b/tools/perf/util/hist.h
@@ -391,6 +391,7 @@ int hists__collapse_resort(struct hists *hists, struct ui_progress *prog);
 
 void hists__decay_entries(struct hists *hists, bool zap_user, bool zap_kernel);
 void hists__delete_entries(struct hists *hists);
+void hists__delete_all_entries(struct hists *hists);
 void hists__output_recalc_col_len(struct hists *hists, int max_rows);
 
 struct hist_entry *hists__get_entry(struct hists *hists, int idx);
-- 
2.54.0

[PATCH 8/9] perf bpf: Validate array presence before casting BPF prog info pointers

[Arnaldo Carvalho de Melo]

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

Several functions cast bpf_prog_info fields (jited_ksyms,
jited_func_lens, jited_prog_insns) from u64 to pointers and
dereference them.  These fields are only valid pointers if
bpil_offs_to_addr() converted their file offsets to addresses, which
only happens when the corresponding PERF_BPIL_* bits are set in
info_linear->arrays.

A crafted perf.data can leave these bits unset while setting non-zero
counts and offset values, causing the functions to dereference raw file
offsets as pointers.

Add array bitmask validation to all perf.data processing paths:

  - __bpf_event__print_bpf_prog_info(): check JITED_KSYMS and
    JITED_FUNC_LENS (changed to take struct perf_bpil *)
  - machine__process_bpf_event_load(): check JITED_KSYMS
  - bpf_read(): check JITED_INSNS before memcpy from jited_prog_insns
  - dso__disassemble_filename(): check JITED_INSNS before returning
    jited_prog_insns pointer

Fixes: f8dfeae009effc0b ("perf bpf: Show more BPF program info in print_bpf_prog_info()")
Reported-by: sashiko-bot <sashiko-bot@kernel.org>
Cc: Song Liu <songliubraving@fb.com>
Assisted-by: Claude <noreply@anthropic.com>
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/util/bpf-event.c | 20 +++++++++++++++++---
 tools/perf/util/bpf-event.h |  4 ++--
 tools/perf/util/dso.c       | 10 ++++++++++
 tools/perf/util/header.c    |  3 +--
 4 files changed, 30 insertions(+), 7 deletions(-)

diff --git a/tools/perf/util/bpf-event.c b/tools/perf/util/bpf-event.c
index 57d53ba848359e12..fa3ebc8ea7f09cdd 100644
--- a/tools/perf/util/bpf-event.c
+++ b/tools/perf/util/bpf-event.c
@@ -59,6 +59,10 @@ static int machine__process_bpf_event_load(struct machine *machine,
 		return 0;
 	info_linear = info_node->info_linear;
 
+	/* jited_ksyms is only valid if bpil_offs_to_addr() converted it */
+	if (!(info_linear->arrays & (1UL << PERF_BPIL_JITED_KSYMS)))
+		return 0;
+
 	for (i = 0; i < info_linear->info.nr_jited_ksyms; i++) {
 		u64 *addrs = (u64 *)(uintptr_t)(info_linear->info.jited_ksyms);
 		u64 addr = addrs[i];
@@ -959,12 +963,15 @@ int evlist__add_bpf_sb_event(struct evlist *evlist, struct perf_env *env)
 	return evlist__add_sb_event(evlist, &attr, bpf_event__sb_cb, env);
 }
 
-void __bpf_event__print_bpf_prog_info(struct bpf_prog_info *info,
+void __bpf_event__print_bpf_prog_info(struct perf_bpil *info_linear,
 				      struct perf_env *env,
 				      FILE *fp)
 {
-	__u32 *prog_lens = (__u32 *)(uintptr_t)(info->jited_func_lens);
-	__u64 *prog_addrs = (__u64 *)(uintptr_t)(info->jited_ksyms);
+	struct bpf_prog_info *info = &info_linear->info;
+	__u64 required_arrays = (1UL << PERF_BPIL_JITED_KSYMS) |
+				(1UL << PERF_BPIL_JITED_FUNC_LENS);
+	__u32 *prog_lens;
+	__u64 *prog_addrs;
 	char name[KSYM_NAME_LEN];
 	struct btf *btf = NULL;
 	u32 sub_prog_cnt, i;
@@ -974,6 +981,13 @@ void __bpf_event__print_bpf_prog_info(struct bpf_prog_info *info,
 	    sub_prog_cnt != info->nr_jited_func_lens)
 		return;
 
+	/* Ensure the arrays were present and converted by bpil_offs_to_addr() */
+	if ((info_linear->arrays & required_arrays) != required_arrays)
+		return;
+
+	prog_lens = (__u32 *)(uintptr_t)(info->jited_func_lens);
+	prog_addrs = (__u64 *)(uintptr_t)(info->jited_ksyms);
+
 	if (info->btf_id) {
 		struct btf_node *node;
 
diff --git a/tools/perf/util/bpf-event.h b/tools/perf/util/bpf-event.h
index 60d2c6637af5d6eb..da4eeb4a1a73208c 100644
--- a/tools/perf/util/bpf-event.h
+++ b/tools/perf/util/bpf-event.h
@@ -40,7 +40,7 @@ struct btf_node {
 int machine__process_bpf(struct machine *machine, union perf_event *event,
 			 struct perf_sample *sample);
 int evlist__add_bpf_sb_event(struct evlist *evlist, struct perf_env *env);
-void __bpf_event__print_bpf_prog_info(struct bpf_prog_info *info,
+void __bpf_event__print_bpf_prog_info(struct perf_bpil *info_linear,
 				      struct perf_env *env,
 				      FILE *fp);
 void bpf_metadata_free(struct bpf_metadata *metadata);
@@ -58,7 +58,7 @@ static inline int evlist__add_bpf_sb_event(struct evlist *evlist __maybe_unused,
 	return 0;
 }
 
-static inline void __bpf_event__print_bpf_prog_info(struct bpf_prog_info *info __maybe_unused,
+static inline void __bpf_event__print_bpf_prog_info(struct perf_bpil *info_linear __maybe_unused,
 						    struct perf_env *env __maybe_unused,
 						    FILE *fp __maybe_unused)
 {
diff --git a/tools/perf/util/dso.c b/tools/perf/util/dso.c
index 1a2fc6d18da74d6c..79f1a30f3683d6b3 100644
--- a/tools/perf/util/dso.c
+++ b/tools/perf/util/dso.c
@@ -880,6 +880,12 @@ static ssize_t bpf_read(struct dso *dso, u64 offset, char *data)
 		return -1;
 	}
 
+	/* jited_prog_insns is only valid if bpil_offs_to_addr() converted it */
+	if (!(node->info_linear->arrays & (1UL << PERF_BPIL_JITED_INSNS))) {
+		dso__data(dso)->status = DSO_DATA_STATUS_ERROR;
+		return -1;
+	}
+
 	len = node->info_linear->info.jited_prog_len;
 	buf = (u8 *)(uintptr_t)node->info_linear->info.jited_prog_insns;
 
@@ -1995,6 +2001,10 @@ const u8 *dso__read_symbol(struct dso *dso, const char *symfs_filename,
 			return NULL;
 		}
 		info_linear = info_node->info_linear;
+		if (!(info_linear->arrays & (1UL << PERF_BPIL_JITED_INSNS))) {
+			errno = SYMBOL_ANNOTATE_ERRNO__BPF_MISSING_BTF;
+			return NULL;
+		}
 		assert(len <= info_linear->info.jited_prog_len);
 		*out_buf_len = len;
 		return (const u8 *)(uintptr_t)(info_linear->info.jited_prog_insns);
diff --git a/tools/perf/util/header.c b/tools/perf/util/header.c
index d7f41db7322cbcb4..091d8f7f6bd2c9d5 100644
--- a/tools/perf/util/header.c
+++ b/tools/perf/util/header.c
@@ -2107,8 +2107,7 @@ static void print_bpf_prog_info(struct feat_fd *ff __maybe_unused, FILE *fp)
 		node = rb_entry(next, struct bpf_prog_info_node, rb_node);
 		next = rb_next(&node->rb_node);
 
-		__bpf_event__print_bpf_prog_info(&node->info_linear->info,
-						 env, fp);
+		__bpf_event__print_bpf_prog_info(node->info_linear, env, fp);
 	}
 
 	up_read(&env->bpf_progs.lock);
-- 
2.54.0

[PATCH 9/9] perf dso: Set standard errno on decompression failure

[Arnaldo Carvalho de Melo]

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

dso__get_filename() sets errno to a negative custom DSO_LOAD_ERRNO
value when kernel module decompression fails:

  errno = *dso__load_errno(dso);  /* e.g. -9996 */

The caller __open_dso() then computes fd = -errno, producing a large
positive value (9996) that looks like a valid file descriptor.  This
can cause close_data_fd() to close an unrelated fd used by another
subsystem.

Set errno to EIO instead.  The detailed error code is already stored
in dso__load_errno(dso) for diagnostic messages.

Fixes: 1d6b3c9ba756a513 ("perf tools: Decompress kernel module when reading DSO data")
Reported-by: sashiko-bot <sashiko-bot@kernel.org>
Cc: Namhyung Kim <namhyung@kernel.org>
Assisted-by: Claude <noreply@anthropic.com>
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/util/dso.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/tools/perf/util/dso.c b/tools/perf/util/dso.c
index 79f1a30f3683d6b3..2309196d8df3111c 100644
--- a/tools/perf/util/dso.c
+++ b/tools/perf/util/dso.c
@@ -600,7 +600,13 @@ static char *dso__get_filename(struct dso *dso, const char *root_dir,
 		size_t len = sizeof(newpath);
 
 		if (dso__decompress_kmodule_path(dso, name, newpath, len) < 0) {
-			errno = *dso__load_errno(dso);
+			/*
+			 * Use a standard errno value, not the negative custom
+			 * DSO_LOAD_ERRNO stored in dso__load_errno(dso):
+			 * __open_dso() computes fd = -errno, so a negative
+			 * errno produces a positive fd that looks valid.
+			 */
+			errno = EIO;
 			goto out;
 		}
 
-- 
2.54.0

Re: [PATCH 1/9] perf machine: Propagate machine__init() error to callers

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] Uninitialized stack variable `vmlinux` is used in the error cleanup path.
--

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

perf machine: Propagate machine__init() error to callers

This commit updates machine__init() to propagate memory allocation errors
and adds return value checks to its callers across the perf tool and tests.

> diff --git a/tools/perf/tests/vmlinux-kallsyms.c b/tools/perf/tests/vmlinux-kallsyms.c
> index 7409abe4aa369..e88e7b2a82c5c 100644
> --- a/tools/perf/tests/vmlinux-kallsyms.c
> +++ b/tools/perf/tests/vmlinux-kallsyms.c
> @@ -203,8 +203,10 @@ static int test__vmlinux_matches_kallsyms(struct test_suite *test __maybe_unused
>  	 * Init the machines that will hold kernel, modules obtained from
>  	 * both vmlinux + .ko files and from /proc/kallsyms split by modules.
>  	 */
> -	machine__init(&args.kallsyms, "", HOST_KERNEL_ID);
> -	machine__init(&vmlinux, "", HOST_KERNEL_ID);
> +	if (machine__init(&args.kallsyms, "", HOST_KERNEL_ID))
> +		goto out;

[Severity: Medium]
Will this cause a crash in the error path?

If the first machine__init() fails here, execution jumps to the "out"
label before the stack variable "vmlinux" is initialized.

In the cleanup path in test__vmlinux_matches_kallsyms():

out:
        machine__exit(&args.kallsyms);
        machine__exit(&vmlinux);
        return err;

Calling machine__exit() on the uninitialized "vmlinux" structure might attempt
to free or clean up garbage pointers. Could "vmlinux" be zero-initialized
when declared, or should the cleanup path be split to avoid this?

> +	if (machine__init(&vmlinux, "", HOST_KERNEL_ID))
> +		goto out;
>  
>  	maps = machine__kernel_maps(&vmlinux);

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

Re: [PATCH 3/9] perf cs-etm: Validate num_cpu before metadata allocation

[sashiko-bot]

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

Pre-existing issues:
- [High] The newly added bounds check only guarantees one 64-bit word per CPU, but the subsequent metadata extraction loop consumes variable-length blocks without enforcing bounds, leading to arbitrary out-of-bounds reads.
--

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

perf cs-etm: Validate num_cpu before metadata allocation

This commit addresses a potential integer overflow and out-of-bounds write in
cs_etm__process_auxtrace_info_full(). It fixes the issue by computing priv_size
early and bounding num_cpu so that it cannot exceed the total number of u64
entries in the event's private data area.

> diff --git a/tools/perf/util/cs-etm.c b/tools/perf/util/cs-etm.c
> index 0927b0b9c06b1..d121c8f22028d 100644
> --- a/tools/perf/util/cs-etm.c
> +++ b/tools/perf/util/cs-etm.c
> @@ -3431,6 +3431,18 @@ int cs_etm__process_auxtrace_info_full(union perf_event *event,
>  	/* First the global part */
>  	ptr = (u64 *) auxtrace_info->priv;
>  	num_cpu = ptr[CS_PMU_TYPE_CPUS] & 0xffffffff;
> +
> +	/*
> +	 * Bound num_cpu by the event size: the global header consumes
> +	 * CS_ETM_HEADER_SIZE bytes, and each CPU needs at least one u64
> +	 * metadata entry after that.
> +	 */
> +	priv_size = total_size - event_header_size - INFO_HEADER_SIZE -
> +		    CS_ETM_HEADER_SIZE;
> +	if (num_cpu <= 0 || priv_size <= 0 ||
> +	    num_cpu > priv_size / (int)sizeof(u64))
> +		return -EINVAL;
> +

[Severity: High]
This is a pre-existing issue, but does the newly added bounds check guarantee
that subsequent accesses in the parsing loop remain within bounds?

The added check only guarantees one 64-bit word per CPU, but each CPU's
metadata block can have a variable length. During the parsing loop:

tools/perf/util/cs-etm.c:cs_etm__process_auxtrace_info_full() {
    ...
    for (int j = 0; j < num_cpu; j++) {
        if (ptr[i] == __perf_cs_etmv3_magic) {
            metadata[j] = cs_etm__create_meta_blk(ptr, &i, ...);
    ...
}

The cs_etm__create_meta_blk() function consumes a variable number of elements
without validating bounds against priv_size:

tools/perf/util/cs-etm.c:cs_etm__create_meta_blk() {
    ...
    nr_in_params = buff_in[i + CS_ETM_NR_TRC_PARAMS];
    ...
    *info_read = i + nr_cmn_params + nr_in_params;
    ...
}

Could this allow a crafted perf.data file to set a large nr_in_params,
causing the shared index i to be incremented past the buffer bounds? On the
next iteration of the loop in cs_etm__process_auxtrace_info_full(), ptr[i]
would then be read out-of-bounds.

The final size validation (i * 8 != priv_size) occurs after the parsing loop
completes, which appears too late to prevent these out-of-bounds reads.

>  	metadata = zalloc(sizeof(*metadata) * num_cpu);
>  	if (!metadata)
>  		return -ENOMEM;

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

Re: [PATCH 4/9] perf cs-etm: Require full global header in auxtrace_info size check

[sashiko-bot]

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

Pre-existing issues:
- [High] Missing bounds checks when parsing variable-length per-CPU metadata in CS-ETM auxtrace_info allows out-of-bounds reads.
- [High] Missing size validations in auxtrace info processing for Intel PT, Intel BTS, and PowerPC VPADTL allow out-of-bounds memory reads from the event payload.
- [High] Insufficient bounds checking and untrusted input usage in Arm SPE auxtrace info processing lead to division by zero and out-of-bounds memory accesses.
--

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

perf cs-etm: Require full global header in auxtrace_info size check

This commit updates cs_etm__process_auxtrace_info() to include
CS_ETM_HEADER_SIZE in the minimum size check. This ensures that the global
header entries fit within the event, fixing an out-of-bounds read issue
with crafted auxtrace_info events.

> diff --git a/tools/perf/util/cs-etm-base.c b/tools/perf/util/cs-etm-base.c
> index 4abe416e3febd..aebef71d3a0a1 100644
> --- a/tools/perf/util/cs-etm-base.c
> +++ b/tools/perf/util/cs-etm-base.c
> @@ -170,7 +170,9 @@ int cs_etm__process_auxtrace_info(union perf_event *event,
>  	u64 *ptr = NULL;
>  	u64 hdr_version;
>  
> -	if (auxtrace_info->header.size < (event_header_size + INFO_HEADER_SIZE))
> +	/* Ensure priv[] is large enough for the global header entries */
> +	if (auxtrace_info->header.size < (event_header_size + INFO_HEADER_SIZE +
> +					  CS_ETM_HEADER_SIZE))
>  		return -EINVAL;
>  
>  	/* First the global part */

[Severity: High]
This isn't a bug introduced by this patch, but the patch attempts to fix a
related out-of-bounds read by validating the global header size. Is the fix
incomplete?

The per-CPU metadata is still read dynamically without verifying that the
event payload (auxtrace_info->header.size) is large enough to contain it.

In cs_etm__process_auxtrace_info(), if dump_trace is true,
cs_etm__print_auxtrace_info() reads at least 7 64-bit words per CPU without
checking the buffer bounds, leading to an out-of-bounds read and exposing heap
memory contents to standard output:

tools/perf/util/cs-etm-base.c:cs_etm__print_cpu_metadata_v1() {
    ...
    total_params = val[i + CS_ETM_NR_TRC_PARAMS] + CS_ETM_COMMON_BLK_MAX_V1;
    ...
}

Similarly, cs_etm__process_auxtrace_info_full() validates that there is at
least 1 word per CPU, but cs_etm__create_meta_blk() iterates and reads multiple
words per CPU based on CS_ETM_NR_TRC_PARAMS, causing out-of-bounds heap reads:

tools/perf/util/cs-etm.c:cs_etm__create_meta_blk() {
    ...
    for (k = CS_ETM_MAGIC; k < nr_out_params; k++)
        metadata[k] = buff_in[i + k];
    ...
}

And later validates the total number of words read only after the out-of-bounds
memory accesses have already occurred:

tools/perf/util/cs-etm.c:cs_etm__process_auxtrace_info_full() {
    ...
    if (i * 8 != priv_size) {
    ...
}

Could a crafted trace file with insufficient per-CPU metadata size crash the
perf tool or leak memory?

[Severity: High]
This is a pre-existing issue, but it exists in similar auxtrace decoders for
Intel PT, Intel BTS, and PowerPC VPADTL. Do these decoders also miss size
validations, allowing out-of-bounds memory reads from the event payload?

In intel_pt_process_auxtrace_info(), min_sz is calculated as sizeof(u64) *
INTEL_PT_PER_CPU_MMAPS (which is sizeof(u64) * 9). This guarantees the payload
has space for indices 0 to 8. The code then reads index 9, which accesses 8
bytes strictly beyond the validated boundary if the size is exactly min_sz:

tools/perf/util/intel-pt.c:intel_pt_process_auxtrace_info() {
    ...
    pt->per_cpu_mmaps = auxtrace_info->priv[INTEL_PT_PER_CPU_MMAPS];
    ...
}

The exact same off-by-one enum indexing pattern causes out-of-bounds reads in
intel_bts_process_auxtrace_info(), which validates up to
INTEL_BTS_SNAPSHOT_MODE but accesses index 5:

tools/perf/util/intel-bts.c:intel_bts_process_auxtrace_info() {
    ...
    bts->snapshot_mode = auxtrace_info->priv[INTEL_BTS_SNAPSHOT_MODE];
    ...
}

And in powerpc_vpadtl_process_auxtrace_info(), which validates up to
POWERPC_VPADTL_TYPE (0 bytes) but reads index 0:

tools/perf/util/powerpc-vpadtl.c:powerpc_vpadtl_process_auxtrace_info() {
    ...
    vpa->pmu_type = auxtrace_info->priv[POWERPC_VPADTL_TYPE];
    ...
}

Can parsing a crafted perf.data file trigger out-of-bounds heap reads and
potential segmentation faults in these decoders?

[Severity: High]
This is a pre-existing issue, but it also exists in the Arm SPE auxtrace
decoder. Are there insufficient bounds checks and untrusted input usage in
arm_spe_process_auxtrace_info() that lead to division by zero and
out-of-bounds memory accesses?

The event size is only verified to be at least ARM_SPE_AUXTRACE_V1_PRIV_SIZE
(16 bytes). If the size is 17 bytes, it skips the V1 branch and reads index 3
(offset 24) from the payload, causing an immediate out-of-bounds read:

tools/perf/util/arm-spe.c:arm_spe__alloc_metadata() {
    ...
    *nr_cpu = ptr[ARM_SPE_CPUS_NUM];
    ...
}

Furthermore, *nr_cpu is taken directly from the untrusted payload without
checking for zero, and allows unsigned wraparound if hdr_sz is larger than
metadata_size:

tools/perf/util/arm-spe.c:arm_spe__alloc_metadata() {
    ...
    per_cpu_sz = (metadata_size - (hdr_sz * sizeof(u64))) / (*nr_cpu);
    ...
}

This could result in a huge per_cpu_sz value passed to memcpy(), leading to a
massive out-of-bounds heap read and segmentation fault:

tools/perf/util/arm-spe.c:arm_spe__alloc_per_cpu_metadata() {
    ...
    memcpy(metadata, buf, per_cpu_size);
    ...
}

Can a crafted perf.data file with nr_cpu == 0 cause a SIGFPE and crash the
perf tool?

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

Re: [PATCH 5/9] perf cs-etm: Bounds-check CPU in cs_etm__get_queue()

[sashiko-bot]

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

Pre-existing issues:
- [Medium] A maliciously crafted perf.data file can trigger a NULL pointer dereference in cs_etm__dump_event() during `perf script -D` or `perf report -D`.
--

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

perf cs-etm: Bounds-check CPU in cs_etm__get_queue()

This commit adds bounds checking for CPU indexes in cs_etm__get_queue()
to prevent out-of-bounds reads from untrusted perf.data payloads. It also
adds NULL checks for queue array elements which can be zero-initialized
after array growth.

> diff --git a/tools/perf/util/cs-etm.c b/tools/perf/util/cs-etm.c
> index d121c8f22028d..5d0664ff73b79 100644
> --- a/tools/perf/util/cs-etm.c
> +++ b/tools/perf/util/cs-etm.c

[ ... ]

> @@ -318,6 +324,9 @@ static int cs_etm__map_trace_id_v0(struct cs_etm_auxtrace *etm, u8 trace_chan_id
>  		int ret;
>  
>  		etmq = etm->queues.queue_array[i].priv;
> +		if (!etmq)
> +			continue;
> +

[Severity: Medium]
This is a pre-existing issue, but since we are handling cases where
queue_array[i].priv can be NULL after auxtrace_queues__grow(), could
this same condition cause a NULL pointer dereference elsewhere?

In tools/perf/util/cs-etm.c:cs_etm__process_auxtrace_event(), the untrusted
idx from the payload can trigger array growth, passing NULL directly:

    cs_etm__dump_event(etm->queues.queue_array[idx].priv, buffer);

A similar queue iteration happens in dump_queued_data():

    cs_etm__dump_event(etm->queues.queue_array[i].priv, buf);

Since cs_etm__dump_event() accesses etmq->decoder without verifying if
etmq is NULL:

    cs_etm_decoder__get_name(etmq->decoder)

Could this cause a crash when processing a crafted perf.data file with
the dump trace option like perf script -D or perf report -D?

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

Re: [PATCH 8/9] perf bpf: Validate array presence before casting BPF prog info pointers

[sashiko-bot]

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

New issues:
- [Medium] Unconditional bitmask validation in `__bpf_event__print_bpf_prog_info` silently drops BPF program info for programs without subprograms (e.g., non-JITed or non-BTF programs).

Pre-existing issues:
- [High] Incomplete validation of BPF prog info array bitmasks allows arbitrary pointer dereferences via crafted `perf.data` files.
--

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

perf bpf: Validate array presence before casting BPF prog info pointers

Several functions cast bpf_prog_info fields (jited_ksyms,
jited_func_lens, jited_prog_insns) from u64 to pointers and
dereference them. These fields are only valid pointers if
bpil_offs_to_addr() converted their file offsets to addresses.

> diff --git a/tools/perf/util/bpf-event.c b/tools/perf/util/bpf-event.c
> index 57d53ba848359..fa3ebc8ea7f09 100644
> --- a/tools/perf/util/bpf-event.c
> +++ b/tools/perf/util/bpf-event.c
[ ... ]
> @@ -959,12 +963,15 @@ int evlist__add_bpf_sb_event(struct evlist *evlist, struct perf_env *env)
>  	return evlist__add_sb_event(evlist, &attr, bpf_event__sb_cb, env);
>  }
>  
> -void __bpf_event__print_bpf_prog_info(struct bpf_prog_info *info,
> +void __bpf_event__print_bpf_prog_info(struct perf_bpil *info_linear,
>  				      struct perf_env *env,
>  				      FILE *fp)
>  {
> -	__u32 *prog_lens = (__u32 *)(uintptr_t)(info->jited_func_lens);
> -	__u64 *prog_addrs = (__u64 *)(uintptr_t)(info->jited_ksyms);
> +	struct bpf_prog_info *info = &info_linear->info;
> +	__u64 required_arrays = (1UL << PERF_BPIL_JITED_KSYMS) |
> +				(1UL << PERF_BPIL_JITED_FUNC_LENS);

[Severity: High]
This isn't a bug introduced by this patch, but does this incomplete bitmask
validation allow arbitrary pointer dereferences via crafted perf.data files?

The required_arrays bitmask enforces PERF_BPIL_JITED_KSYMS and
PERF_BPIL_JITED_FUNC_LENS but omits PERF_BPIL_PROG_TAGS and
PERF_BPIL_FUNC_INFO.

Later in this function, synthesize_bpf_prog_name() is called, which blindly
casts and dereferences info->prog_tags and info->func_info without checking
if their array presence bits were set.

Additionally, symbol__disassemble_bpf_libbfd() in tools/perf/util/libbfd.c
lacks any array bitmask validation and blindly casts and dereferences
jited_prog_insns and jited_ksyms. A crafted perf.data file can provide
these fields as raw file offsets while unsetting their corresponding bits,
bypassing the bpil_offs_to_addr() conversion and leading to segmentation
faults.

> +	__u32 *prog_lens;
> +	__u64 *prog_addrs;
>  	char name[KSYM_NAME_LEN];
>  	struct btf *btf = NULL;
>  	u32 sub_prog_cnt, i;
> @@ -974,6 +981,13 @@ void __bpf_event__print_bpf_prog_info(struct bpf_prog_info *info,
>  	    sub_prog_cnt != info->nr_jited_func_lens)
>  		return;
>  
> +	/* Ensure the arrays were present and converted by bpil_offs_to_addr() */
> +	if ((info_linear->arrays & required_arrays) != required_arrays)
> +		return;

[Severity: Medium]
Will this unconditional check silently drop BPF program info for programs
without subprograms?

The function bpil_offs_to_addr() has an edge case where it clears an array's
presence bit if its count is 0 and it is placed at the end of the data buffer
(offset equals data_len).

Valid BPF programs that legitimately have nr_jited_ksyms == 0 and
nr_jited_func_lens == 0 (such as non-JITed programs) will have these bits
unset. The new validation check will unconditionally fail here, causing
perf report --header to silently omit their basic program info.

Should this check only be enforced when sub_prog_cnt > 0?

> +
> +	prog_lens = (__u32 *)(uintptr_t)(info->jited_func_lens);
> +	prog_addrs = (__u64 *)(uintptr_t)(info->jited_ksyms);
> +
>  	if (info->btf_id) {

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

Re: [PATCH 6/9] perf c2c: Free format list entries when c2c_hists__init() fails

[sashiko-bot]

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

Pre-existing issues:
- [High] The `PARSE_LIST` macro and `hpp_list__parse()` silently mask and overwrite parsing and memory allocation errors.
- [High] Silent failure on `strdup()` memory allocation failures in `hpp_list__parse()`.
- [High] `perf c2c` lacks `.feature` and `.build_id` callbacks in `struct perf_tool`, breaking pipe mode.
- [High] `perf_hpp__setup_output_field` unconditionally adds sort fields to the global `perf_hpp_list`.
--

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

perf c2c: Free format list entries when c2c_hists__init() fails

When c2c_hists__init() fails partway through hpp_list__parse(),
dynamically allocated format structures that were already added to
hists->list are leaked because he__get_c2c_hists() frees the hists
container without first unregistering the format entries.

> diff --git a/tools/perf/builtin-c2c.c b/tools/perf/builtin-c2c.c
> index 07c7e8fb315e6..eabb922ef295e 100644
> --- a/tools/perf/builtin-c2c.c
> +++ b/tools/perf/builtin-c2c.c
> @@ -226,6 +226,7 @@ he__get_c2c_hists(struct hist_entry *he,
>  
>  	ret = c2c_hists__init(hists, sort, nr_header_lines, env);

[Severity: High]
This isn't a bug introduced by this patch, but looking at how
c2c_hists__init() calls hpp_list__parse(), it seems parsing and memory
allocation failures can be silently masked.

Inside hpp_list__parse(), PARSE_LIST() initializes ret = 0. When it is
invoked sequentially:

    PARSE_LIST(output, c2c_hists__init_output);
    PARSE_LIST(sort,   c2c_hists__init_sort);

If the first macro fails (e.g., returning -EINVAL), the second macro
invocation overwrites the error code. Additionally, the macro's loop only
breaks on -EINVAL or -ESRCH, failing to catch -ENOMEM. Could this result
in hpp_list__parse() returning success even when field initialization
fails?

[Severity: High]
This is a pre-existing issue, but I noticed another potential problem inside
hpp_list__parse() during memory allocation:

    char *output = output_ ? strdup(output_) : NULL;
    char *sort   = sort_   ? strdup(sort_) : NULL;

If strdup() fails due to OOM, it returns NULL. The code doesn't check
for NULL before passing it to PARSE_LIST(), which then evaluates
if (!_list) break; and exits with ret = 0. Does this cause the function
to silently set up empty dimension lists and report success rather than
propagating -ENOMEM?

[Severity: High]
This is also a pre-existing issue, but when c2c_hists__init() sets up
the output fields, there might be a use-after-free risk related to the
global list.

In perf_hpp__setup_output_field(), when adding a new field, it calls:

    perf_hpp__column_register(fmt);

This wrapper unconditionally targets the global perf_hpp_list:

    perf_hpp_list__column_register(&perf_hpp_list, format);

Since hists->list is a dynamically allocated local list, does this end
up polluting the global list with local format pointers that might later
be freed on error paths?

>  	if (ret) {
> +		perf_hpp__reset_output_field(&hists->list);
>  		c2c_he->hists = NULL;
>  		free(hists);
>  		return NULL;

[Severity: High]
This is a pre-existing issue, but while looking at perf c2c tool
initialization in cmd_c2c(), I noticed it lacks .feature and .build_id
callbacks.

    perf_tool__init(&c2c.tool, /*ordered_events=*/true);
    c2c.tool.sample = process_sample_event;

In pipe mode, system features like NUMA nodes are synthesized into
PERF_RECORD_HEADER_FEATURE events. Because the feature callback is
missing, perf_env isn't populated.

Later, setup_nodes() checks:

    n = env->numa_nodes;
    if (!n)
        return -EINVAL;

Does the missing feature callback cause piped inputs and cross-analysis
to completely fail by leaving env->numa_nodes as 0?

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

[PATCH 6/9] perf c2c: Free format list entries when c2c_hists__init() fails

[Arnaldo Carvalho de Melo]

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

When c2c_hists__init() fails partway through hpp_list__parse(),
dynamically allocated format structures that were already added to
hists->list are leaked because he__get_c2c_hists() frees the hists
container without first unregistering the format entries.

Call perf_hpp__reset_output_field() before freeing the hists container
on the error path, matching what c2c_he_free() already does on the
normal destruction path.

Fixes: 17a7c5946d79a12c ("perf c2c report: Decode c2c_stats for hist entries")
Reported-by: sashiko-bot <sashiko-bot@kernel.org>
Cc: Jiri Olsa <jolsa@kernel.org>
Assisted-by: Claude <noreply@anthropic.com>
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/builtin-c2c.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/tools/perf/builtin-c2c.c b/tools/perf/builtin-c2c.c
index 07c7e8fb315e6cf3..eabb922ef295ef86 100644
--- a/tools/perf/builtin-c2c.c
+++ b/tools/perf/builtin-c2c.c
@@ -226,6 +226,7 @@ he__get_c2c_hists(struct hist_entry *he,
 
 	ret = c2c_hists__init(hists, sort, nr_header_lines, env);
 	if (ret) {
+		perf_hpp__reset_output_field(&hists->list);
 		c2c_he->hists = NULL;
 		free(hists);
 		return NULL;
-- 
2.54.0