[PATCH v5 10/23] perf trace: Do not read sample padding as an augmented argument

[Ian Rogers]

syscall__augmented_args() treats everything after the fixed tracepoint
payload as augmented arguments, taking any non-zero trailing length as a
struct augmented_arg. A record with no augmented arguments at all still
has a trailing run, because the raw payload is padded.

perf_sample_save_raw_data() sizes the raw data as:

	size = round_up(sum + sizeof(u32), sizeof(u64));
	raw->size = size - sizeof(u32);
	frag->pad = raw->size - sum;

and the kernel writes that padding with __output_skip(), which advances
over it rather than zeroing it, so the bytes are whatever the ring buffer
last held there. A 64 byte struct syscall_enter_args therefore arrives
with raw_size of 68, and the 4 bytes past the end are stale memory that
syscall__augmented_args() copies out and hands to a beautifier as the
size and int_arg of an augmented argument.

A trailing run shorter than a struct augmented_arg cannot be one, so
recognise it as the padding it is. The length prefix and the payload it
describes are checked separately by syscall_arg__augmented_args_valid();
this stops the padding being offered as augmented data in the first
place, so that the syscall appears with no augmented arguments as it
should rather than with one whose contents happen to pass validation.

Reported-by: Arnaldo Carvalho de Melo <acme@xxxxxxxxxx>
Closes: https://lore.kernel.org/linux-perf-users/arJ-gpzqOHk-gF8T@x2/
Assisted-by: Antigravity:gemini-3.1-pro
Signed-off-by: Ian Rogers <irogers@xxxxxxxxxx>
---
 tools/perf/builtin-trace.c | 47 ++++++++++++++++++++++++++------------
 1 file changed, 32 insertions(+), 15 deletions(-)

diff --git a/tools/perf/builtin-trace.c b/tools/perf/builtin-trace.c
index eeaaab44016c..aa2d64eb56bd 100644
--- a/tools/perf/builtin-trace.c
+++ b/tools/perf/builtin-trace.c
@@ -3022,6 +3022,7 @@ static void *syscall__augmented_args(struct trace *trace, struct syscall *sc,
 	 * traffic to just what is needed for each syscall.
 	 */
 	int args_size = raw_augmented_args_size ?: sc->args_size;
+	static uintptr_t argbuf[1024]; /* assuming single-threaded */
 
 	/*
 	 * Augmented arguments are a perf trace specific payload, they are only
@@ -3042,24 +3043,40 @@ static void *syscall__augmented_args(struct trace *trace, struct syscall *sc,
 		return NULL;
 
 	*augmented_args_size = sample->raw_size - args_size;
-	if (*augmented_args_size > 0) {
-		static uintptr_t argbuf[1024]; /* assuming single-threaded */
 
-		if ((size_t)(*augmented_args_size) > sizeof(argbuf))
-			return NULL;
-
-		/*
-		 * The perf ring-buffer is 8-byte aligned but sample->raw_data
-		 * is not because it's preceded by u32 size.  Later, beautifier
-		 * will use the augmented args with stricter alignments like in
-		 * some struct.  To make sure it's aligned, let's copy the args
-		 * into a static buffer as it's single-threaded for now.
-		 */
-		memcpy(argbuf, sample->raw_data + args_size, *augmented_args_size);
+	/*
+	 * perf_sample_save_raw_data() rounds the raw payload up to a multiple
+	 * of 8 bytes less the u32 that holds its size, and the kernel skips
+	 * over that padding rather than zeroing it, so those bytes are stale
+	 * ring buffer contents.
+	 *
+	 * A record that carries no augmented arguments at all therefore still
+	 * arrives with up to 7 trailing bytes, e.g. the 64 byte struct
+	 * syscall_enter_args that an unaugmented syscall emits comes back with
+	 * raw_size of 68. Anything shorter than a struct augmented_arg cannot
+	 * be one, so drop it instead of letting a beautifier read a length out
+	 * of uninitialised memory.
+	 */
+	if (*augmented_args_size < (int)sizeof(struct augmented_arg)) {
+		*augmented_args_size = 0;
+		return NULL;
+	}
 
-		return argbuf;
+	if ((size_t)(*augmented_args_size) > sizeof(argbuf)) {
+		*augmented_args_size = 0;
+		return NULL;
 	}
-	return NULL;
+
+	/*
+	 * The perf ring-buffer is 8-byte aligned but sample->raw_data
+	 * is not because it's preceded by u32 size.  Later, beautifier
+	 * will use the augmented args with stricter alignments like in
+	 * some struct.  To make sure it's aligned, let's copy the args
+	 * into a static buffer as it's single-threaded for now.
+	 */
+	memcpy(argbuf, sample->raw_data + args_size, *augmented_args_size);
+
+	return argbuf;
 }
 
 static int trace__sys_enter(struct trace *trace,
-- 
2.56.0.rc1.315.gc6ed9934b7-goog