Re: [PATCHES v4 00/29] perf: Harden perf.data parsing against crafted/corrupted files

[Arnaldo Carvalho de Melo]

On Tue, May 26, 2026 at 10:06:10PM -0300, Arnaldo Carvalho de Melo wrote:
> On Tue, May 26, 2026 at 06:17:36PM -0300, Arnaldo Carvalho de Melo wrote:
> > perf.data validation and hardening (29 patches)
> > 
> > A crafted or corrupted perf.data file can cause out-of-bounds
> > reads/writes, infinite loops, heap overflows, and segfaults in perf
> > report, perf script, perf inject, perf timechart, and perf kwork.
> > This series adds defense-in-depth validation for file parsing:
> 
> The analysis about Sashiko's remaining comments is below, unless someone
> has something not related by Sashiko, I'll merge this tomorrow and
> continue processing the other outstanding patches.

Merged into perf-tools-next,

- Arnaldo
 
> ● Here's the v4 sashiko.dev review triage — 13 of 29 patches got reviews:
> 
>   Patches with findings:
> 
>   Patch: 02 (peek_event bounds)
>   Findings: 1 High: mmap_size - page_offset underflow
>   Verdict: Pre-existing — reader__init() validates data_size, page_offset can't exceed mmap_size
>   ────────────────────────────────────────
>   Patch: 04 (zstd compress)
>   Findings: 1 Critical + 3 High: multi-record header overflow, AIO data_size, flush return, decompress pos
>   Verdict: All pre-existing — the Critical is about process_header() aggregate size, and the decompress issue is fixed later in patch 05
>   ────────────────────────────────────────
>   Patch: 05 (zstd decompress)
>   Findings: 1 High: O_NONBLOCK missing on file opens
>   Verdict: Pre-existing — not introduced by this patch, unrelated to zstd
> 
> This one IIRC Ian sent a patch for review-prompts, merged recently that
> will make its way to Sashiko and will stop being flagged as a problem:
> 
>   "kernel/subsystem/perf.md: Remove section describing non-blocking IO"
>   https://github.com/masoncl/review-prompts/commit/261d73261dbb11f38ff9c653da3608b162741e03
> 
>   ────────────────────────────────────────
>   Patch: 08 (swap infra)
>   Findings: 2 High: mmap2 prot/flags not swapped, event_update union not swapped
>   Verdict: Both pre-existing — correct observations for follow-up series
>   ────────────────────────────────────────
>   Patch: 10 (HEADER_ATTR)
>   Findings: same as v3 — already triaged
>   Verdict:
>   ────────────────────────────────────────
>   Patch: 11 (nr validation)
>   Findings: 1 Medium: native path aborts vs swap path skips on bad THREAD_MAP
>   Verdict: Valid observation — but intentional: native path returns -EINVAL to catch corruption, swap path skips to keep session alive.
>   ────────────────────────────────────────
>   Patch: 12 (build_id_swap)
>   Findings: same as v3 — already fixed in v4
>   Verdict:
>   ────────────────────────────────────────
>   Patch: 15 (BPF_METADATA)
>   Findings: 1 High new: double-fetch of header.size in swap path; 2 High pre-existing: TOCTOU on native path
>   Verdict: The double-fetch is valid for swap but swap runs on MAP_PRIVATE (writable copy), so no concurrent modification possible.
>   ────────────────────────────────────────
>   Patch: 24 (compressed hardening)
>   Findings: 1 Medium: double-fetch of event->header.size in tool.c
>   Verdict: Same TOCTOU pattern
>   ────────────────────────────────────────
>   Patch: 26 (CPU bounds)
>   Findings: 1 High: global clamp corrupts data for >4096 CPUs
>   Verdict: Known limitation — memory [[MAX_NR_CPUS dynamic allocation]] TODO
>   ────────────────────────────────────────
>   Patch: 28 (READ_ONCE snapshot)
>   Findings: 4 High: incomplete TOCTOU fix, type confusion, array count re-reads
>   Verdict: All pre-existing MAP_SHARED TOCTOU. The full fix would be MAP_PRIVATE, noted as follow-up
>   ────────────────────────────────────────
>   Patch: 29 (shell test)
> 
>   Fixed and sent the diff in response to Sashiko's review e-mail.
> 
>   Summary: 1 new actionable issue in v4. All the other findings are
>   either pre-existing (documented in the cover letter), already fixed in
>   this version, or intentional design decisions. The mmap2 prot/flags
>   and event_update union swap gaps (patch 08) are valid pre-existing
>   bugs for a follow-up series.
> 
> -------------------------------------------------------------------------