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

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

[Aaron Tomlin]

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@atomlin.com/

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

[PATCH v2 perf-tools-next 1/6] perf trace: Add upper bound checks for augmented BTF struct printing

[Aaron Tomlin]

When pretty-printing augmented struct payloads using BTF via
btf_struct_scnprintf(), augmented_arg->size is currently only validated
against values <= 0.

However, several edge cases can result in size mismatches or buffer
over-reads:
    1.  If arg->augmented.size is smaller than sizeof(*augmented_arg),
        dereferencing augmented_arg->size reads past the available
        buffer.

    2.  If augmented_arg->size exceeds arg->augmented.size -
        sizeof(*augmented_arg), calculating consumed =
        sizeof(*augmented_arg) + augmented_arg->size can overflow signed
        integer limits (e.g., with crafted INT_MAX values in an
        untrusted perf.data file) or cause arg->augmented.size to
        underflow. This advances arg->augmented.args out of bounds,
        corrupting the parsing state for subsequent arguments in
        multi-argument syscalls.

    3.  If the captured payload is truncated (e.g., short reads in BPF,
        or during cross-architecture analysis such as replaying a 32-bit
        perf.data on a 64-bit host where host BTF type->size exceeds the
        32-bit target payload), passing type->size to
        btf_dump__dump_type_data() causes libbpf to read past the end of
        the payload buffer.

Enforce an upper bound on augmented_arg->size against the remaining
buffer (arg->augmented.size - sizeof(*augmented_arg)) and verify that the
captured payload contains at least type->size bytes before passing it to
btf_dump__dump_type_data().

Fixes: cb32035214b9 ("perf trace: Pretty print struct data")
Reported-by: sashiko-bot <sashiko-bot@kernel.org>
Signed-off-by: Aaron Tomlin <atomlin@atomlin.com>
---
 tools/perf/builtin-trace.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/tools/perf/builtin-trace.c b/tools/perf/builtin-trace.c
index be19d70eba09..20fffc24507b 100644
--- a/tools/perf/builtin-trace.c
+++ b/tools/perf/builtin-trace.c
@@ -1133,12 +1133,13 @@ static size_t btf_struct_scnprintf(const struct btf_type *type, struct btf *btf,
 	LIBBPF_OPTS(btf_dump_opts, dump_opts);
 	LIBBPF_OPTS(btf_dump_type_data_opts, dump_data_opts);
 
-	if (arg == NULL || arg->augmented.args == NULL || arg->augmented.size <= 0 ||
+	if (arg == NULL || arg->augmented.args == NULL || arg->augmented.size < (int)sizeof(*augmented_arg) ||
 	    arg->fmt == NULL || !arg->fmt->from_user)
 		return 0;
 
 	augmented_arg = arg->augmented.args;
-	if (augmented_arg->size <= 0)
+	if (augmented_arg->size <= 0 || augmented_arg->size > arg->augmented.size - (int)sizeof(*augmented_arg) ||
+	    (size_t)augmented_arg->size < type->size)
 		return 0;
 
 	dump_data_opts.compact	  = true;
-- 
2.55.0

[PATCH v2 perf-tools-next 2/6] perf trace: Validate payload bounds in augmented string beautifier

[Aaron Tomlin]

When pretty-printing augmented string arguments via
syscall_arg__scnprintf_augmented_string(), augmented_arg->size is not
validated against the remaining buffer size (arg->augmented.size).

If a malformed or truncated perf.data record provides an invalid or
excessively large augmented_arg->size:
    1.  If arg->augmented.size is smaller than sizeof(*augmented_arg),
        dereferencing augmented_arg->size reads past the available
        buffer.

    2.  Calculating consumed = sizeof(*augmented_arg) +
        augmented_arg->size can overflow signed integer bounds or cause
        arg->augmented.size to underflow, advancing arg->augmented.args
        out of bounds and corrupting the parsing state for subsequent
        arguments in multi-argument syscalls.

Validate that arg->augmented.size is large enough to hold
sizeof(*augmented_arg) and that augmented_arg->size is within the bounds
of the remaining buffer before printing or calculating consumed bytes. If
validation fails, fall back to printing the raw pointer value.

Fixes: 8195168e8779 ("perf trace: Consume the augmented_raw_syscalls payload")
Reported-by: sashiko-bot <sashiko-bot@kernel.org>
Signed-off-by: Aaron Tomlin <atomlin@atomlin.com>
---
 tools/perf/builtin-trace.c | 21 +++++++++++++++++----
 1 file changed, 17 insertions(+), 4 deletions(-)

diff --git a/tools/perf/builtin-trace.c b/tools/perf/builtin-trace.c
index 20fffc24507b..91461ab927b6 100644
--- a/tools/perf/builtin-trace.c
+++ b/tools/perf/builtin-trace.c
@@ -1898,12 +1898,21 @@ static void thread__set_filename_pos(struct thread *thread, const char *bf,
 static size_t syscall_arg__scnprintf_augmented_string(struct syscall_arg *arg, char *bf, size_t size)
 {
 	struct augmented_arg *augmented_arg = arg->augmented.args;
-	size_t printed = scnprintf(bf, size, "\"%.*s\"", augmented_arg->size, augmented_arg->value);
+	size_t printed;
+	int consumed;
+
+	if (arg->augmented.size < (int)sizeof(*augmented_arg))
+		return 0;
+
+	if (augmented_arg->size <= 0 || augmented_arg->size > arg->augmented.size - (int)sizeof(*augmented_arg))
+		return 0;
+
+	printed = scnprintf(bf, size, "\"%.*s\"", augmented_arg->size, augmented_arg->value);
 	/*
 	 * So that the next arg with a payload can consume its augmented arg, i.e. for rename* syscalls
 	 * we would have two strings, each prefixed by its size.
 	 */
-	int consumed = sizeof(*augmented_arg) + augmented_arg->size;
+	consumed = sizeof(*augmented_arg) + augmented_arg->size;
 
 	arg->augmented.args = ((void *)arg->augmented.args) + consumed;
 	arg->augmented.size -= consumed;
@@ -1916,8 +1925,12 @@ static size_t syscall_arg__scnprintf_filename(char *bf, size_t size,
 {
 	unsigned long ptr = arg->val;
 
-	if (arg->augmented.args)
-		return syscall_arg__scnprintf_augmented_string(arg, bf, size);
+	if (arg->augmented.args) {
+		size_t printed = syscall_arg__scnprintf_augmented_string(arg, bf, size);
+
+		if (printed)
+			return printed;
+	}
 
 	if (!arg->trace->vfs_getname)
 		return scnprintf(bf, size, "%#x", ptr);
-- 
2.55.0

[PATCH v2 perf-tools-next 3/6] perf trace: Validate payload bounds in augmented buffer beautifier

[Aaron Tomlin]

When pretty-printing non-string augmented buffers via
syscall_arg__scnprintf_buf(), augmented_arg->size is not validated
against the remaining buffer size (arg->augmented.size).

If a malformed or truncated perf.data record provides an invalid or
excessively large augmented_arg->size:
    1.  If arg->augmented.size is smaller than sizeof(*augmented_arg),
        dereferencing augmented_arg->size reads past the available
        buffer.

    2.  The loop reads up to augmented_arg->size bytes from
        augmented_arg->value without verifying that they exist within
        the buffer, leading to buffer over-reads.

    3.  Calculating consumed = sizeof(*augmented_arg) +
        augmented_arg->size can overflow signed integer bounds or cause
        arg->augmented.size to underflow, advancing arg->augmented.args
        out of bounds and corrupting the parsing state for subsequent
        arguments in multi-argument syscalls.

Validate that arg->augmented.size is large enough to hold
sizeof(*augmented_arg) and that augmented_arg->size is within the bounds
of the remaining buffer before iterating or calculating consumed bytes.
If validation fails, fall back to printing the raw pointer value.

Fixes: b257fac12f38 ("perf trace: Pretty print buffer data")
Reported-by: sashiko-bot <sashiko-bot@kernel.org>
Signed-off-by: Aaron Tomlin <atomlin@atomlin.com>
---
 tools/perf/builtin-trace.c | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/tools/perf/builtin-trace.c b/tools/perf/builtin-trace.c
index 91461ab927b6..4277de761a54 100644
--- a/tools/perf/builtin-trace.c
+++ b/tools/perf/builtin-trace.c
@@ -1945,13 +1945,17 @@ static size_t syscall_arg__scnprintf_filename(char *bf, size_t size,
 static size_t syscall_arg__scnprintf_buf(char *bf, size_t size, struct syscall_arg *arg)
 {
 	struct augmented_arg *augmented_arg = arg->augmented.args;
-	unsigned char *orig = (unsigned char *)augmented_arg->value;
+	unsigned char *orig;
 	size_t printed = 0;
 	int consumed;
 
-	if (augmented_arg == NULL)
-		return 0;
+	if (augmented_arg == NULL || arg->augmented.size < (int)sizeof(*augmented_arg))
+		return scnprintf(bf, size, "%#lx", arg->val);
+
+	if (augmented_arg->size <= 0 || augmented_arg->size > arg->augmented.size - (int)sizeof(*augmented_arg))
+		return scnprintf(bf, size, "%#lx", arg->val);
 
+	orig = (unsigned char *)augmented_arg->value;
 	for (int j = 0; j < augmented_arg->size; ++j) {
 		bool control_char = orig[j] <= MAX_CONTROL_CHAR || orig[j] >= MAX_ASCII;
 		/* print control characters (0~31 and 127), and non-ascii characters in \(digits) */
-- 
2.55.0

[PATCH v2 perf-tools-next 4/6] perf trace beauty: Validate payload size in augmented timespec beautifier

[Aaron Tomlin]

When pretty-printing augmented timespec arguments via
syscall_arg__scnprintf_augmented_timespec(), arg->augmented.args->value
is cast to struct timespec without verifying that the captured payload
is large enough to hold the structure.

If a malformed or truncated perf.data record provides an augmented
payload smaller than sizeof(struct timespec), accessing ts->tv_sec or
ts->tv_nsec reads memory past the end of the available buffer.

Validate that arg->augmented.size is at least sizeof(struct augmented_arg)
and that augmented_arg->size is at least sizeof(struct timespec) while
remaining within the available buffer. If validation fails, fall back to
printing the raw pointer value.

Fixes: 6ac73820993c ("perf trace: Add augmenter for clock_gettime's rqtp timespec arg")
Reported-by: sashiko-bot <sashiko-bot@kernel.org>
Signed-off-by: Aaron Tomlin <atomlin@atomlin.com>
---
 tools/perf/trace/beauty/timespec.c | 19 ++++++++++++++++---
 1 file changed, 16 insertions(+), 3 deletions(-)

diff --git a/tools/perf/trace/beauty/timespec.c b/tools/perf/trace/beauty/timespec.c
index b14ab72a2738..fad503bd953a 100644
--- a/tools/perf/trace/beauty/timespec.c
+++ b/tools/perf/trace/beauty/timespec.c
@@ -7,15 +7,28 @@
 
 static size_t syscall_arg__scnprintf_augmented_timespec(struct syscall_arg *arg, char *bf, size_t size)
 {
-	struct timespec *ts = (struct timespec *)arg->augmented.args->value;
+	struct augmented_arg *augmented_arg = arg->augmented.args;
+	struct timespec *ts;
 
+	if (arg->augmented.size < (int)sizeof(*augmented_arg))
+		return 0;
+
+	if (augmented_arg->size < (int)sizeof(*ts) ||
+	    augmented_arg->size > arg->augmented.size - (int)sizeof(*augmented_arg))
+		return 0;
+
+	ts = (struct timespec *)augmented_arg->value;
 	return scnprintf(bf, size, "{ .tv_sec: %" PRIu64 ", .tv_nsec: %" PRIu64 " }", ts->tv_sec, ts->tv_nsec);
 }
 
 size_t syscall_arg__scnprintf_timespec(char *bf, size_t size, struct syscall_arg *arg)
 {
-	if (arg->augmented.args)
-		return syscall_arg__scnprintf_augmented_timespec(arg, bf, size);
+	if (arg->augmented.args) {
+		size_t printed = syscall_arg__scnprintf_augmented_timespec(arg, bf, size);
+
+		if (printed)
+			return printed;
+	}
 
 	return scnprintf(bf, size, "%#lx", arg->val);
 }
-- 
2.55.0

[PATCH v2 perf-tools-next 5/6] perf trace beauty: Validate payload size in augmented sockaddr beautifier

[Aaron Tomlin]

When pretty-printing augmented sockaddr arguments via
syscall_arg__scnprintf_augmented_sockaddr(), &arg->augmented.args->value
is cast to struct sockaddr without checking whether the captured payload
is large enough to hold the socket address.

If a malformed or truncated perf.data record provides an augmented
payload smaller than sizeof(sa->sa_family), accessing sa->sa_family or
dereferencing family-specific fields (e.g., struct sockaddr_in,
sockaddr_in6, or sockaddr_un) reads memory past the available buffer.

Validate that arg->augmented.size is at least sizeof(struct augmented_arg)
and that augmented_arg->size is at least sizeof(sa->sa_family) while
remaining within the available buffer. Furthermore, ensure that the
payload contains sufficient bytes for the given sa->sa_family before
invoking the family-specific formatter. If validation fails, fall back
to printing the raw pointer value.

Fixes: d5a7e6613b00 ("perf trace augmented_syscalls: Augment connect's 'sockaddr' arg")
Reported-by: sashiko-bot <sashiko-bot@kernel.org>
Signed-off-by: Aaron Tomlin <atomlin@atomlin.com>
---
 tools/perf/trace/beauty/sockaddr.c | 35 +++++++++++++++++++++++++-----
 1 file changed, 30 insertions(+), 5 deletions(-)

diff --git a/tools/perf/trace/beauty/sockaddr.c b/tools/perf/trace/beauty/sockaddr.c
index a17a27ac2a6f..00c702ff4465 100644
--- a/tools/perf/trace/beauty/sockaddr.c
+++ b/tools/perf/trace/beauty/sockaddr.c
@@ -47,23 +47,48 @@ static size_t (*af_scnprintfs[])(struct sockaddr *sa, char *bf, size_t size) = {
 
 static size_t syscall_arg__scnprintf_augmented_sockaddr(struct syscall_arg *arg, char *bf, size_t size)
 {
-	struct sockaddr *sa = (struct sockaddr *)&arg->augmented.args->value;
+	struct augmented_arg *augmented_arg = arg->augmented.args;
+	struct sockaddr *sa;
 	char family[32];
 	size_t printed;
 
+	if (arg->augmented.size < (int)sizeof(*augmented_arg))
+		return 0;
+
+	if (augmented_arg->size < (int)sizeof(sa->sa_family) ||
+	    augmented_arg->size > arg->augmented.size - (int)sizeof(*augmented_arg))
+		return 0;
+
+	sa = (struct sockaddr *)&augmented_arg->value;
 	strarray__scnprintf(&strarray__socket_families, family, sizeof(family), "%d", arg->show_string_prefix, sa->sa_family);
 	printed = scnprintf(bf, size, "{ .family: %s", family);
 
-	if (sa->sa_family < ARRAY_SIZE(af_scnprintfs) && af_scnprintfs[sa->sa_family])
-		printed += af_scnprintfs[sa->sa_family](sa, bf + printed, size - printed);
+	if (sa->sa_family < ARRAY_SIZE(af_scnprintfs) && af_scnprintfs[sa->sa_family]) {
+		bool valid = false;
+
+		if (sa->sa_family == AF_INET && augmented_arg->size >= (int)sizeof(struct sockaddr_in))
+			valid = true;
+		else if (sa->sa_family == AF_INET6 && augmented_arg->size >= (int)sizeof(struct sockaddr_in6))
+			valid = true;
+		else if (sa->sa_family == AF_LOCAL &&
+			 augmented_arg->size > (int)offsetof(struct sockaddr_un, sun_path))
+			valid = true;
+
+		if (valid)
+			printed += af_scnprintfs[sa->sa_family](sa, bf + printed, size - printed);
+	}
 
 	return printed + scnprintf(bf + printed, size - printed, " }");
 }
 
 size_t syscall_arg__scnprintf_sockaddr(char *bf, size_t size, struct syscall_arg *arg)
 {
-	if (arg->augmented.args)
-		return syscall_arg__scnprintf_augmented_sockaddr(arg, bf, size);
+	if (arg->augmented.args) {
+		size_t printed = syscall_arg__scnprintf_augmented_sockaddr(arg, bf, size);
+
+		if (printed)
+			return printed;
+	}
 
 	return scnprintf(bf, size, "%#lx", arg->val);
 }
-- 
2.55.0

[PATCH v2 perf-tools-next 6/6] perf trace beauty: Validate payload size in augmented perf_event_open beautifier

[Aaron Tomlin]

When pretty-printing augmented perf_event_attr arguments via
syscall_arg__scnprintf_augmented_perf_event_attr(),
arg->augmented.args->value is cast to struct perf_event_attr and read
without verifying that the captured payload is large enough to contain
at least PERF_ATTR_SIZE_VER0 bytes.

If a malformed or truncated perf.data record provides an augmented
payload smaller than PERF_ATTR_SIZE_VER0, accessing attr->size or
executing memcpy(&local_attr, attr, PERF_ATTR_SIZE_VER0) reads memory
past the end of the available buffer.

Validate that arg->augmented.size is at least sizeof(struct augmented_arg)
and that augmented_arg->size is at least PERF_ATTR_SIZE_VER0 while
remaining within the available buffer. If validation fails, fall back to
printing the raw pointer value.

Fixes: a9cd6c676685 ("perf trace: Add BPF augmenter to perf_event_open()'s 'struct perf_event_attr' arg")
Reported-by: sashiko-bot <sashiko-bot@kernel.org>
Signed-off-by: Aaron Tomlin <atomlin@atomlin.com>
---
 tools/perf/trace/beauty/perf_event_open.c | 19 ++++++++++++++++---
 1 file changed, 16 insertions(+), 3 deletions(-)

diff --git a/tools/perf/trace/beauty/perf_event_open.c b/tools/perf/trace/beauty/perf_event_open.c
index 6315b46bcdf0..846738225abd 100644
--- a/tools/perf/trace/beauty/perf_event_open.c
+++ b/tools/perf/trace/beauty/perf_event_open.c
@@ -81,9 +81,18 @@ static size_t perf_event_attr___scnprintf(struct perf_event_attr *attr, char *bf
 
 static size_t syscall_arg__scnprintf_augmented_perf_event_attr(struct syscall_arg *arg, char *bf, size_t size)
 {
-	struct perf_event_attr *attr = (void *)arg->augmented.args->value;
+	struct augmented_arg *augmented_arg = arg->augmented.args;
+	struct perf_event_attr *attr;
 	struct perf_event_attr local_attr;
 
+	if (arg->augmented.size < (int)sizeof(*augmented_arg))
+		return 0;
+
+	if (augmented_arg->size < (int)PERF_ATTR_SIZE_VER0 ||
+	    augmented_arg->size > arg->augmented.size - (int)sizeof(*augmented_arg))
+		return 0;
+
+	attr = (void *)augmented_arg->value;
 	/*
 	 * augmented_raw_syscalls.bpf.c (shipped with perf) copies
 	 * PERF_ATTR_SIZE_VER0 bytes when the tracee passes size=0,
@@ -107,8 +116,12 @@ static size_t syscall_arg__scnprintf_augmented_perf_event_attr(struct syscall_ar
 
 size_t syscall_arg__scnprintf_perf_event_attr(char *bf, size_t size, struct syscall_arg *arg)
 {
-	if (arg->augmented.args)
-		return syscall_arg__scnprintf_augmented_perf_event_attr(arg, bf, size);
+	if (arg->augmented.args) {
+		size_t printed = syscall_arg__scnprintf_augmented_perf_event_attr(arg, bf, size);
+
+		if (printed)
+			return printed;
+	}
 
 	return scnprintf(bf, size, "%#lx", arg->val);
 }
-- 
2.55.0

Re: [PATCH v2 perf-tools-next 1/6] perf trace: Add upper bound checks for augmented BTF struct printing

[sashiko-bot]

> When pretty-printing augmented struct payloads using BTF via
> btf_struct_scnprintf(), augmented_arg->size is currently only validated
> against values <= 0.
> 
> However, several edge cases can result in size mismatches or buffer
> over-reads:
>     1.  If arg->augmented.size is smaller than sizeof(*augmented_arg),
>         dereferencing augmented_arg->size reads past the available
>         buffer.
> 
>     2.  If augmented_arg->size exceeds arg->augmented.size -
>         sizeof(*augmented_arg), calculating consumed =
>         sizeof(*augmented_arg) + augmented_arg->size can overflow signed
>         integer limits (e.g., with crafted INT_MAX values in an
>         untrusted perf.data file) or cause arg->augmented.size to
> [ ... ]
> btf_dump__dump_type_data().
> 
> Fixes: cb32035214b9 ("perf trace: Pretty print struct data")
> Reported-by: sashiko-bot <sashiko-bot@kernel.org>
> Signed-off-by: Aaron Tomlin <atomlin@atomlin.com>

Sashiko has reviewed this patch and found no issues. It looks great!

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260907015140.363076-1-atomlin@atomlin.com?part=1

Re: [PATCH v2 perf-tools-next 5/6] perf trace beauty: Validate payload size in augmented sockaddr beautifier

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:

New issues:
- [Medium] Hardcoded address families in the validation logic break extensibility for future formatters.
- [Low] Missing explicit POSIX / Standard C header inclusions for newly introduced types and macros.

Pre-existing issues:
- [High] Missing null-termination enforcement in AF_LOCAL socket paths leads to out-of-bounds reads and abstract socket logic errors.
--

commit 229e56bcf8519abca8347b48fb3e2d0bdc74e111
Author: Aaron Tomlin <atomlin@atomlin.com>

perf trace beauty: Validate payload size in augmented sockaddr beautifier

This commit adds size validation to the augmented sockaddr beautifier to
ensure the captured payload is large enough before dereferencing address
family-specific fields. It verifies the payload size against the augmented
argument bounds and checks for sufficient bytes for AF_INET, AF_INET6, and
AF_LOCAL before formatting.

> diff --git a/tools/perf/trace/beauty/sockaddr.c b/tools/perf/trace/beauty/sockaddr.c
> index a17a27ac2a6ff..00c702ff4465b 100644
> --- a/tools/perf/trace/beauty/sockaddr.c
> +++ b/tools/perf/trace/beauty/sockaddr.c
[ ... ]
> @@ -47,23 +47,48 @@ static size_t (*af_scnprintfs[])(struct sockaddr *sa, char *bf, size_t size) = {
[ ... ]
>  	if (sa->sa_family < ARRAY_SIZE(af_scnprintfs) && af_scnprintfs[sa->sa_family]) {
> +		bool valid = false;

[Severity: Low]
Does this file miss the <stdbool.h> header for the bool type?

The patch introduces the usage of bool in tools/perf/trace/beauty/sockaddr.c,
but the explicit POSIX standard header is missing, which can cause compilation
failures on systems using musl libc.

> +
> +		if (sa->sa_family == AF_INET && augmented_arg->size >= (int)sizeof(struct sockaddr_in))
> +			valid = true;
> +		else if (sa->sa_family == AF_INET6 && augmented_arg->size >= (int)sizeof(struct sockaddr_in6))
> +			valid = true;

[Severity: Medium]
Does this hardcoded validation bypass the extensibility of the af_scnprintfs
dispatch table?

If a new address family formatter (e.g., AF_NETLINK) is added to the
af_scnprintfs dispatch array in the future, it appears it will never be
invoked because the hardcoded validation logic only explicitly recognizes
AF_INET, AF_INET6, and AF_LOCAL.

> +		else if (sa->sa_family == AF_LOCAL &&
> +			 augmented_arg->size > (int)offsetof(struct sockaddr_un, sun_path))

[Severity: Low]
Does this file miss the <stddef.h> header for the offsetof macro?

Similar to bool, the patch introduces offsetof but fails to explicitly include
<stddef.h>, which is required for musl compatibility.

> +			valid = true;

[Severity: High]
This is a pre-existing issue, but does this newly introduced validation fail
to enforce NUL-termination or bound the string length for AF_LOCAL paths?

This explicitly marks the payload as valid without ensuring NUL termination.
Later in syscall_arg__scnprintf_augmented_sockaddr(), the formatter is called
without passing the actual bounds of the captured sockaddr payload:

tools/perf/trace/beauty/sockaddr.c:syscall_arg__scnprintf_augmented_sockaddr() {
    ...
    if (valid)
        printed += af_scnprintfs[sa->sa_family](sa, bf + printed, size - printed);
    ...
}

This leaves af_local__scnprintf() unable to safely bound the read when
formatting sun->sun_path with the "%s" specifier. If the captured payload in
perf.data lacks a NUL terminator, scnprintf() will read past the validated
bounds of the augmented_arg buffer.

> +
> +		if (valid)
> +			printed += af_scnprintfs[sa->sa_family](sa, bf + printed, size - printed);
> +	}
>  
>  	return printed + scnprintf(bf + printed, size - printed, " }");
>  }
[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260907015140.363076-1-atomlin@atomlin.com?part=5

Re: [PATCH v2 perf-tools-next 4/6] perf trace beauty: Validate payload size in augmented timespec beautifier

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Medium] Validation logic relies on the host's native `struct timespec` layout instead of dynamically querying the trace environment, breaking cross-platform analysis.
- [Medium] Validation relies on an uninitialized/stale field from the BPF tracer, silently breaking timespec formatting.
--

commit 8e3c4b6ee60070e35799a1584520842d1e0f8b14
Author: Aaron Tomlin <atomlin@atomlin.com>

perf trace beauty: Validate payload size in augmented timespec beautifier

This commit adds bounds checking to the augmented timespec beautifier in perf 
trace to prevent reading past the end of the payload buffer for malformed or 
truncated perf.data records.

> diff --git a/tools/perf/trace/beauty/timespec.c b/tools/perf/trace/beauty/timespec.c
> index b14ab72a2738e..fad503bd953a6 100644
> --- a/tools/perf/trace/beauty/timespec.c
> +++ b/tools/perf/trace/beauty/timespec.c
> @@ -7,15 +7,28 @@
>  
>  static size_t syscall_arg__scnprintf_augmented_timespec(struct syscall_arg *arg, char *bf, size_t size)
>  {
> -	struct timespec *ts = (struct timespec *)arg->augmented.args->value;
> +	struct augmented_arg *augmented_arg = arg->augmented.args;
> +	struct timespec *ts;
>  
> +	if (arg->augmented.size < (int)sizeof(*augmented_arg))
> +		return 0;
> +
> +	if (augmented_arg->size < (int)sizeof(*ts) ||
> +	    augmented_arg->size > arg->augmented.size - (int)sizeof(*augmented_arg))
> +		return 0;

[Severity: Medium]
Does this validation break cross-architecture trace analysis? The check
relies strictly on the host's sizeof(*ts), which evaluates to 16 on a
64-bit host. If a perf.data file recorded on a 32-bit architecture is
analyzed here, wouldn't the tracee's valid, smaller payload be incorrectly
rejected by this bounds check?

[Severity: Medium]
Can this bounds check fail randomly due to uninitialized data from the
eBPF tracer? Looking at the eBPF side in sys_enter_clock_nanosleep:

tools/perf/util/bpf_skel/augmented_raw_syscalls.bpf.c:sys_enter_clock_nanosleep() {
    ...
    bpf_probe_read_user(&augmented_args->arg.value, size, rqtp_arg);
    return augmented__output(args, augmented_args, len + size);
}

The eBPF tracer does not appear to initialize augmented_args->arg.size.
Since the augmented_args_tmp per-CPU array is reused across tracepoints,
if a prior syscall (like open) set the size to a path length, wouldn't
the new check here read that stale size and reject valid payloads?

> +
> +	ts = (struct timespec *)augmented_arg->value;
>  	return scnprintf(bf, size, "{ .tv_sec: %" PRIu64 ", .tv_nsec: %" PRIu64 " }", ts->tv_sec, ts->tv_nsec);
>  }

[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260907015140.363076-1-atomlin@atomlin.com?part=4

Re: [PATCH v2 perf-tools-next 6/6] perf trace beauty: Validate payload size in augmented perf_event_open beautifier

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

New issues:
- [High] The newly added bounds check relies on the `augmented_arg->size` field, but the BPF program for `sys_enter_perf_event_open` leaves this field uninitialized, randomly breaking `perf trace` augmentation.

Pre-existing issues:
- [High] The patch fails to validate that the embedded `attr->size` does not exceed the actual augmented payload size, allowing out-of-bounds reads if the tracee mutates the field (TOCTOU) or if a malformed `perf.data` file is parsed.
--

commit f9180137a1c578e9c897496a107baa521c327ae0
Author: Aaron Tomlin <atomlin@atomlin.com>

perf trace beauty: Validate payload size in augmented perf_event_open beautifier

This commit adds bounds checking to the augmented payload of perf_event_open
in perf trace. It attempts to verify that the augmented argument size is large
enough to contain at least PERF_ATTR_SIZE_VER0 bytes before accessing the
perf_event_attr payload.

> diff --git a/tools/perf/trace/beauty/perf_event_open.c b/tools/perf/trace/beauty/perf_event_open.c
> index 6315b46bcdf02..846738225abd7 100644
> --- a/tools/perf/trace/beauty/perf_event_open.c
> +++ b/tools/perf/trace/beauty/perf_event_open.c
> @@ -81,9 +81,18 @@ static size_t perf_event_attr___scnprintf(struct perf_event_attr *attr, char *bf
>  
>  static size_t syscall_arg__scnprintf_augmented_perf_event_attr(struct syscall_arg *arg, char *bf, size_t size)
>  {
> -	struct perf_event_attr *attr = (void *)arg->augmented.args->value;
> +	struct augmented_arg *augmented_arg = arg->augmented.args;
> +	struct perf_event_attr *attr;
>  	struct perf_event_attr local_attr;
>  
> +	if (arg->augmented.size < (int)sizeof(*augmented_arg))
> +		return 0;
> +
> +	if (augmented_arg->size < (int)PERF_ATTR_SIZE_VER0 ||
> +	    augmented_arg->size > arg->augmented.size - (int)sizeof(*augmented_arg))
> +		return 0;

[Severity: High]
Does this newly added bounds check read an uninitialized field?

Looking at sys_enter_perf_event_open() in
tools/perf/util/bpf_skel/augmented_raw_syscalls.bpf.c, the BPF program reads
the payload and outputs it:

    if (bpf_probe_read_user(&augmented_args->arg.value, size, attr) < 0)
        goto failure;

    return augmented__output(args, augmented_args, len + size);

However, it does not appear to populate augmented_args->arg.size. Since
augmented_args is allocated from a reused per-CPU map, will size contain
junk data from previous syscalls (like an openat length), causing this
check to randomly fail and break the trace augmentation?

> +
> +	attr = (void *)augmented_arg->value;

[Severity: High]
This is a pre-existing issue, but does the newly added bounds check completely
prevent out-of-bounds reads if the tracee mutates the embedded attr->size
field?

While this patch validates that augmented_arg->size fits within the buffer,
attr is eventually passed down to perf_event_attr__fprintf() which trusts
attr->size directly:

    u32 attr_size = attr->size ?: PERF_ATTR_SIZE_VER0;
    ...
    if (attr_size > sizeof(*attr))
        attr_size = sizeof(*attr);

If a malformed perf.data record or a TOCTOU mutation provides an attr->size
that is larger than the actual augmented payload (e.g., 136 bytes while the
buffer only contains PERF_ATTR_SIZE_VER0 bytes), won't the formatter read
past the end of the available buffer and leak adjacent memory into the
trace?

[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260907015140.363076-1-atomlin@atomlin.com?part=6

Re: [PATCH v2 perf-tools-next 2/6] perf trace: Validate payload bounds in augmented string beautifier

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] Failing to validate memory alignment of malformed payload sizes causes SIGBUS traps on subsequent arguments.
--

commit 2e5b760b204b32d343eb2501c4ff84ed16a4618a
Author: Aaron Tomlin <atomlin@atomlin.com>
Subject: perf trace: Validate payload bounds in augmented string beautifier

This commit adds bounds validation for augmented_arg->size when 
pretty-printing augmented string arguments in the perf trace tool. It 
ensures the payload size is within the remaining buffer bounds before 
calculating consumed bytes, preventing potential buffer over-reads and 
integer overflows.

> diff --git a/tools/perf/builtin-trace.c b/tools/perf/builtin-trace.c
> index 20fffc24507b5..91461ab927b63 100644
> --- a/tools/perf/builtin-trace.c
> +++ b/tools/perf/builtin-trace.c

[ ... ]

> @@ -1898,12 +1898,21 @@ static void thread__set_filename_pos(struct thread *thread, const char *bf,
>  static size_t syscall_arg__scnprintf_augmented_string(struct syscall_arg *arg, char *bf, size_t size)
>  {
>  	struct augmented_arg *augmented_arg = arg->augmented.args;
> -	size_t printed = scnprintf(bf, size, "\"%.*s\"", augmented_arg->size, augmented_arg->value);
> +	size_t printed;
> +	int consumed;
> +
> +	if (arg->augmented.size < (int)sizeof(*augmented_arg))
> +		return 0;
> +
> +	if (augmented_arg->size <= 0 || augmented_arg->size > arg->augmented.size - (int)sizeof(*augmented_arg))
> +		return 0;
> +
> +	printed = scnprintf(bf, size, "\"%.*s\"", augmented_arg->size, augmented_arg->value);
>  	/*
>  	 * So that the next arg with a payload can consume its augmented arg, i.e. for rename* syscalls
>  	 * we would have two strings, each prefixed by its size.
>  	 */
> -	int consumed = sizeof(*augmented_arg) + augmented_arg->size;
> +	consumed = sizeof(*augmented_arg) + augmented_arg->size;
>  
>  	arg->augmented.args = ((void *)arg->augmented.args) + consumed;

[Severity: High]
This is a pre-existing issue, but does failing to validate the memory 
alignment of malformed payload sizes cause SIGBUS traps on subsequent
arguments?

If a user uses the perf trace tool to read a malformed or maliciously 
crafted perf.data file where augmented_arg->size is not naturally 
aligned, the tool advances the pointer by this unaligned size without 
correcting it. 

Can the next argument read dereference a misaligned pointer and crash 
the perf tool on architectures requiring strict memory alignment?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260907015140.363076-1-atomlin@atomlin.com?part=2

Re: [PATCH v2 perf-tools-next 3/6] perf trace: Validate payload bounds in augmented buffer beautifier

[sashiko-bot]

> When pretty-printing non-string augmented buffers via
> syscall_arg__scnprintf_buf(), augmented_arg->size is not validated
> against the remaining buffer size (arg->augmented.size).
> 
> If a malformed or truncated perf.data record provides an invalid or
> excessively large augmented_arg->size:
>     1.  If arg->augmented.size is smaller than sizeof(*augmented_arg),
>         dereferencing augmented_arg->size reads past the available
>         buffer.
> 
>     2.  The loop reads up to augmented_arg->size bytes from
>         augmented_arg->value without verifying that they exist within
>         the buffer, leading to buffer over-reads.
> 
>     3.  Calculating consumed = sizeof(*augmented_arg) +
>         augmented_arg->size can overflow signed integer bounds or cause
>         arg->augmented.size to underflow, advancing arg->augmented.args
>         out of bounds and corrupting the parsing state for subsequent
>         arguments in multi-argument syscalls.
> 
> Validate that arg->augmented.size is large enough to hold
> sizeof(*augmented_arg) and that augmented_arg->size is within the bounds
> of the remaining buffer before iterating or calculating consumed bytes.
> If validation fails, fall back to printing the raw pointer value.
> 
> Fixes: b257fac12f38 ("perf trace: Pretty print buffer data")
> Reported-by: sashiko-bot <sashiko-bot@kernel.org>
> Signed-off-by: Aaron Tomlin <atomlin@atomlin.com>

Sashiko has reviewed this patch and found no issues. It looks great!

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260907015140.363076-1-atomlin@atomlin.com?part=3