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.

(cherry picked from commit 9ca600e2bfacc52a65c89f3485723b2c27394e55)
(cherry picked from commit 95ee2c6b481b7a1f953cb720c35df568b7a6cb70)

diff --git a/src/core/bpf-firewall.c b/src/core/bpf-firewall.c
index 5874541575d1a1c219587ea6b179e4ef43a32682..0143589ab1d6aa017754419a93599c360e1ef019 100644 (file)
--- a/src/core/bpf-firewall.c
+++ b/src/core/bpf-firewall.c
@@ -845,11 +845,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) {
@@ -869,12 +872,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 8ae45bf3f1fe13d2c39e6921865cb94b9760852f..a36631d07ed2cada90b1714d81e38e54dd4ff9a9 100644 (file)
--- a/src/shared/bpf-program.c
+++ b/src/shared/bpf-program.c
@@ -82,15 +82,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)
@@ -108,9 +106,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)
@@ -166,12 +162,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;
@@ -203,11 +197,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;
@@ -219,15 +211,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;
@@ -236,12 +229,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;
@@ -250,12 +243,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 71aed125583d34ad4a42588a19c7672275ef86ab..d41be0d7b8a19d63e13b1dd5024763f5f7a592eb 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);