Re: [PATCH bpf-next v2] libbpf: Fix potential uninitialized tail padding with LIBBPF_OPTS_RESET

[Martin KaFai Lau <martin.lau@linux.dev>]

On 11/6/23 10:29 PM, Yonghong Song wrote:
> Martin reported that there is a libbpf complaining of non-zero-value tail
> padding with LIBBPF_OPTS_RESET macro if struct bpf_netkit_opts is modified
> to have a 4-byte tail padding. This only happens to clang compiler.
> The commend line is: ./test_progs -t tc_netkit_multi_links
> Martin and I did some investigation and found this indeed the case and
> the following are the investigation details.
> 
> Clang 18:
>    clang version 18.0.0
>    <I tried clang15/16/17 and they all have similar results>
> 
> tools/lib/bpf/libbpf_common.h:
>    #define LIBBPF_OPTS_RESET(NAME, ...)                                      \
>          do {                                                                \
>                  memset(&NAME, 0, sizeof(NAME));                             \
>                  NAME = (typeof(NAME)) {                                     \
>                          .sz = sizeof(NAME),                                 \
>                          __VA_ARGS__                                         \
>                  };                                                          \
>          } while (0)
> 
>    #endif
> 
> tools/lib/bpf/libbpf.h:
>    struct bpf_netkit_opts {
>          /* size of this struct, for forward/backward compatibility */
>          size_t sz;
>          __u32 flags;
>          __u32 relative_fd;
>          __u32 relative_id;
>          __u64 expected_revision;
>          size_t :0;
>    };
>    #define bpf_netkit_opts__last_field expected_revision
> In the above struct bpf_netkit_opts, there is no tail padding.
> 
> prog_tests/tc_netkit.c:
>    static void serial_test_tc_netkit_multi_links_target(int mode, int target)
>    {
>          ...
>          LIBBPF_OPTS(bpf_netkit_opts, optl);
>          ...
>          LIBBPF_OPTS_RESET(optl,
>                  .flags = BPF_F_BEFORE,
>                  .relative_fd = bpf_program__fd(skel->progs.tc1),
>          );
>          ...
>    }
> 
> Let us make the following source change, note that we have a 4-byte
> tailing padding now.
>    diff --git a/tools/lib/bpf/libbpf.h b/tools/lib/bpf/libbpf.h
>    index 6cd9c501624f..0dd83910ae9a 100644
>    --- a/tools/lib/bpf/libbpf.h
>    +++ b/tools/lib/bpf/libbpf.h
>    @@ -803,13 +803,13 @@ bpf_program__attach_tcx(const struct bpf_program *prog, int ifindex,
>     struct bpf_netkit_opts {
>          /* size of this struct, for forward/backward compatibility */
>          size_t sz;
>    -       __u32 flags;
>          __u32 relative_fd;
>          __u32 relative_id;
>          __u64 expected_revision;
>    +       __u32 flags;
>          size_t :0;
>     };
>    -#define bpf_netkit_opts__last_field expected_revision
>    +#define bpf_netkit_opts__last_field flags

The bpf_netkit_ops is in the bpf tree. If avoiding a hole in bpf_netkit_opts 
like above is preferred, probably the fix in this patch and the bpf_netkit_ops 
change should be in the same libbpf version?

Ran the test in a loop. It resolved the issue.

Tested-by: Martin KaFai Lau <martin.lau@kernel.org>