mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH v2 0/2] perf evsel: Validate cross-endian samples before byte swapping
@ 2026-09-03 11:04 Mark Amirkan via B4 Relay
  2026-09-03 11:04 ` [PATCH v2 1/2] perf evsel: Validate branch stack " Mark Amirkan via B4 Relay
                   ` (2 more replies)
  0 siblings, 3 replies; 5+ messages in thread
From: Mark Amirkan via B4 Relay @ 2026-09-03 11:04 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
  Cc: Mark Amirkan, linux-perf-users, linux-kernel, stable

Two paths in __evsel__parse_sample() use values from an
opposite-endian perf.data record to modify the record before checking that
the data fits within event->header.size.

Patch 1 checks the complete branch stack before swapping entry flags.
Patch 2 checks the RAW payload and the complete 64-bit words touched by
mem_bswap_64() before swapping the data.

The malformed branch-stack and RAW records reproduce as out-of-bounds
reads and writes. Each patch adds a Sample parsing regression test that
checks rejected records do not modify data past the declared event.

Signed-off-by: Mark Amirkan <markdamirkan@gmail.com>
---
Changes in v2:
- Add the adjacent PERF_SAMPLE_RAW fix found during v1 review.
- Keep the branch-stack fix unchanged.
- Link to v1: https://patch.msgid.link/20260903-sympwn-linux-002-final-v2-v1-1-7c6e4166b814@gmail.com

To: Peter Zijlstra <peterz@infradead.org>
To: Ingo Molnar <mingo@redhat.com>
To: Arnaldo Carvalho de Melo <acme@kernel.org>
To: Namhyung Kim <namhyung@kernel.org>
To: Mark Rutland <mark.rutland@arm.com>
To: Alexander Shishkin <alexander.shishkin@linux.intel.com>
To: Jiri Olsa <jolsa@kernel.org>
To: Ian Rogers <irogers@google.com>
To: Adrian Hunter <adrian.hunter@intel.com>
To: James Clark <james.clark@linaro.org>
Cc: linux-perf-users@vger.kernel.org
Cc: linux-kernel@vger.kernel.org

---
Mark Amirkan (2):
      perf evsel: Validate branch stack before byte swapping
      perf evsel: Validate RAW sample before byte swapping

 tools/perf/tests/sample-parsing.c | 109 ++++++++++++++++++++++++++++++++++++++
 tools/perf/util/evsel.c           |  20 +++----
 2 files changed, 120 insertions(+), 9 deletions(-)
---
base-commit: aadea57f532882d8bab444646863c7ef8a778ff1
change-id: 20260903-sympwn-linux-002-final-v2-e30a8df210c1

Best regards,
--  
Mark Amirkan <markdamirkan@gmail.com>



^ permalink raw reply	[flat|nested] 5+ messages in thread

* [PATCH v2 1/2] perf evsel: Validate branch stack before byte swapping
  2026-09-03 11:04 [PATCH v2 0/2] perf evsel: Validate cross-endian samples before byte swapping Mark Amirkan via B4 Relay
@ 2026-09-03 11:04 ` Mark Amirkan via B4 Relay
  2026-09-03 11:04 ` [PATCH v2 2/2] perf evsel: Validate RAW sample " Mark Amirkan via B4 Relay
  2026-09-03 20:19 ` [PATCH v2 0/2] perf evsel: Validate cross-endian samples " Ian Rogers
  2 siblings, 0 replies; 5+ messages in thread
From: Mark Amirkan via B4 Relay @ 2026-09-03 11:04 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
  Cc: Mark Amirkan, linux-perf-users, linux-kernel, stable

From: Mark Amirkan <markdamirkan@gmail.com>

When perf reads an opposite-endian branch stack, __evsel__parse_sample()
swaps each entry before checking whether all entries fit in the event. A
truncated sample can therefore make the swap loop read and write past the
event boundary.

A truncated perf.data file makes perf report crash with SIGSEGV. ASan
reports an out-of-bounds read. A regression test puts an entry just past
the declared end and shows that its flags are changed before the parser
returns -EFAULT.

Move the bounds check before the byte-swap loop. Valid samples are handled
as before.

Fixes: 63c12ae2f246 ("perf evsel: Add bitfield_swap() to handle branch_stack endian issue")
Cc: stable@vger.kernel.org
Assisted-by: Symbolic
Signed-off-by: Mark Amirkan <markdamirkan@gmail.com>
---
 tools/perf/tests/sample-parsing.c | 48 +++++++++++++++++++++++++++++++++++++++
 tools/perf/util/evsel.c           |  3 ++-
 2 files changed, 50 insertions(+), 1 deletion(-)

diff --git a/tools/perf/tests/sample-parsing.c b/tools/perf/tests/sample-parsing.c
index 32dbc484487a..583951534937 100644
--- a/tools/perf/tests/sample-parsing.c
+++ b/tools/perf/tests/sample-parsing.c
@@ -1,5 +1,6 @@
 // SPDX-License-Identifier: GPL-2.0
 #include <stdbool.h>
+#include <errno.h>
 #include <inttypes.h>
 #include <stdlib.h>
 #include <string.h>
@@ -417,6 +418,49 @@ static int do_test(u64 sample_type, u64 sample_regs, u64 read_format)
 	return ret;
 }
 
+static int test_truncated_branch_stack(void)
+{
+	struct perf_event_attr attr = {
+		.sample_type = PERF_SAMPLE_BRANCH_STACK,
+	};
+	struct {
+		struct perf_event_header header;
+		u64 nr;
+		struct branch_entry entry;
+	} input = {
+		.header = {
+			.type = PERF_RECORD_SAMPLE,
+			.size = sizeof(input.header) + sizeof(input.nr),
+		},
+		.nr = 1,
+	};
+	struct perf_sample sample;
+	struct evsel *evsel;
+	u64 flags = 1;
+	int err;
+
+	input.entry.flags.value = flags;
+	evsel = evsel__new(&attr);
+	if (!evsel)
+		return -1;
+
+	evsel->sample_size = __evsel__sample_size(attr.sample_type);
+	err = __evsel__parse_sample(evsel, (union perf_event *)&input,
+				    &sample, /*needs_swap=*/true);
+	perf_sample__exit(&sample);
+	evsel__put(evsel);
+
+	if (err != -EFAULT) {
+		pr_debug("truncated branch stack returned %d, expected -EFAULT\n", err);
+		return -1;
+	}
+	if (input.entry.flags.value != flags) {
+		pr_debug("truncated branch stack modified data past the event\n");
+		return -1;
+	}
+	return 0;
+}
+
 /**
  * test__sample_parsing - test sample parsing.
  *
@@ -433,6 +477,10 @@ static int test__sample_parsing(struct test_suite *test __maybe_unused, int subt
 	size_t i;
 	int err;
 
+	err = test_truncated_branch_stack();
+	if (err)
+		return err;
+
 	/*
 	 * Fail the test if it has not been updated when new sample format bits
 	 * were added.  Please actually update the test rather than just change
diff --git a/tools/perf/util/evsel.c b/tools/perf/util/evsel.c
index d4cb455f4a7d..cc0bc0857754 100644
--- a/tools/perf/util/evsel.c
+++ b/tools/perf/util/evsel.c
@@ -3639,6 +3639,8 @@ int __evsel__parse_sample(struct evsel *evsel, union perf_event *event,
 			e = (struct branch_entry *)&data->branch_stack->hw_idx;
 		}
 
+		OVERFLOW_CHECK(array, sz, max_size);
+
 		if (swapped) {
 			/*
 			 * struct branch_flag does not have endian
@@ -3654,7 +3656,6 @@ int __evsel__parse_sample(struct evsel *evsel, union perf_event *event,
 				e->flags.value = evsel__bitfield_swap_branch_flags(e->flags.value);
 		}
 
-		OVERFLOW_CHECK(array, sz, max_size);
 		array = (void *)array + sz;
 
 		if (evsel__has_branch_counters(evsel)) {

-- 
Git-146)



^ permalink raw reply	[flat|nested] 5+ messages in thread

* [PATCH v2 2/2] perf evsel: Validate RAW sample before byte swapping
  2026-09-03 11:04 [PATCH v2 0/2] perf evsel: Validate cross-endian samples before byte swapping Mark Amirkan via B4 Relay
  2026-09-03 11:04 ` [PATCH v2 1/2] perf evsel: Validate branch stack " Mark Amirkan via B4 Relay
@ 2026-09-03 11:04 ` Mark Amirkan via B4 Relay
  2026-09-03 20:19 ` [PATCH v2 0/2] perf evsel: Validate cross-endian samples " Ian Rogers
  2 siblings, 0 replies; 5+ messages in thread
From: Mark Amirkan via B4 Relay @ 2026-09-03 11:04 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
  Cc: Mark Amirkan, linux-perf-users, linux-kernel, stable

From: Mark Amirkan <markdamirkan@gmail.com>

For an opposite-endian RAW sample, __evsel__parse_sample() passes the
input-controlled size to mem_bswap_64() before checking whether the
payload fits in the event. A truncated record can therefore make the helper
read and write past the event boundary.

A crafted perf.data file makes perf report crash with SIGSEGV. ASan
reports the out-of-bounds access. A regression test puts backed data past
the declared end and shows that it is changed before the parser returns
-EFAULT.

Move the bounds checks before mem_bswap_64(). Check the rounded length too,
because the helper accesses complete 64-bit words. Complete records are
handled as before.

Fixes: f9d8adb345d7 ("perf evsel: Fix swap for samples with raw data")
Cc: stable@vger.kernel.org
Assisted-by: Symbolic
Signed-off-by: Mark Amirkan <markdamirkan@gmail.com>
---
 tools/perf/tests/sample-parsing.c | 61 +++++++++++++++++++++++++++++++++++++++
 tools/perf/util/evsel.c           | 17 ++++++-----
 2 files changed, 70 insertions(+), 8 deletions(-)

diff --git a/tools/perf/tests/sample-parsing.c b/tools/perf/tests/sample-parsing.c
index 583951534937..bd30f6d4c31b 100644
--- a/tools/perf/tests/sample-parsing.c
+++ b/tools/perf/tests/sample-parsing.c
@@ -461,6 +461,55 @@ static int test_truncated_branch_stack(void)
 	return 0;
 }
 
+static int test_truncated_swapped_raw(u16 event_size, u32 raw_size)
+{
+	struct perf_event_attr attr = {
+		.sample_type = PERF_SAMPLE_RAW,
+	};
+	struct {
+		struct perf_event_header header;
+		union {
+			u64 value;
+			u32 words[2];
+		} raw;
+		u64 canary;
+	} input = {
+		.header = {
+			.type = PERF_RECORD_SAMPLE,
+			.size = event_size,
+		},
+		/* Parsing a pre-swapped word exchanges these two u32 values. */
+		.raw.words = { 0x12345678, raw_size },
+		.canary = 0x8877665544332211ULL,
+	};
+	struct perf_sample sample;
+	struct evsel *evsel;
+	u64 raw = input.raw.value;
+	u64 canary = input.canary;
+	int err;
+
+	evsel = evsel__new(&attr);
+	if (!evsel)
+		return -1;
+
+	evsel->sample_size = __evsel__sample_size(attr.sample_type);
+	err = __evsel__parse_sample(evsel, (union perf_event *)&input,
+				    &sample, /*needs_swap=*/true);
+	perf_sample__exit(&sample);
+	evsel__put(evsel);
+
+	if (err != -EFAULT) {
+		pr_debug("truncated swapped RAW sample (size %u, raw %u) returned %d, expected -EFAULT\n",
+			 event_size, raw_size, err);
+		return -1;
+	}
+	if (input.raw.value != raw || input.canary != canary) {
+		pr_debug("truncated swapped RAW sample modified data before validation\n");
+		return -1;
+	}
+	return 0;
+}
+
 /**
  * test__sample_parsing - test sample parsing.
  *
@@ -481,6 +530,18 @@ static int test__sample_parsing(struct test_suite *test __maybe_unused, int subt
 	if (err)
 		return err;
 
+	/* The declared RAW payload extends past an otherwise aligned event. */
+	err = test_truncated_swapped_raw(sizeof(struct perf_event_header) +
+					 sizeof(u64), 16);
+	if (err)
+		return err;
+
+	/* The final complete word touched by mem_bswap_64() extends past it. */
+	err = test_truncated_swapped_raw(sizeof(struct perf_event_header) +
+					 sizeof(u32) + 9, 9);
+	if (err)
+		return err;
+
 	/*
 	 * Fail the test if it has not been updated when new sample format bits
 	 * were added.  Please actually update the test rather than just change
diff --git a/tools/perf/util/evsel.c b/tools/perf/util/evsel.c
index cc0bc0857754..ce429eb247b6 100644
--- a/tools/perf/util/evsel.c
+++ b/tools/perf/util/evsel.c
@@ -3584,7 +3584,10 @@ int __evsel__parse_sample(struct evsel *evsel, union perf_event *event,
 	}
 
 	if (type & PERF_SAMPLE_RAW) {
+		const __u64 *raw;
+
 		OVERFLOW_CHECK_u64(array);
+		raw = array;
 		u.val64 = *array;
 
 		/*
@@ -3600,16 +3603,14 @@ int __evsel__parse_sample(struct evsel *evsel, union perf_event *event,
 		}
 		data->raw_size = u.val32[0];
 
-		/*
-		 * The raw data is aligned on 64bits including the
-		 * u32 size, so it's safe to use mem_bswap_64.
-		 */
-		if (swapped)
-			mem_bswap_64((void *) array, data->raw_size);
-
 		array = (void *)array + sizeof(u32);
-
 		OVERFLOW_CHECK(array, data->raw_size, max_size);
+		if (swapped) {
+			/* mem_bswap_64() accesses complete 64-bit words. */
+			sz = roundup((u64)data->raw_size, sizeof(u64));
+			OVERFLOW_CHECK(raw, sz, max_size);
+			mem_bswap_64((void *)raw, data->raw_size);
+		}
 		data->raw_data = (void *)array;
 		array = (void *)array + data->raw_size;
 	}

-- 
Git-146)



^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH v2 0/2] perf evsel: Validate cross-endian samples before byte swapping
  2026-09-03 11:04 [PATCH v2 0/2] perf evsel: Validate cross-endian samples before byte swapping Mark Amirkan via B4 Relay
  2026-09-03 11:04 ` [PATCH v2 1/2] perf evsel: Validate branch stack " Mark Amirkan via B4 Relay
  2026-09-03 11:04 ` [PATCH v2 2/2] perf evsel: Validate RAW sample " Mark Amirkan via B4 Relay
@ 2026-09-03 20:19 ` Ian Rogers
  2026-09-05  0:32   ` Arnaldo Carvalho de Melo
  2 siblings, 1 reply; 5+ messages in thread
From: Ian Rogers @ 2026-09-03 20:19 UTC (permalink / raw)
  To: markdamirkan
  Cc: Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo,
	Namhyung Kim, Mark Rutland, Alexander Shishkin, Jiri Olsa,
	Adrian Hunter, James Clark, linux-perf-users, linux-kernel,
	stable

On Thu, Sep 3, 2026 at 4:05 AM Mark Amirkan via B4 Relay
<devnull+markdamirkan.gmail.com@kernel.org> wrote:
>
> Two paths in __evsel__parse_sample() use values from an
> opposite-endian perf.data record to modify the record before checking that
> the data fits within event->header.size.
>
> Patch 1 checks the complete branch stack before swapping entry flags.
> Patch 2 checks the RAW payload and the complete 64-bit words touched by
> mem_bswap_64() before swapping the data.
>
> The malformed branch-stack and RAW records reproduce as out-of-bounds
> reads and writes. Each patch adds a Sample parsing regression test that
> checks rejected records do not modify data past the declared event.
>
> Signed-off-by: Mark Amirkan <markdamirkan@gmail.com>

Reviewed-by: Ian Rogers <irogers@google.com>

Thanks,
Ian

> ---
> Changes in v2:
> - Add the adjacent PERF_SAMPLE_RAW fix found during v1 review.
> - Keep the branch-stack fix unchanged.
> - Link to v1: https://patch.msgid.link/20260903-sympwn-linux-002-final-v2-v1-1-7c6e4166b814@gmail.com
>
> To: Peter Zijlstra <peterz@infradead.org>
> To: Ingo Molnar <mingo@redhat.com>
> To: Arnaldo Carvalho de Melo <acme@kernel.org>
> To: Namhyung Kim <namhyung@kernel.org>
> To: Mark Rutland <mark.rutland@arm.com>
> To: Alexander Shishkin <alexander.shishkin@linux.intel.com>
> To: Jiri Olsa <jolsa@kernel.org>
> To: Ian Rogers <irogers@google.com>
> To: Adrian Hunter <adrian.hunter@intel.com>
> To: James Clark <james.clark@linaro.org>
> Cc: linux-perf-users@vger.kernel.org
> Cc: linux-kernel@vger.kernel.org
>
> ---
> Mark Amirkan (2):
>       perf evsel: Validate branch stack before byte swapping
>       perf evsel: Validate RAW sample before byte swapping
>
>  tools/perf/tests/sample-parsing.c | 109 ++++++++++++++++++++++++++++++++++++++
>  tools/perf/util/evsel.c           |  20 +++----
>  2 files changed, 120 insertions(+), 9 deletions(-)
> ---
> base-commit: aadea57f532882d8bab444646863c7ef8a778ff1
> change-id: 20260903-sympwn-linux-002-final-v2-e30a8df210c1
>
> Best regards,
> --
> Mark Amirkan <markdamirkan@gmail.com>
>
>

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH v2 0/2] perf evsel: Validate cross-endian samples before byte swapping
  2026-09-03 20:19 ` [PATCH v2 0/2] perf evsel: Validate cross-endian samples " Ian Rogers
@ 2026-09-05  0:32   ` Arnaldo Carvalho de Melo
  0 siblings, 0 replies; 5+ messages in thread
From: Arnaldo Carvalho de Melo @ 2026-09-05  0:32 UTC (permalink / raw)
  To: Ian Rogers
  Cc: markdamirkan, Peter Zijlstra, Ingo Molnar, Namhyung Kim,
	Mark Rutland, Alexander Shishkin, Jiri Olsa, Adrian Hunter,
	James Clark, linux-perf-users, linux-kernel, stable

On Thu, Sep 03, 2026 at 01:19:07PM -0700, Ian Rogers wrote:
> On Thu, Sep 3, 2026 at 4:05 AM Mark Amirkan via B4 Relay
> <devnull+markdamirkan.gmail.com@kernel.org> wrote:
> >
> > Two paths in __evsel__parse_sample() use values from an
> > opposite-endian perf.data record to modify the record before checking that
> > the data fits within event->header.size.
> >
> > Patch 1 checks the complete branch stack before swapping entry flags.
> > Patch 2 checks the RAW payload and the complete 64-bit words touched by
> > mem_bswap_64() before swapping the data.
> >
> > The malformed branch-stack and RAW records reproduce as out-of-bounds
> > reads and writes. Each patch adds a Sample parsing regression test that
> > checks rejected records do not modify data past the declared event.
> >
> > Signed-off-by: Mark Amirkan <markdamirkan@gmail.com>
> 
> Reviewed-by: Ian Rogers <irogers@google.com>

Thanks, applied to perf-tools-next, for v7.4.

- Arnaldo

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2026-09-05  0:32 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-09-03 11:04 [PATCH v2 0/2] perf evsel: Validate cross-endian samples before byte swapping Mark Amirkan via B4 Relay
2026-09-03 11:04 ` [PATCH v2 1/2] perf evsel: Validate branch stack " Mark Amirkan via B4 Relay
2026-09-03 11:04 ` [PATCH v2 2/2] perf evsel: Validate RAW sample " Mark Amirkan via B4 Relay
2026-09-03 20:19 ` [PATCH v2 0/2] perf evsel: Validate cross-endian samples " Ian Rogers
2026-09-05  0:32   ` Arnaldo Carvalho de Melo

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®