* [PATCH v3 1/2] perf record: Return the written size from process_comp_header()
2026-07-08 13:38 [PATCH v3 0/2] perf record: fix multi-record Zstd compression Dmitry Ilvokhin
@ 2026-07-08 13:38 ` Dmitry Ilvokhin
2026-07-08 13:38 ` [PATCH v3 2/2] perf record: Fix multiple PERF_RECORD_COMPRESSED2 records per push Dmitry Ilvokhin
1 sibling, 0 replies; 3+ messages in thread
From: Dmitry Ilvokhin @ 2026-07-08 13:38 UTC (permalink / raw)
To: Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo,
Namhyung Kim, Mark Rutland, Alexander Shishkin, Jiri Olsa,
Ian Rogers, Adrian Hunter, James Clark, Nick Terrell,
David Sterba
Cc: linux-kernel, linux-perf-users, kernel-team, Farid Zakaria,
Dmitry Ilvokhin
process_comp_header() is called from zstd_compress_stream_to_records()
twice per record: once with data_size == 0 to write the record header,
and once with the payload size to finalize it. It returns the increment
it was passed, and the loop separately decides whether a record still
fits by comparing the remaining 'dst_size' against the header size.
With the fit check split from the code that writes the record,
process_comp_header() cannot reject a record on its own, so any bytes it
writes into 'dst' have to be bounds-checked by the caller instead of
where they are produced.
Pass the space left in 'dst' to process_comp_header(), let it return the
number of bytes written or -1 when the header does not fit, and account
the compressed payload in the loop.
No functional change intended.
Signed-off-by: Dmitry Ilvokhin <d@ilvokhin.com>
---
tools/perf/builtin-record.c | 17 +++++++++++++----
tools/perf/util/compress.h | 6 ++++--
tools/perf/util/zstd.c | 25 ++++++++++++++-----------
3 files changed, 31 insertions(+), 17 deletions(-)
diff --git a/tools/perf/builtin-record.c b/tools/perf/builtin-record.c
index ebd3ed0c9b3e..757bf4486898 100644
--- a/tools/perf/builtin-record.c
+++ b/tools/perf/builtin-record.c
@@ -1592,16 +1592,25 @@ static void record__adjust_affinity(struct record *rec, struct mmap *map)
}
}
-static size_t process_comp_header(void *record, size_t increment)
+/*
+ * Called once with data_size == 0 to start a record, then once with
+ * data_size == compressed payload size to finalize.
+ * Returns the bytes written, or -1 if it won't fit.
+ */
+static ssize_t process_comp_header(void *record, size_t dst_size,
+ size_t data_size)
{
struct perf_record_compressed2 *event = record;
size_t size = sizeof(*event);
- if (increment) {
- event->header.size += increment;
- return increment;
+ if (data_size) {
+ event->header.size += data_size;
+ return 0;
}
+ if (size > dst_size)
+ return -1;
+
event->header.type = PERF_RECORD_COMPRESSED2;
event->header.size = size;
diff --git a/tools/perf/util/compress.h b/tools/perf/util/compress.h
index 6cfecfca16f2..ec6c38129e24 100644
--- a/tools/perf/util/compress.h
+++ b/tools/perf/util/compress.h
@@ -54,7 +54,8 @@ int zstd_fini(struct zstd_data *data);
ssize_t zstd_compress_stream_to_records(struct zstd_data *data, void *dst, size_t dst_size,
void *src, size_t src_size, size_t max_record_size,
- size_t process_header(void *record, size_t increment));
+ ssize_t process_header(void *record, size_t dst_size,
+ size_t data_size));
size_t zstd_decompress_stream(struct zstd_data *data, void *src, size_t src_size,
void *dst, size_t dst_size);
@@ -75,7 +76,8 @@ ssize_t zstd_compress_stream_to_records(struct zstd_data *data __maybe_unused,
void *dst __maybe_unused, size_t dst_size __maybe_unused,
void *src __maybe_unused, size_t src_size __maybe_unused,
size_t max_record_size __maybe_unused,
- size_t process_header(void *record, size_t increment) __maybe_unused)
+ ssize_t process_header(void *record, size_t dst_size,
+ size_t data_size) __maybe_unused)
{
return 0;
}
diff --git a/tools/perf/util/zstd.c b/tools/perf/util/zstd.c
index 21a0eb58597c..d98014902f01 100644
--- a/tools/perf/util/zstd.c
+++ b/tools/perf/util/zstd.c
@@ -31,9 +31,11 @@ int zstd_fini(struct zstd_data *data)
ssize_t zstd_compress_stream_to_records(struct zstd_data *data, void *dst, size_t dst_size,
void *src, size_t src_size, size_t max_record_size,
- size_t process_header(void *record, size_t increment))
+ ssize_t process_header(void *record, size_t dst_size,
+ size_t data_size))
{
- size_t ret, size, compressed = 0;
+ size_t ret, compressed = 0;
+ ssize_t size;
ZSTD_inBuffer input = { src, src_size, 0 };
ZSTD_outBuffer output;
void *record;
@@ -55,12 +57,9 @@ ssize_t zstd_compress_stream_to_records(struct zstd_data *data, void *dst, size_
while (input.pos < input.size) {
record = dst;
- /* process_header writes the event header into record */
- if (dst_size < sizeof(struct perf_event_header))
- goto reset;
- size = process_header(record, 0);
+ size = process_header(record, dst_size, 0);
/* Output buffer full — cannot fit even the record header */
- if (size > dst_size)
+ if (size < 0)
goto reset;
compressed += size;
dst += size;
@@ -74,17 +73,21 @@ ssize_t zstd_compress_stream_to_records(struct zstd_data *data, void *dst, size_
(long)src_size, ZSTD_getErrorName(ret));
goto reset;
}
- size = output.pos;
+ compressed += output.pos;
+ dst += output.pos;
+ dst_size -= output.pos;
/*
* No progress: ZSTD couldn't emit any bytes into the
* remaining output buffer. Calling process_header
- * with size=0 would re-trigger header initialization,
+ * with output.pos=0 would re-trigger header initialization,
* double-subtracting the header size from dst_size and
* underflowing the unsigned counter.
*/
- if (size == 0)
+ if (output.pos == 0)
+ goto reset;
+ size = process_header(record, dst_size, output.pos);
+ if (size < 0)
goto reset;
- size = process_header(record, size);
compressed += size;
dst += size;
dst_size -= size;
--
2.53.0-Meta
^ permalink raw reply related [flat|nested] 3+ messages in thread* [PATCH v3 2/2] perf record: Fix multiple PERF_RECORD_COMPRESSED2 records per push
2026-07-08 13:38 [PATCH v3 0/2] perf record: fix multi-record Zstd compression Dmitry Ilvokhin
2026-07-08 13:38 ` [PATCH v3 1/2] perf record: Return the written size from process_comp_header() Dmitry Ilvokhin
@ 2026-07-08 13:38 ` Dmitry Ilvokhin
1 sibling, 0 replies; 3+ messages in thread
From: Dmitry Ilvokhin @ 2026-07-08 13:38 UTC (permalink / raw)
To: Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo,
Namhyung Kim, Mark Rutland, Alexander Shishkin, Jiri Olsa,
Ian Rogers, Adrian Hunter, James Clark, Nick Terrell,
David Sterba
Cc: linux-kernel, linux-perf-users, kernel-team, Farid Zakaria,
Dmitry Ilvokhin
With Zstd compression enabled ('perf record -z'), a single mmap push
whose compressed output exceeds the maximum record size makes
zstd_compress_stream_to_records() emit several PERF_RECORD_COMPRESSED2
records back to back. record__pushfn() however rewrote only the first
record's header to describe the whole blob as one record:
event->data_size = compressed - sizeof(struct perf_record_compressed2);
event->header.size = PERF_ALIGN(compressed, sizeof(u64));
padding = event->header.size - compressed;
...
record__write(rec, map, &pad, padding);
perf_event_header::size is a __u16, so once the compressed blob no
longer fits in it the header.size assignment truncates and 'padding'
(size_t) underflows. write() is then handed that bogus length and fails
with EFAULT, aborting the recording:
failed to write perf data, error: Bad address
The bytes that did reach the file are mis-framed, so reading it back
cannot be decompressed.
This is easy to hit with a high event rate and a large buffer, e.g.:
perf record -z -F max -m 32M --per-thread -- perf test -w thloop 5 1
The single-record fixup is wrong by construction: because header.size is
16 bits a compressed record cannot exceed 64KB, so the compressor must
split a push into a chain of records, and the session reader already
consumes them as such.
Frame each record where it is produced instead: make
process_comp_header() set the per-record data_size, 8-byte-align
header.size and zero the trailing padding, and let record__pushfn()
write the resulting blob, as the AIO path already does. Reduce
max_record_size by sizeof(u64) so the per-record alignment padding
cannot push header.size past its u16 field. process_comp_header()
returns -1 when that padding would not fit the space left in 'dst', so
the compressor stops instead of overrunning the output buffer.
There is no on-disk format change; a perf.data written by the fixed tool
is still read by existing perf.
Fixes: 208c0e168344 ("perf record: Add 8-byte aligned event type PERF_RECORD_COMPRESSED2")
Reported-by: Farid Zakaria <fmzakari@meta.com>
Signed-off-by: Dmitry Ilvokhin <d@ilvokhin.com>
---
tools/perf/builtin-record.c | 38 +++++------
.../record+zstd_comp_decomp_multi_record.sh | 63 +++++++++++++++++++
2 files changed, 83 insertions(+), 18 deletions(-)
create mode 100755 tools/perf/tests/shell/record+zstd_comp_decomp_multi_record.sh
diff --git a/tools/perf/builtin-record.c b/tools/perf/builtin-record.c
index 757bf4486898..3a4702eb88fa 100644
--- a/tools/perf/builtin-record.c
+++ b/tools/perf/builtin-record.c
@@ -64,6 +64,7 @@
#include <poll.h>
#include <pthread.h>
#include <unistd.h>
+#include <string.h>
#ifndef HAVE_GETTID
#include <syscall.h>
#endif
@@ -653,27 +654,14 @@ static int record__pushfn(struct mmap *map, void *to, void *bf, size_t size)
struct record *rec = to;
if (record__comp_enabled(rec)) {
- struct perf_record_compressed2 *event = map->data;
- size_t padding = 0;
- u8 pad[8] = {0};
ssize_t compressed = zstd_compress(rec->session, map, map->data,
mmap__mmap_len(map), bf, size);
if (compressed < 0)
return (int)compressed;
- bf = event;
thread->samples++;
-
- /*
- * The record from `zstd_compress` is not 8 bytes aligned, which would cause asan
- * error. We make it aligned here.
- */
- event->data_size = compressed - sizeof(struct perf_record_compressed2);
- event->header.size = PERF_ALIGN(compressed, sizeof(u64));
- padding = event->header.size - compressed;
- return record__write(rec, map, bf, compressed) ||
- record__write(rec, map, &pad, padding);
+ return record__write(rec, map, map->data, compressed);
}
thread->samples++;
@@ -1594,7 +1582,8 @@ static void record__adjust_affinity(struct record *rec, struct mmap *map)
/*
* Called once with data_size == 0 to start a record, then once with
- * data_size == compressed payload size to finalize.
+ * data_size == compressed payload size to finalize and 8-byte-pad it
+ * (unaligned records trip ASan in the reader).
* Returns the bytes written, or -1 if it won't fit.
*/
static ssize_t process_comp_header(void *record, size_t dst_size,
@@ -1604,8 +1593,15 @@ static ssize_t process_comp_header(void *record, size_t dst_size,
size_t size = sizeof(*event);
if (data_size) {
- event->header.size += data_size;
- return 0;
+ size_t padding;
+
+ event->data_size = data_size;
+ event->header.size = PERF_ALIGN(size + data_size, sizeof(u64));
+ padding = event->header.size - size - data_size;
+ if (padding > dst_size)
+ return -1;
+ memset(record + size + data_size, 0, padding);
+ return padding;
}
if (size > dst_size)
@@ -1613,6 +1609,7 @@ static ssize_t process_comp_header(void *record, size_t dst_size,
event->header.type = PERF_RECORD_COMPRESSED2;
event->header.size = size;
+ event->data_size = 0;
return size;
}
@@ -1621,7 +1618,12 @@ static ssize_t zstd_compress(struct perf_session *session, struct mmap *map,
void *dst, size_t dst_size, void *src, size_t src_size)
{
ssize_t compressed;
- size_t max_record_size = PERF_SAMPLE_MAX_SIZE - sizeof(struct perf_record_compressed2) - 1;
+ /*
+ * Reserve space so per-record PERF_ALIGN() padding keeps header.size
+ * within u16.
+ */
+ size_t max_record_size = PERF_SAMPLE_MAX_SIZE
+ - sizeof(struct perf_record_compressed2) - sizeof(u64);
struct zstd_data *zstd_data = &session->zstd_data;
if (map && map->file)
diff --git a/tools/perf/tests/shell/record+zstd_comp_decomp_multi_record.sh b/tools/perf/tests/shell/record+zstd_comp_decomp_multi_record.sh
new file mode 100755
index 000000000000..c05ace8214ca
--- /dev/null
+++ b/tools/perf/tests/shell/record+zstd_comp_decomp_multi_record.sh
@@ -0,0 +1,63 @@
+#!/bin/bash
+# Zstd perf.data compression/decompression of multi-record data
+# SPDX-License-Identifier: GPL-2.0
+
+perfdata=$(mktemp /tmp/__perf_test.perf.data.XXXXX)
+recout=$(mktemp /tmp/__perf_test.zstd.rec.XXXXX)
+injout=$(mktemp /tmp/__perf_test.zstd.inj.XXXXX)
+perf_tool=perf
+
+cleanup() {
+ rm -f "${perfdata}" "${perfdata}".old "${perfdata}".decomp "${recout}" "${injout}"
+}
+trap cleanup EXIT TERM INT
+
+skip_if_no_z_record() {
+ $perf_tool record -h 2>&1 | grep -q -- '-z, --compression-level'
+}
+
+collect_z_record() {
+ echo "Collecting compressed record file:"
+ [ "$(uname -m)" != s390x ] && gflag='-g'
+ $perf_tool record -o "${perfdata}" $gflag -z -F max -m 32M --per-thread -- \
+ $perf_tool test -w thloop 5 1 \
+ >/dev/null 2>"${recout}"
+}
+
+check_record() {
+ echo "Checking record did not fail to write data:"
+ if grep -q "failed to write perf data" "${recout}"; then
+ cat "${recout}"
+ return 1
+ fi
+}
+
+check_decompress() {
+ echo "Checking compressed file decompresses cleanly:"
+ if ! $perf_tool inject -i "${perfdata}" -o "${perfdata}".decomp 2>"${injout}"; then
+ cat "${injout}"
+ return 1
+ fi
+ if grep -Eqi "decompress|corrupt|failed to process type" "${injout}"; then
+ cat "${injout}"
+ return 1
+ fi
+}
+
+skip_if_no_z_record || exit 2
+collect_z_record
+check_record || exit 1
+
+# Need >1 record, else the multi-record path wasn't exercised.
+# Skip rather than pass/fail spuriously.
+nr=$($perf_tool report -i "${perfdata}" --stats 2>/dev/null |
+ awk '/COMPRESSED2 events:/ { print $3 }')
+if [ -z "${nr}" ] || [ "${nr}" -lt 2 ]; then
+ echo "less than two compressed records (${nr:-0}), skipping"
+ exit 2
+fi
+echo "Produced ${nr} compressed records"
+
+check_decompress
+err=$?
+exit $err
--
2.53.0-Meta
^ permalink raw reply related [flat|nested] 3+ messages in thread