Re: [PATCH v3 perf-tools-next 3/7] perf trace: Align pointer advance in augmented string beautifier

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] perf trace: unpadded augmented arguments corruption due to unconditional PERF_ALIGN
--

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

perf trace: Align pointer advance in augmented string beautifier

This commit updates the augmented string beautifier in perf trace to
round up the consumed payload bytes to an 8-byte boundary using PERF_ALIGN.
It also adds bounds checking to prevent reading out of bounds if the aligned
consumed offset exceeds the remaining buffer.

> diff --git a/tools/perf/builtin-trace.c b/tools/perf/builtin-trace.c
> --- a/tools/perf/builtin-trace.c
> +++ b/tools/perf/builtin-trace.c
> @@ -1912,10 +1912,15 @@ static size_t syscall_arg__scnprintf_augmented_string(struct syscall_arg *arg, c
>  	 * 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.
>  	 */
> -	consumed = sizeof(*augmented_arg) + augmented_arg->size;
> +	consumed = sizeof(*augmented_arg) + PERF_ALIGN(augmented_arg->size, sizeof(u64));

[Severity: High]
Does this unconditional alignment cause corruption when reading arguments 
produced by the generic BPF tracepoint?

The generic BPF producer, specifically augment_arg() in 
tools/perf/util/bpf_skel/augmented_raw_syscalls.bpf.c, emits unpadded, 
tightly-packed arguments. When tracing syscalls with multiple string or 
pointer arguments (such as linkat, symlinkat, or move_mount), the BPF 
program packs the strings consecutively.

Since PERF_ALIGN skips non-existent padding bytes here, could this 
over-advance the pointer and misalign the reader for the second argument, 
causing it to read arbitrary payload bytes as the size and value fields? 

Additionally, might this affect backward compatibility when analyzing 
older, unpadded perf.data files where augmented string arguments are not 
aligned to an 8-byte boundary?

>  
> -	arg->augmented.args = ((void *)arg->augmented.args) + consumed;
> -	arg->augmented.size -= consumed;
> +	if (consumed > arg->augmented.size) {
> +		arg->augmented.args = NULL;
> +		arg->augmented.size = 0;
> +	} else {
> +		arg->augmented.args = ((void *)arg->augmented.args) + consumed;
> +		arg->augmented.size -= consumed;
> +	}
>  
>  	return printed;
>  }

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