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

[Namhyung Kim]

On Tue, May 26, 2026 at 06:17:37PM -0300, Arnaldo Carvalho de Melo wrote:
> From: Arnaldo Carvalho de Melo <acme@xxxxxxxxxx>
> 
> Add a per-type minimum size table (perf_event__min_size[]) and
> enforce it before swap and processing, so that both cross-endian
> and native-endian paths are protected from accessing fields past
> the event boundary.
> 
> The table uses offsetof() for types with trailing variable-length
> fields (filenames, strings, msg arrays) and sizeof() for
> fixed-size types.  Zero entries mean no minimum beyond the 8-byte
> header already enforced by the reader.
> 
> Undersized events are skipped with a warning in process_event
> and rejected in peek_event — both checked before the swap
> handler runs, preventing OOB access on crafted event fields.
> 
> Also reject events whose header.size is not 8-byte aligned.  The
> kernel aligns all event sizes to sizeof(u64) — see
> perf_event_comm_event() (ALIGN), perf_event_mmap_event(),
> perf_event_cgroup(), perf_event_ksymbol() (IS_ALIGNED loops),
> and perf_event_text_poke() (ALIGN) in kernel/events/core.c.
> An unaligned size means the file is corrupted or crafted; reject
> early so downstream code that divides by sizeof(u64) to compute
> array element counts gets exact results.
> 
> Three legacy user events are exempted from the alignment check:
> TRACING_DATA (66) had a 12-byte struct before commit b39c915a4f36
> ("libperf event: Ensure tracing data is multiple of 8 sized")
> added padding, COMPRESSED (81) carries raw ZSTD output (already
> superseded by COMPRESSED2 with PERF_ALIGN), and HEADER_FEATURE
> (80) uses do_write_string() with a 4-byte length prefix.
> 
> Also guard event_swap() against crafted event types >=
> PERF_RECORD_HEADER_MAX to prevent OOB reads on the
> perf_event__swap_ops[] array.
> 
> Changes in v2:
> - Fix double-skip for unsupported event types: return 0 instead
>   of event->header.size in perf_session__process_event() for
>   HEADER_MAX, since reader__read_event() already advances by
>   event->header.size (Reported-by: sashiko-bot@xxxxxxxxxx)
> - Exempt TRACING_DATA, COMPRESSED, and HEADER_FEATURE from the
>   alignment check — these legacy user events predate the 8-byte
>   alignment rule (Reported-by: sashiko-bot@xxxxxxxxxx)
> - peek_event: return 0 (skip) for unknown event types instead of
>   -1 (error), consistent with process_event which already skips
>   unsupported types gracefully (Reported-by: sashiko-bot@xxxxxxxxxx)
> 
> Reported-by: sashiko-bot@xxxxxxxxxx # Running on a local machine
> Cc: Adrian Hunter <adrian.hunter@xxxxxxxxx>
> Cc: Ian Rogers <irogers@xxxxxxxxxx>
> Cc: Jiri Olsa <jolsa@xxxxxxxxxx>
> Cc: Namhyung Kim <namhyung@xxxxxxxxxx>
> Assisted-by: Claude Opus 4.6 (1M context) <noreply@xxxxxxxxxxxxx>
> Signed-off-by: Arnaldo Carvalho de Melo <acme@xxxxxxxxxx>
> ---
[SNIP]
> +	/*
> +	 * Check alignment before type: an unaligned size misaligns the
> +	 * stream for all subsequent reads regardless of event type.
> +	 * Three legacy user events predate the 8-byte rule — exempt them.
> +	 */
> +	if (event->header.size % sizeof(u64) &&
> +	    event->header.type != PERF_RECORD_HEADER_TRACING_DATA &&
> +	    event->header.type != PERF_RECORD_COMPRESSED &&
> +	    event->header.type != PERF_RECORD_HEADER_FEATURE) {

Do we still allow misaligned data for these types?  I think we should
maintain 8 bytes alignment.  Can we add some paddings?

Thanks,
Namhyung


> +		pr_warning("WARNING: peek_event: event type %u size %u not aligned to %zu\n",
> +			   event->header.type,
> +			   event->header.size, sizeof(u64));
>  		return -1;
> +	}