From: "Maciej Żenczykowski" <zenczykowski@gmail.com>
To: "Maciej Żenczykowski" <maze@google.com>,
"Pablo Neira Ayuso" <pablo@netfilter.org>,
"Florian Westphal" <fw@strlen.de>
Cc: Linux Network Development Mailing List <netdev@vger.kernel.org>,
Netfilter Development Mailing List
<netfilter-devel@vger.kernel.org>, Chenbo Feng <fengc@google.com>,
Alexei Starovoitov <ast@kernel.org>,
Willem de Bruijn <willemb@google.com>
Subject: [PATCH v3] iptables: open eBPF programs in read only mode
Date: Tue, 31 Mar 2020 09:07:03 -0700 [thread overview]
Message-ID: <20200331160703.56842-1-zenczykowski@gmail.com> (raw)
In-Reply-To: <20200320030015.195806-1-zenczykowski@gmail.com>
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.
This matches Linux 5.2's:
commit e547ff3f803e779a3898f1f48447b29f43c54085
Author: Chenbo Feng <fengc@google.com>
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 <fengc@google.com>
Cc: Alexei Starovoitov <ast@kernel.org>
Cc: Willem de Bruijn <willemb@google.com>
Signed-off-by: Maciej Żenczykowski <maze@google.com>
---
extensions/libxt_bpf.c | 25 ++++++++++++++++++-------
1 file changed, 18 insertions(+), 7 deletions(-)
diff --git a/extensions/libxt_bpf.c b/extensions/libxt_bpf.c
index 92958247..4aea477a 100644
--- a/extensions/libxt_bpf.c
+++ b/extensions/libxt_bpf.c
@@ -61,14 +61,25 @@ 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 +136,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");
--
2.26.0.rc2.310.g2932bb562d-goog
prev parent reply other threads:[~2020-03-31 16:07 UTC|newest]
Thread overview: 12+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-03-20 3:00 [PATCH] iptables: open eBPF programs in read only mode Maciej Żenczykowski
2020-03-26 13:59 ` Pablo Neira Ayuso
2020-03-26 14:08 ` Maciej Żenczykowski
2020-03-26 14:13 ` Maciej Żenczykowski
2020-03-26 14:16 ` Maciej Żenczykowski
2020-03-26 14:19 ` Maciej Żenczykowski
2020-03-26 14:28 ` [PATCH v2] " Maciej Żenczykowski
2020-03-26 18:30 ` Jakub Kicinski
2020-03-26 18:34 ` Maciej Żenczykowski
2020-03-26 19:22 ` Alexei Starovoitov
2020-03-26 14:22 ` [PATCH] " Pablo Neira Ayuso
2020-03-31 16:07 ` Maciej Żenczykowski [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20200331160703.56842-1-zenczykowski@gmail.com \
--to=zenczykowski@gmail.com \
--cc=ast@kernel.org \
--cc=fengc@google.com \
--cc=fw@strlen.de \
--cc=maze@google.com \
--cc=netdev@vger.kernel.org \
--cc=netfilter-devel@vger.kernel.org \
--cc=pablo@netfilter.org \
--cc=willemb@google.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.