From 247c055f54bcc23c3d038d88862ff1549401b40d Mon Sep 17 00:00:00 2001 From: Luca Boccassi Date: Sun, 10 Jan 2021 15:36:31 +0000 Subject: [PATCH] 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) --- src/core/bpf-firewall.c | 24 ++++++------ src/shared/bpf-program.c | 73 ++++++++++++++++-------------------- src/test/test-bpf-firewall.c | 10 ++--- 3 files changed, 51 insertions(+), 56 deletions(-) diff --git a/src/core/bpf-firewall.c b/src/core/bpf-firewall.c index 5874541575..0143589ab1 100644 --- 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 8ae45bf3f1..a36631d07e 100644 --- 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 71aed12558..d41be0d7b8 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); -- 2.25.1