From: Arnaldo Carvalho de Melo <acme@kernel.org>
To: Namhyung Kim <namhyung@kernel.org>
Cc: Ingo Molnar <mingo@kernel.org>,
Thomas Gleixner <tglx@linutronix.de>,
James Clark <james.clark@linaro.org>,
Jiri Olsa <jolsa@kernel.org>, Ian Rogers <irogers@google.com>,
Adrian Hunter <adrian.hunter@intel.com>,
Clark Williams <williams@redhat.com>,
linux-kernel@vger.kernel.org, linux-perf-users@vger.kernel.org,
Arnaldo Carvalho de Melo <acme@redhat.com>,
sashiko-bot@kernel.org, Blake Jones <blakejones@google.com>,
"Claude Opus 4.6 (1M context)" <noreply@anthropic.com>
Subject: [PATCH 15/29] perf session: Add byte-swap and bounds check for PERF_RECORD_BPF_METADATA events
Date: Sun, 24 May 2026 22:05:35 -0300 [thread overview]
Message-ID: <20260525010550.1100375-16-acme@kernel.org> (raw)
In-Reply-To: <20260525010550.1100375-1-acme@kernel.org>
From: Arnaldo Carvalho de Melo <acme@redhat.com>
PERF_RECORD_BPF_METADATA has no entry in perf_event__swap_ops[], so its
nr_entries field is never byte-swapped when reading a cross-endian
perf.data file. Downstream processing in
perf_event__fprintf_bpf_metadata() loops over nr_entries, so a
foreign-endian value causes out-of-bounds reads.
Add a swap handler that byte-swaps nr_entries after validating that
header.size is large enough. The entries[] array contains only char
arrays (key/value strings), so no per-entry swap is needed — but ensure
NUL-termination on the writable cross-endian path.
Validate header.size, nr_entries, and string NUL-termination in the
common event delivery path so that native-endian files with malicious
values are also rejected. Snapshot nr_entries via READ_ONCE() before
validation — the event is on a MAP_SHARED mmap that could theoretically
change between the bounds check and the loop.
Changes in v2:
- Snapshot event->header.size via READ_ONCE() into a local variable
to prevent a double-fetch underflow in the max_entries calculation
(Reported-by: sashiko-bot@kernel.org)
- Write back clamped nr_entries to the event on the swap path,
consistent with NAMESPACES and STAT_CONFIG handlers — without
writeback the native path sees the inflated nr and skips the
event entirely (Reported-by: sashiko-bot@kernel.org)
Fixes: ab38e84ba9a8 ("perf record: collect BPF metadata from existing BPF programs")
Reported-by: sashiko-bot@kernel.org # Running on a local machine
Cc: Blake Jones <blakejones@google.com>
Cc: Ian Rogers <irogers@google.com>
Cc: Jiri Olsa <jolsa@kernel.org>
Cc: Namhyung Kim <namhyung@kernel.org>
Assisted-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
tools/perf/util/session.c | 89 ++++++++++++++++++++++++++++++++++++++-
1 file changed, 88 insertions(+), 1 deletion(-)
diff --git a/tools/perf/util/session.c b/tools/perf/util/session.c
index 092fccbea8f8017e..95eb793026de6d8d 100644
--- a/tools/perf/util/session.c
+++ b/tools/perf/util/session.c
@@ -961,6 +961,48 @@ static int perf_event__time_conv_swap(union perf_event *event,
return 0;
}
+static int perf_event__bpf_metadata_swap(union perf_event *event,
+ bool sample_id_all __maybe_unused)
+{
+ u64 i, nr, max_nr;
+
+ /* Fixed header must fit before accessing nr_entries or prog_name */
+ if (event->header.size < sizeof(event->bpf_metadata))
+ return -1;
+
+ event->bpf_metadata.nr_entries = bswap_64(event->bpf_metadata.nr_entries);
+
+ /*
+ * Ensure NUL-termination on the cross-endian path where the
+ * mapping is writable (MAP_PRIVATE + PROT_WRITE). Fixing
+ * the string in place is preferred over rejecting because it
+ * preserves the event for downstream processing — only the
+ * last byte is lost.
+ *
+ * The native-endian path (MAP_SHARED + PROT_READ) cannot
+ * write, so it validates and skips unterminated events in
+ * perf_session__process_user_event() instead. The two
+ * strategies produce different outcomes for the same
+ * malformed input (fix vs skip), which is inherent in the
+ * writable-vs-read-only mapping model.
+ */
+ event->bpf_metadata.prog_name[BPF_PROG_NAME_LEN - 1] = '\0';
+
+ nr = event->bpf_metadata.nr_entries;
+ max_nr = (event->header.size - sizeof(event->bpf_metadata)) /
+ sizeof(event->bpf_metadata.entries[0]);
+ if (nr > max_nr) {
+ /* Persist clamped value so the native path processes entries, not skips */
+ nr = max_nr;
+ event->bpf_metadata.nr_entries = nr;
+ }
+
+ for (i = 0; i < nr; i++) {
+ event->bpf_metadata.entries[i].key[BPF_METADATA_KEY_LEN - 1] = '\0';
+ event->bpf_metadata.entries[i].value[BPF_METADATA_VALUE_LEN - 1] = '\0';
+ }
+ return 0;
+}
static int
perf_event__schedstat_cpu_swap(union perf_event *event __maybe_unused,
bool sample_id_all __maybe_unused)
@@ -1060,6 +1102,7 @@ static perf_event__swap_op perf_event__swap_ops[] = {
[PERF_RECORD_STAT_ROUND] = perf_event__stat_round_swap,
[PERF_RECORD_EVENT_UPDATE] = perf_event__event_update_swap,
[PERF_RECORD_TIME_CONV] = perf_event__time_conv_swap,
+ [PERF_RECORD_BPF_METADATA] = perf_event__bpf_metadata_swap,
[PERF_RECORD_SCHEDSTAT_CPU] = perf_event__schedstat_cpu_swap,
[PERF_RECORD_SCHEDSTAT_DOMAIN] = perf_event__schedstat_domain_swap,
[PERF_RECORD_HEADER_MAX] = NULL,
@@ -2203,9 +2246,53 @@ static s64 perf_session__process_user_event(struct perf_session *session,
case PERF_RECORD_FINISHED_INIT:
err = tool->finished_init(tool, session, event);
break;
- case PERF_RECORD_BPF_METADATA:
+ case PERF_RECORD_BPF_METADATA: {
+ u64 nr_entries, max_entries;
+ u32 hdr_size = READ_ONCE(event->header.size);
+
+ if (hdr_size < sizeof(event->bpf_metadata)) {
+ pr_warning("WARNING: PERF_RECORD_BPF_METADATA: header.size (%u) too small, skipping\n",
+ hdr_size);
+ err = 0;
+ break;
+ }
+
+ /*
+ * Native-endian files are mmap'd read-only — validate
+ * NUL-termination instead of writing.
+ */
+ if (strnlen(event->bpf_metadata.prog_name,
+ BPF_PROG_NAME_LEN) == BPF_PROG_NAME_LEN) {
+ pr_warning("WARNING: PERF_RECORD_BPF_METADATA: prog_name not null-terminated, skipping\n");
+ err = 0;
+ break;
+ }
+
+ /* Snapshot — event is mmap'd and could change between reads */
+ nr_entries = READ_ONCE(event->bpf_metadata.nr_entries);
+ max_entries = (hdr_size - sizeof(event->bpf_metadata)) /
+ sizeof(event->bpf_metadata.entries[0]);
+ if (nr_entries > max_entries) {
+ pr_warning("WARNING: PERF_RECORD_BPF_METADATA: nr_entries %" PRIu64 " exceeds max %" PRIu64 ", skipping\n",
+ nr_entries, max_entries);
+ err = 0;
+ break;
+ }
+
+ for (u64 i = 0; i < nr_entries; i++) {
+ if (strnlen(event->bpf_metadata.entries[i].key,
+ BPF_METADATA_KEY_LEN) == BPF_METADATA_KEY_LEN ||
+ strnlen(event->bpf_metadata.entries[i].value,
+ BPF_METADATA_VALUE_LEN) == BPF_METADATA_VALUE_LEN) {
+ pr_warning("WARNING: PERF_RECORD_BPF_METADATA: entry %" PRIu64 " key/value not null-terminated, skipping\n", i);
+ err = 0;
+ goto out;
+ }
+ }
+
err = tool->bpf_metadata(tool, session, event);
break;
+ }
case PERF_RECORD_SCHEDSTAT_CPU:
err = tool->schedstat_cpu(tool, session, event);
break;
--
2.54.0
next prev parent reply other threads:[~2026-05-25 1:06 UTC|newest]
Thread overview: 49+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-05-25 1:05 [PATCHES v3 00/29] perf: Harden perf.data parsing against crafted/corrupted files Arnaldo Carvalho de Melo
2026-05-25 1:05 ` [PATCH 01/29] perf session: Add minimum event size and alignment validation Arnaldo Carvalho de Melo
2026-05-25 1:45 ` sashiko-bot
2026-05-25 1:05 ` [PATCH 02/29] perf session: Bounds-check one_mmap event pointer in peek_event Arnaldo Carvalho de Melo
2026-05-25 1:41 ` sashiko-bot
2026-05-25 1:05 ` [PATCH 03/29] perf tools: Fix event_contains() macro to verify full field extent Arnaldo Carvalho de Melo
2026-05-25 1:05 ` [PATCH 04/29] perf zstd: Fix compression error path in zstd_compress_stream_to_records() Arnaldo Carvalho de Melo
2026-05-25 1:52 ` sashiko-bot
2026-05-25 1:05 ` [PATCH 05/29] perf zstd: Fix multi-iteration decompression and error handling Arnaldo Carvalho de Melo
2026-05-25 1:05 ` [PATCH 06/29] perf session: Fix PERF_RECORD_READ swap and dump for variable-length events Arnaldo Carvalho de Melo
2026-05-25 1:05 ` [PATCH 07/29] perf session: Fix swap_sample_id_all() crash on crafted events Arnaldo Carvalho de Melo
2026-05-25 1:05 ` [PATCH 08/29] perf session: Add validated swap infrastructure with null-termination checks Arnaldo Carvalho de Melo
2026-05-25 1:05 ` [PATCH 09/29] perf session: Use bounded copy for PERF_RECORD_TIME_CONV Arnaldo Carvalho de Melo
2026-05-25 1:05 ` [PATCH 10/29] perf session: Validate HEADER_ATTR attr.size before swapping Arnaldo Carvalho de Melo
2026-05-25 1:56 ` sashiko-bot
2026-05-25 1:05 ` [PATCH 11/29] perf session: Validate nr fields against event size on both swap and common paths Arnaldo Carvalho de Melo
2026-05-25 1:05 ` [PATCH 12/29] perf header: Byte-swap build ID event pid and bounds check section entries Arnaldo Carvalho de Melo
2026-05-25 1:05 ` [PATCH 13/29] perf cpumap: Reject RANGE_CPUS with start_cpu > end_cpu Arnaldo Carvalho de Melo
2026-05-25 1:40 ` sashiko-bot
2026-05-25 1:05 ` [PATCH 14/29] perf auxtrace: Harden auxtrace_error event handling Arnaldo Carvalho de Melo
2026-05-25 1:05 ` Arnaldo Carvalho de Melo [this message]
2026-05-25 1:54 ` [PATCH 15/29] perf session: Add byte-swap and bounds check for PERF_RECORD_BPF_METADATA events sashiko-bot
2026-05-25 1:05 ` [PATCH 16/29] perf header: Validate null-termination in PERF_RECORD_EVENT_UPDATE string fields Arnaldo Carvalho de Melo
2026-05-25 1:05 ` [PATCH 17/29] perf tools: Bounds check perf_event_attr fields against attr.size before printing Arnaldo Carvalho de Melo
2026-05-25 1:59 ` sashiko-bot
2026-05-25 1:05 ` [PATCH 18/29] perf header: Propagate feature section processing errors Arnaldo Carvalho de Melo
2026-05-25 3:01 ` sashiko-bot
2026-05-25 1:05 ` [PATCH 19/29] perf header: Validate f_attr.ids section before use in perf_session__read_header() Arnaldo Carvalho de Melo
2026-05-25 2:39 ` sashiko-bot
2026-05-25 1:05 ` [PATCH 20/29] perf header: Validate feature section size and add read path bounds checking Arnaldo Carvalho de Melo
2026-05-25 1:05 ` [PATCH 21/29] perf header: Sanity check HEADER_EVENT_DESC attr.size before swap Arnaldo Carvalho de Melo
2026-05-25 1:05 ` [PATCH 22/29] perf header: Validate bitmap size before allocating in do_read_bitmap() Arnaldo Carvalho de Melo
2026-05-25 2:29 ` sashiko-bot
2026-05-25 15:38 ` Arnaldo Carvalho de Melo
2026-05-25 1:05 ` [PATCH 23/29] perf session: Add byte-swap handler for PERF_RECORD_COMPRESSED2 Arnaldo Carvalho de Melo
2026-05-25 1:05 ` [PATCH 24/29] perf tools: Harden compressed event processing Arnaldo Carvalho de Melo
2026-05-25 2:17 ` sashiko-bot
2026-05-25 1:05 ` [PATCH 25/29] perf session: Check for decompression buffer size overflow Arnaldo Carvalho de Melo
2026-05-25 1:05 ` [PATCH 26/29] perf session: Bound nr_cpus_avail and validate sample CPU Arnaldo Carvalho de Melo
2026-05-25 1:05 ` [PATCH 27/29] perf kwork: Bounds check work->cpu before indexing cpus_runtime[] Arnaldo Carvalho de Melo
2026-05-25 1:05 ` [PATCH 28/29] perf session: Snapshot event->header.size in process_user_event() Arnaldo Carvalho de Melo
2026-05-25 2:39 ` sashiko-bot
2026-05-25 1:05 ` [PATCH 29/29] perf test: Add truncated perf.data robustness test Arnaldo Carvalho de Melo
2026-05-25 2:04 ` sashiko-bot
2026-05-25 15:41 ` Arnaldo Carvalho de Melo
-- strict thread matches above, loose matches on Subject: below --
2026-05-26 21:17 [PATCHES v4 00/29] perf: Harden perf.data parsing against crafted/corrupted files Arnaldo Carvalho de Melo
2026-05-26 21:17 ` [PATCH 15/29] perf session: Add byte-swap and bounds check for PERF_RECORD_BPF_METADATA events Arnaldo Carvalho de Melo
2026-05-26 21:56 ` sashiko-bot
2026-05-24 3:26 [PATCHES v2 00/29] perf: Harden perf.data parsing against crafted/corrupted files Arnaldo Carvalho de Melo
2026-05-24 3:26 ` [PATCH 15/29] perf session: Add byte-swap and bounds check for PERF_RECORD_BPF_METADATA events Arnaldo Carvalho de Melo
2026-05-24 4:13 ` sashiko-bot
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20260525010550.1100375-16-acme@kernel.org \
--to=acme@kernel.org \
--cc=acme@redhat.com \
--cc=adrian.hunter@intel.com \
--cc=blakejones@google.com \
--cc=irogers@google.com \
--cc=james.clark@linaro.org \
--cc=jolsa@kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-perf-users@vger.kernel.org \
--cc=mingo@kernel.org \
--cc=namhyung@kernel.org \
--cc=noreply@anthropic.com \
--cc=sashiko-bot@kernel.org \
--cc=tglx@linutronix.de \
--cc=williams@redhat.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.