bpf.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH bpf-next 0/6] bpf: add netfilter program type
@ 2023-04-05 16:11 Florian Westphal
  2023-04-05 16:11 ` [PATCH bpf-next 1/6] bpf: add bpf_link support for BPF_NETFILTER programs Florian Westphal
                   ` (6 more replies)
  0 siblings, 7 replies; 14+ messages in thread
From: Florian Westphal @ 2023-04-05 16:11 UTC (permalink / raw)
  To: netdev; +Cc: netfilter-devel, bpf, dxu, qde, Florian Westphal

Add minimal support to hook bpf programs to netfilter hooks, e.g.
PREROUTING or FORWARD.

For this the most relevant parts for registering a netfilter
hook via the in-kernel api are exposed to userspace via bpf_link.

The new program type is 'tracing style', i.e. there is no context
access rewrite done by verifier, the function argument (struct bpf_nf_ctx)
isn't stable.
There is no support for direct packet access, dynptr api should be used
instead.

With this its possible to build a small test program such as:

 #include "vmlinux.h"
extern int bpf_dynptr_from_skb(struct __sk_buff *skb, __u64 flags,
                               struct bpf_dynptr *ptr__uninit) __ksym;
extern void *bpf_dynptr_slice(const struct bpf_dynptr *ptr, uint32_t offset,
                                   void *buffer, uint32_t buffer__sz) __ksym;
SEC("netfilter")
int nf_test(struct bpf_nf_ctx *ctx)
{
	struct nf_hook_state *state = ctx->state;
	struct sk_buff *skb = ctx->skb;
	const struct iphdr *iph, _iph;
	const struct tcphdr *th, _th;
	struct bpf_dynptr ptr;

	if (bpf_dynptr_from_skb(skb, 0, &ptr))
		return NF_DROP;

	iph = bpf_dynptr_slice(&ptr, 0, &_iph, sizeof(_iph));
	if (!iph)
		return NF_DROP;

	th = bpf_dynptr_slice(&ptr, iph->ihl << 2, &_th, sizeof(_th));
	if (!th)
		return NF_DROP;

	bpf_printk("accept %x:%d->%x:%d, hook %d ifin %d\n", iph->saddr, bpf_ntohs(th->source), iph->daddr, bpf_ntohs(th->dest), state->hook, state->in->ifindex);
        return NF_ACCEPT;
}

Then, tail /sys/kernel/tracing/trace_pipe.

Changes since last RFC version:
1. extend 'bpftool link show' to print prio/hooknum etc
2. extend 'nft list hooks' so it can print the bpf program id
3. Add an extra patch to artificially restrict bpf progs with
   same priority.  Its fine from a technical pov but it will
   cause ordering issues (most recent one comes first).
   Can be removed later.
4. Add test_run support for netfilter prog type and a small
   extension to verifier tests to make sure we can't return
   verdicts like NF_STOLEN.
5. Alter the netfilter part of the bpf_link uapi struct:
   - add flags/reserved members.
  Not used here except returning errors when they are nonzero.
  Plan is to allow the bpf_link users to enable netfilter
  defrag or conntrack engine by setting feature flags at
  link create time in the future.

Let me know if there is anything missing that has to be addressed
before this can be merged.

Thanks!

Florian Westphal (6):
  bpf: add bpf_link support for BPF_NETFILTER programs
  bpf: minimal support for programs hooked into netfilter framework
  netfilter: nfnetlink hook: dump bpf prog id
  netfilter: disallow bpf hook attachment at same priority
  tools: bpftool: print netfilter link info
  bpf: add test_run support for netfilter program type

 include/linux/bpf.h                           |   3 +
 include/linux/bpf_types.h                     |   4 +
 include/linux/netfilter.h                     |   1 +
 include/net/netfilter/nf_bpf_link.h           |   8 +
 include/uapi/linux/bpf.h                      |  15 ++
 include/uapi/linux/netfilter/nfnetlink_hook.h |  20 +-
 kernel/bpf/btf.c                              |   6 +
 kernel/bpf/syscall.c                          |   6 +
 kernel/bpf/verifier.c                         |   3 +
 net/bpf/test_run.c                            | 143 +++++++++++++
 net/core/filter.c                             |   1 +
 net/netfilter/Kconfig                         |   3 +
 net/netfilter/Makefile                        |   1 +
 net/netfilter/core.c                          |  12 ++
 net/netfilter/nf_bpf_link.c                   | 190 ++++++++++++++++++
 net/netfilter/nfnetlink_hook.c                |  81 ++++++--
 tools/bpf/bpftool/link.c                      |  24 +++
 tools/include/uapi/linux/bpf.h                |  15 ++
 tools/lib/bpf/libbpf.c                        |   1 +
 .../selftests/bpf/verifier/netfilter.c        |  23 +++
 20 files changed, 546 insertions(+), 14 deletions(-)
 create mode 100644 include/net/netfilter/nf_bpf_link.h
 create mode 100644 net/netfilter/nf_bpf_link.c
 create mode 100644 tools/testing/selftests/bpf/verifier/netfilter.c

-- 
2.39.2


^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PATCH bpf-next 1/6] bpf: add bpf_link support for BPF_NETFILTER programs
  2023-04-05 16:11 [PATCH bpf-next 0/6] bpf: add netfilter program type Florian Westphal
@ 2023-04-05 16:11 ` Florian Westphal
  2023-04-06  4:50   ` kernel test robot
  2023-04-05 16:11 ` [PATCH bpf-next 2/6] bpf: minimal support for programs hooked into netfilter framework Florian Westphal
                   ` (5 subsequent siblings)
  6 siblings, 1 reply; 14+ messages in thread
From: Florian Westphal @ 2023-04-05 16:11 UTC (permalink / raw)
  To: netdev; +Cc: netfilter-devel, bpf, dxu, qde, Florian Westphal

Add bpf_link support skeleton.  To keep this reviewable, no bpf program
can be invoked yet, if a program is attached only a c-stub is called and
not the actual bpf program.

Defaults to 'y' if both netfilter and bpf syscall are enabled in kconfig.

Uapi example usage:
	union bpf_attr attr = { };

	attr.link_create.prog_fd = progfd;
	attr.link_create.attach_type = 0; /* unused */
	attr.link_create.netfilter.pf = PF_INET;
	attr.link_create.netfilter.hooknum = NF_INET_LOCAL_IN;
	attr.link_create.netfilter.priority = -128;

	err = bpf(BPF_LINK_CREATE, &attr, sizeof(attr));

... this would attach progfd to ipv4:input hook.

Such hook gets removed automatically if the calling program exits.

BPF_NETFILTER program invocation is added in followup change.

NF_HOOK_OP_BPF enum will eventually be read from nfnetlink_hook, it
allows to tell userspace which program is attached at the given hook
when user runs 'nft hook list' command rather than just the priority
and not-very-helpful 'this hook runs a bpf prog but I can't tell which
one'.

Will also be used to disallow registration of two bpf programs with
same priority in a followup patch.

Signed-off-by: Florian Westphal <fw@strlen.de>
---
 include/linux/netfilter.h           |   1 +
 include/net/netfilter/nf_bpf_link.h |   2 +
 include/uapi/linux/bpf.h            |  15 ++++
 kernel/bpf/syscall.c                |   6 ++
 net/netfilter/Kconfig               |   3 +
 net/netfilter/Makefile              |   1 +
 net/netfilter/nf_bpf_link.c         | 121 ++++++++++++++++++++++++++++
 7 files changed, 149 insertions(+)
 create mode 100644 include/net/netfilter/nf_bpf_link.h
 create mode 100644 net/netfilter/nf_bpf_link.c

diff --git a/include/linux/netfilter.h b/include/linux/netfilter.h
index c8e03bcaecaa..0762444e3767 100644
--- a/include/linux/netfilter.h
+++ b/include/linux/netfilter.h
@@ -80,6 +80,7 @@ typedef unsigned int nf_hookfn(void *priv,
 enum nf_hook_ops_type {
 	NF_HOOK_OP_UNDEFINED,
 	NF_HOOK_OP_NF_TABLES,
+	NF_HOOK_OP_BPF,
 };
 
 struct nf_hook_ops {
diff --git a/include/net/netfilter/nf_bpf_link.h b/include/net/netfilter/nf_bpf_link.h
new file mode 100644
index 000000000000..9d1b338e89d7
--- /dev/null
+++ b/include/net/netfilter/nf_bpf_link.h
@@ -0,0 +1,2 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+int bpf_nf_link_attach(const union bpf_attr *attr, struct bpf_prog *prog);
diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
index e3d3b5160d26..7d1a615ce78e 100644
--- a/include/uapi/linux/bpf.h
+++ b/include/uapi/linux/bpf.h
@@ -986,6 +986,7 @@ enum bpf_prog_type {
 	BPF_PROG_TYPE_LSM,
 	BPF_PROG_TYPE_SK_LOOKUP,
 	BPF_PROG_TYPE_SYSCALL, /* a program that can execute syscalls */
+	BPF_PROG_TYPE_NETFILTER,
 };
 
 enum bpf_attach_type {
@@ -1050,6 +1051,7 @@ enum bpf_link_type {
 	BPF_LINK_TYPE_PERF_EVENT = 7,
 	BPF_LINK_TYPE_KPROBE_MULTI = 8,
 	BPF_LINK_TYPE_STRUCT_OPS = 9,
+	BPF_LINK_TYPE_NETFILTER = 10,
 
 	MAX_BPF_LINK_TYPE,
 };
@@ -1550,6 +1552,13 @@ union bpf_attr {
 				 */
 				__u64		cookie;
 			} tracing;
+			struct {
+				__u32		pf;
+				__u32		hooknum;
+				__s32		prio;
+				__u32		flags;
+				__u64		reserved[2];
+			} netfilter;
 		};
 	} link_create;
 
@@ -6400,6 +6409,12 @@ struct bpf_link_info {
 		struct {
 			__u32 map_id;
 		} struct_ops;
+		struct {
+			__u32 pf;
+			__u32 hooknum;
+			__s32 priority;
+			__u32 flags;
+		} netfilter;
 	};
 } __attribute__((aligned(8)));
 
diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c
index e18ac7fdc210..bf0cce7071ef 100644
--- a/kernel/bpf/syscall.c
+++ b/kernel/bpf/syscall.c
@@ -35,6 +35,7 @@
 #include <linux/rcupdate_trace.h>
 #include <linux/memcontrol.h>
 #include <linux/trace_events.h>
+#include <net/netfilter/nf_bpf_link.h>
 
 #define IS_FD_ARRAY(map) ((map)->map_type == BPF_MAP_TYPE_PERF_EVENT_ARRAY || \
 			  (map)->map_type == BPF_MAP_TYPE_CGROUP_ARRAY || \
@@ -2472,6 +2473,7 @@ static bool is_net_admin_prog_type(enum bpf_prog_type prog_type)
 	case BPF_PROG_TYPE_CGROUP_SYSCTL:
 	case BPF_PROG_TYPE_SOCK_OPS:
 	case BPF_PROG_TYPE_EXT: /* extends any prog */
+	case BPF_PROG_TYPE_NETFILTER:
 		return true;
 	case BPF_PROG_TYPE_CGROUP_SKB:
 		/* always unpriv */
@@ -4598,6 +4600,7 @@ static int link_create(union bpf_attr *attr, bpfptr_t uattr)
 
 	switch (prog->type) {
 	case BPF_PROG_TYPE_EXT:
+	case BPF_PROG_TYPE_NETFILTER:
 		break;
 	case BPF_PROG_TYPE_PERF_EVENT:
 	case BPF_PROG_TYPE_TRACEPOINT:
@@ -4664,6 +4667,9 @@ static int link_create(union bpf_attr *attr, bpfptr_t uattr)
 	case BPF_PROG_TYPE_XDP:
 		ret = bpf_xdp_link_attach(attr, prog);
 		break;
+	case BPF_PROG_TYPE_NETFILTER:
+		ret = bpf_nf_link_attach(attr, prog);
+		break;
 #endif
 	case BPF_PROG_TYPE_PERF_EVENT:
 	case BPF_PROG_TYPE_TRACEPOINT:
diff --git a/net/netfilter/Kconfig b/net/netfilter/Kconfig
index 4d6737160857..bea06f62a30e 100644
--- a/net/netfilter/Kconfig
+++ b/net/netfilter/Kconfig
@@ -30,6 +30,9 @@ config NETFILTER_FAMILY_BRIDGE
 config NETFILTER_FAMILY_ARP
 	bool
 
+config NETFILTER_BPF_LINK
+	def_bool BPF_SYSCALL
+
 config NETFILTER_NETLINK_HOOK
 	tristate "Netfilter base hook dump support"
 	depends on NETFILTER_ADVANCED
diff --git a/net/netfilter/Makefile b/net/netfilter/Makefile
index 5ffef1cd6143..d4958e7e7631 100644
--- a/net/netfilter/Makefile
+++ b/net/netfilter/Makefile
@@ -22,6 +22,7 @@ nf_conntrack-$(CONFIG_DEBUG_INFO_BTF) += nf_conntrack_bpf.o
 endif
 
 obj-$(CONFIG_NETFILTER) = netfilter.o
+obj-$(CONFIG_NETFILTER_BPF_LINK) += nf_bpf_link.o
 
 obj-$(CONFIG_NETFILTER_NETLINK) += nfnetlink.o
 obj-$(CONFIG_NETFILTER_NETLINK_ACCT) += nfnetlink_acct.o
diff --git a/net/netfilter/nf_bpf_link.c b/net/netfilter/nf_bpf_link.c
new file mode 100644
index 000000000000..c68b2cb70fd4
--- /dev/null
+++ b/net/netfilter/nf_bpf_link.c
@@ -0,0 +1,121 @@
+// SPDX-License-Identifier: GPL-2.0
+#include <linux/bpf.h>
+#include <linux/netfilter.h>
+
+#include <net/netfilter/nf_bpf_link.h>
+
+static unsigned int nf_hook_run_bpf(void *bpf_prog, struct sk_buff *skb, const struct nf_hook_state *s)
+{
+	return NF_ACCEPT;
+}
+
+struct bpf_nf_link {
+	struct bpf_link link;
+	struct nf_hook_ops hook_ops;
+	struct net *net;
+};
+
+static void bpf_nf_link_release(struct bpf_link *link)
+{
+	struct bpf_nf_link *nf_link = container_of(link, struct bpf_nf_link, link);
+
+	nf_unregister_net_hook(nf_link->net, &nf_link->hook_ops);
+}
+
+static void bpf_nf_link_dealloc(struct bpf_link *link)
+{
+	struct bpf_nf_link *nf_link = container_of(link, struct bpf_nf_link, link);
+
+	kfree(nf_link);
+}
+
+static int bpf_nf_link_detach(struct bpf_link *link)
+{
+	bpf_nf_link_release(link);
+	return 0;
+}
+
+static void bpf_nf_link_show_info(const struct bpf_link *link,
+				  struct seq_file *seq)
+{
+	struct bpf_nf_link *nf_link = container_of(link, struct bpf_nf_link, link);
+
+	seq_printf(seq, "pf:\t%u\thooknum:\t%u\tprio:\t%d\n",
+		   nf_link->hook_ops.pf, nf_link->hook_ops.hooknum,
+		   nf_link->hook_ops.priority);
+}
+
+static int bpf_nf_link_fill_link_info(const struct bpf_link *link,
+				      struct bpf_link_info *info)
+{
+	struct bpf_nf_link *nf_link = container_of(link, struct bpf_nf_link, link);
+
+	info->netfilter.pf = nf_link->hook_ops.pf;
+	info->netfilter.hooknum = nf_link->hook_ops.hooknum;
+	info->netfilter.priority = nf_link->hook_ops.priority;
+	info->netfilter.flags = 0;
+
+	return 0;
+}
+
+static int bpf_nf_link_update(struct bpf_link *link, struct bpf_prog *new_prog,
+			      struct bpf_prog *old_prog)
+{
+	return -EOPNOTSUPP;
+}
+
+static const struct bpf_link_ops bpf_nf_link_lops = {
+	.release = bpf_nf_link_release,
+	.dealloc = bpf_nf_link_dealloc,
+	.detach = bpf_nf_link_detach,
+	.show_fdinfo = bpf_nf_link_show_info,
+	.fill_link_info = bpf_nf_link_fill_link_info,
+	.update_prog = bpf_nf_link_update,
+};
+
+int bpf_nf_link_attach(const union bpf_attr *attr, struct bpf_prog *prog)
+{
+	struct net *net = current->nsproxy->net_ns;
+	struct bpf_link_primer link_primer;
+	struct bpf_nf_link *link;
+	int err;
+
+	if (attr->link_create.flags)
+		return -EINVAL;
+
+	if (attr->link_create.netfilter.flags)
+		return -EOPNOTSUPP;
+
+	if (attr->link_create.netfilter.reserved[0] | attr->link_create.netfilter.reserved[1])
+		return -EINVAL;
+
+	link = kzalloc(sizeof(*link), GFP_USER);
+	if (!link)
+		return -ENOMEM;
+
+	bpf_link_init(&link->link, BPF_LINK_TYPE_NETFILTER, &bpf_nf_link_lops, prog);
+
+	link->hook_ops.hook = nf_hook_run_bpf;
+	link->hook_ops.hook_ops_type = NF_HOOK_OP_BPF;
+	link->hook_ops.priv = prog;
+
+	link->hook_ops.pf = attr->link_create.netfilter.pf;
+	link->hook_ops.priority = attr->link_create.netfilter.prio;
+	link->hook_ops.hooknum = attr->link_create.netfilter.hooknum;
+
+	link->net = net;
+
+	err = bpf_link_prime(&link->link, &link_primer);
+	if (err) {
+		kfree(link);
+		return err;
+	}
+
+	err = nf_register_net_hook(net, &link->hook_ops);
+	if (err) {
+		bpf_link_cleanup(&link_primer);
+		return err;
+	}
+
+	return bpf_link_settle(&link_primer);
+}
-- 
2.39.2


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH bpf-next 2/6] bpf: minimal support for programs hooked into netfilter framework
  2023-04-05 16:11 [PATCH bpf-next 0/6] bpf: add netfilter program type Florian Westphal
  2023-04-05 16:11 ` [PATCH bpf-next 1/6] bpf: add bpf_link support for BPF_NETFILTER programs Florian Westphal
@ 2023-04-05 16:11 ` Florian Westphal
  2023-04-05 16:11 ` [PATCH bpf-next 3/6] netfilter: nfnetlink hook: dump bpf prog id Florian Westphal
                   ` (4 subsequent siblings)
  6 siblings, 0 replies; 14+ messages in thread
From: Florian Westphal @ 2023-04-05 16:11 UTC (permalink / raw)
  To: netdev; +Cc: netfilter-devel, bpf, dxu, qde, Florian Westphal

This adds minimal support for BPF_PROG_TYPE_NETFILTER bpf programs
that will be invoked via the NF_HOOK() points in the ip stack.

Invocation incurs an indirect call.  This is not a necessity: Its
possible to add 'DEFINE_BPF_DISPATCHER(nf_progs)' and handle the
program invocation with the same method already done for xdp progs.

This isn't done here to keep the size of this chunk down.

Verifier restricts verdicts to either DROP or ACCEPT.

Signed-off-by: Florian Westphal <fw@strlen.de>
---
 include/linux/bpf_types.h           |  4 ++
 include/net/netfilter/nf_bpf_link.h |  6 +++
 kernel/bpf/btf.c                    |  6 +++
 kernel/bpf/verifier.c               |  3 ++
 net/core/filter.c                   |  1 +
 net/netfilter/nf_bpf_link.c         | 70 ++++++++++++++++++++++++++++-
 6 files changed, 89 insertions(+), 1 deletion(-)

diff --git a/include/linux/bpf_types.h b/include/linux/bpf_types.h
index d4ee3ccd3753..39a999abb0ce 100644
--- a/include/linux/bpf_types.h
+++ b/include/linux/bpf_types.h
@@ -79,6 +79,10 @@ BPF_PROG_TYPE(BPF_PROG_TYPE_LSM, lsm,
 #endif
 BPF_PROG_TYPE(BPF_PROG_TYPE_SYSCALL, bpf_syscall,
 	      void *, void *)
+#ifdef CONFIG_NETFILTER
+BPF_PROG_TYPE(BPF_PROG_TYPE_NETFILTER, netfilter,
+	      struct bpf_nf_ctx, struct bpf_nf_ctx)
+#endif
 
 BPF_MAP_TYPE(BPF_MAP_TYPE_ARRAY, array_map_ops)
 BPF_MAP_TYPE(BPF_MAP_TYPE_PERCPU_ARRAY, percpu_array_map_ops)
diff --git a/include/net/netfilter/nf_bpf_link.h b/include/net/netfilter/nf_bpf_link.h
index 9d1b338e89d7..863cbbcc66f9 100644
--- a/include/net/netfilter/nf_bpf_link.h
+++ b/include/net/netfilter/nf_bpf_link.h
@@ -1,2 +1,8 @@
 /* SPDX-License-Identifier: GPL-2.0 */
+
+struct bpf_nf_ctx {
+	const struct nf_hook_state *state;
+	struct sk_buff *skb;
+};
+
 int bpf_nf_link_attach(const union bpf_attr *attr, struct bpf_prog *prog);
diff --git a/kernel/bpf/btf.c b/kernel/bpf/btf.c
index b7e5a5510b91..03abc1643567 100644
--- a/kernel/bpf/btf.c
+++ b/kernel/bpf/btf.c
@@ -25,6 +25,9 @@
 #include <linux/bsearch.h>
 #include <linux/kobject.h>
 #include <linux/sysfs.h>
+
+#include <net/netfilter/nf_bpf_link.h>
+
 #include <net/sock.h>
 #include "../tools/lib/bpf/relo_core.h"
 
@@ -212,6 +215,7 @@ enum btf_kfunc_hook {
 	BTF_KFUNC_HOOK_SK_SKB,
 	BTF_KFUNC_HOOK_SOCKET_FILTER,
 	BTF_KFUNC_HOOK_LWT,
+	BTF_KFUNC_HOOK_NETFILTER,
 	BTF_KFUNC_HOOK_MAX,
 };
 
@@ -7849,6 +7853,8 @@ static int bpf_prog_type_to_kfunc_hook(enum bpf_prog_type prog_type)
 	case BPF_PROG_TYPE_LWT_XMIT:
 	case BPF_PROG_TYPE_LWT_SEG6LOCAL:
 		return BTF_KFUNC_HOOK_LWT;
+	case BPF_PROG_TYPE_NETFILTER:
+		return BTF_KFUNC_HOOK_NETFILTER;
 	default:
 		return BTF_KFUNC_HOOK_MAX;
 	}
diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
index eaf9c5291cf0..0dfaca75f8a9 100644
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -13803,6 +13803,9 @@ static int check_return_code(struct bpf_verifier_env *env)
 		}
 		break;
 
+	case BPF_PROG_TYPE_NETFILTER:
+		range = tnum_range(NF_DROP, NF_ACCEPT);
+		break;
 	case BPF_PROG_TYPE_EXT:
 		/* freplace program can return anything as its return value
 		 * depends on the to-be-replaced kernel func or bpf program.
diff --git a/net/core/filter.c b/net/core/filter.c
index 3370efad1dda..24a5c62e5979 100644
--- a/net/core/filter.c
+++ b/net/core/filter.c
@@ -11721,6 +11721,7 @@ static int __init bpf_kfunc_init(void)
 	ret = ret ?: register_btf_kfunc_id_set(BPF_PROG_TYPE_LWT_IN, &bpf_kfunc_set_skb);
 	ret = ret ?: register_btf_kfunc_id_set(BPF_PROG_TYPE_LWT_XMIT, &bpf_kfunc_set_skb);
 	ret = ret ?: register_btf_kfunc_id_set(BPF_PROG_TYPE_LWT_SEG6LOCAL, &bpf_kfunc_set_skb);
+	ret = ret ?: register_btf_kfunc_id_set(BPF_PROG_TYPE_NETFILTER, &bpf_kfunc_set_skb);
 	return ret ?: register_btf_kfunc_id_set(BPF_PROG_TYPE_XDP, &bpf_kfunc_set_xdp);
 }
 late_initcall(bpf_kfunc_init);
diff --git a/net/netfilter/nf_bpf_link.c b/net/netfilter/nf_bpf_link.c
index c68b2cb70fd4..4b22a31d6df5 100644
--- a/net/netfilter/nf_bpf_link.c
+++ b/net/netfilter/nf_bpf_link.c
@@ -1,12 +1,19 @@
 // SPDX-License-Identifier: GPL-2.0
 #include <linux/bpf.h>
+#include <linux/filter.h>
 #include <linux/netfilter.h>
 
 #include <net/netfilter/nf_bpf_link.h>
 
 static unsigned int nf_hook_run_bpf(void *bpf_prog, struct sk_buff *skb, const struct nf_hook_state *s)
 {
-	return NF_ACCEPT;
+	const struct bpf_prog *prog = bpf_prog;
+	struct bpf_nf_ctx ctx = {
+		.state = s,
+		.skb = skb,
+	};
+
+	return bpf_prog_run(prog, &ctx);
 }
 
 struct bpf_nf_link {
@@ -119,3 +126,64 @@ int bpf_nf_link_attach(const union bpf_attr *attr, struct bpf_prog *prog)
 
 	return bpf_link_settle(&link_primer);
 }
+
+const struct bpf_prog_ops netfilter_prog_ops = {
+};
+
+static bool nf_ptr_to_btf_id(struct bpf_insn_access_aux *info, const char *name)
+{
+	struct btf *btf;
+	s32 type_id;
+
+	btf = bpf_get_btf_vmlinux();
+	if (IS_ERR_OR_NULL(btf))
+		return false;
+
+	type_id = btf_find_by_name_kind(btf, name, BTF_KIND_STRUCT);
+	if (WARN_ON_ONCE(type_id < 0))
+		return false;
+
+	info->btf = btf;
+	info->btf_id = type_id;
+	info->reg_type = PTR_TO_BTF_ID | PTR_TRUSTED;
+	return true;
+}
+
+static bool nf_is_valid_access(int off, int size, enum bpf_access_type type,
+			       const struct bpf_prog *prog,
+			       struct bpf_insn_access_aux *info)
+{
+	if (off < 0 || off >= sizeof(struct bpf_nf_ctx))
+		return false;
+
+	if (type == BPF_WRITE)
+		return false;
+
+	switch (off) {
+	case bpf_ctx_range(struct bpf_nf_ctx, skb):
+		if (size != sizeof_field(struct bpf_nf_ctx, skb))
+			return false;
+
+		return nf_ptr_to_btf_id(info, "sk_buff");
+	case bpf_ctx_range(struct bpf_nf_ctx, state):
+		if (size != sizeof_field(struct bpf_nf_ctx, state))
+			return false;
+
+		return nf_ptr_to_btf_id(info, "nf_hook_state");
+	default:
+		return false;
+	}
+
+	return false;
+}
+
+static const struct bpf_func_proto *
+bpf_nf_func_proto(enum bpf_func_id func_id, const struct bpf_prog *prog)
+{
+	return bpf_base_func_proto(func_id);
+}
+
+const struct bpf_verifier_ops netfilter_verifier_ops = {
+	.is_valid_access	= nf_is_valid_access,
+	.get_func_proto		= bpf_nf_func_proto,
+};
-- 
2.39.2


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH bpf-next 3/6] netfilter: nfnetlink hook: dump bpf prog id
  2023-04-05 16:11 [PATCH bpf-next 0/6] bpf: add netfilter program type Florian Westphal
  2023-04-05 16:11 ` [PATCH bpf-next 1/6] bpf: add bpf_link support for BPF_NETFILTER programs Florian Westphal
  2023-04-05 16:11 ` [PATCH bpf-next 2/6] bpf: minimal support for programs hooked into netfilter framework Florian Westphal
@ 2023-04-05 16:11 ` Florian Westphal
  2023-04-05 16:11 ` [PATCH bpf-next 4/6] netfilter: disallow bpf hook attachment at same priority Florian Westphal
                   ` (3 subsequent siblings)
  6 siblings, 0 replies; 14+ messages in thread
From: Florian Westphal @ 2023-04-05 16:11 UTC (permalink / raw)
  To: netdev; +Cc: netfilter-devel, bpf, dxu, qde, Florian Westphal

This allows userspace ("nft list hooks") to show which bpf program
is attached to which hook.

Without this, user only knows bpf prog is attached at prio
x, y, z at INPUT and FORWARD, but can't tell which program is where.

Signed-off-by: Florian Westphal <fw@strlen.de>
---
 include/uapi/linux/netfilter/nfnetlink_hook.h | 20 ++++-
 net/netfilter/nfnetlink_hook.c                | 81 ++++++++++++++++---
 2 files changed, 87 insertions(+), 14 deletions(-)

diff --git a/include/uapi/linux/netfilter/nfnetlink_hook.h b/include/uapi/linux/netfilter/nfnetlink_hook.h
index bbcd285b22e1..63b7dbddf0b1 100644
--- a/include/uapi/linux/netfilter/nfnetlink_hook.h
+++ b/include/uapi/linux/netfilter/nfnetlink_hook.h
@@ -32,8 +32,12 @@ enum nfnl_hook_attributes {
 /**
  * enum nfnl_hook_chain_info_attributes - chain description
  *
- * NFNLA_HOOK_INFO_DESC: nft chain and table name (enum nft_table_attributes) (NLA_NESTED)
+ * NFNLA_HOOK_INFO_DESC: nft chain and table name (NLA_NESTED)
  * NFNLA_HOOK_INFO_TYPE: chain type (enum nfnl_hook_chaintype) (NLA_U32)
+ *
+ * NFNLA_HOOK_INFO_DESC depends on NFNLA_HOOK_INFO_TYPE value:
+ *   NFNL_HOOK_TYPE_NFTABLES: enum nft_table_attributes
+ *   NFNL_HOOK_TYPE_BPF: enum nfnl_hook_bpf_info_attributes
  */
 enum nfnl_hook_chain_info_attributes {
 	NFNLA_HOOK_INFO_UNSPEC,
@@ -56,9 +60,23 @@ enum nfnl_hook_chain_desc_attributes {
  * enum nfnl_hook_chaintype - chain type
  *
  * @NFNL_HOOK_TYPE_NFTABLES nf_tables base chain
+ * @NFNL_HOOK_TYPE_BPF bpf program
  */
 enum nfnl_hook_chaintype {
 	NFNL_HOOK_TYPE_NFTABLES = 0x1,
+	NFNL_HOOK_TYPE_BPF,
+};
+
+/**
+ * enum nfnl_hook_bpf_info_attributes - bpf prog description
+ *
+ * NFNLA_BPF_INFO_ID: bpf program id (NLA_U32)
+ */
+enum nfnl_hook_bpf_attributes {
+	NFNLA_HOOK_BPF_UNSPEC,
+	NFNLA_HOOK_BPF_ID,
+	__NFNLA_HOOK_BPF_MAX,
 };
+#define NFNLA_HOOK_BPF_MAX (__NFNLA_HOOK_BPF_MAX - 1)
 
 #endif /* _NFNL_HOOK_H */
diff --git a/net/netfilter/nfnetlink_hook.c b/net/netfilter/nfnetlink_hook.c
index 8120aadf6a0f..ade8ee1988b1 100644
--- a/net/netfilter/nfnetlink_hook.c
+++ b/net/netfilter/nfnetlink_hook.c
@@ -5,6 +5,7 @@
  * Author: Florian Westphal <fw@strlen.de>
  */
 
+#include <linux/bpf.h>
 #include <linux/module.h>
 #include <linux/kallsyms.h>
 #include <linux/kernel.h>
@@ -57,35 +58,76 @@ struct nfnl_dump_hook_data {
 	u8 hook;
 };
 
+static struct nlattr *nfnl_start_info_type(struct sk_buff *nlskb, enum nfnl_hook_chaintype t)
+{
+	struct nlattr *nest = nla_nest_start(nlskb, NFNLA_HOOK_CHAIN_INFO);
+	int ret;
+
+	if (!nest)
+		return NULL;
+
+	ret = nla_put_be32(nlskb, NFNLA_HOOK_INFO_TYPE, htonl(t));
+	if (ret == 0)
+		return nest;
+
+	nla_nest_cancel(nlskb, nest);
+	return NULL;
+}
+
+static int nfnl_hook_put_bpf_prog_info(struct sk_buff *nlskb,
+				       const struct nfnl_dump_hook_data *ctx,
+				       unsigned int seq,
+				       const struct bpf_prog *prog)
+{
+	struct nlattr *nest, *nest2;
+	int ret;
+
+	if (!IS_ENABLED(CONFIG_NETFILTER_BPF_LINK))
+		return 0;
+
+	if (WARN_ON_ONCE(!prog))
+		return 0;
+
+	nest = nfnl_start_info_type(nlskb, NFNL_HOOK_TYPE_BPF);
+	if (!nest)
+		return -EMSGSIZE;
+
+	nest2 = nla_nest_start(nlskb, NFNLA_HOOK_INFO_DESC);
+	if (!nest2)
+		goto cancel_nest;
+
+	ret = nla_put_be32(nlskb, NFNLA_HOOK_BPF_ID, htonl(prog->aux->id));
+	if (ret)
+		goto cancel_nest;
+
+	nla_nest_end(nlskb, nest2);
+	nla_nest_end(nlskb, nest);
+	return 0;
+
+cancel_nest:
+	nla_nest_cancel(nlskb, nest);
+	return -EMSGSIZE;
+}
+
 static int nfnl_hook_put_nft_chain_info(struct sk_buff *nlskb,
 					const struct nfnl_dump_hook_data *ctx,
 					unsigned int seq,
-					const struct nf_hook_ops *ops)
+					struct nft_chain *chain)
 {
 	struct net *net = sock_net(nlskb->sk);
 	struct nlattr *nest, *nest2;
-	struct nft_chain *chain;
 	int ret = 0;
 
-	if (ops->hook_ops_type != NF_HOOK_OP_NF_TABLES)
-		return 0;
-
-	chain = ops->priv;
 	if (WARN_ON_ONCE(!chain))
 		return 0;
 
 	if (!nft_is_active(net, chain))
 		return 0;
 
-	nest = nla_nest_start(nlskb, NFNLA_HOOK_CHAIN_INFO);
+	nest = nfnl_start_info_type(nlskb, NFNL_HOOK_TYPE_NFTABLES);
 	if (!nest)
 		return -EMSGSIZE;
 
-	ret = nla_put_be32(nlskb, NFNLA_HOOK_INFO_TYPE,
-			   htonl(NFNL_HOOK_TYPE_NFTABLES));
-	if (ret)
-		goto cancel_nest;
-
 	nest2 = nla_nest_start(nlskb, NFNLA_HOOK_INFO_DESC);
 	if (!nest2)
 		goto cancel_nest;
@@ -171,7 +213,20 @@ static int nfnl_hook_dump_one(struct sk_buff *nlskb,
 	if (ret)
 		goto nla_put_failure;
 
-	ret = nfnl_hook_put_nft_chain_info(nlskb, ctx, seq, ops);
+	switch (ops->hook_ops_type) {
+	case NF_HOOK_OP_NF_TABLES:
+		ret = nfnl_hook_put_nft_chain_info(nlskb, ctx, seq, ops->priv);
+		break;
+	case NF_HOOK_OP_BPF:
+		ret = nfnl_hook_put_bpf_prog_info(nlskb, ctx, seq, ops->priv);
+		break;
+	case NF_HOOK_OP_UNDEFINED:
+		break;
+	default:
+		WARN_ON_ONCE(1);
+		break;
+	}
+
 	if (ret)
 		goto nla_put_failure;
 
-- 
2.39.2


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH bpf-next 4/6] netfilter: disallow bpf hook attachment at same priority
  2023-04-05 16:11 [PATCH bpf-next 0/6] bpf: add netfilter program type Florian Westphal
                   ` (2 preceding siblings ...)
  2023-04-05 16:11 ` [PATCH bpf-next 3/6] netfilter: nfnetlink hook: dump bpf prog id Florian Westphal
@ 2023-04-05 16:11 ` Florian Westphal
  2023-04-05 16:11 ` [PATCH bpf-next 5/6] tools: bpftool: print netfilter link info Florian Westphal
                   ` (2 subsequent siblings)
  6 siblings, 0 replies; 14+ messages in thread
From: Florian Westphal @ 2023-04-05 16:11 UTC (permalink / raw)
  To: netdev; +Cc: netfilter-devel, bpf, dxu, qde, Florian Westphal

This is just to avoid ordering issues between multiple bpf programs,
this could be removed later in case it turns out to be too cautious.

bpf prog could still be shared with non-bpf hook, otherwise we'd have to
make conntrack hook registration fail just because a bpf program has
same priority.

Signed-off-by: Florian Westphal <fw@strlen.de>
---
 net/netfilter/core.c | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/net/netfilter/core.c b/net/netfilter/core.c
index 358220b58521..f0783e42108b 100644
--- a/net/netfilter/core.c
+++ b/net/netfilter/core.c
@@ -119,6 +119,18 @@ nf_hook_entries_grow(const struct nf_hook_entries *old,
 		for (i = 0; i < old_entries; i++) {
 			if (orig_ops[i] != &dummy_ops)
 				alloc_entries++;
+
+			/* Restrict BPF hook type to force a unique priority, not
+			 * shared at attach time.
+			 *
+			 * This is mainly to avoid ordering issues between two
+			 * different bpf programs, this doesn't prevent a normal
+			 * hook at same priority as a bpf one (we don't want to
+			 * prevent defrag, conntrack, iptables etc from attaching).
+			 */
+			if (reg->priority == orig_ops[i]->priority &&
+			    reg->hook_ops_type == NF_HOOK_OP_BPF)
+				return ERR_PTR(-EBUSY);
 		}
 	}
 
-- 
2.39.2


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH bpf-next 5/6] tools: bpftool: print netfilter link info
  2023-04-05 16:11 [PATCH bpf-next 0/6] bpf: add netfilter program type Florian Westphal
                   ` (3 preceding siblings ...)
  2023-04-05 16:11 ` [PATCH bpf-next 4/6] netfilter: disallow bpf hook attachment at same priority Florian Westphal
@ 2023-04-05 16:11 ` Florian Westphal
  2023-04-05 16:11 ` [PATCH bpf-next 6/6] bpf: add test_run support for netfilter program type Florian Westphal
  2023-04-12  8:20 ` [PATCH bpf-next 0/6] bpf: add " Quentin Deslandes
  6 siblings, 0 replies; 14+ messages in thread
From: Florian Westphal @ 2023-04-05 16:11 UTC (permalink / raw)
  To: netdev; +Cc: netfilter-devel, bpf, dxu, qde, Florian Westphal

Dump protocol family, hook and priority value:
$ bpftool link
2: type 10  prog 20
        pf: 2, hook 1, prio -128

Signed-off-by: Florian Westphal <fw@strlen.de>
---
 tools/bpf/bpftool/link.c       | 24 ++++++++++++++++++++++++
 tools/include/uapi/linux/bpf.h | 15 +++++++++++++++
 tools/lib/bpf/libbpf.c         |  1 +
 3 files changed, 40 insertions(+)

diff --git a/tools/bpf/bpftool/link.c b/tools/bpf/bpftool/link.c
index f985b79cca27..a2ea85d1ebbf 100644
--- a/tools/bpf/bpftool/link.c
+++ b/tools/bpf/bpftool/link.c
@@ -135,6 +135,18 @@ static void show_iter_json(struct bpf_link_info *info, json_writer_t *wtr)
 	}
 }
 
+static void show_netfilter_json(const struct bpf_link_info *info, json_writer_t *wtr)
+{
+	jsonw_uint_field(json_wtr, "pf",
+			 info->netfilter.pf);
+	jsonw_uint_field(json_wtr, "hook",
+			 info->netfilter.hooknum);
+	jsonw_int_field(json_wtr, "prio",
+			 info->netfilter.priority);
+	jsonw_uint_field(json_wtr, "flags",
+			 info->netfilter.flags);
+}
+
 static int get_prog_info(int prog_id, struct bpf_prog_info *info)
 {
 	__u32 len = sizeof(*info);
@@ -195,6 +207,10 @@ static int show_link_close_json(int fd, struct bpf_link_info *info)
 				 info->netns.netns_ino);
 		show_link_attach_type_json(info->netns.attach_type, json_wtr);
 		break;
+	case BPF_LINK_TYPE_NETFILTER:
+		show_netfilter_json(info, json_wtr);
+		break;
+
 	default:
 		break;
 	}
@@ -301,6 +317,14 @@ static int show_link_close_plain(int fd, struct bpf_link_info *info)
 		printf("\n\tnetns_ino %u  ", info->netns.netns_ino);
 		show_link_attach_type_plain(info->netns.attach_type);
 		break;
+	case BPF_LINK_TYPE_NETFILTER:
+		printf("\n\tpf: %d, hook %u, prio %d",
+		       info->netfilter.pf,
+		       info->netfilter.hooknum,
+		       info->netfilter.priority);
+		if (info->netfilter.flags)
+			printf(" flags 0x%x", info->netfilter.flags);
+		break;
 	default:
 		break;
 	}
diff --git a/tools/include/uapi/linux/bpf.h b/tools/include/uapi/linux/bpf.h
index d6c5a022ae28..dcd2d5ec4918 100644
--- a/tools/include/uapi/linux/bpf.h
+++ b/tools/include/uapi/linux/bpf.h
@@ -986,6 +986,7 @@ enum bpf_prog_type {
 	BPF_PROG_TYPE_LSM,
 	BPF_PROG_TYPE_SK_LOOKUP,
 	BPF_PROG_TYPE_SYSCALL, /* a program that can execute syscalls */
+	BPF_PROG_TYPE_NETFILTER,
 };
 
 enum bpf_attach_type {
@@ -1050,6 +1051,7 @@ enum bpf_link_type {
 	BPF_LINK_TYPE_PERF_EVENT = 7,
 	BPF_LINK_TYPE_KPROBE_MULTI = 8,
 	BPF_LINK_TYPE_STRUCT_OPS = 9,
+	BPF_LINK_TYPE_NETFILTER = 10,
 
 	MAX_BPF_LINK_TYPE,
 };
@@ -1550,6 +1552,13 @@ union bpf_attr {
 				 */
 				__u64		cookie;
 			} tracing;
+			struct {
+				__u32		pf;
+				__u32		hooknum;
+				__s32		prio;
+				__u32		flags;
+				__u64		reserved[2];
+			} netfilter;
 		};
 	} link_create;
 
@@ -6400,6 +6409,12 @@ struct bpf_link_info {
 		struct {
 			__u32 map_id;
 		} struct_ops;
+		struct {
+			__u32 pf;
+			__u32 hooknum;
+			__s32 priority;
+			__u32 flags;
+		} netfilter;
 	};
 } __attribute__((aligned(8)));
 
diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
index 49cd304ae3bc..ae27451002ae 100644
--- a/tools/lib/bpf/libbpf.c
+++ b/tools/lib/bpf/libbpf.c
@@ -8641,6 +8641,7 @@ static const struct bpf_sec_def section_defs[] = {
 	SEC_DEF("struct_ops+",		STRUCT_OPS, 0, SEC_NONE),
 	SEC_DEF("struct_ops.s+",	STRUCT_OPS, 0, SEC_SLEEPABLE),
 	SEC_DEF("sk_lookup",		SK_LOOKUP, BPF_SK_LOOKUP, SEC_ATTACHABLE),
+	SEC_DEF("netfilter",		NETFILTER, 0, SEC_NONE),
 };
 
 static size_t custom_sec_def_cnt;
-- 
2.39.2


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH bpf-next 6/6] bpf: add test_run support for netfilter program type
  2023-04-05 16:11 [PATCH bpf-next 0/6] bpf: add netfilter program type Florian Westphal
                   ` (4 preceding siblings ...)
  2023-04-05 16:11 ` [PATCH bpf-next 5/6] tools: bpftool: print netfilter link info Florian Westphal
@ 2023-04-05 16:11 ` Florian Westphal
  2023-04-05 18:22   ` kernel test robot
  2023-04-07  1:36   ` Alexei Starovoitov
  2023-04-12  8:20 ` [PATCH bpf-next 0/6] bpf: add " Quentin Deslandes
  6 siblings, 2 replies; 14+ messages in thread
From: Florian Westphal @ 2023-04-05 16:11 UTC (permalink / raw)
  To: netdev; +Cc: netfilter-devel, bpf, dxu, qde, Florian Westphal

also add two simple retval tests: as-is, a return value other
than accept or drop will cause issues.

NF_QUEUE could be implemented later IFF we can guarantee that
attachment of such programs can be rejected if they get attached
to a pf/hook that doesn't support async reinjection.

NF_STOLEN could be implemented via trusted helpers that will eventually
free the skb, else this would leak the skb reference.

Signed-off-by: Florian Westphal <fw@strlen.de>
---
 include/linux/bpf.h                           |   3 +
 net/bpf/test_run.c                            | 143 ++++++++++++++++++
 net/netfilter/nf_bpf_link.c                   |   1 +
 .../selftests/bpf/verifier/netfilter.c        |  23 +++
 4 files changed, 170 insertions(+)
 create mode 100644 tools/testing/selftests/bpf/verifier/netfilter.c

diff --git a/include/linux/bpf.h b/include/linux/bpf.h
index 2d8f3f639e68..453cee1efdd3 100644
--- a/include/linux/bpf.h
+++ b/include/linux/bpf.h
@@ -2235,6 +2235,9 @@ int bpf_prog_test_run_raw_tp(struct bpf_prog *prog,
 int bpf_prog_test_run_sk_lookup(struct bpf_prog *prog,
 				const union bpf_attr *kattr,
 				union bpf_attr __user *uattr);
+int bpf_prog_test_run_nf(struct bpf_prog *prog,
+			 const union bpf_attr *kattr,
+			 union bpf_attr __user *uattr);
 bool btf_ctx_access(int off, int size, enum bpf_access_type type,
 		    const struct bpf_prog *prog,
 		    struct bpf_insn_access_aux *info);
diff --git a/net/bpf/test_run.c b/net/bpf/test_run.c
index f1652f5fbd2e..c14f577fd987 100644
--- a/net/bpf/test_run.c
+++ b/net/bpf/test_run.c
@@ -19,7 +19,9 @@
 #include <linux/error-injection.h>
 #include <linux/smp.h>
 #include <linux/sock_diag.h>
+#include <linux/netfilter.h>
 #include <net/xdp.h>
+#include <net/netfilter/nf_bpf_link.h>
 
 #define CREATE_TRACE_POINTS
 #include <trace/events/bpf_test_run.h>
@@ -1690,6 +1692,147 @@ int bpf_prog_test_run_syscall(struct bpf_prog *prog,
 	return err;
 }
 
+static int verify_and_copy_hook_state(struct nf_hook_state *state,
+				      const struct nf_hook_state *user,
+				      struct net_device *dev)
+{
+	if (user->in || user->out)
+		return -EINVAL;
+
+	if (user->net || user->sk || user->okfn)
+		return -EINVAL;
+
+	switch (user->pf) {
+	case NFPROTO_IPV4:
+	case NFPROTO_IPV6:
+		switch (state->hook) {
+		case NF_INET_PRE_ROUTING:
+			state->in = dev;
+			break;
+		case NF_INET_LOCAL_IN:
+			state->in = dev;
+			break;
+		case NF_INET_FORWARD:
+			state->in = dev;
+			state->out = dev;
+			break;
+		case NF_INET_LOCAL_OUT:
+			state->out = dev;
+			break;
+		case NF_INET_POST_ROUTING:
+			state->out = dev;
+			break;
+		}
+
+		break;
+	default:
+		return -EINVAL;
+	}
+
+	state->pf = user->pf;
+	state->hook = user->hook;
+
+	return 0;
+}
+
+int bpf_prog_test_run_nf(struct bpf_prog *prog,
+			 const union bpf_attr *kattr,
+			 union bpf_attr __user *uattr)
+{
+	struct net *net = current->nsproxy->net_ns;
+	struct net_device *dev = net->loopback_dev;
+	struct nf_hook_state *user_ctx, hook_state = {
+		.pf = NFPROTO_IPV4,
+		.hook = NF_INET_PRE_ROUTING,
+	};
+	u32 size = kattr->test.data_size_in;
+	u32 repeat = kattr->test.repeat;
+	const struct ethhdr *eth;
+	struct bpf_nf_ctx ctx = {
+		.state = &hook_state,
+	};
+	struct sk_buff *skb = NULL;
+	u32 retval, duration;
+	void *data;
+	int ret;
+
+	if (kattr->test.flags || kattr->test.cpu || kattr->test.batch_size)
+		return -EINVAL;
+
+	if (size < ETH_HLEN + sizeof(struct iphdr))
+		return -EINVAL;
+
+	data = bpf_test_init(kattr, kattr->test.data_size_in, size,
+			     NET_SKB_PAD + NET_IP_ALIGN,
+			     SKB_DATA_ALIGN(sizeof(struct skb_shared_info)));
+	if (IS_ERR(data))
+		return PTR_ERR(data);
+
+	eth = (struct ethhdr *)data;
+
+	if (!repeat)
+		repeat = 1;
+
+	user_ctx = bpf_ctx_init(kattr, sizeof(struct nf_hook_state));
+	if (IS_ERR(user_ctx)) {
+		kfree(data);
+		return PTR_ERR(user_ctx);
+	}
+
+	if (user_ctx) {
+		ret = verify_and_copy_hook_state(&hook_state, user_ctx, dev);
+		if (ret)
+			goto out;
+	}
+
+	skb = slab_build_skb(data);
+	if (!skb) {
+		ret = -ENOMEM;
+		goto out;
+	}
+
+	data = NULL; /* data released via kfree_skb */
+
+	skb_reserve(skb, NET_SKB_PAD + NET_IP_ALIGN);
+	__skb_put(skb, size);
+
+	skb->protocol = eth_type_trans(skb, dev);
+
+	skb_reset_network_header(skb);
+
+	ret = -EINVAL;
+
+	switch (skb->protocol) {
+	case htons(ETH_P_IP):
+		if (hook_state.pf == NFPROTO_IPV4)
+			break;
+		goto out;
+	case htons(ETH_P_IPV6):
+		if (size < ETH_HLEN + sizeof(struct ipv6hdr))
+			goto out;
+		if (hook_state.pf == NFPROTO_IPV6)
+			break;
+		goto out;
+	default:
+		ret = -EPROTO;
+		goto out;
+	}
+
+	ctx.skb = skb;
+
+	ret = bpf_test_run(prog, &ctx, repeat, &retval, &duration, false);
+	if (ret)
+		goto out;
+
+	ret = bpf_test_finish(kattr, uattr, NULL, NULL, 0, retval, duration);
+
+out:
+	kfree(user_ctx);
+	kfree_skb(skb);
+	kfree(data);
+	return ret;
+}
+
 static const struct btf_kfunc_id_set bpf_prog_test_kfunc_set = {
 	.owner = THIS_MODULE,
 	.set   = &test_sk_check_kfunc_ids,
diff --git a/net/netfilter/nf_bpf_link.c b/net/netfilter/nf_bpf_link.c
index 4b22a31d6df5..c27fd569adf1 100644
--- a/net/netfilter/nf_bpf_link.c
+++ b/net/netfilter/nf_bpf_link.c
@@ -128,6 +128,7 @@ int bpf_nf_link_attach(const union bpf_attr *attr, struct bpf_prog *prog)
 }
 
 const struct bpf_prog_ops netfilter_prog_ops = {
+	.test_run = bpf_prog_test_run_nf,
 };
 
 static bool nf_ptr_to_btf_id(struct bpf_insn_access_aux *info, const char *name)
diff --git a/tools/testing/selftests/bpf/verifier/netfilter.c b/tools/testing/selftests/bpf/verifier/netfilter.c
new file mode 100644
index 000000000000..deeb87afdf50
--- /dev/null
+++ b/tools/testing/selftests/bpf/verifier/netfilter.c
@@ -0,0 +1,23 @@
+{
+	"netfilter, accept all",
+	.insns = {
+	BPF_MOV64_IMM(BPF_REG_0, 1),
+	BPF_EXIT_INSN(),
+	},
+	.result = ACCEPT,
+	.prog_type = BPF_PROG_TYPE_NETFILTER,
+	.retval = 1,
+	.data = {
+		0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0x08, 0x00,
+	},
+},
+{
+	"netfilter, stolen verdict",
+	.insns = {
+	BPF_MOV64_IMM(BPF_REG_0, 2),
+	BPF_EXIT_INSN(),
+	},
+	.result = REJECT,
+	.errstr = "At program exit the register R0 has value (0x2; 0x0) should have been in (0x0; 0x1)",
+	.prog_type = BPF_PROG_TYPE_NETFILTER,
+},
-- 
2.39.2


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* Re: [PATCH bpf-next 6/6] bpf: add test_run support for netfilter program type
  2023-04-05 16:11 ` [PATCH bpf-next 6/6] bpf: add test_run support for netfilter program type Florian Westphal
@ 2023-04-05 18:22   ` kernel test robot
  2023-04-07  1:36   ` Alexei Starovoitov
  1 sibling, 0 replies; 14+ messages in thread
From: kernel test robot @ 2023-04-05 18:22 UTC (permalink / raw)
  To: Florian Westphal, netdev
  Cc: oe-kbuild-all, netfilter-devel, bpf, dxu, qde, Florian Westphal

Hi Florian,

kernel test robot noticed the following build warnings:

[auto build test WARNING on bpf-next/master]

url:    https://github.com/intel-lab-lkp/linux/commits/Florian-Westphal/bpf-add-bpf_link-support-for-BPF_NETFILTER-programs/20230406-001447
base:   https://git.kernel.org/pub/scm/linux/kernel/git/bpf/bpf-next.git master
patch link:    https://lore.kernel.org/r/20230405161116.13565-7-fw%40strlen.de
patch subject: [PATCH bpf-next 6/6] bpf: add test_run support for netfilter program type
config: s390-allyesconfig (https://download.01.org/0day-ci/archive/20230406/202304060207.JawhnyR9-lkp@intel.com/config)
compiler: s390-linux-gcc (GCC) 12.1.0
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # https://github.com/intel-lab-lkp/linux/commit/7fba218dfc4942aa6781f4d1b5c475a0569cfd2e
        git remote add linux-review https://github.com/intel-lab-lkp/linux
        git fetch --no-tags linux-review Florian-Westphal/bpf-add-bpf_link-support-for-BPF_NETFILTER-programs/20230406-001447
        git checkout 7fba218dfc4942aa6781f4d1b5c475a0569cfd2e
        # save the config file
        mkdir build_dir && cp config build_dir/.config
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross W=1 O=build_dir ARCH=s390 olddefconfig
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross W=1 O=build_dir ARCH=s390 SHELL=/bin/bash net/

If you fix the issue, kindly add following tag where applicable
| Reported-by: kernel test robot <lkp@intel.com>
| Link: https://lore.kernel.org/oe-kbuild-all/202304060207.JawhnyR9-lkp@intel.com/

All warnings (new ones prefixed by >>):

   net/bpf/test_run.c: In function 'bpf_prog_test_run_nf':
>> net/bpf/test_run.c:1750:30: warning: variable 'eth' set but not used [-Wunused-but-set-variable]
    1750 |         const struct ethhdr *eth;
         |                              ^~~


vim +/eth +1750 net/bpf/test_run.c

  1737	
  1738	int bpf_prog_test_run_nf(struct bpf_prog *prog,
  1739				 const union bpf_attr *kattr,
  1740				 union bpf_attr __user *uattr)
  1741	{
  1742		struct net *net = current->nsproxy->net_ns;
  1743		struct net_device *dev = net->loopback_dev;
  1744		struct nf_hook_state *user_ctx, hook_state = {
  1745			.pf = NFPROTO_IPV4,
  1746			.hook = NF_INET_PRE_ROUTING,
  1747		};
  1748		u32 size = kattr->test.data_size_in;
  1749		u32 repeat = kattr->test.repeat;
> 1750		const struct ethhdr *eth;
  1751		struct bpf_nf_ctx ctx = {
  1752			.state = &hook_state,
  1753		};
  1754		struct sk_buff *skb = NULL;
  1755		u32 retval, duration;
  1756		void *data;
  1757		int ret;
  1758	
  1759		if (kattr->test.flags || kattr->test.cpu || kattr->test.batch_size)
  1760			return -EINVAL;
  1761	
  1762		if (size < ETH_HLEN + sizeof(struct iphdr))
  1763			return -EINVAL;
  1764	
  1765		data = bpf_test_init(kattr, kattr->test.data_size_in, size,
  1766				     NET_SKB_PAD + NET_IP_ALIGN,
  1767				     SKB_DATA_ALIGN(sizeof(struct skb_shared_info)));
  1768		if (IS_ERR(data))
  1769			return PTR_ERR(data);
  1770	
  1771		eth = (struct ethhdr *)data;
  1772	
  1773		if (!repeat)
  1774			repeat = 1;
  1775	
  1776		user_ctx = bpf_ctx_init(kattr, sizeof(struct nf_hook_state));
  1777		if (IS_ERR(user_ctx)) {
  1778			kfree(data);
  1779			return PTR_ERR(user_ctx);
  1780		}
  1781	
  1782		if (user_ctx) {
  1783			ret = verify_and_copy_hook_state(&hook_state, user_ctx, dev);
  1784			if (ret)
  1785				goto out;
  1786		}
  1787	
  1788		skb = slab_build_skb(data);
  1789		if (!skb) {
  1790			ret = -ENOMEM;
  1791			goto out;
  1792		}
  1793	
  1794		data = NULL; /* data released via kfree_skb */
  1795	
  1796		skb_reserve(skb, NET_SKB_PAD + NET_IP_ALIGN);
  1797		__skb_put(skb, size);
  1798	
  1799		skb->protocol = eth_type_trans(skb, dev);
  1800	
  1801		skb_reset_network_header(skb);
  1802	
  1803		ret = -EINVAL;
  1804	
  1805		switch (skb->protocol) {
  1806		case htons(ETH_P_IP):
  1807			if (hook_state.pf == NFPROTO_IPV4)
  1808				break;
  1809			goto out;
  1810		case htons(ETH_P_IPV6):
  1811			if (size < ETH_HLEN + sizeof(struct ipv6hdr))
  1812				goto out;
  1813			if (hook_state.pf == NFPROTO_IPV6)
  1814				break;
  1815			goto out;
  1816		default:
  1817			ret = -EPROTO;
  1818			goto out;
  1819		}
  1820	
  1821		ctx.skb = skb;
  1822	
  1823		ret = bpf_test_run(prog, &ctx, repeat, &retval, &duration, false);
  1824		if (ret)
  1825			goto out;
  1826	
  1827		ret = bpf_test_finish(kattr, uattr, NULL, NULL, 0, retval, duration);
  1828	
  1829	out:
  1830		kfree(user_ctx);
  1831		kfree_skb(skb);
  1832		kfree(data);
  1833		return ret;
  1834	}
  1835	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH bpf-next 1/6] bpf: add bpf_link support for BPF_NETFILTER programs
  2023-04-05 16:11 ` [PATCH bpf-next 1/6] bpf: add bpf_link support for BPF_NETFILTER programs Florian Westphal
@ 2023-04-06  4:50   ` kernel test robot
  0 siblings, 0 replies; 14+ messages in thread
From: kernel test robot @ 2023-04-06  4:50 UTC (permalink / raw)
  To: Florian Westphal, netdev
  Cc: oe-kbuild-all, netfilter-devel, bpf, dxu, qde, Florian Westphal

Hi Florian,

kernel test robot noticed the following build errors:

[auto build test ERROR on bpf-next/master]

url:    https://github.com/intel-lab-lkp/linux/commits/Florian-Westphal/bpf-add-bpf_link-support-for-BPF_NETFILTER-programs/20230406-001447
base:   https://git.kernel.org/pub/scm/linux/kernel/git/bpf/bpf-next.git master
patch link:    https://lore.kernel.org/r/20230405161116.13565-2-fw%40strlen.de
patch subject: [PATCH bpf-next 1/6] bpf: add bpf_link support for BPF_NETFILTER programs
config: x86_64-kexec (https://download.01.org/0day-ci/archive/20230406/202304061228.XRcVvxoL-lkp@intel.com/config)
compiler: gcc-11 (Debian 11.3.0-8) 11.3.0
reproduce (this is a W=1 build):
        # https://github.com/intel-lab-lkp/linux/commit/f373efb623e6ff708403b172fafb506028de6cb8
        git remote add linux-review https://github.com/intel-lab-lkp/linux
        git fetch --no-tags linux-review Florian-Westphal/bpf-add-bpf_link-support-for-BPF_NETFILTER-programs/20230406-001447
        git checkout f373efb623e6ff708403b172fafb506028de6cb8
        # save the config file
        mkdir build_dir && cp config build_dir/.config
        make W=1 O=build_dir ARCH=x86_64 olddefconfig
        make W=1 O=build_dir ARCH=x86_64 SHELL=/bin/bash

If you fix the issue, kindly add following tag where applicable
| Reported-by: kernel test robot <lkp@intel.com>
| Link: https://lore.kernel.org/oe-kbuild-all/202304061228.XRcVvxoL-lkp@intel.com/

All errors (new ones prefixed by >>):

   ld: vmlinux.o: in function `link_create':
>> kernel/bpf/syscall.c:4671: undefined reference to `bpf_nf_link_attach'


vim +4671 kernel/bpf/syscall.c

  4578	
  4579	#define BPF_LINK_CREATE_LAST_FIELD link_create.kprobe_multi.cookies
  4580	static int link_create(union bpf_attr *attr, bpfptr_t uattr)
  4581	{
  4582		enum bpf_prog_type ptype;
  4583		struct bpf_prog *prog;
  4584		int ret;
  4585	
  4586		if (CHECK_ATTR(BPF_LINK_CREATE))
  4587			return -EINVAL;
  4588	
  4589		if (attr->link_create.attach_type == BPF_STRUCT_OPS)
  4590			return bpf_struct_ops_link_create(attr);
  4591	
  4592		prog = bpf_prog_get(attr->link_create.prog_fd);
  4593		if (IS_ERR(prog))
  4594			return PTR_ERR(prog);
  4595	
  4596		ret = bpf_prog_attach_check_attach_type(prog,
  4597							attr->link_create.attach_type);
  4598		if (ret)
  4599			goto out;
  4600	
  4601		switch (prog->type) {
  4602		case BPF_PROG_TYPE_EXT:
  4603		case BPF_PROG_TYPE_NETFILTER:
  4604			break;
  4605		case BPF_PROG_TYPE_PERF_EVENT:
  4606		case BPF_PROG_TYPE_TRACEPOINT:
  4607			if (attr->link_create.attach_type != BPF_PERF_EVENT) {
  4608				ret = -EINVAL;
  4609				goto out;
  4610			}
  4611			break;
  4612		case BPF_PROG_TYPE_KPROBE:
  4613			if (attr->link_create.attach_type != BPF_PERF_EVENT &&
  4614			    attr->link_create.attach_type != BPF_TRACE_KPROBE_MULTI) {
  4615				ret = -EINVAL;
  4616				goto out;
  4617			}
  4618			break;
  4619		default:
  4620			ptype = attach_type_to_prog_type(attr->link_create.attach_type);
  4621			if (ptype == BPF_PROG_TYPE_UNSPEC || ptype != prog->type) {
  4622				ret = -EINVAL;
  4623				goto out;
  4624			}
  4625			break;
  4626		}
  4627	
  4628		switch (prog->type) {
  4629		case BPF_PROG_TYPE_CGROUP_SKB:
  4630		case BPF_PROG_TYPE_CGROUP_SOCK:
  4631		case BPF_PROG_TYPE_CGROUP_SOCK_ADDR:
  4632		case BPF_PROG_TYPE_SOCK_OPS:
  4633		case BPF_PROG_TYPE_CGROUP_DEVICE:
  4634		case BPF_PROG_TYPE_CGROUP_SYSCTL:
  4635		case BPF_PROG_TYPE_CGROUP_SOCKOPT:
  4636			ret = cgroup_bpf_link_attach(attr, prog);
  4637			break;
  4638		case BPF_PROG_TYPE_EXT:
  4639			ret = bpf_tracing_prog_attach(prog,
  4640						      attr->link_create.target_fd,
  4641						      attr->link_create.target_btf_id,
  4642						      attr->link_create.tracing.cookie);
  4643			break;
  4644		case BPF_PROG_TYPE_LSM:
  4645		case BPF_PROG_TYPE_TRACING:
  4646			if (attr->link_create.attach_type != prog->expected_attach_type) {
  4647				ret = -EINVAL;
  4648				goto out;
  4649			}
  4650			if (prog->expected_attach_type == BPF_TRACE_RAW_TP)
  4651				ret = bpf_raw_tp_link_attach(prog, NULL);
  4652			else if (prog->expected_attach_type == BPF_TRACE_ITER)
  4653				ret = bpf_iter_link_attach(attr, uattr, prog);
  4654			else if (prog->expected_attach_type == BPF_LSM_CGROUP)
  4655				ret = cgroup_bpf_link_attach(attr, prog);
  4656			else
  4657				ret = bpf_tracing_prog_attach(prog,
  4658							      attr->link_create.target_fd,
  4659							      attr->link_create.target_btf_id,
  4660							      attr->link_create.tracing.cookie);
  4661			break;
  4662		case BPF_PROG_TYPE_FLOW_DISSECTOR:
  4663		case BPF_PROG_TYPE_SK_LOOKUP:
  4664			ret = netns_bpf_link_create(attr, prog);
  4665			break;
  4666	#ifdef CONFIG_NET
  4667		case BPF_PROG_TYPE_XDP:
  4668			ret = bpf_xdp_link_attach(attr, prog);
  4669			break;
  4670		case BPF_PROG_TYPE_NETFILTER:
> 4671			ret = bpf_nf_link_attach(attr, prog);
  4672			break;
  4673	#endif
  4674		case BPF_PROG_TYPE_PERF_EVENT:
  4675		case BPF_PROG_TYPE_TRACEPOINT:
  4676			ret = bpf_perf_link_attach(attr, prog);
  4677			break;
  4678		case BPF_PROG_TYPE_KPROBE:
  4679			if (attr->link_create.attach_type == BPF_PERF_EVENT)
  4680				ret = bpf_perf_link_attach(attr, prog);
  4681			else
  4682				ret = bpf_kprobe_multi_link_attach(attr, prog);
  4683			break;
  4684		default:
  4685			ret = -EINVAL;
  4686		}
  4687	
  4688	out:
  4689		if (ret < 0)
  4690			bpf_prog_put(prog);
  4691		return ret;
  4692	}
  4693	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH bpf-next 6/6] bpf: add test_run support for netfilter program type
  2023-04-05 16:11 ` [PATCH bpf-next 6/6] bpf: add test_run support for netfilter program type Florian Westphal
  2023-04-05 18:22   ` kernel test robot
@ 2023-04-07  1:36   ` Alexei Starovoitov
  2023-04-08 21:38     ` Florian Westphal
  1 sibling, 1 reply; 14+ messages in thread
From: Alexei Starovoitov @ 2023-04-07  1:36 UTC (permalink / raw)
  To: Florian Westphal; +Cc: netdev, netfilter-devel, bpf, dxu, qde

On Wed, Apr 05, 2023 at 06:11:16PM +0200, Florian Westphal wrote:
> also add two simple retval tests: as-is, a return value other
> than accept or drop will cause issues.
> 
> NF_QUEUE could be implemented later IFF we can guarantee that
> attachment of such programs can be rejected if they get attached
> to a pf/hook that doesn't support async reinjection.
> 
> NF_STOLEN could be implemented via trusted helpers that will eventually
> free the skb, else this would leak the skb reference.
> 
> Signed-off-by: Florian Westphal <fw@strlen.de>
> ---
>  include/linux/bpf.h                           |   3 +
>  net/bpf/test_run.c                            | 143 ++++++++++++++++++
>  net/netfilter/nf_bpf_link.c                   |   1 +
>  .../selftests/bpf/verifier/netfilter.c        |  23 +++
>  4 files changed, 170 insertions(+)
>  create mode 100644 tools/testing/selftests/bpf/verifier/netfilter.c
> 
> diff --git a/include/linux/bpf.h b/include/linux/bpf.h
> index 2d8f3f639e68..453cee1efdd3 100644
> --- a/include/linux/bpf.h
> +++ b/include/linux/bpf.h
> @@ -2235,6 +2235,9 @@ int bpf_prog_test_run_raw_tp(struct bpf_prog *prog,
>  int bpf_prog_test_run_sk_lookup(struct bpf_prog *prog,
>  				const union bpf_attr *kattr,
>  				union bpf_attr __user *uattr);
> +int bpf_prog_test_run_nf(struct bpf_prog *prog,
> +			 const union bpf_attr *kattr,
> +			 union bpf_attr __user *uattr);
>  bool btf_ctx_access(int off, int size, enum bpf_access_type type,
>  		    const struct bpf_prog *prog,
>  		    struct bpf_insn_access_aux *info);
> diff --git a/net/bpf/test_run.c b/net/bpf/test_run.c
> index f1652f5fbd2e..c14f577fd987 100644
> --- a/net/bpf/test_run.c
> +++ b/net/bpf/test_run.c
> @@ -19,7 +19,9 @@
>  #include <linux/error-injection.h>
>  #include <linux/smp.h>
>  #include <linux/sock_diag.h>
> +#include <linux/netfilter.h>
>  #include <net/xdp.h>
> +#include <net/netfilter/nf_bpf_link.h>
>  
>  #define CREATE_TRACE_POINTS
>  #include <trace/events/bpf_test_run.h>
> @@ -1690,6 +1692,147 @@ int bpf_prog_test_run_syscall(struct bpf_prog *prog,
>  	return err;
>  }
>  
> +static int verify_and_copy_hook_state(struct nf_hook_state *state,
> +				      const struct nf_hook_state *user,
> +				      struct net_device *dev)
> +{
> +	if (user->in || user->out)
> +		return -EINVAL;
> +
> +	if (user->net || user->sk || user->okfn)
> +		return -EINVAL;
> +
> +	switch (user->pf) {
> +	case NFPROTO_IPV4:
> +	case NFPROTO_IPV6:
> +		switch (state->hook) {
> +		case NF_INET_PRE_ROUTING:
> +			state->in = dev;
> +			break;
> +		case NF_INET_LOCAL_IN:
> +			state->in = dev;
> +			break;
> +		case NF_INET_FORWARD:
> +			state->in = dev;
> +			state->out = dev;
> +			break;
> +		case NF_INET_LOCAL_OUT:
> +			state->out = dev;
> +			break;
> +		case NF_INET_POST_ROUTING:
> +			state->out = dev;
> +			break;
> +		}
> +
> +		break;
> +	default:
> +		return -EINVAL;
> +	}
> +
> +	state->pf = user->pf;
> +	state->hook = user->hook;
> +
> +	return 0;
> +}
> +
> +int bpf_prog_test_run_nf(struct bpf_prog *prog,
> +			 const union bpf_attr *kattr,
> +			 union bpf_attr __user *uattr)
> +{
> +	struct net *net = current->nsproxy->net_ns;
> +	struct net_device *dev = net->loopback_dev;
> +	struct nf_hook_state *user_ctx, hook_state = {
> +		.pf = NFPROTO_IPV4,
> +		.hook = NF_INET_PRE_ROUTING,
> +	};
> +	u32 size = kattr->test.data_size_in;
> +	u32 repeat = kattr->test.repeat;
> +	const struct ethhdr *eth;
> +	struct bpf_nf_ctx ctx = {
> +		.state = &hook_state,
> +	};
> +	struct sk_buff *skb = NULL;
> +	u32 retval, duration;
> +	void *data;
> +	int ret;
> +
> +	if (kattr->test.flags || kattr->test.cpu || kattr->test.batch_size)
> +		return -EINVAL;
> +
> +	if (size < ETH_HLEN + sizeof(struct iphdr))
> +		return -EINVAL;
> +
> +	data = bpf_test_init(kattr, kattr->test.data_size_in, size,
> +			     NET_SKB_PAD + NET_IP_ALIGN,
> +			     SKB_DATA_ALIGN(sizeof(struct skb_shared_info)));
> +	if (IS_ERR(data))
> +		return PTR_ERR(data);
> +
> +	eth = (struct ethhdr *)data;
> +
> +	if (!repeat)
> +		repeat = 1;
> +
> +	user_ctx = bpf_ctx_init(kattr, sizeof(struct nf_hook_state));
> +	if (IS_ERR(user_ctx)) {
> +		kfree(data);
> +		return PTR_ERR(user_ctx);
> +	}
> +
> +	if (user_ctx) {
> +		ret = verify_and_copy_hook_state(&hook_state, user_ctx, dev);
> +		if (ret)
> +			goto out;
> +	}
> +
> +	skb = slab_build_skb(data);
> +	if (!skb) {
> +		ret = -ENOMEM;
> +		goto out;
> +	}
> +
> +	data = NULL; /* data released via kfree_skb */
> +
> +	skb_reserve(skb, NET_SKB_PAD + NET_IP_ALIGN);
> +	__skb_put(skb, size);
> +
> +	skb->protocol = eth_type_trans(skb, dev);
> +
> +	skb_reset_network_header(skb);
> +
> +	ret = -EINVAL;
> +
> +	switch (skb->protocol) {
> +	case htons(ETH_P_IP):
> +		if (hook_state.pf == NFPROTO_IPV4)
> +			break;
> +		goto out;
> +	case htons(ETH_P_IPV6):
> +		if (size < ETH_HLEN + sizeof(struct ipv6hdr))
> +			goto out;
> +		if (hook_state.pf == NFPROTO_IPV6)
> +			break;
> +		goto out;
> +	default:
> +		ret = -EPROTO;
> +		goto out;
> +	}
> +
> +	ctx.skb = skb;
> +
> +	ret = bpf_test_run(prog, &ctx, repeat, &retval, &duration, false);
> +	if (ret)
> +		goto out;
> +
> +	ret = bpf_test_finish(kattr, uattr, NULL, NULL, 0, retval, duration);
> +
> +out:
> +	kfree(user_ctx);
> +	kfree_skb(skb);
> +	kfree(data);
> +	return ret;
> +}
> +
>  static const struct btf_kfunc_id_set bpf_prog_test_kfunc_set = {
>  	.owner = THIS_MODULE,
>  	.set   = &test_sk_check_kfunc_ids,
> diff --git a/net/netfilter/nf_bpf_link.c b/net/netfilter/nf_bpf_link.c
> index 4b22a31d6df5..c27fd569adf1 100644
> --- a/net/netfilter/nf_bpf_link.c
> +++ b/net/netfilter/nf_bpf_link.c
> @@ -128,6 +128,7 @@ int bpf_nf_link_attach(const union bpf_attr *attr, struct bpf_prog *prog)
>  }
>  
>  const struct bpf_prog_ops netfilter_prog_ops = {
> +	.test_run = bpf_prog_test_run_nf,
>  };
>  
>  static bool nf_ptr_to_btf_id(struct bpf_insn_access_aux *info, const char *name)
> diff --git a/tools/testing/selftests/bpf/verifier/netfilter.c b/tools/testing/selftests/bpf/verifier/netfilter.c
> new file mode 100644
> index 000000000000..deeb87afdf50
> --- /dev/null
> +++ b/tools/testing/selftests/bpf/verifier/netfilter.c
> @@ -0,0 +1,23 @@
> +{
> +	"netfilter, accept all",
> +	.insns = {
> +	BPF_MOV64_IMM(BPF_REG_0, 1),
> +	BPF_EXIT_INSN(),
> +	},
> +	.result = ACCEPT,
> +	.prog_type = BPF_PROG_TYPE_NETFILTER,
> +	.retval = 1,
> +	.data = {
> +		0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0x08, 0x00,
> +	},
> +},
> +{
> +	"netfilter, stolen verdict",
> +	.insns = {
> +	BPF_MOV64_IMM(BPF_REG_0, 2),
> +	BPF_EXIT_INSN(),
> +	},
> +	.result = REJECT,
> +	.errstr = "At program exit the register R0 has value (0x2; 0x0) should have been in (0x0; 0x1)",
> +	.prog_type = BPF_PROG_TYPE_NETFILTER,

We're adding all new asm tests to test_progs now instead of test_verifier. See progs/verifier_*.c.

Other than this nit and build bot complains it looks good to me.

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH bpf-next 6/6] bpf: add test_run support for netfilter program type
  2023-04-07  1:36   ` Alexei Starovoitov
@ 2023-04-08 21:38     ` Florian Westphal
  0 siblings, 0 replies; 14+ messages in thread
From: Florian Westphal @ 2023-04-08 21:38 UTC (permalink / raw)
  To: Alexei Starovoitov
  Cc: Florian Westphal, netdev, netfilter-devel, bpf, dxu, qde

Alexei Starovoitov <alexei.starovoitov@gmail.com> wrote:
> >  static bool nf_ptr_to_btf_id(struct bpf_insn_access_aux *info, const char *name)
> > diff --git a/tools/testing/selftests/bpf/verifier/netfilter.c b/tools/testing/selftests/bpf/verifier/netfilter.c
> > new file mode 100644
> > index 000000000000..deeb87afdf50
> > --- /dev/null
> > +++ b/tools/testing/selftests/bpf/verifier/netfilter.c
> > @@ -0,0 +1,23 @@
> > +{
> > +	"netfilter, accept all",

[..]

> We're adding all new asm tests to test_progs now instead of test_verifier. See progs/verifier_*.c.

Thanks for the pointer, I'll have a look at this for v2.

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH bpf-next 0/6] bpf: add netfilter program type
  2023-04-05 16:11 [PATCH bpf-next 0/6] bpf: add netfilter program type Florian Westphal
                   ` (5 preceding siblings ...)
  2023-04-05 16:11 ` [PATCH bpf-next 6/6] bpf: add test_run support for netfilter program type Florian Westphal
@ 2023-04-12  8:20 ` Quentin Deslandes
  2023-04-12  9:45   ` Florian Westphal
  6 siblings, 1 reply; 14+ messages in thread
From: Quentin Deslandes @ 2023-04-12  8:20 UTC (permalink / raw)
  To: Florian Westphal, netdev; +Cc: netfilter-devel, bpf, dxu

On 05/04/2023 18:11, Florian Westphal wrote:
> Add minimal support to hook bpf programs to netfilter hooks, e.g.
> PREROUTING or FORWARD.
> 
> For this the most relevant parts for registering a netfilter
> hook via the in-kernel api are exposed to userspace via bpf_link.
> 
> The new program type is 'tracing style', i.e. there is no context
> access rewrite done by verifier, the function argument (struct bpf_nf_ctx)
> isn't stable.
> There is no support for direct packet access, dynptr api should be used
> instead.

Does this mean the verifier will reject any program accessing ctx->skb
(e.g. ctx->skb + X)?

> With this its possible to build a small test program such as:
> 
>  #include "vmlinux.h"
> extern int bpf_dynptr_from_skb(struct __sk_buff *skb, __u64 flags,
>                                struct bpf_dynptr *ptr__uninit) __ksym;
> extern void *bpf_dynptr_slice(const struct bpf_dynptr *ptr, uint32_t offset,
>                                    void *buffer, uint32_t buffer__sz) __ksym;
> SEC("netfilter")
> int nf_test(struct bpf_nf_ctx *ctx)
> {
> 	struct nf_hook_state *state = ctx->state;
> 	struct sk_buff *skb = ctx->skb;
> 	const struct iphdr *iph, _iph;
> 	const struct tcphdr *th, _th;
> 	struct bpf_dynptr ptr;
> 
> 	if (bpf_dynptr_from_skb(skb, 0, &ptr))
> 		return NF_DROP;
> 
> 	iph = bpf_dynptr_slice(&ptr, 0, &_iph, sizeof(_iph));
> 	if (!iph)
> 		return NF_DROP;
> 
> 	th = bpf_dynptr_slice(&ptr, iph->ihl << 2, &_th, sizeof(_th));
> 	if (!th)
> 		return NF_DROP;
> 
> 	bpf_printk("accept %x:%d->%x:%d, hook %d ifin %d\n", iph->saddr, bpf_ntohs(th->source), iph->daddr, bpf_ntohs(th->dest), state->hook, state->in->ifindex);
>         return NF_ACCEPT;
> }
> 
> Then, tail /sys/kernel/tracing/trace_pipe.
> 
> Changes since last RFC version:
> 1. extend 'bpftool link show' to print prio/hooknum etc
> 2. extend 'nft list hooks' so it can print the bpf program id
> 3. Add an extra patch to artificially restrict bpf progs with
>    same priority.  Its fine from a technical pov but it will
>    cause ordering issues (most recent one comes first).
>    Can be removed later.
> 4. Add test_run support for netfilter prog type and a small
>    extension to verifier tests to make sure we can't return
>    verdicts like NF_STOLEN.
> 5. Alter the netfilter part of the bpf_link uapi struct:
>    - add flags/reserved members.
>   Not used here except returning errors when they are nonzero.
>   Plan is to allow the bpf_link users to enable netfilter
>   defrag or conntrack engine by setting feature flags at
>   link create time in the future.
> 
> Let me know if there is anything missing that has to be addressed
> before this can be merged.
> 
> Thanks!
> 
> Florian Westphal (6):
>   bpf: add bpf_link support for BPF_NETFILTER programs
>   bpf: minimal support for programs hooked into netfilter framework
>   netfilter: nfnetlink hook: dump bpf prog id
>   netfilter: disallow bpf hook attachment at same priority
>   tools: bpftool: print netfilter link info
>   bpf: add test_run support for netfilter program type
> 
>  include/linux/bpf.h                           |   3 +
>  include/linux/bpf_types.h                     |   4 +
>  include/linux/netfilter.h                     |   1 +
>  include/net/netfilter/nf_bpf_link.h           |   8 +
>  include/uapi/linux/bpf.h                      |  15 ++
>  include/uapi/linux/netfilter/nfnetlink_hook.h |  20 +-
>  kernel/bpf/btf.c                              |   6 +
>  kernel/bpf/syscall.c                          |   6 +
>  kernel/bpf/verifier.c                         |   3 +
>  net/bpf/test_run.c                            | 143 +++++++++++++
>  net/core/filter.c                             |   1 +
>  net/netfilter/Kconfig                         |   3 +
>  net/netfilter/Makefile                        |   1 +
>  net/netfilter/core.c                          |  12 ++
>  net/netfilter/nf_bpf_link.c                   | 190 ++++++++++++++++++
>  net/netfilter/nfnetlink_hook.c                |  81 ++++++--
>  tools/bpf/bpftool/link.c                      |  24 +++
>  tools/include/uapi/linux/bpf.h                |  15 ++
>  tools/lib/bpf/libbpf.c                        |   1 +
>  .../selftests/bpf/verifier/netfilter.c        |  23 +++
>  20 files changed, 546 insertions(+), 14 deletions(-)
>  create mode 100644 include/net/netfilter/nf_bpf_link.h
>  create mode 100644 net/netfilter/nf_bpf_link.c
>  create mode 100644 tools/testing/selftests/bpf/verifier/netfilter.c
> 


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH bpf-next 0/6] bpf: add netfilter program type
  2023-04-12  8:20 ` [PATCH bpf-next 0/6] bpf: add " Quentin Deslandes
@ 2023-04-12  9:45   ` Florian Westphal
  2023-04-13  9:26     ` Quentin Deslandes
  0 siblings, 1 reply; 14+ messages in thread
From: Florian Westphal @ 2023-04-12  9:45 UTC (permalink / raw)
  To: Quentin Deslandes; +Cc: Florian Westphal, netdev, netfilter-devel, bpf, dxu

Quentin Deslandes <qde@naccy.de> wrote:
> On 05/04/2023 18:11, Florian Westphal wrote:
> > Add minimal support to hook bpf programs to netfilter hooks, e.g.
> > PREROUTING or FORWARD.
> > 
> > For this the most relevant parts for registering a netfilter
> > hook via the in-kernel api are exposed to userspace via bpf_link.
> > 
> > The new program type is 'tracing style', i.e. there is no context
> > access rewrite done by verifier, the function argument (struct bpf_nf_ctx)
> > isn't stable.
> > There is no support for direct packet access, dynptr api should be used
> > instead.
> 
> Does this mean the verifier will reject any program accessing ctx->skb
> (e.g. ctx->skb + X)?

Do you mean access to ctx->skb->data + X?  If so, yes, that won't work.

Otherwise, then no, it just means that programs might have to be recompiled
if they lack needed relocation information, but only if bpf_nf_ctx structure is
changed.

Initial version used "__sk_buff *skb", like e.g. clsact.  I was told
to not do that and expose the real kernel-side structure instead and to
not bother with direct packet access (skb->data access) support.

> >  #include "vmlinux.h"
> > extern int bpf_dynptr_from_skb(struct __sk_buff *skb, __u64 flags,
> >                                struct bpf_dynptr *ptr__uninit) __ksym;
> > extern void *bpf_dynptr_slice(const struct bpf_dynptr *ptr, uint32_t offset,
> >                                    void *buffer, uint32_t buffer__sz) __ksym;
> > SEC("netfilter")
> > int nf_test(struct bpf_nf_ctx *ctx)
> > {
> > 	struct nf_hook_state *state = ctx->state;
> > 	struct sk_buff *skb = ctx->skb;

ctx->skb is dereferenced...

> > 	if (bpf_dynptr_from_skb(skb, 0, &ptr))
> > 		return NF_DROP;

... dynptr is created ...

> > 	iph = bpf_dynptr_slice(&ptr, 0, &_iph, sizeof(_iph));
> > 	if (!iph)
> > 		return NF_DROP;
> > 	th = bpf_dynptr_slice(&ptr, iph->ihl << 2, &_th, sizeof(_th));

ip header access.

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH bpf-next 0/6] bpf: add netfilter program type
  2023-04-12  9:45   ` Florian Westphal
@ 2023-04-13  9:26     ` Quentin Deslandes
  0 siblings, 0 replies; 14+ messages in thread
From: Quentin Deslandes @ 2023-04-13  9:26 UTC (permalink / raw)
  To: Florian Westphal; +Cc: netdev, netfilter-devel, bpf, dxu

On 12/04/2023 11:45, Florian Westphal wrote:
> Quentin Deslandes <qde@naccy.de> wrote:
>> On 05/04/2023 18:11, Florian Westphal wrote:
>>> Add minimal support to hook bpf programs to netfilter hooks, e.g.
>>> PREROUTING or FORWARD.
>>>
>>> For this the most relevant parts for registering a netfilter
>>> hook via the in-kernel api are exposed to userspace via bpf_link.
>>>
>>> The new program type is 'tracing style', i.e. there is no context
>>> access rewrite done by verifier, the function argument (struct bpf_nf_ctx)
>>> isn't stable.
>>> There is no support for direct packet access, dynptr api should be used
>>> instead.
>>
>> Does this mean the verifier will reject any program accessing ctx->skb
>> (e.g. ctx->skb + X)?
> 
> Do you mean access to ctx->skb->data + X?  If so, yes, that won't work.
> 
> Otherwise, then no, it just means that programs might have to be recompiled
> if they lack needed relocation information, but only if bpf_nf_ctx structure is
> changed.
> 
> Initial version used "__sk_buff *skb", like e.g. clsact.  I was told
> to not do that and expose the real kernel-side structure instead and to
> not bother with direct packet access (skb->data access) support.
> 

That's exactly what I meant, thanks.

>>>  #include "vmlinux.h"
>>> extern int bpf_dynptr_from_skb(struct __sk_buff *skb, __u64 flags,
>>>                                struct bpf_dynptr *ptr__uninit) __ksym;
>>> extern void *bpf_dynptr_slice(const struct bpf_dynptr *ptr, uint32_t offset,
>>>                                    void *buffer, uint32_t buffer__sz) __ksym;
>>> SEC("netfilter")
>>> int nf_test(struct bpf_nf_ctx *ctx)
>>> {
>>> 	struct nf_hook_state *state = ctx->state;
>>> 	struct sk_buff *skb = ctx->skb;
> 
> ctx->skb is dereferenced...
> 
>>> 	if (bpf_dynptr_from_skb(skb, 0, &ptr))
>>> 		return NF_DROP;
> 
> ... dynptr is created ...
> 
>>> 	iph = bpf_dynptr_slice(&ptr, 0, &_iph, sizeof(_iph));
>>> 	if (!iph)
>>> 		return NF_DROP;
>>> 	th = bpf_dynptr_slice(&ptr, iph->ihl << 2, &_th, sizeof(_th));
> 
> ip header access.


^ permalink raw reply	[flat|nested] 14+ messages in thread

end of thread, other threads:[~2023-04-13  9:41 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-04-05 16:11 [PATCH bpf-next 0/6] bpf: add netfilter program type Florian Westphal
2023-04-05 16:11 ` [PATCH bpf-next 1/6] bpf: add bpf_link support for BPF_NETFILTER programs Florian Westphal
2023-04-06  4:50   ` kernel test robot
2023-04-05 16:11 ` [PATCH bpf-next 2/6] bpf: minimal support for programs hooked into netfilter framework Florian Westphal
2023-04-05 16:11 ` [PATCH bpf-next 3/6] netfilter: nfnetlink hook: dump bpf prog id Florian Westphal
2023-04-05 16:11 ` [PATCH bpf-next 4/6] netfilter: disallow bpf hook attachment at same priority Florian Westphal
2023-04-05 16:11 ` [PATCH bpf-next 5/6] tools: bpftool: print netfilter link info Florian Westphal
2023-04-05 16:11 ` [PATCH bpf-next 6/6] bpf: add test_run support for netfilter program type Florian Westphal
2023-04-05 18:22   ` kernel test robot
2023-04-07  1:36   ` Alexei Starovoitov
2023-04-08 21:38     ` Florian Westphal
2023-04-12  8:20 ` [PATCH bpf-next 0/6] bpf: add " Quentin Deslandes
2023-04-12  9:45   ` Florian Westphal
2023-04-13  9:26     ` Quentin Deslandes

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).