Re: [PATCH v6 03/26] perf trace: Don't read sample padding as an augmented argument

[Namhyung Kim <namhyung@kernel.org>]

On Mon, Sep 28, 2026 at 11:25:42AM -0700, Ian Rogers wrote:
> The kernel pads PERF_SAMPLE_RAW data to a u64 boundary without zeroing
> the padding, so a syscall record without augmented arguments still has
> up to 7 trailing bytes of stale data. syscall__augmented_args() passes
> these to the beautifiers as a struct augmented_arg, whose size is then
> garbage, causing a crash in syscall_arg__scnprintf_buf().
> 
> Ignore trailing data shorter than a struct augmented_arg.
> 
> Reported-by: Arnaldo Carvalho de Melo <acme@kernel.org>
> 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@google.com>
> ---
>  tools/perf/builtin-trace.c | 35 +++++++++++++++++++----------------
>  1 file changed, 19 insertions(+), 16 deletions(-)
> 
> diff --git a/tools/perf/builtin-trace.c b/tools/perf/builtin-trace.c
> index d327603ae454..c39de91140a0 100644
> --- a/tools/perf/builtin-trace.c
> +++ b/tools/perf/builtin-trace.c
> @@ -2961,26 +2961,29 @@ static void *syscall__augmented_args(struct syscall *sc, struct perf_sample *sam
>  	 * 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_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 raw data is padded to a u64 boundary with stale bytes, so less
> +	 * than a struct augmented_arg is only padding.
> +	 */

Confusingly we have a very large struct augmented_arg in the BPF code
and it made me wonder..  Maybe we need to rename one of them later.

Anyway this looks good now.

Reviewed-by: Namhyung Kim <namhyung@kernel.org>

Thanks,
Namhyung


> +	if (*augmented_args_size < (int)sizeof(struct augmented_arg) ||
> +	    (size_t)(*augmented_args_size) > sizeof(argbuf)) {
> +		*augmented_args_size = 0;
> +		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);
> +	/*
> +	 * 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;
> -	}
> -	return NULL;
> +	return argbuf;
>  }
>  
>  static int trace__sys_enter(struct trace *trace,
> -- 
> 2.56.0.rc1.315.gc6ed9934b7-goog
>