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

[Arnaldo Carvalho de Melo]

On Thu, May 28, 2026 at 03:07:13PM -0700, Ian Rogers wrote:
> On Tue, May 26, 2026 at 6:06 PM Arnaldo Carvalho de Melo
> <acme@xxxxxxxxxx> 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.
> >
> > ● 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.
> >
> > -------------------------------------------------------------------------
> 
> I'm more than happy with this merge, good not being the enemy of
> perfect and things like that.
> 
> Reviewed-by: Ian Rogers <irogers@xxxxxxxxxx>

Thanks, added to the csets, now to go on those other pre-existing
bugs...

- Arnaldo