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

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] perf trace: btf_struct_scnprintf() fails to consume short payloads
--

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

perf trace: Add upper bound checks for augmented BTF struct printing

This commit adds upper bound checks for augmented_arg->size against the
remaining buffer size. It prevents buffer over-reads and integer
overflows when pretty-printing augmented struct payloads using BTF.

> 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

[ ... ]

> @@ -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;

[Severity: Medium]
Does this early return fail to consume the short payload?

By returning 0 here when the augmented payload is smaller than type->size, it
appears the parser abandons the payload without updating arg->augmented.args.

When the next syscall argument is formatted, will it read this stale struct
payload as its own augmented data, corrupting the parsing state for all
subsequent arguments?

Since short reads can occur legitimately during BPF tracing when capturing
large structs, should this gracefully advance past the short payload rather
than abandoning the update entirely?

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