Re: [PATCH v2 perf-tools-next 0/6] perf trace: Validate payload bounds across augmented argument beautifiers

[Aaron Tomlin]

On Sun, Sep 06, 2026 at 09:51:34PM -0400, Aaron Tomlin wrote:
> When pretty-printing augmented syscall arguments in perf trace, raw payload
> data captured from BPF programs is passed to various argument formatters
> via struct syscall_arg.
> 
> However, when processing malformed, truncated, or untrusted perf.data
> records (e.g., truncated reads in BPF ringbuffers, cross-architecture
> replays, or crafted sample records), the payload can be shorter than
> expected or contain invalid size fields:
>     1.  Dereferencing augmented_arg fields before validating that
>         arg->augmented.size is at least sizeof(struct augmented_arg) can read
>         past the available buffer.
> 
>     2.  Passing augmented_arg->size to formatters or loop counters without
>         bounding it against the remaining buffer can cause out-of-bounds memory
>         reads.
> 
>     3.  In multi-argument syscalls, calculating consumed bytes as:
> 
>             consumed = sizeof(*augmented_arg) + augmented_arg->size;
> 
>         without bounds checking can overflow signed integer bounds or cause
>         arg->augmented.size to underflow. This advances arg->augmented.args
>         out of bounds, corrupting parsing state for all subsequent
>         arguments in the same syscall.
> 
>     4.  Type/family-specific beautifiers (i.e., BTF struct dump, sockaddr,
>         timespec, perf_event_attr) can dereference structure fields without
>         verifying that the payload contains sufficient bytes for the target
>         type.
> 
> This series adds comprehensive upper-bound and payload-size checks across
> all augmented argument beautifiers in perf trace. If validation fails in
> any beautifier, it cleanly falls back to printing the raw pointer/hex
> value.
> 
> To facilitate clean, conflict-free backports across active LTS kernels,
> each formatter fix is isolated to its own commit.
> 
> Changes since v1:
> 
>  - Expanded the original single patch into a 6-patch series in response to
>    reviewer feedback from sashiko-bot regarding similar bounds check
>    omissions across other augmented formatters in perf trace
> 
>  - Added new patch validating payload bounds and consumed offset
>    calculations in syscall_arg__scnprintf_augmented_string()
> 
>  - Added new patch validating payload bounds before byte traversal in
>    syscall_arg__scnprintf_buf(), isolated to ensure an independent Fixes:
>    tag for stable backports
> 
>  - Added new patch validating payload size against sizeof(struct timespec)
>    in syscall_arg__scnprintf_augmented_timespec()
> 
>  - Added new patch validating payload bounds and family-specific lengths in
>    syscall_arg__scnprintf_augmented_sockaddr()
> 
>  - Added new patch validating payload size against at least
>    PERF_ATTR_SIZE_VER0 in
>    syscall_arg__scnprintf_augmented_perf_event_attr()
> 
>  - Link to v1: https://lore.kernel.org/all/20260906011132.279321-1-atomlin@xxxxxxxxxxx/
> 
> Aaron Tomlin (6):
>   perf trace: Add upper bound checks for augmented BTF struct printing
>   perf trace: Validate payload bounds in augmented string beautifier
>   perf trace: Validate payload bounds in augmented buffer beautifier
>   perf trace beauty: Validate payload size in augmented timespec
>     beautifier
>   perf trace beauty: Validate payload size in augmented sockaddr
>     beautifier
>   perf trace beauty: Validate payload size in augmented perf_event_open
>     beautifier
> 
>  tools/perf/builtin-trace.c                | 36 +++++++++++++++++------
>  tools/perf/trace/beauty/perf_event_open.c | 19 ++++++++++--
>  tools/perf/trace/beauty/sockaddr.c        | 35 ++++++++++++++++++----
>  tools/perf/trace/beauty/timespec.c        | 19 ++++++++++--
>  4 files changed, 89 insertions(+), 20 deletions(-)
> 
> 
> base-commit: 02f6847e1822714a4201b87e42f92b0d43e8549d
> -- 
> 2.55.0
> 

Kindly disregard this iteration, as per Sashiko [1].

The observations regarding uninitialised augmented_arg->size reads, arising
from the fact that several eBPF probes in augmented_raw_syscalls.bpf.c omit
populating this field from the reused per-CPU map, are indeed entirely
valid.

Likewise, the points concerning dispatch extensibility and variable-length
path handling in the sockaddr beautifier, alongside potential TOCTOU
discrepancies in perf_event_open, have been taken on board. Additionally,
regarding the suggested inclusion of <stdbool.h> and <stddef.h> in
sockaddr.c, these have been intentionally omitted as the requisite types
and macros (bool and offsetof()) are already provided unconditionally via
trace/beauty/beauty.h.

[1]: https://sashiko.dev/#/patchset/20260907015140.363076-1-atomlin%40atomlin.com

-- 
Aaron Tomlin