bpf: do not use structured initialization for bpf_attr

It looks like zero'ing the struct is not enough, and with some level
of optimizations there is still non-zero padding left over.
Switch to member-by-member initialization. Also convert all remaining
bpf_attr variables in other files.

diff --git a/src/core/bpf-firewall.c b/src/core/bpf-firewall.c
index 5952eaf2f73bcbb683720fb5f535ebd6504fed5b..0f588b6ca5fce2e64aa23f04ff6bdacc2068b376 100644 (file)
--- 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 9da8c109b8463941af18962243dcf96f6c197968..10239142af30898fc556b476fa553868d10fc530 100644 (file)
--- 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 cb7d8398a81c59297eade2ea8e9110efd523f194..b6fd22904fa17d8c686cb84f89cbe8d5064b560e 100644 (file)
--- 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);