Re: [PATCH perf-tools-next v6 1/5] perf trace: Fix error checking in btf_struct_scnprintf()

[Aaron Tomlin]

On Fri, Sep 04, 2026 at 06:17:54PM -0300, Arnaldo Carvalho de Melo wrote:
> On Mon, Aug 24, 2026 at 09:31:18AM -0400, Aaron Tomlin wrote:
> > btf_dump__dump_type_data() returns the positive number of bytes dumped on
> > success, or a negative error code (e.g., -EINVAL) on failure.
> > 
> > Currently, btf_struct_scnprintf() checks if btf_dump__dump_type_data()
> > returns 0. When a negative error code is returned on failure, this check
> > evaluates to false, causing the function to proceed down the success path
> > and advance arg->augmented.args past the unprinted struct data before
> > returning 0.
> > 
> > Consequently, when syscall__scnprintf_args() falls back to the default
> > argument beautifier upon receiving 0 from trace__btf_scnprintf(), the
> > fallback beautifier or subsequent arguments read from an erroneously
> > advanced augmented arguments buffer.
> > 
> > Fix this by checking for '<= 0' ensuring that on any failure or 0-byte
> > dump, 0 is returned without modifying the augmented arguments pointer.
> > 
> > Fixes: cb32035214b9 ("perf trace: Pretty print augmented struct args using BTF")
> > Reported-by: sashiko-bot <sashiko-bot@xxxxxxxxxx>
> > Signed-off-by: Aaron Tomlin <atomlin@xxxxxxxxxxx>
> > ---
> >  tools/perf/builtin-trace.c | 4 +++-
> >  1 file changed, 3 insertions(+), 1 deletion(-)
> > 
> > diff --git a/tools/perf/builtin-trace.c b/tools/perf/builtin-trace.c
> > index c3c7f1f85c53..1bc39f674f11 100644
> > --- a/tools/perf/builtin-trace.c
> > +++ b/tools/perf/builtin-trace.c
> > @@ -1066,7 +1066,9 @@ static size_t btf_struct_scnprintf(const struct btf_type *type, struct btf *btf,
> >  		return 0;
> >  
> >  	/* pretty print the struct data here */
> > -	if (btf_dump__dump_type_data(btf_dump, type_id, arg->augmented.args->value, type->size, &dump_data_opts) == 0)
> > +	if (btf_dump__dump_type_data(btf_dump, type_id,
> > +				     arg->augmented.args->value,
> > +				     type->size, &dump_data_opts) <= 0)
> >  		return 0;
> 
> Applied the series, only change I made was:
> 
> diff --git a/tools/perf/builtin-trace.c b/tools/perf/builtin-trace.c
> index c3c7f1f85c530196..4c8f91f0f854b382 100644
> --- a/tools/perf/builtin-trace.c
> +++ b/tools/perf/builtin-trace.c
> @@ -1066,7 +1066,7 @@ static size_t btf_struct_scnprintf(const struct btf_type *type, struct btf *btf,
>                 return 0;
> 
>         /* pretty print the struct data here */
> -       if (btf_dump__dump_type_data(btf_dump, type_id, arg->augmented.args->value, type->size, &dump_data_opts) == 0)
> +       if (btf_dump__dump_type_data(btf_dump, type_id, arg->augmented.args->value, type->size, &dump_data_opts) <= 0)
>                 return 0;
> 
>         consumed = sizeof(*augmented_arg) + augmented_arg->size;
> (END)
> acme@number:~/git/pahole$
> 
> As this source code allows for longer lines, having it like above helps
> in reviewing, we can see straight away what the change was. There was a
> clash a few patches later that expected that reflow to be in place, I
> adjusted that as well, no code changes in both cases.
> 
> Please avoid reflowing things.
> 
> Thanks for your work, appreciated!
> 
> - Arnaldo

Hi Arnaldo,

Noted and thank you.

Kind regards,
-- 
Aaron Tomlin