Commit 98589a09 authored by Shmulik Ladkani's avatar Shmulik Ladkani Committed by Pablo Neira Ayuso

netfilter: xt_bpf: Fix XT_BPF_MODE_FD_PINNED mode of 'xt_bpf_info_v1'

Commit 2c16d603 ("netfilter: xt_bpf: support ebpf") introduced
support for attaching an eBPF object by an fd, with the
'bpf_mt_check_v1' ABI expecting the '.fd' to be specified upon each
IPT_SO_SET_REPLACE call.

However this breaks subsequent iptables calls:

 # iptables -A INPUT -m bpf --object-pinned /sys/fs/bpf/xxx -j ACCEPT
 # iptables -A INPUT -s 5.6.7.8 -j ACCEPT
 iptables: Invalid argument. Run `dmesg' for more information.

That's because iptables works by loading existing rules using
IPT_SO_GET_ENTRIES to userspace, then issuing IPT_SO_SET_REPLACE with
the replacement set.

However, the loaded 'xt_bpf_info_v1' has an arbitrary '.fd' number
(from the initial "iptables -m bpf" invocation) - so when 2nd invocation
occurs, userspace passes a bogus fd number, which leads to
'bpf_mt_check_v1' to fail.

One suggested solution [1] was to hack iptables userspace, to perform a
"entries fixup" immediatley after IPT_SO_GET_ENTRIES, by opening a new,
process-local fd per every 'xt_bpf_info_v1' entry seen.

However, in [2] both Pablo Neira Ayuso and Willem de Bruijn suggested to
depricate the xt_bpf_info_v1 ABI dealing with pinned ebpf objects.

This fix changes the XT_BPF_MODE_FD_PINNED behavior to ignore the given
'.fd' and instead perform an in-kernel lookup for the bpf object given
the provided '.path'.

It also defines an alias for the XT_BPF_MODE_FD_PINNED mode, named
XT_BPF_MODE_PATH_PINNED, to better reflect the fact that the user is
expected to provide the path of the pinned object.

Existing XT_BPF_MODE_FD_ELF behavior (non-pinned fd mode) is preserved.

References: [1] https://marc.info/?l=netfilter-devel&m=150564724607440&w=2
            [2] https://marc.info/?l=netfilter-devel&m=150575727129880&w=2Reported-by: default avatarRafael Buchbinder <rafi@rbk.ms>
Signed-off-by: default avatarShmulik Ladkani <shmulik.ladkani@gmail.com>
Acked-by: default avatarWillem de Bruijn <willemb@google.com>
Acked-by: default avatarDaniel Borkmann <daniel@iogearbox.net>
Signed-off-by: default avatarPablo Neira Ayuso <pablo@netfilter.org>
parent 49f817d7
...@@ -368,6 +368,11 @@ static inline void __bpf_prog_uncharge(struct user_struct *user, u32 pages) ...@@ -368,6 +368,11 @@ static inline void __bpf_prog_uncharge(struct user_struct *user, u32 pages)
{ {
} }
static inline int bpf_obj_get_user(const char __user *pathname)
{
return -EOPNOTSUPP;
}
static inline struct net_device *__dev_map_lookup_elem(struct bpf_map *map, static inline struct net_device *__dev_map_lookup_elem(struct bpf_map *map,
u32 key) u32 key)
{ {
......
...@@ -23,6 +23,7 @@ enum xt_bpf_modes { ...@@ -23,6 +23,7 @@ enum xt_bpf_modes {
XT_BPF_MODE_FD_PINNED, XT_BPF_MODE_FD_PINNED,
XT_BPF_MODE_FD_ELF, XT_BPF_MODE_FD_ELF,
}; };
#define XT_BPF_MODE_PATH_PINNED XT_BPF_MODE_FD_PINNED
struct xt_bpf_info_v1 { struct xt_bpf_info_v1 {
__u16 mode; __u16 mode;
......
...@@ -363,6 +363,7 @@ int bpf_obj_get_user(const char __user *pathname) ...@@ -363,6 +363,7 @@ int bpf_obj_get_user(const char __user *pathname)
putname(pname); putname(pname);
return ret; return ret;
} }
EXPORT_SYMBOL_GPL(bpf_obj_get_user);
static void bpf_evict_inode(struct inode *inode) static void bpf_evict_inode(struct inode *inode)
{ {
......
...@@ -8,6 +8,7 @@ ...@@ -8,6 +8,7 @@
*/ */
#include <linux/module.h> #include <linux/module.h>
#include <linux/syscalls.h>
#include <linux/skbuff.h> #include <linux/skbuff.h>
#include <linux/filter.h> #include <linux/filter.h>
#include <linux/bpf.h> #include <linux/bpf.h>
...@@ -49,6 +50,22 @@ static int __bpf_mt_check_fd(int fd, struct bpf_prog **ret) ...@@ -49,6 +50,22 @@ static int __bpf_mt_check_fd(int fd, struct bpf_prog **ret)
return 0; return 0;
} }
static int __bpf_mt_check_path(const char *path, struct bpf_prog **ret)
{
mm_segment_t oldfs = get_fs();
int retval, fd;
set_fs(KERNEL_DS);
fd = bpf_obj_get_user(path);
set_fs(oldfs);
if (fd < 0)
return fd;
retval = __bpf_mt_check_fd(fd, ret);
sys_close(fd);
return retval;
}
static int bpf_mt_check(const struct xt_mtchk_param *par) static int bpf_mt_check(const struct xt_mtchk_param *par)
{ {
struct xt_bpf_info *info = par->matchinfo; struct xt_bpf_info *info = par->matchinfo;
...@@ -66,9 +83,10 @@ static int bpf_mt_check_v1(const struct xt_mtchk_param *par) ...@@ -66,9 +83,10 @@ static int bpf_mt_check_v1(const struct xt_mtchk_param *par)
return __bpf_mt_check_bytecode(info->bpf_program, return __bpf_mt_check_bytecode(info->bpf_program,
info->bpf_program_num_elem, info->bpf_program_num_elem,
&info->filter); &info->filter);
else if (info->mode == XT_BPF_MODE_FD_PINNED || else if (info->mode == XT_BPF_MODE_FD_ELF)
info->mode == XT_BPF_MODE_FD_ELF)
return __bpf_mt_check_fd(info->fd, &info->filter); return __bpf_mt_check_fd(info->fd, &info->filter);
else if (info->mode == XT_BPF_MODE_PATH_PINNED)
return __bpf_mt_check_path(info->path, &info->filter);
else else
return -EINVAL; return -EINVAL;
} }
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment