Message ID | 20200320030015.195806-1-zenczykowski@gmail.com |
---|---|
State | Not Applicable |
Delegated to: | David Miller |
Headers | show |
Series | iptables: open eBPF programs in read only mode | expand |
On Thu, Mar 19, 2020 at 08:00:15PM -0700, Maciej Żenczykowski wrote: > From: Maciej Żenczykowski <maze@google.com> > > Adjust the mode eBPF programs are opened in so 0400 pinned bpf programs > work without requiring CAP_DAC_OVERRIDE. Unfortunately this is breaking stuff: libxt_bpf.c: In function ‘bpf_obj_get_readonly’: libxt_bpf.c:70:6: error: ‘union bpf_attr’ has no member named ‘file_flags’ 70 | attr.file_flags = BPF_F_RDONLY; | ^ libxt_bpf.c:70:20: error: ‘BPF_F_RDONLY’ undeclared (first use in this function) 70 | attr.file_flags = BPF_F_RDONLY; | ^~~~~~~~~~~~
I don't get it. It builds for me. And it doesn't if I insert an intentional syntax error in the same line, so I'm definitely compiling exactly that code.
I think your build system's kernel headers are old. Linux 4.15-rc1 commit 6e71b04a82248ccf13a94b85cbc674a9fefe53f5 Author: Chenbo Feng <fengc@google.com> Date: Wed Oct 18 13:00:22 2017 -0700 bpf: Add file mode configuration into bpf maps --- a/include/uapi/linux/bpf.h +++ b/include/uapi/linux/bpf.h @@ -218,6 +218,10 @@ enum bpf_attach_type { #define BPF_OBJ_NAME_LEN 16U +/* Flags for accessing BPF object */ +#define BPF_F_RDONLY (1U << 3) +#define BPF_F_WRONLY (1U << 4) + union bpf_attr { struct { /* anonymous struct used by BPF_MAP_CREATE command */ __u32 map_type; /* one of enum bpf_map_type */ @@ -260,6 +264,7 @@ union bpf_attr { struct { /* anonymous struct used by BPF_OBJ_* commands */ __aligned_u64 pathname; __u32 bpf_fd; + __u32 file_flags; }; struct { /* anonymous struct used by BPF_PROG_ATTACH/DETACH commands */ On Thu, Mar 26, 2020 at 7:08 AM Maciej Żenczykowski <zenczykowski@gmail.com> wrote: > > I don't get it. It builds for me. > > And it doesn't if I insert an intentional syntax error in the same line, > so I'm definitely compiling exactly that code.
I guess maybe we could wrap it in a #ifdef BPF_F_RDONLY attr.file_flags = BPF_F_RDONLY; #endif if we want to continue supporting building against pre-4.15 kernel headers...
Ugh, and I guess that on a pre-4.15 kernel it would cause failures due to unknown flag... Maybe we need to try with flag and fallback to without...
On Thu, Mar 26, 2020 at 07:16:16AM -0700, Maciej Żenczykowski wrote: > I guess maybe we could wrap it in a > > #ifdef BPF_F_RDONLY > attr.file_flags = BPF_F_RDONLY; > #endif > > if we want to continue supporting building against pre-4.15 kernel headers... You can probably add a cached copy of this header file to the iptables tree via your patch like. This is done in other existing extensions to not rely on the available kernel headers. There is no parity between userspace iptables and kernel version, it is good if you make sure this compiles for older kernels are still supported. Thank you.
diff --git a/extensions/libxt_bpf.c b/extensions/libxt_bpf.c index 92958247..bf46505c 100644 --- a/extensions/libxt_bpf.c +++ b/extensions/libxt_bpf.c @@ -61,12 +61,13 @@ static const struct xt_option_entry bpf_opts_v1[] = { XTOPT_TABLEEND, }; -static int bpf_obj_get(const char *filepath) +static int bpf_obj_get_readonly(const char *filepath) { #if defined HAVE_LINUX_BPF_H && defined __NR_bpf && defined BPF_FS_MAGIC union bpf_attr attr; memset(&attr, 0, sizeof(attr)); + attr.file_flags = BPF_F_RDONLY; attr.pathname = (__u64) filepath; return syscall(__NR_bpf, BPF_OBJ_GET, &attr, sizeof(attr)); @@ -125,7 +126,7 @@ static void bpf_parse_string(struct sock_filter *pc, __u16 *lenp, __u16 len_max, static void bpf_parse_obj_pinned(struct xt_bpf_info_v1 *bi, const char *filepath) { - bi->fd = bpf_obj_get(filepath); + bi->fd = bpf_obj_get_readonly(filepath); if (bi->fd < 0) xtables_error(PARAMETER_PROBLEM, "bpf: failed to get bpf object");