Re: [PATCH v6 06/26] perf trace: Copy sockaddr arguments by their length

[Arnaldo Carvalho de Melo <acme@kernel.org>]

On Mon, Sep 28, 2026 at 11:25:45AM -0700, Ian Rogers wrote:
> The BTF augmenter copies sizeof(struct sockaddr), 16 bytes, for the
> sockaddr arguments of connect, bind and sendto, and runs before the
> sys_enter_connect and sys_enter_sendto programs that copy their length.
> An IPv6 address needs 24 or 28 bytes, so the rest was read from past the
> payload, and now that the printers are bounded only its family is shown.
> 
> Copy them as buffers sized by the length argument after them, up to the
> 32 bytes augmented buffers are limited to. That holds an IPv6 address
> and doubles the AF_LOCAL path shown.
> 
> Fixes: a68fd6a6cdd3 ("perf trace: Collect augmented data using BPF")
> Assisted-by: Antigravity:gemini-3.1-pro
> Signed-off-by: Ian Rogers <irogers@google.com>
> ---
>  tools/perf/builtin-trace.c | 7 ++++++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
> 
> diff --git a/tools/perf/builtin-trace.c b/tools/perf/builtin-trace.c
> index 85db74965280..f94745a60f4a 100644
> --- a/tools/perf/builtin-trace.c
> +++ b/tools/perf/builtin-trace.c
> @@ -4159,7 +4159,12 @@ static int trace__bpf_sys_enter_beauty_map(struct trace *trace, int e_machine, i
>  				continue;
>  
>  			bt = sc->arg_fmt[i].type;
> -			beauty_array[i] = bt->size;
> +			/* Copy a sockaddr as a buffer sized by the next argument, e.g. addrlen. */
> +			if (strcmp(name, "sockaddr") == 0 && field->next &&
> +			    strstr(field->next->name, "len"))
> +				beauty_array[i] = -((i + 1) + 1);
> +			else
> +				beauty_array[i] = bt->size;


Humm, I thought that this would be called in BPF handlers like:

SEC("tp/syscalls/sys_enter_connect")
int sys_enter_connect(struct syscall_enter_args *args)
{
        struct augmented_args_payload *augmented_args = augmented_args_payload();
        const void *sockaddr_arg = (const void *)args->args[1];
        unsigned int socklen = args->args[2];
        unsigned int len = sizeof(u64) + sizeof(augmented_args->args); // the size + err in all 'augmented_arg' structs

        if (augmented_args == NULL)
                return 1; /* Failure: don't filter */

        _Static_assert(is_power_of_2(sizeof(augmented_args->arg.saddr)), "sizeof(augmented_args->arg.saddr) needs to be a power of two");
        socklen &= sizeof(augmented_args->arg.saddr) - 1;

        bpf_probe_read_user(&augmented_args->arg.saddr, socklen, sockaddr_arg);
        augmented_args->arg.size = socklen;
        augmented_args->arg.err = 0;

        return augmented__output(args, augmented_args, len + socklen);
}

And it knows how many bytes to read by looking at socklen
(args->args[2]), i.e. not use the generic BPF handler that uses this
beauty_array, because knowing how many bytes to read in this case is
dynamic, varies with each syscall, according to one of its arguments :-\

What am I missing?

- Arnaldo


>  			can_augment = true;
>  		} else if (field->flags & TEP_FIELD_IS_POINTER && /* string */
>  			   strcmp(field->type, "const char *") == 0 &&
> -- 
> 2.56.0.rc1.315.gc6ed9934b7-goog