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

[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

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

[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

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

[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

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

[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

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

[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 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 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

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

[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

[PATCHES v4 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 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/thread-maps-share.c |  2 +-
 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 ++++---
 18 files changed, 120 insertions(+), 39 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/thread-maps-share.c |  2 +-
 tools/perf/util/aslr.c               | 12 +++++++++---
 tools/perf/util/machine.c            | 24 ++++++++++++++----------
 tools/perf/util/machine.h            |  2 +-
 tools/perf/util/session.c            |  7 ++++---
 9 files changed, 37 insertions(+), 22 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/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/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

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] The return value of the newly fallible `machine__init()` function is ignored in tests, leading to potential null pointer dereferences on initialization failure.
--

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

perf machine: Propagate machine__init() error to callers

machine__init() always returns 0 even when memory allocation fails,
because commit 81f981d7ec43ed93 introduced 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.

> diff --git a/tools/perf/util/machine.c b/tools/perf/util/machine.c
> index 31715366e29ff..9329d319bd033 100644
> --- a/tools/perf/util/machine.c
> +++ b/tools/perf/util/machine.c
[ ... ]
> @@ -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;
>  }

[Severity: Medium]
Since this patch changes machine__init() to return its internal error instead
of returning 0 on failure, do we also need to update the direct callers in the
tests to check this return value?

For example, test__kallsyms_split() ignores the return value:

tools/perf/tests/kallsyms-split.c:test__kallsyms_split() {
    ...
	machine__init(&m, root_dir, HOST_KERNEL_ID);
	if (machine__create_kernel_maps(&m) < 0) {
    ...
}

And test__vmlinux_matches_kallsyms() does as well:

tools/perf/tests/vmlinux-kallsyms.c:test__vmlinux_matches_kallsyms() {
    ...
	machine__init(&args.kallsyms, "", HOST_KERNEL_ID);
	machine__init(&vmlinux, "", HOST_KERNEL_ID);
    ...
}

If machine__init() fails, it cleans up and leaves machine.kmaps as NULL.

Could this lead to a NULL pointer dereference in functions like
machine__create_kernel_maps() that expect kmaps to be valid?

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

[PATCHES v3 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 get_kernel_version() 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 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/thread-maps-share.c |  2 +-
 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 ++++---
 18 files changed, 120 insertions(+), 39 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/thread-maps-share.c |  2 +-
 tools/perf/util/aslr.c               | 12 +++++++++---
 tools/perf/util/machine.c            | 24 ++++++++++++++----------
 tools/perf/util/machine.h            |  2 +-
 tools/perf/util/session.c            |  7 ++++---
 9 files changed, 37 insertions(+), 22 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/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/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

[PATCHES v2 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 get_kernel_version() 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 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/thread-maps-share.c |  2 +-
 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            |  4 +++-
 18 files changed, 119 insertions(+), 37 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/thread-maps-share.c |  2 +-
 tools/perf/util/aslr.c               | 12 +++++++++---
 tools/perf/util/machine.c            | 24 ++++++++++++++----------
 tools/perf/util/machine.h            |  2 +-
 tools/perf/util/session.c            |  4 +++-
 9 files changed, 36 insertions(+), 20 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/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/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..26bacb6c1a572a62 100644
--- a/tools/perf/util/session.c
+++ b/tools/perf/util/session.c
@@ -160,7 +160,9 @@ 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);
+	if (machines__init(&session->machines))
+		goto out_delete;
+
 	ordered_events__init(&session->ordered_events,
 			     ordered_events__deliver_event, NULL);
 
-- 
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] Calling `perf_session__delete()` on early failure before `perf_env__init()` is called attempts to lock and destroy uninitialized mutexes and rw_semaphores.
--

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

perf machine: Propagate machine__init() error to callers

This commit updates machine__init() to return an error code upon failure
instead of 0. It modifies callers like machines__init() to check the
return value and propagate the error further up the stack.

> diff --git a/tools/perf/util/session.c b/tools/perf/util/session.c
> index 1a9a008ddda35..26bacb6c1a572 100644
> --- a/tools/perf/util/session.c
> +++ b/tools/perf/util/session.c
> @@ -160,7 +160,9 @@ 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);
> +	if (machines__init(&session->machines))
> +		goto out_delete;
> +

[Severity: Medium]
Will jumping to out_delete here cause a crash when cleaning up the
environment?

When this goto is taken, perf_session__delete() is called, which in turn
calls perf_env__exit().

However, perf_env__init(&session->header.env) is only called later in
__perf_session__new(), meaning the environment is still zero-initialized
from the zalloc() of the session.

When perf_env__exit() runs, it does:

perf_env__exit()
    mutex_destroy(&env->lock);
    perf_env__purge_bpf(env);

And perf_env__purge_bpf() calls down_write(&env->bpf_progs.lock).

Could attempting to destroy and lock these uninitialized synchronization
primitives lead to undefined behavior depending on the underlying libc
implementation?

>  	ordered_events__init(&session->ordered_events,
>  			     ordered_events__deliver_event, NULL);
>

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

[PATCHES v1 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 get_kernel_version() 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.

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/thread-maps-share.c |  2 +-
 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            | 23 +++++++++++++---------
 tools/perf/util/machine.h            |  2 +-
 tools/perf/util/session.c            |  4 +++-
 17 files changed, 106 insertions(+), 29 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() 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().

Fixes: 81f981d7ec43ed93 ("perf machine: Free root_dir in machine__init() error path")
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/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/thread-maps-share.c |  2 +-
 tools/perf/util/machine.c            | 15 ++++++++++-----
 tools/perf/util/machine.h            |  2 +-
 tools/perf/util/session.c            |  4 +++-
 8 files changed, 23 insertions(+), 12 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/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/util/machine.c b/tools/perf/util/machine.c
index 31715366e29ff704..7cf20ace3dbb5e55 100644
--- a/tools/perf/util/machine.c
+++ b/tools/perf/util/machine.c
@@ -124,11 +124,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 +138,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 +234,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..26bacb6c1a572a62 100644
--- a/tools/perf/util/session.c
+++ b/tools/perf/util/session.c
@@ -160,7 +160,9 @@ 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);
+	if (machines__init(&session->machines))
+		goto out_delete;
+
 	ordered_events__init(&session->ordered_events,
 			     ordered_events__deliver_event, NULL);
 
-- 
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 2 potential issue(s) to consider:
- [High] Error paths introduced by checking machines__init() result in cleanup routines destroying uninitialized mutexes and rwlocks.
- [High] Missing error handling for the newly propagated return value of machines__init() in aslr_tool__init().
--

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

perf machine: Propagate machine__init() error to callers

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'.

[ ... ]
> diff --git a/tools/perf/tests/hists_cumulate.c b/tools/perf/tests/hists_cumulate.c
> index 267cbc24691ac..9f4d4e281e11e 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;

[Severity: High]
If machines__init() fails here, we jump to out:

out:
	/* tear down everything */
	evlist__delete(evlist);
	machines__exit(&machines);
	put_fake_samples();

Since machines is allocated on the stack and machines__init() can fail before
fully initializing it, will machines__exit() try to clean up uninitialized
state, such as calling down_write() on an uninitialized rwlock?

[ ... ]
> diff --git a/tools/perf/util/session.c b/tools/perf/util/session.c
> index 1a9a008ddda35..26bacb6c1a572 100644
> --- a/tools/perf/util/session.c
> +++ b/tools/perf/util/session.c
> @@ -160,7 +160,9 @@ 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);
> +	if (machines__init(&session->machines))
> +		goto out_delete;
> +

[Severity: High]
If we jump to out_delete here, perf_session__delete(session) is called, which
unconditionally calls perf_env__exit(&session->header.env).

Because this goto happens before perf_env__init(&session->header.env) is called,
could perf_env__exit() end up destroying uninitialized rwlocks and mutexes in
the zero-allocated session->header.env?

[ ... ]

[Severity: High]
Although not modified in this patch, the newly propagated return value of
machines__init() seems to be ignored in tools/perf/util/aslr.c in the
aslr_tool__init() function.

If machines__init() fails there, could aslr_tool__init() silently proceed
with an uninitialized machines structure, leading to crashes when it is
subsequently used or destroyed?

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