strace for m68k bpf_prog_info mismatch

[Geert Uytterhoeven]

Hi Baruch,

On Fri, May 3, 2019 at 6:06 AM Baruch Siach <baruch at tkos.co.il> wrote:
> strace 5.0 fails to build for m86k/5208 with the Buildroot generated
> toolchain:
>
> In file included from bpf_attr_check.c:6:0:
> static_assert.h:20:25: error: static assertion failed: "bpf_prog_info_struct.nr_jited_ksyms offset mismatch"
>  #  define static_assert _Static_assert
>                          ^
> bpf_attr_check.c:913:2: note: in expansion of macro ‘static_assert’
>   static_assert(offsetof(struct bpf_prog_info_struct, nr_jited_ksyms) == offsetof(struct bpf_prog_info, nr_jited_ksyms),
>   ^~~~~~~~~~~~~
>
> The direct cause is a difference in the hole after the gpl_compatible
> field. Here is pahole output for the kernel struct (from v4.19):
>
> struct bpf_prog_info {
>         ...
>         __u32                      ifindex;              /*    80     4 */
>         __u32                      gpl_compatible:1;     /*    84: 0  4 */
>
>         /* XXX 15 bits hole, try to pack */
>         /* Bitfield combined with next fields */
>
>         __u64                      netns_dev;            /*    86     8 */

I guess that should be "__aligned_u64 netns_dev;", to not rely on
implicit alignment.

> And this is for the strace struct:
>
> struct bpf_prog_info_struct {
>         ...
>         uint32_t                   ifindex;              /*    80     4 */
>         uint32_t                   gpl_compatible:1;     /*    84: 0  4 */
>
>         /* XXX 31 bits hole, try to pack */

How come the uint64_t below is 8-byte aligned, not 2-byte aligned?
Does strace use a special definition of uint64_t?

>
>         uint64_t                   netns_dev;            /*    88     8 */
>
> How should this be fixed?

IMHO all "__u64" in structs tagged "__attribute__((aligned(8)))" should
be replaced by "__aligned_u64", which is what the (whitespace-damaged)
diff below does.

Note that changing __bpf_md_ptr() may have a visible effect, as it is
used with struct bpf_sock, which has a size that is not a multiple of
8 bytes.  So depending on architecture, a hole may appear.

--- a/include/uapi/linux/bpf.h
+++ b/include/uapi/linux/bpf.h
@@ -362,7 +362,7 @@ union bpf_attr {
                        __aligned_u64 value;
                        __aligned_u64 next_key;
                };
-               __u64           flags;
+               __aligned_u64   flags;
        };

        struct { /* anonymous struct used by BPF_PROG_LOAD command */
@@ -452,7 +452,7 @@ union bpf_attr {
        } query;

        struct {
-               __u64 name;
+               __aligned_u64 name;
                __u32 prog_fd;
        } raw_tracepoint;

@@ -476,8 +476,8 @@ union bpf_attr {
                                                 */
                __u32           prog_id;        /* output: prod_id */
                __u32           fd_type;        /* output: BPF_FD_TYPE_* */
-               __u64           probe_offset;   /* output: probe_offset */
-               __u64           probe_addr;     /* output: probe_addr */
+               __aligned_u64   probe_offset;   /* output: probe_offset */
+               __aligned_u64   probe_addr;     /* output: probe_addr */
        } task_fd_query;
 } __attribute__((aligned(8)));

@@ -2878,7 +2878,7 @@ enum bpf_lwt_encap_mode {
 #define __bpf_md_ptr(type, name)       \
 union {                                        \
        type name;                      \
-       __u64 :64;                      \
+       __aligned_u64 :64;              \
 } __attribute__((aligned(8)))

 /* user accessible mirror of in-kernel sk_buff.
@@ -3129,15 +3129,15 @@ struct bpf_prog_info {
        __u32 xlated_prog_len;
        __aligned_u64 jited_prog_insns;
        __aligned_u64 xlated_prog_insns;
-       __u64 load_time;        /* ns since boottime */
+       __aligned_u64 load_time;        /* ns since boottime */
        __u32 created_by_uid;
        __u32 nr_map_ids;
        __aligned_u64 map_ids;
        char name[BPF_OBJ_NAME_LEN];
        __u32 ifindex;
        __u32 gpl_compatible:1;
-       __u64 netns_dev;
-       __u64 netns_ino;
+       __aligned_u64 netns_dev;
+       __aligned_u64 netns_ino;
        __u32 nr_jited_ksyms;
        __u32 nr_jited_func_lens;
        __aligned_u64 jited_ksyms;
@@ -3154,8 +3154,8 @@ struct bpf_prog_info {
        __u32 jited_line_info_rec_size;
        __u32 nr_prog_tags;
        __aligned_u64 prog_tags;
-       __u64 run_time_ns;
-       __u64 run_cnt;
+       __aligned_u64 run_time_ns;
+       __aligned_u64 run_cnt;
 } __attribute__((aligned(8)));

 struct bpf_map_info {
@@ -3168,8 +3168,8 @@ struct bpf_map_info {
        char  name[BPF_OBJ_NAME_LEN];
        __u32 ifindex;
        __u32 :32;
-       __u64 netns_dev;
-       __u64 netns_ino;
+       __aligned_u64 netns_dev;
+       __aligned_u64 netns_ino;
        __u32 btf_id;
        __u32 btf_key_type_id;
        __u32 btf_value_type_id;

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert at linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds