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>,
Kan Liang <kan.liang@linux.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>,
"Claude Opus 4.6 (1M context)" <noreply@anthropic.com>
Subject: [PATCH 05/28] perf session: Fix PERF_RECORD_READ swap and dump for variable-length events
Date: Sun, 10 May 2026 00:33:56 -0300 [thread overview]
Message-ID: <20260510033424.255812-6-acme@kernel.org> (raw)
In-Reply-To: <20260510033424.255812-1-acme@kernel.org>
From: Arnaldo Carvalho de Melo <acme@redhat.com>
The kernel dynamically sizes PERF_RECORD_READ based on
attr.read_format: only the fields enabled by PERF_FORMAT_TOTAL_TIME_ENABLED,
PERF_FORMAT_TOTAL_TIME_RUNNING, PERF_FORMAT_ID, and PERF_FORMAT_LOST
are emitted, packed with no gaps.
perf_event__read_swap() unconditionally byte-swapped time_enabled,
time_running, and id at their fixed struct offsets, causing
out-of-bounds access on smaller events and swapping the wrong
bytes when not all format fields are present. It also dropped
the sample_id_all swap entirely.
Replace the individual field swaps with a single mem_bswap_64()
over the entire tail from value onward. Since every field after
pid/tid is u64 regardless of which combination is present, this
correctly handles any read_format combination and any trailing
sample_id_all fields.
Similarly, dump_read() accessed optional fields via fixed struct
offsets, displaying values from wrong positions when not all
format bits are set. Walk the packed u64 array sequentially
instead, with bounds checks against event->header.size.
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 | 61 +++++++++++++++++++++++++++------------
1 file changed, 43 insertions(+), 18 deletions(-)
diff --git a/tools/perf/util/session.c b/tools/perf/util/session.c
index aae0651fb6f025a1..20b70d6fb7cc8ed4 100644
--- a/tools/perf/util/session.c
+++ b/tools/perf/util/session.c
@@ -354,17 +354,22 @@ static void perf_event__task_swap(union perf_event *event, bool sample_id_all)
swap_sample_id_all(event, &event->fork + 1);
}
-static void perf_event__read_swap(union perf_event *event, bool sample_id_all)
+static void perf_event__read_swap(union perf_event *event,
+ bool sample_id_all __maybe_unused)
{
- event->read.pid = bswap_32(event->read.pid);
- event->read.tid = bswap_32(event->read.tid);
- event->read.value = bswap_64(event->read.value);
- event->read.time_enabled = bswap_64(event->read.time_enabled);
- event->read.time_running = bswap_64(event->read.time_running);
- event->read.id = bswap_64(event->read.id);
+ size_t tail;
- if (sample_id_all)
- swap_sample_id_all(event, &event->read + 1);
+ event->read.pid = bswap_32(event->read.pid);
+ event->read.tid = bswap_32(event->read.tid);
+ /*
+ * Everything after pid/tid is u64: the read values (variable
+ * set determined by attr.read_format, which we don't have
+ * here) optionally followed by sample_id_all fields.
+ * Since all are u64, swap the entire remaining tail at once.
+ */
+ tail = event->header.size - offsetof(struct perf_record_read, value);
+ tail &= ~(size_t)(sizeof(__u64) - 1);
+ mem_bswap_64(&event->read.value, tail);
}
static void perf_event__aux_swap(union perf_event *event, bool sample_id_all)
@@ -1198,8 +1203,9 @@ static void dump_deferred_callchain(struct evsel *evsel, union perf_event *event
static void dump_read(struct evsel *evsel, union perf_event *event)
{
- struct perf_record_read *read_event = &event->read;
u64 read_format;
+ __u64 *array;
+ void *end;
if (!dump_trace)
return;
@@ -1211,18 +1217,37 @@ static void dump_read(struct evsel *evsel, union perf_event *event)
return;
read_format = evsel->core.attr.read_format;
+ /*
+ * The kernel packs only the enabled read_format fields
+ * after value, with no gaps. Walk the packed array
+ * instead of using fixed struct offsets.
+ */
+ array = &event->read.value + 1;
+ end = (void *)event + event->header.size;
- if (read_format & PERF_FORMAT_TOTAL_TIME_ENABLED)
- printf("... time enabled : %" PRI_lu64 "\n", read_event->time_enabled);
+ if (read_format & PERF_FORMAT_TOTAL_TIME_ENABLED) {
+ if ((void *)(array + 1) > end)
+ return;
+ printf("... time enabled : %" PRI_lu64 "\n", *array++);
+ }
- if (read_format & PERF_FORMAT_TOTAL_TIME_RUNNING)
- printf("... time running : %" PRI_lu64 "\n", read_event->time_running);
+ if (read_format & PERF_FORMAT_TOTAL_TIME_RUNNING) {
+ if ((void *)(array + 1) > end)
+ return;
+ printf("... time running : %" PRI_lu64 "\n", *array++);
+ }
- if (read_format & PERF_FORMAT_ID)
- printf("... id : %" PRI_lu64 "\n", read_event->id);
+ if (read_format & PERF_FORMAT_ID) {
+ if ((void *)(array + 1) > end)
+ return;
+ printf("... id : %" PRI_lu64 "\n", *array++);
+ }
- if (read_format & PERF_FORMAT_LOST)
- printf("... lost : %" PRI_lu64 "\n", read_event->lost);
+ if (read_format & PERF_FORMAT_LOST) {
+ if ((void *)(array + 1) > end)
+ return;
+ printf("... lost : %" PRI_lu64 "\n", *array++);
+ }
}
static struct machine *machines__find_for_cpumode(struct machines *machines,
--
2.54.0
next prev parent reply other threads:[~2026-05-10 3:35 UTC|newest]
Thread overview: 30+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-05-10 3:33 [PATCH 00/28] perf: Harden perf.data parsing against crafted/corrupted files Arnaldo Carvalho de Melo
2026-05-10 3:33 ` [PATCH 01/28] perf session: Add minimum event size validation table Arnaldo Carvalho de Melo
2026-05-11 19:01 ` Ian Rogers
2026-05-10 3:33 ` [PATCH 02/28] perf tools: Fix event_contains() macro to verify full field extent Arnaldo Carvalho de Melo
2026-05-10 3:33 ` [PATCH 03/28] perf zstd: Fix compression error path in zstd_compress_stream_to_records() Arnaldo Carvalho de Melo
2026-05-10 3:33 ` [PATCH 04/28] perf zstd: Fix multi-iteration decompression and error handling Arnaldo Carvalho de Melo
2026-05-10 3:33 ` Arnaldo Carvalho de Melo [this message]
2026-05-10 3:33 ` [PATCH 06/28] perf session: Align auxtrace_info priv size before byte-swapping Arnaldo Carvalho de Melo
2026-05-10 3:33 ` [PATCH 07/28] perf session: Add validated swap infrastructure with null-termination checks Arnaldo Carvalho de Melo
2026-05-10 3:33 ` [PATCH 08/28] perf session: Use bounded copy for PERF_RECORD_TIME_CONV Arnaldo Carvalho de Melo
2026-05-10 3:34 ` [PATCH 09/28] perf session: Validate HEADER_ATTR alignment and attr.size before swapping Arnaldo Carvalho de Melo
2026-05-10 3:34 ` [PATCH 10/28] perf session: Validate nr fields against event size on both swap and common paths Arnaldo Carvalho de Melo
2026-05-10 3:34 ` [PATCH 11/28] perf header: Byte-swap build ID event pid and bounds check section entries Arnaldo Carvalho de Melo
2026-05-10 3:34 ` [PATCH 12/28] perf cpumap: Reject RANGE_CPUS with start_cpu > end_cpu Arnaldo Carvalho de Melo
2026-05-10 3:34 ` [PATCH 13/28] perf auxtrace: Harden auxtrace_error event handling Arnaldo Carvalho de Melo
2026-05-10 3:34 ` [PATCH 14/28] perf session: Add byte-swap and bounds check for PERF_RECORD_BPF_METADATA events Arnaldo Carvalho de Melo
2026-05-10 3:34 ` [PATCH 15/28] perf header: Validate null-termination in PERF_RECORD_EVENT_UPDATE string fields Arnaldo Carvalho de Melo
2026-05-10 3:34 ` [PATCH 16/28] perf tools: Bounds check perf_event_attr fields against attr.size before printing Arnaldo Carvalho de Melo
2026-05-10 3:34 ` [PATCH 17/28] perf header: Propagate feature section processing errors Arnaldo Carvalho de Melo
2026-05-10 3:34 ` [PATCH 18/28] perf header: Validate f_attr.ids section before use in perf_session__read_header() Arnaldo Carvalho de Melo
2026-05-10 3:34 ` [PATCH 19/28] perf header: Validate feature section size and add read path bounds checking Arnaldo Carvalho de Melo
2026-05-10 3:34 ` [PATCH 20/28] perf header: Sanity check HEADER_EVENT_DESC attr.size before swap Arnaldo Carvalho de Melo
2026-05-10 3:34 ` [PATCH 21/28] perf header: Validate bitmap size before allocating in do_read_bitmap() Arnaldo Carvalho de Melo
2026-05-10 3:34 ` [PATCH 22/28] perf session: Add byte-swap for PERF_RECORD_COMPRESSED2 events Arnaldo Carvalho de Melo
2026-05-10 3:34 ` [PATCH 23/28] perf tools: Harden compressed event processing Arnaldo Carvalho de Melo
2026-05-10 3:34 ` [PATCH 24/28] perf session: Check for decompression buffer size overflow Arnaldo Carvalho de Melo
2026-05-10 3:34 ` [PATCH 25/28] perf session: Bound nr_cpus_avail and validate sample CPU Arnaldo Carvalho de Melo
2026-05-10 3:34 ` [PATCH 26/28] perf timechart: Bounds check cpu_id and fix topology_map allocation Arnaldo Carvalho de Melo
2026-05-10 3:34 ` [PATCH 27/28] perf kwork: Bounds check work->cpu before indexing cpus_runtime[] Arnaldo Carvalho de Melo
2026-05-10 3:34 ` [PATCH 28/28] perf test: Add truncated perf.data robustness test Arnaldo Carvalho de Melo
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=20260510033424.255812-6-acme@kernel.org \
--to=acme@kernel.org \
--cc=acme@redhat.com \
--cc=adrian.hunter@intel.com \
--cc=irogers@google.com \
--cc=james.clark@linaro.org \
--cc=jolsa@kernel.org \
--cc=kan.liang@linux.intel.com \
--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=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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox