Thread (43 messages) 43 messages, 2 authors, 1d ago

Re: [PATCH 01/29] perf session: Add minimum event size and alignment validation

From: sashiko-bot@kernel.org
Date: 2026-05-24 04:13:14

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] Callers of `perf_session__peek_event()` completely abort processing on undersized events, breaking the intended 'skip' behavior.
--

commit b42afb24726ad9701717fde2cf945b69f1cdd644
Author: Arnaldo Carvalho de Melo [off-list ref]

perf session: Add minimum event size and alignment validation

This commit adds a per-type minimum size table for perf events to prevent
out-of-bounds accesses during swapping and processing. It enforces this by
checking event sizes before swap operations and verifying that event sizes
are 8-byte aligned.
quoted hunk ↗ jump to hunk
diff --git a/tools/perf/util/session.c b/tools/perf/util/session.c
index 1e25892963b78..0523fd243e02c 100644
--- a/tools/perf/util/session.c
+++ b/tools/perf/util/session.c
[ ... ]
+/*
+ * Read and validate the event at @file_offset.
+ *
+ * Returns:
+ *   0  — success: *event_ptr is set and safe to access.
+ *  -1  — error; check *event_ptr to decide whether to advance or abort:
+ *          *event_ptr set  — event header was read but the event is
+ *                            malformed (too small for its type, or byte-swap
+ *                            failed).  header.size is still valid, so the
+ *                            caller can advance past the event.
+ *          *event_ptr NULL — fatal: couldn't read the header at all
+ *                            (I/O error, offset out of range, pipe mode).
+ *                            Caller must abort.
+ */
 int perf_session__peek_event(struct perf_session *session, off_t file_offset,
 			     void *buf, size_t buf_sz,
 			     union perf_event **event_ptr,
[ ... ]
+	if (perf_event__too_small(event, &min_sz)) {
+		pr_warning("WARNING: peek_event: %s event size %u too small (min %u)\n",
+			   perf_event__name(event->header.type),
+			   event->header.size, min_sz);
 		return -1;
+	}
[Severity: Medium]
The API contract for perf_session__peek_event() was changed to return -1 on
undersized events, requiring callers to inspect *event_ptr to advance.

Does this break bisectability if the callers (such as
perf_session__peek_events()) were not updated in this commit to perform
this check?

If a caller hasn't been updated, it might completely abort the file parsing
loop when encountering a single undersized event, breaking the intended skip
behavior.

I see this is resolved in a later commit (37f83323fcf82: "perf session: Add
validated swap infrastructure with null-termination checks"), but wanted to
ask about the intermediate state in this commit.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260524032709.1080771-1-acme@kernel.org?part=1
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help