[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@xxxxxxxxxx>
Signed-off-by: Aaron Tomlin <atomlin@xxxxxxxxxxx>
---
 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