* [PATCH 1/9] perf machine: Propagate machine__init() error to callers
2026-06-15 22:32 [PATCHES v2 0/9] perf tools: Fix pre-existing bugs in machine, cs-etm, c2c, bpf, and dso Arnaldo Carvalho de Melo
@ 2026-06-15 22:32 ` Arnaldo Carvalho de Melo
2026-06-15 22:52 ` sashiko-bot
2026-06-15 22:32 ` [PATCH 2/9] perf machine: Use snprintf() for guestmount path construction Arnaldo Carvalho de Melo
` (7 subsequent siblings)
8 siblings, 1 reply; 20+ messages in thread
From: Arnaldo Carvalho de Melo @ 2026-06-15 22:32 UTC (permalink / raw)
To: Namhyung Kim
Cc: Ingo Molnar, Thomas Gleixner, James Clark, Jiri Olsa, Ian Rogers,
Adrian Hunter, Clark Williams, linux-kernel, linux-perf-users,
Arnaldo Carvalho de Melo, sashiko-bot, Claude
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
^ permalink raw reply related [flat|nested] 20+ messages in thread* Re: [PATCH 1/9] perf machine: Propagate machine__init() error to callers
2026-06-15 22:32 ` [PATCH 1/9] perf machine: Propagate machine__init() error to callers Arnaldo Carvalho de Melo
@ 2026-06-15 22:52 ` sashiko-bot
0 siblings, 0 replies; 20+ messages in thread
From: sashiko-bot @ 2026-06-15 22:52 UTC (permalink / raw)
To: Arnaldo Carvalho de Melo; +Cc: linux-perf-users
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
^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH 2/9] perf machine: Use snprintf() for guestmount path construction
2026-06-15 22:32 [PATCHES v2 0/9] perf tools: Fix pre-existing bugs in machine, cs-etm, c2c, bpf, and dso Arnaldo Carvalho de Melo
2026-06-15 22:32 ` [PATCH 1/9] perf machine: Propagate machine__init() error to callers Arnaldo Carvalho de Melo
@ 2026-06-15 22:32 ` Arnaldo Carvalho de Melo
2026-06-15 22:32 ` [PATCH 3/9] perf cs-etm: Validate num_cpu before metadata allocation Arnaldo Carvalho de Melo
` (6 subsequent siblings)
8 siblings, 0 replies; 20+ messages in thread
From: Arnaldo Carvalho de Melo @ 2026-06-15 22:32 UTC (permalink / raw)
To: Namhyung Kim
Cc: Ingo Molnar, Thomas Gleixner, James Clark, Jiri Olsa, Ian Rogers,
Adrian Hunter, Clark Williams, linux-kernel, linux-perf-users,
Arnaldo Carvalho de Melo, sashiko-bot, Zhang, Yanmin, Claude
From: Arnaldo Carvalho de Melo <acme@redhat.com>
machines__findnew(), machines__create_guest_kernel_maps(), and
get_kernel_version() use sprintf() to build paths by prepending
symbol_conf.guestmount or root_dir. All 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 — and a truncated root_dir propagated to
get_kernel_version() would overflow its own version[PATH_MAX] buffer
when "/proc/version" is appended.
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
^ permalink raw reply related [flat|nested] 20+ messages in thread* [PATCH 3/9] perf cs-etm: Validate num_cpu before metadata allocation
2026-06-15 22:32 [PATCHES v2 0/9] perf tools: Fix pre-existing bugs in machine, cs-etm, c2c, bpf, and dso Arnaldo Carvalho de Melo
2026-06-15 22:32 ` [PATCH 1/9] perf machine: Propagate machine__init() error to callers Arnaldo Carvalho de Melo
2026-06-15 22:32 ` [PATCH 2/9] perf machine: Use snprintf() for guestmount path construction Arnaldo Carvalho de Melo
@ 2026-06-15 22:32 ` Arnaldo Carvalho de Melo
2026-06-15 22:48 ` sashiko-bot
2026-06-15 22:32 ` [PATCH 4/9] perf cs-etm: Require full global header in auxtrace_info size check Arnaldo Carvalho de Melo
` (5 subsequent siblings)
8 siblings, 1 reply; 20+ messages in thread
From: Arnaldo Carvalho de Melo @ 2026-06-15 22:32 UTC (permalink / raw)
To: Namhyung Kim
Cc: Ingo Molnar, Thomas Gleixner, James Clark, Jiri Olsa, Ian Rogers,
Adrian Hunter, Clark Williams, linux-kernel, linux-perf-users,
Arnaldo Carvalho de Melo, sashiko-bot, James Clark, Leo Yan,
Tor Jeremiassen, Claude
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
^ permalink raw reply related [flat|nested] 20+ messages in thread* Re: [PATCH 3/9] perf cs-etm: Validate num_cpu before metadata allocation
2026-06-15 22:32 ` [PATCH 3/9] perf cs-etm: Validate num_cpu before metadata allocation Arnaldo Carvalho de Melo
@ 2026-06-15 22:48 ` sashiko-bot
0 siblings, 0 replies; 20+ messages in thread
From: sashiko-bot @ 2026-06-15 22:48 UTC (permalink / raw)
To: Arnaldo Carvalho de Melo; +Cc: linux-perf-users
Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
Pre-existing issues:
- [Medium] Insufficient bounds check for the CPU metadata parsing loop leading to out-of-bounds memory reads from untrusted data.
- [Medium] Missing bounds validation on `num_cpu` before printing trace metadata causes out-of-bounds memory reads.
- [Medium] Memory leak of `cs_etm_queue` structures and RB trees on initialization error paths.
--
commit d0500b50aa50740d3a89612a4e4d97ba8f0c3129
Author: Arnaldo Carvalho de Melo <acme@redhat.com>
perf cs-etm: Validate num_cpu before metadata allocation
This commit bounds num_cpu by checking it against the total available private
data size to prevent an integer overflow and subsequent out-of-bounds writes
during metadata allocation.
> 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;
[Severity: Medium]
This is a pre-existing issue, but does dump_trace in
cs_etm__process_auxtrace_info() extract and use num_cpu for
cs_etm__print_auxtrace_info() before this new validation occurs?
tools/perf/util/cs-etm-base.c:cs_etm__process_auxtrace_info() {
...
if (dump_trace) {
num_cpu = ptr[CS_PMU_TYPE_CPUS] & 0xffffffff;
cs_etm__print_auxtrace_info(ptr, num_cpu);
...
If so, an attacker could still trigger out-of-bounds reads by specifying an
excessively large num_cpu when perf report -D is run.
> +
> + /*
> + * 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: Medium]
This is a pre-existing issue, but does the metadata parsing loop later in this
function lack a bounds check for the buffer size?
While num_cpu is now bounded, the loop iterates j < num_cpu and calls
cs_etm__create_meta_blk() which advances the index i:
tools/perf/util/cs-etm.c:cs_etm__create_meta_blk() {
...
nr_in_params = buff_in[i + CS_ETM_NR_TRC_PARAMS];
...
i += nr_in_params + nr_cmn_params;
*buff_in_offset = i;
...
Can a crafted nr_in_params bypass the new bounds check and cause an
out-of-bounds read on the next iteration?
> metadata = zalloc(sizeof(*metadata) * num_cpu);
> if (!metadata)
> return -ENOMEM;
[Severity: Medium]
This is a pre-existing issue, but does the err_free_queues path in this
function leak the queue->priv allocations?
If an error occurs later in cs_etm__process_auxtrace_info_full(), it cleans
up via:
tools/perf/util/cs-etm.c:cs_etm__process_auxtrace_info_full() {
...
err_free_queues:
auxtrace_queues__free(&etm->queues);
...
auxtrace_queues__free() frees the arrays but doesn't free the dynamically
allocated cs_etm_queue structures stored in queue->priv from
cs_etm__setup_queue(). Is an additional cs_etm__free_events() style cleanup
needed to prevent memory leaks?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260615223249.36598-1-acme@kernel.org?part=3
^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH 4/9] perf cs-etm: Require full global header in auxtrace_info size check
2026-06-15 22:32 [PATCHES v2 0/9] perf tools: Fix pre-existing bugs in machine, cs-etm, c2c, bpf, and dso Arnaldo Carvalho de Melo
` (2 preceding siblings ...)
2026-06-15 22:32 ` [PATCH 3/9] perf cs-etm: Validate num_cpu before metadata allocation Arnaldo Carvalho de Melo
@ 2026-06-15 22:32 ` Arnaldo Carvalho de Melo
2026-06-15 22:46 ` sashiko-bot
2026-06-15 22:32 ` [PATCH 5/9] perf cs-etm: Bounds-check CPU in cs_etm__get_queue() Arnaldo Carvalho de Melo
` (4 subsequent siblings)
8 siblings, 1 reply; 20+ messages in thread
From: Arnaldo Carvalho de Melo @ 2026-06-15 22:32 UTC (permalink / raw)
To: Namhyung Kim
Cc: Ingo Molnar, Thomas Gleixner, James Clark, Jiri Olsa, Ian Rogers,
Adrian Hunter, Clark Williams, linux-kernel, linux-perf-users,
Arnaldo Carvalho de Melo, sashiko-bot, James Clark, Leo Yan,
Claude
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
^ permalink raw reply related [flat|nested] 20+ messages in thread* Re: [PATCH 4/9] perf cs-etm: Require full global header in auxtrace_info size check
2026-06-15 22:32 ` [PATCH 4/9] perf cs-etm: Require full global header in auxtrace_info size check Arnaldo Carvalho de Melo
@ 2026-06-15 22:46 ` sashiko-bot
0 siblings, 0 replies; 20+ messages in thread
From: sashiko-bot @ 2026-06-15 22:46 UTC (permalink / raw)
To: Arnaldo Carvalho de Melo; +Cc: linux-perf-users
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
Pre-existing issues:
- [High] Insufficient bounds checking for per-CPU metadata allows out-of-bounds reads when parsing and printing crafted auxtrace_info events.
--
commit 3efe458a25dcafb887e671334329fd63dbfdb114
Author: Arnaldo Carvalho de Melo <acme@redhat.com>
perf cs-etm: Require full global header in auxtrace_info size check
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 are guaranteed to fit.
> 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 is a pre-existing issue, but I have a question about the subsequent
parsing of the event payload in cs_etm__process_auxtrace_info().
When dump_trace is enabled, it extracts num_cpu from the payload and passes
it to cs_etm__print_auxtrace_info():
tools/perf/util/cs-etm-base.c:cs_etm__process_auxtrace_info() {
...
if (dump_trace) {
num_cpu = ptr[CS_PMU_TYPE_CPUS] & 0xffffffff;
cs_etm__print_auxtrace_info(ptr, num_cpu);
}
...
}
Does cs_etm__print_auxtrace_info() or its children validate that the event
is actually large enough to contain this many CPUs?
It looks like cs_etm__print_auxtrace_info() iterates num_cpu times, calling
cs_etm__print_cpu_metadata_v1(), which accesses array elements directly:
tools/perf/util/cs-etm-base.c:cs_etm__print_cpu_metadata_v1() {
...
magic = val[i + CS_ETM_MAGIC];
/* total params to print is NR_PARAMS + common block size for v1 */
total_params = val[i + CS_ETM_NR_TRC_PARAMS] + CS_ETM_COMMON_BLK_MAX_V1;
...
}
Since i is advanced based on the payload-provided parameter count, could
these operations read out-of-bounds if a crafted auxtrace_info event has a
large num_cpu but a short payload?
Additionally, a similar bounds issue seems to exist in the non-dump_trace path
in cs_etm__process_auxtrace_info_full(). While it verifies there is at least
one u64 per CPU, the loop processing each CPU reads further into the buffer:
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) {
...
err = cs_etm__create_meta_blk(ptr, &i,
...
}
Where cs_etm__create_meta_blk() accesses multiple elements without checking
against the total event size:
tools/perf/util/cs-etm.c:cs_etm__create_meta_blk() {
...
nr_in_params = buff_in[i + CS_ETM_NR_TRC_PARAMS];
...
}
The strict event size check (i * 8 != priv_size) in
cs_etm__process_auxtrace_info_full() only occurs after the loop finishes.
Would it be safer to check the current offset i and the required size against
auxtrace_info->header.size before reading elements from ptr[] or buff_in[]?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260615223249.36598-1-acme@kernel.org?part=4
^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH 5/9] perf cs-etm: Bounds-check CPU in cs_etm__get_queue()
2026-06-15 22:32 [PATCHES v2 0/9] perf tools: Fix pre-existing bugs in machine, cs-etm, c2c, bpf, and dso Arnaldo Carvalho de Melo
` (3 preceding siblings ...)
2026-06-15 22:32 ` [PATCH 4/9] perf cs-etm: Require full global header in auxtrace_info size check Arnaldo Carvalho de Melo
@ 2026-06-15 22:32 ` Arnaldo Carvalho de Melo
2026-06-15 22:54 ` sashiko-bot
2026-06-15 22:32 ` [PATCH 6/9] perf c2c: Free format list entries when c2c_hists__init() fails Arnaldo Carvalho de Melo
` (3 subsequent siblings)
8 siblings, 1 reply; 20+ messages in thread
From: Arnaldo Carvalho de Melo @ 2026-06-15 22:32 UTC (permalink / raw)
To: Namhyung Kim
Cc: Ingo Molnar, Thomas Gleixner, James Clark, Jiri Olsa, Ian Rogers,
Adrian Hunter, Clark Williams, linux-kernel, linux-perf-users,
Arnaldo Carvalho de Melo, sashiko-bot, James Clark, Leo Yan,
Claude
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
^ permalink raw reply related [flat|nested] 20+ messages in thread* Re: [PATCH 5/9] perf cs-etm: Bounds-check CPU in cs_etm__get_queue()
2026-06-15 22:32 ` [PATCH 5/9] perf cs-etm: Bounds-check CPU in cs_etm__get_queue() Arnaldo Carvalho de Melo
@ 2026-06-15 22:54 ` sashiko-bot
0 siblings, 0 replies; 20+ messages in thread
From: sashiko-bot @ 2026-06-15 22:54 UTC (permalink / raw)
To: Arnaldo Carvalho de Melo; +Cc: linux-perf-users
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
Pre-existing issues:
- [High] Missing NULL checks on `etmq` and `etmq->decoder` lead to segmentation faults on malformed or out-of-bounds auxtrace queues.
--
commit 97b84b25fe94e957ad4cb91dac4d3e8d49fe5b1d
Author: Arnaldo Carvalho de Melo <acme@redhat.com>
perf cs-etm: Bounds-check CPU in cs_etm__get_queue()
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.
> 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: High]
This is a pre-existing issue, but since we are adding NULL checks for the priv
field here because auxtrace_queues__grow() zero-initializes new entries, are
there other places that might still be vulnerable to this?
For example, in cs_etm__process_auxtrace_event(), we index into the queue array
and pass the priv field directly to cs_etm__dump_event():
err = auxtrace_queues__add_event(&etm->queues, session,
event, data_offset, &buffer);
if (err)
return err;
if (dump_trace)
if (auxtrace_buffer__get_data(buffer, fd)) {
cs_etm__dump_event(etm->queues.queue_array[idx].priv, buffer);
auxtrace_buffer__put_data(buffer);
}
Then cs_etm__dump_event() unconditionally dereferences etmq->decoder, which
causes a segmentation fault if etmq is NULL:
static void cs_etm__dump_event(struct cs_etm_queue *etmq,
struct auxtrace_buffer *buffer)
{
...
color_fprintf(stdout, color,
". ... CoreSight %s Trace data: size %#zx bytes\n",
cs_etm_decoder__get_name(etmq->decoder), buffer->size);
Additionally, if the queue is empty during decoder creation, etmq->decoder can
be NULL. Later in cs_etm__decode_data_block(), etmq->decoder is passed without
a NULL check:
ret = cs_etm_decoder__process_data_block(etmq->decoder,
etmq->offset,
Could this be triggered by a malformed or out-of-bounds auxtrace index,
resulting in a crash?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260615223249.36598-1-acme@kernel.org?part=5
^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH 6/9] perf c2c: Free format list entries when c2c_hists__init() fails
2026-06-15 22:32 [PATCHES v2 0/9] perf tools: Fix pre-existing bugs in machine, cs-etm, c2c, bpf, and dso Arnaldo Carvalho de Melo
` (4 preceding siblings ...)
2026-06-15 22:32 ` [PATCH 5/9] perf cs-etm: Bounds-check CPU in cs_etm__get_queue() Arnaldo Carvalho de Melo
@ 2026-06-15 22:32 ` Arnaldo Carvalho de Melo
2026-06-15 22:32 ` [PATCH 7/9] perf c2c: Fix hist entry and format list leaks in c2c_he_free() Arnaldo Carvalho de Melo
` (2 subsequent siblings)
8 siblings, 0 replies; 20+ messages in thread
From: Arnaldo Carvalho de Melo @ 2026-06-15 22:32 UTC (permalink / raw)
To: Namhyung Kim
Cc: Ingo Molnar, Thomas Gleixner, James Clark, Jiri Olsa, Ian Rogers,
Adrian Hunter, Clark Williams, linux-kernel, linux-perf-users,
Arnaldo Carvalho de Melo, sashiko-bot, Claude
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
^ permalink raw reply related [flat|nested] 20+ messages in thread* [PATCH 7/9] perf c2c: Fix hist entry and format list leaks in c2c_he_free()
2026-06-15 22:32 [PATCHES v2 0/9] perf tools: Fix pre-existing bugs in machine, cs-etm, c2c, bpf, and dso Arnaldo Carvalho de Melo
` (5 preceding siblings ...)
2026-06-15 22:32 ` [PATCH 6/9] perf c2c: Free format list entries when c2c_hists__init() fails Arnaldo Carvalho de Melo
@ 2026-06-15 22:32 ` Arnaldo Carvalho de Melo
2026-06-15 23:04 ` sashiko-bot
2026-06-15 22:32 ` [PATCH 8/9] perf bpf: Validate array presence before casting BPF prog info pointers Arnaldo Carvalho de Melo
2026-06-15 22:32 ` [PATCH 9/9] perf dso: Set standard errno on decompression failure Arnaldo Carvalho de Melo
8 siblings, 1 reply; 20+ messages in thread
From: Arnaldo Carvalho de Melo @ 2026-06-15 22:32 UTC (permalink / raw)
To: Namhyung Kim
Cc: Ingo Molnar, Thomas Gleixner, James Clark, Jiri Olsa, Ian Rogers,
Adrian Hunter, Clark Williams, linux-kernel, linux-perf-users,
Arnaldo Carvalho de Melo, sashiko-bot, Claude
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
^ permalink raw reply related [flat|nested] 20+ messages in thread* Re: [PATCH 7/9] perf c2c: Fix hist entry and format list leaks in c2c_he_free()
2026-06-15 22:32 ` [PATCH 7/9] perf c2c: Fix hist entry and format list leaks in c2c_he_free() Arnaldo Carvalho de Melo
@ 2026-06-15 23:04 ` sashiko-bot
0 siblings, 0 replies; 20+ messages in thread
From: sashiko-bot @ 2026-06-15 23:04 UTC (permalink / raw)
To: Arnaldo Carvalho de Melo; +Cc: linux-perf-users
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
Pre-existing issues:
- [Medium] perf_hpp__setup_output_field() and perf_hpp__append_sort_keys() append formats to the global perf_hpp_list instead of the provided local list, polluting global state and causing missing output columns.
--
commit a4f0bc9a81bce084555c8dfe98ae2f6675da0ef8
Author: Arnaldo Carvalho de Melo <acme@redhat.com>
perf c2c: Fix hist entry and format list leaks in c2c_he_free()
This commit addresses leaks in c2c_he_free() by replacing
hists__delete_entries() with hists__delete_all_entries() to walk and free
all inner hist_entry objects across all rb_root trees. It also adds a call
to perf_hpp__reset_output_field() to unregister and free dynamically
allocated format entries on the hists->list.
> diff --git a/tools/perf/builtin-c2c.c b/tools/perf/builtin-c2c.c
> index eabb922ef295e..c9584dbedf77a 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);
[Severity: Medium]
This isn't a bug introduced by this patch, but does the setup for these local
hpp lists append formats to the global perf_hpp_list by mistake?
Looking at perf_hpp__setup_output_field() in tools/perf/ui/hist.c, it takes a
local list argument but uses the global wrapper perf_hpp__column_register(fmt)
instead of perf_hpp_list__column_register(list, fmt).
A similar issue appears in perf_hpp__append_sort_keys() in the same file,
which uses perf_hpp__register_sort_field(fmt) instead of
perf_hpp_list__register_sort_field(list, fmt).
Does this inject sort formats into the global list (polluting global state
during processing) while leaving the local list missing those columns and
breaking the output display for inner hists?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260615223249.36598-1-acme@kernel.org?part=7
^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH 8/9] perf bpf: Validate array presence before casting BPF prog info pointers
2026-06-15 22:32 [PATCHES v2 0/9] perf tools: Fix pre-existing bugs in machine, cs-etm, c2c, bpf, and dso Arnaldo Carvalho de Melo
` (6 preceding siblings ...)
2026-06-15 22:32 ` [PATCH 7/9] perf c2c: Fix hist entry and format list leaks in c2c_he_free() Arnaldo Carvalho de Melo
@ 2026-06-15 22:32 ` Arnaldo Carvalho de Melo
2026-06-15 23:01 ` sashiko-bot
2026-06-15 22:32 ` [PATCH 9/9] perf dso: Set standard errno on decompression failure Arnaldo Carvalho de Melo
8 siblings, 1 reply; 20+ messages in thread
From: Arnaldo Carvalho de Melo @ 2026-06-15 22:32 UTC (permalink / raw)
To: Namhyung Kim
Cc: Ingo Molnar, Thomas Gleixner, James Clark, Jiri Olsa, Ian Rogers,
Adrian Hunter, Clark Williams, linux-kernel, linux-perf-users,
Arnaldo Carvalho de Melo, sashiko-bot, Song Liu, Claude
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 ++++++++++
| 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);
--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
^ permalink raw reply related [flat|nested] 20+ messages in thread* Re: [PATCH 8/9] perf bpf: Validate array presence before casting BPF prog info pointers
2026-06-15 22:32 ` [PATCH 8/9] perf bpf: Validate array presence before casting BPF prog info pointers Arnaldo Carvalho de Melo
@ 2026-06-15 23:01 ` sashiko-bot
0 siblings, 0 replies; 20+ messages in thread
From: sashiko-bot @ 2026-06-15 23:01 UTC (permalink / raw)
To: Arnaldo Carvalho de Melo; +Cc: linux-perf-users
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
Pre-existing issues:
- [High] The newly added array bitmask validation in `__bpf_event__print_bpf_prog_info()` is incomplete, missing critical validations for `PERF_BPIL_PROG_TAGS` and `PERF_BPIL_FUNC_INFO` arrays.
- [High] The libbfd annotation path completely misses the newly added array bitmask validations, allowing arbitrary file offsets for `jited_prog_insns`, `line_info`, and `jited_line_info` to be dereferenced.
--
commit 0cf9a385d10d4f34d342db1cd59a01ae41698542
Author: Arnaldo Carvalho de Melo <acme@redhat.com>
perf bpf: Validate array presence before casting BPF prog info pointers
This commit adds bitmask validations to ensure that BPF program info fields
like jited_ksyms and jited_func_lens are actually valid pointers before
they are dereferenced. It aims to prevent issues where crafted perf.data
files leave these bits unset, causing functions to dereference raw file
offsets.
> 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:
[Severity: High]
This is a pre-existing issue, but does the libbfd annotation path also need
similar validation?
The commit message states validations are added to all perf.data processing
paths, but symbol__disassemble_bpf_libbfd() in tools/perf/util/libbfd.c
accesses info_linear->info.jited_prog_insns and passes it to
bpf_prog_linfo__new().
bpf_prog_linfo__new() then casts info->line_info and info->jited_line_info
to pointers and calls memcpy():
tools/lib/bpf/bpf_prog_linfo.c:bpf_prog_linfo__new() {
...
memcpy(prog_linfo->raw_linfo, (void *)(long)info->line_info, data_sz);
...
memcpy(prog_linfo->raw_jited_linfo,
(void *)(long)info->jited_line_info, data_sz);
...
}
Can a crafted perf.data file omitting PERF_BPIL_JITED_INSNS,
PERF_BPIL_LINE_INFO, or PERF_BPIL_JITED_LINE_INFO bits cause an
out-of-bounds read or segmentation fault here?
> - __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
[ ... ]
> 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 is a pre-existing issue, but is this required_arrays bitmask missing
validations for PERF_BPIL_PROG_TAGS and PERF_BPIL_FUNC_INFO?
__bpf_event__print_bpf_prog_info() calls synthesize_bpf_prog_name(), which
casts info->prog_tags and info->func_info to pointers and dereferences them:
tools/perf/util/bpf-event.c:synthesize_bpf_prog_name() {
...
u8 (*prog_tags)[BPF_TAG_SIZE] = (void *)(uintptr_t)(info->prog_tags);
void *func_infos = (void *)(uintptr_t)(info->func_info);
...
name_len += snprintf_hex(buf + name_len, size - name_len,
prog_tags[sub_id], BPF_TAG_SIZE);
...
}
If a crafted perf.data file leaves the PERF_BPIL_PROG_TAGS and
PERF_BPIL_FUNC_INFO bits unset but provides non-zero offset values, will
untranslated raw offsets be dereferenced here?
> + __u32 *prog_lens;
> + __u64 *prog_addrs;
> char name[KSYM_NAME_LEN];
> struct btf *btf = NULL;
> u32 sub_prog_cnt, i;
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260615223249.36598-1-acme@kernel.org?part=8
^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH 9/9] perf dso: Set standard errno on decompression failure
2026-06-15 22:32 [PATCHES v2 0/9] perf tools: Fix pre-existing bugs in machine, cs-etm, c2c, bpf, and dso Arnaldo Carvalho de Melo
` (7 preceding siblings ...)
2026-06-15 22:32 ` [PATCH 8/9] perf bpf: Validate array presence before casting BPF prog info pointers Arnaldo Carvalho de Melo
@ 2026-06-15 22:32 ` Arnaldo Carvalho de Melo
8 siblings, 0 replies; 20+ messages in thread
From: Arnaldo Carvalho de Melo @ 2026-06-15 22:32 UTC (permalink / raw)
To: Namhyung Kim
Cc: Ingo Molnar, Thomas Gleixner, James Clark, Jiri Olsa, Ian Rogers,
Adrian Hunter, Clark Williams, linux-kernel, linux-perf-users,
Arnaldo Carvalho de Melo, sashiko-bot, Claude
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
^ permalink raw reply related [flat|nested] 20+ messages in thread