Merge pull request #18195 from bluca/bpf_init

bpf: do not use structured initialization for bpf_attr

5 files changed, 53 insertions, 60 deletions

diff --git a/.github/workflows/ubuntu-unit-tests.sh b/.github/workflows/ubuntu-unit-tests.sh
index 831eab97f8..705b147b96 100755
--- a/.github/workflows/ubuntu-unit-tests.sh
+++ b/.github/workflows/ubuntu-unit-tests.sh
@@ -41,9 +41,8 @@ for phase in "${PHASES[@]}"; do
             if [[ "$phase" = "RUN_CLANG" ]]; then
                 export CC=clang
                 export CXX=clang++
-                MESON_ARGS=(--optimization=1)
             fi
-            meson --werror -Dtests=unsafe -Dslow-tests=true -Dfuzz-tests=true -Dman=true "${MESON_ARGS[@]}" build
+            meson --werror -Dtests=unsafe -Dslow-tests=true -Dfuzz-tests=true -Dman=true build
             ninja -C build -v
             meson test -C build --print-errorlogs
             ;;
diff --git a/src/core/bpf-firewall.c b/src/core/bpf-firewall.c
index 5952eaf2f7..0f588b6ca5 100644
--- a/src/core/bpf-firewall.c
+++ b/src/core/bpf-firewall.c
@@ -840,11 +840,14 @@ int bpf_firewall_supported(void) {
          * CONFIG_CGROUP_BPF is turned off, then the call will fail early with EINVAL. If it is turned on the
          * parameters are validated however, and that'll fail with EBADF then. */
 
-        attr = (union bpf_attr) {
-                .attach_type = BPF_CGROUP_INET_EGRESS,
-                .target_fd = -1,
-                .attach_bpf_fd = -1,
-        };
+        // FIXME: Clang doesn't 0-pad with structured initialization, causing
+        // the kernel to reject the bpf_attr as invalid. See:
+        // https://github.com/torvalds/linux/blob/v5.9/kernel/bpf/syscall.c#L65
+        // Ideally it should behave like GCC, so that we can remove these workarounds.
+        zero(attr);
+        attr.attach_type = BPF_CGROUP_INET_EGRESS;
+        attr.target_fd = -1;
+        attr.attach_bpf_fd = -1;
 
         if (bpf(BPF_PROG_DETACH, &attr, sizeof(attr)) < 0) {
                 if (errno != EBADF) {
@@ -864,12 +867,11 @@ int bpf_firewall_supported(void) {
          * bpf() call and the BPF_F_ALLOW_MULTI flags value. Since the flags are checked early in the system call we'll
          * get EINVAL if it's not supported, and EBADF as before if it is available. */
 
-        attr = (union bpf_attr) {
-                .attach_type = BPF_CGROUP_INET_EGRESS,
-                .target_fd = -1,
-                .attach_bpf_fd = -1,
-                .attach_flags = BPF_F_ALLOW_MULTI,
-        };
+        zero(attr);
+        attr.attach_type = BPF_CGROUP_INET_EGRESS;
+        attr.target_fd = -1;
+        attr.attach_bpf_fd = -1;
+        attr.attach_flags = BPF_F_ALLOW_MULTI;
 
         if (bpf(BPF_PROG_ATTACH, &attr, sizeof(attr)) < 0) {
                 if (errno == EBADF) {
diff --git a/src/shared/bpf-program.c b/src/shared/bpf-program.c
index 9da8c109b8..10239142af 100644
--- a/src/shared/bpf-program.c
+++ b/src/shared/bpf-program.c
@@ -81,15 +81,13 @@ int bpf_program_load_kernel(BPFProgram *p, char *log_buf, size_t log_size) {
         // https://github.com/torvalds/linux/blob/v5.9/kernel/bpf/syscall.c#L65
         // Ideally it should behave like GCC, so that we can remove these workarounds.
         zero(attr);
-        attr = (union bpf_attr) {
-                .prog_type = p->prog_type,
-                .insns = PTR_TO_UINT64(p->instructions),
-                .insn_cnt = p->n_instructions,
-                .license = PTR_TO_UINT64("GPL"),
-                .log_buf = PTR_TO_UINT64(log_buf),
-                .log_level = !!log_buf,
-                .log_size = log_size,
-        };
+        attr.prog_type = p->prog_type;
+        attr.insns = PTR_TO_UINT64(p->instructions);
+        attr.insn_cnt = p->n_instructions;
+        attr.license = PTR_TO_UINT64("GPL");
+        attr.log_buf = PTR_TO_UINT64(log_buf);
+        attr.log_level = !!log_buf;
+        attr.log_size = log_size;
 
         p->kernel_fd = bpf(BPF_PROG_LOAD, &attr, sizeof(attr));
         if (p->kernel_fd < 0)
@@ -107,9 +105,7 @@ int bpf_program_load_from_bpf_fs(BPFProgram *p, const char *path) {
                 return -EBUSY;
 
         zero(attr);
-        attr = (union bpf_attr) {
-                .pathname = PTR_TO_UINT64(path),
-        };
+        attr.pathname = PTR_TO_UINT64(path);
 
         p->kernel_fd = bpf(BPF_OBJ_GET, &attr, sizeof(attr));
         if (p->kernel_fd < 0)
@@ -165,12 +161,10 @@ int bpf_program_cgroup_attach(BPFProgram *p, int type, const char *path, uint32_
                 return -errno;
 
         zero(attr);
-        attr = (union bpf_attr) {
-                .attach_type = type,
-                .target_fd = fd,
-                .attach_bpf_fd = p->kernel_fd,
-                .attach_flags = flags,
-        };
+        attr.attach_type = type;
+        attr.target_fd = fd;
+        attr.attach_bpf_fd = p->kernel_fd;
+        attr.attach_flags = flags;
 
         if (bpf(BPF_PROG_ATTACH, &attr, sizeof(attr)) < 0)
                 return -errno;
@@ -202,11 +196,9 @@ int bpf_program_cgroup_detach(BPFProgram *p) {
                 union bpf_attr attr;
 
                 zero(attr);
-                attr = (union bpf_attr) {
-                        .attach_type = p->attached_type,
-                        .target_fd = fd,
-                        .attach_bpf_fd = p->kernel_fd,
-                };
+                attr.attach_type = p->attached_type;
+                attr.target_fd = fd;
+                attr.attach_bpf_fd = p->kernel_fd;
 
                 if (bpf(BPF_PROG_DETACH, &attr, sizeof(attr)) < 0)
                         return -errno;
@@ -218,15 +210,16 @@ int bpf_program_cgroup_detach(BPFProgram *p) {
 }
 
 int bpf_map_new(enum bpf_map_type type, size_t key_size, size_t value_size, size_t max_entries, uint32_t flags) {
-        union bpf_attr attr = {
-                .map_type = type,
-                .key_size = key_size,
-                .value_size = value_size,
-                .max_entries = max_entries,
-                .map_flags = flags,
-        };
+        union bpf_attr attr;
         int fd;
 
+        zero(attr);
+        attr.map_type = type;
+        attr.key_size = key_size;
+        attr.value_size = value_size;
+        attr.max_entries = max_entries;
+        attr.map_flags = flags;
+
         fd = bpf(BPF_MAP_CREATE, &attr, sizeof(attr));
         if (fd < 0)
                 return -errno;
@@ -235,12 +228,12 @@ int bpf_map_new(enum bpf_map_type type, size_t key_size, size_t value_size, size
 }
 
 int bpf_map_update_element(int fd, const void *key, void *value) {
+        union bpf_attr attr;
 
-        union bpf_attr attr = {
-                .map_fd = fd,
-                .key = PTR_TO_UINT64(key),
-                .value = PTR_TO_UINT64(value),
-        };
+        zero(attr);
+        attr.map_fd = fd;
+        attr.key = PTR_TO_UINT64(key);
+        attr.value = PTR_TO_UINT64(value);
 
         if (bpf(BPF_MAP_UPDATE_ELEM, &attr, sizeof(attr)) < 0)
                 return -errno;
@@ -249,12 +242,12 @@ int bpf_map_update_element(int fd, const void *key, void *value) {
 }
 
 int bpf_map_lookup_element(int fd, const void *key, void *value) {
+        union bpf_attr attr;
 
-        union bpf_attr attr = {
-                .map_fd = fd,
-                .key = PTR_TO_UINT64(key),
-                .value = PTR_TO_UINT64(value),
-        };
+        zero(attr);
+        attr.map_fd = fd;
+        attr.key = PTR_TO_UINT64(key);
+        attr.value = PTR_TO_UINT64(value);
 
         if (bpf(BPF_MAP_LOOKUP_ELEM, &attr, sizeof(attr)) < 0)
                 return -errno;
diff --git a/src/test/test-bpf-firewall.c b/src/test/test-bpf-firewall.c
index cb7d8398a8..b6fd22904f 100644
--- a/src/test/test-bpf-firewall.c
+++ b/src/test/test-bpf-firewall.c
@@ -8,6 +8,7 @@
 #include "bpf-program.h"
 #include "load-fragment.h"
 #include "manager.h"
+#include "memory-util.h"
 #include "rm-rf.h"
 #include "service.h"
 #include "tests.h"
@@ -77,11 +78,10 @@ int main(int argc, char *argv[]) {
         assert(r >= 0);
 
         if (test_custom_filter) {
-                attr = (union bpf_attr) {
-                        .pathname = PTR_TO_UINT64(test_prog),
-                        .bpf_fd = p->kernel_fd,
-                        .file_flags = 0,
-                };
+                zero(attr);
+                attr.pathname = PTR_TO_UINT64(test_prog);
+                attr.bpf_fd = p->kernel_fd;
+                attr.file_flags = 0;
 
                 (void) unlink(test_prog);
 
diff --git a/travis-ci/managers/debian.sh b/travis-ci/managers/debian.sh
index ad6306744c..39fbb3d65c 100755
--- a/travis-ci/managers/debian.sh
+++ b/travis-ci/managers/debian.sh
@@ -62,9 +62,8 @@ for phase in "${PHASES[@]}"; do
         RUN|RUN_GCC|RUN_CLANG)
             if [[ "$phase" = "RUN_CLANG" ]]; then
                 ENV_VARS="-e CC=clang -e CXX=clang++"
-                MESON_ARGS="--optimization=1"
             fi
-            docker exec $ENV_VARS -it $CONT_NAME meson --werror -Dtests=unsafe -Dslow-tests=true -Dfuzz-tests=true -Dsplit-usr=true -Dman=true $MESON_ARGS build
+            docker exec $ENV_VARS -it $CONT_NAME meson --werror -Dtests=unsafe -Dslow-tests=true -Dfuzz-tests=true -Dsplit-usr=true -Dman=true build
             $DOCKER_EXEC ninja -v -C build
             docker exec -e "TRAVIS=$TRAVIS" -it $CONT_NAME ninja -C build test
             ;;