From 9fda8cfbce691bde502e3dbad0bff9076d33f3dc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Maciej=20=C5=BBenczykowski?= Date: Tue, 21 Apr 2020 04:28:28 +0000 Subject: UPSTREAM: iptables: open eBPF programs in read only mode MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adjust the mode eBPF programs are opened in so 0400 pinned bpf programs work without requiring CAP_DAC_OVERRIDE. This matches Linux 5.2's: commit e547ff3f803e779a3898f1f48447b29f43c54085 Author: Chenbo Feng Date: Tue May 14 19:42:57 2019 -0700 bpf: relax inode permission check for retrieving bpf program For iptable module to load a bpf program from a pinned location, it only retrieve a loaded program and cannot change the program content so requiring a write permission for it might not be necessary. Also when adding or removing an unrelated iptable rule, it might need to flush and reload the xt_bpf related rules as well and triggers the inode permission check. It might be better to remove the write premission check for the inode so we won't need to grant write access to all the processes that flush and restore iptables rules. kernel/bpf/inode.c: - int ret = inode_permission(inode, MAY_READ | MAY_WRITE); + int ret = inode_permission(inode, MAY_READ); In practice, AFAICT, the xt_bpf match .fd field isn't even used by new kernels, but I believe it might be needed for compatibility with old ones (though I'm pretty sure table modifications on them will outright fail). Test: builds, passes Android test suite (albeit on an older iptables base), git grep bpf_obj_get - finds no other users Cc: Chenbo Feng Cc: Alexei Starovoitov Cc: Willem de Bruijn Signed-off-by: Maciej Żenczykowski Signed-off-by: Pablo Neira Ayuso (cherry picked from commit 74ef6f1c16ff672139031330dc71c274300dfb2e) Bug: 150112553 Change-Id: Ic282b4c6f1f8b1302ef79b84831bfec831279bc9 Merged-In: Ic282b4c6f1f8b1302ef79b84831bfec831279bc9 --- extensions/libxt_bpf.c | 26 +++++++++++++++++++------- 1 file changed, 19 insertions(+), 7 deletions(-) diff --git a/extensions/libxt_bpf.c b/extensions/libxt_bpf.c index 92958247..eeae86e5 100644 --- a/extensions/libxt_bpf.c +++ b/extensions/libxt_bpf.c @@ -61,14 +61,26 @@ 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.pathname = (__u64) filepath; - + /* union bpf_attr includes this in an anonymous struct, but the + * file_flags field and the BPF_F_RDONLY constant are only present + * in Linux 4.15+ kernel headers (include/uapi/linux/bpf.h) + */ + struct { // this part of union bpf_attr is for BPF_OBJ_* commands + __aligned_u64 pathname; + __u32 bpf_fd; + __u32 file_flags; + } attr = { + .pathname = (__u64)filepath, + .file_flags = (1U << 3), // BPF_F_RDONLY + }; + int fd = syscall(__NR_bpf, BPF_OBJ_GET, &attr, sizeof(attr)); + if (fd >= 0) return fd; + + /* on any error fallback to default R/W access for pre-4.15-rc1 kernels */ + attr.file_flags = 0; return syscall(__NR_bpf, BPF_OBJ_GET, &attr, sizeof(attr)); #else xtables_error(OTHER_PROBLEM, @@ -125,7 +137,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"); -- cgit v1.2.3 From 7628ec22c1e35f7ed9bb6f634a312556a6ac2fbb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Maciej=20=C5=BBenczykowski?= Date: Tue, 21 Apr 2020 08:07:34 +0000 Subject: libiptc.c: remove ipt_error_target MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This is a spurious change vs upstream, and it is apparently no longer needed, presumably due to our kernel header files catching up. aosp/bionic$ git grep 'ipt_error_target' libc/kernel/uapi/linux/netfilter_ipv4/ip_tables.h:35: aosp/bionic$ git grep -A3 'xt_error_target \{' libc/kernel/uapi/linux/netfilter/x_tables.h:63: struct xt_error_target { struct xt_entry_target target; char errorname[XT_FUNCTION_MAXNAMELEN]; }; compare to iptables own headers aosp/external/iptables$ git grep ipt_error_target include/linux/netfilter_ipv4/ip_tables.h:33:#define ipt_error_target xt_error_target libiptc/libip4tc.c:294: == ALIGN(sizeof(struct ipt_error_target))); aosp/external/iptables$ git grep ip6t_error_target include/linux/netfilter_ipv6/ip6_tables.h:33:#define ip6t_error_target xt_error_target (Also note that only the #undef is Android specific, the actual struct was upstream and was later deleted, it stuck around in Android iptables due to incorrect merge conflict resolution) Test: builds Signed-off-by: Maciej Żenczykowski Bug: 150112553 Change-Id: I97981de75a8f0bc0cf75306610560f24404d987f Merged-In: I97981de75a8f0bc0cf75306610560f24404d987f --- libiptc/libiptc.c | 7 ------- 1 file changed, 7 deletions(-) diff --git a/libiptc/libiptc.c b/libiptc/libiptc.c index c3142424..58882015 100644 --- a/libiptc/libiptc.c +++ b/libiptc/libiptc.c @@ -67,13 +67,6 @@ static const char *hooknames[] = { }; /* Convenience structures */ -#undef ipt_error_target /* uapi includes this already. */ -struct ipt_error_target -{ - STRUCT_ENTRY_TARGET t; - char error[TABLE_MAXNAMELEN]; -}; - struct chain_head; struct rule_head; -- cgit v1.2.3