[PATCH bpf-next 0/2] libbpf: add netfilter link attach helper

[PATCH bpf-next 0/2] libbpf: add netfilter link attach helper

[Florian Westphal]

When initial netfilter bpf program type support got added one
suggestion was to extend libbpf with a helper to ease attachment
of nf programs to the hook locations.

Add such a helper and a demo test case that attaches a dummy
program to various combinations.

I tested that the selftest fails when changing the expected
outcome (i.e., set 'success' when it should fail and v.v.).

Florian Westphal (2):
  tools: libbpf: add netfilter link attach helper
  selftests/bpf: Add bpf_program__attach_netfilter_opts helper test

 tools/lib/bpf/libbpf.c                        | 51 +++++++++++
 tools/lib/bpf/libbpf.h                        | 15 ++++
 tools/lib/bpf/libbpf.map                      |  1 +
 .../bpf/prog_tests/netfilter_basic.c          | 87 +++++++++++++++++++
 .../bpf/progs/test_netfilter_link_attach.c    | 14 +++
 5 files changed, 168 insertions(+)
 create mode 100644 tools/testing/selftests/bpf/prog_tests/netfilter_basic.c
 create mode 100644 tools/testing/selftests/bpf/progs/test_netfilter_link_attach.c

-- 
2.39.3

[PATCH bpf-next 1/2] tools: libbpf: add netfilter link attach helper

[Florian Westphal]

Add new api function: bpf_program__attach_netfilter_opts.

It takes a bpf program (netfilter type), and a pointer to a option struct
that contains the desired attachment (protocol family, priority, hook
location, ...).

It returns a pointer to a 'bpf_link' structure or NULL on error.

Next patch adds new netfilter_basic test that uses this function to
attach a program to a few pf/hook/priority combinations.

Suggested-by: Andrii Nakryiko <andrii.nakryiko@gmail.com>
Signed-off-by: Florian Westphal <fw@strlen.de>
---
 tools/lib/bpf/libbpf.c   | 51 ++++++++++++++++++++++++++++++++++++++++
 tools/lib/bpf/libbpf.h   | 15 ++++++++++++
 tools/lib/bpf/libbpf.map |  1 +
 3 files changed, 67 insertions(+)

diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
index 5cca00979aae..033447aa0773 100644
--- a/tools/lib/bpf/libbpf.c
+++ b/tools/lib/bpf/libbpf.c
@@ -11811,6 +11811,57 @@ static int attach_iter(const struct bpf_program *prog, long cookie, struct bpf_l
 	return libbpf_get_error(*link);
 }
 
+struct bpf_link *bpf_program__attach_netfilter_opts(const struct bpf_program *prog,
+						    const struct bpf_netfilter_opts *opts)
+{
+	const size_t attr_sz = offsetofend(union bpf_attr, link_create);
+	struct bpf_link *link;
+	int prog_fd, link_fd;
+	union bpf_attr attr;
+
+	if (!OPTS_VALID(opts, bpf_netfilter_opts))
+		return libbpf_err_ptr(-EINVAL);
+
+	prog_fd = bpf_program__fd(prog);
+	if (prog_fd < 0) {
+		pr_warn("prog '%s': can't attach before loaded\n", prog->name);
+		return libbpf_err_ptr(-EINVAL);
+	}
+
+	link = calloc(1, sizeof(*link));
+	if (!link)
+		return libbpf_err_ptr(-ENOMEM);
+	link->detach = &bpf_link__detach_fd;
+
+	memset(&attr, 0, attr_sz);
+
+	attr.link_create.prog_fd = prog_fd;
+	attr.link_create.netfilter.pf = OPTS_GET(opts, pf, 0);
+	attr.link_create.netfilter.hooknum = OPTS_GET(opts, hooknum, 0);
+	attr.link_create.netfilter.priority = OPTS_GET(opts, priority, 0);
+	attr.link_create.netfilter.flags = OPTS_GET(opts, flags, 0);
+
+	link_fd = syscall(__NR_bpf, BPF_LINK_CREATE, &attr, attr_sz);
+
+	link->fd = ensure_good_fd(link_fd);
+
+	if (link->fd < 0) {
+		char errmsg[STRERR_BUFSIZE];
+
+		link_fd = -errno;
+		free(link);
+		pr_warn("prog '%s': failed to attach to pf:%d,hooknum:%d:prio:%d: %s\n",
+			prog->name,
+			OPTS_GET(opts, pf, 0),
+			OPTS_GET(opts, hooknum, 0),
+			OPTS_GET(opts, priority, 0),
+			libbpf_strerror_r(link_fd, errmsg, sizeof(errmsg)));
+		return libbpf_err_ptr(link_fd);
+	}
+
+	return link;
+}
+
 struct bpf_link *bpf_program__attach(const struct bpf_program *prog)
 {
 	struct bpf_link *link = NULL;
diff --git a/tools/lib/bpf/libbpf.h b/tools/lib/bpf/libbpf.h
index 754da73c643b..081beb95a097 100644
--- a/tools/lib/bpf/libbpf.h
+++ b/tools/lib/bpf/libbpf.h
@@ -718,6 +718,21 @@ LIBBPF_API struct bpf_link *
 bpf_program__attach_freplace(const struct bpf_program *prog,
 			     int target_fd, const char *attach_func_name);
 
+struct bpf_netfilter_opts {
+	/* size of this struct, for forward/backward compatibility */
+	size_t sz;
+
+	__u32 pf;
+	__u32 hooknum;
+	__s32 priority;
+	__u32 flags;
+};
+#define bpf_netfilter_opts__last_field flags
+
+LIBBPF_API struct bpf_link *
+bpf_program__attach_netfilter_opts(const struct bpf_program *prog,
+				   const struct bpf_netfilter_opts *opts);
+
 struct bpf_map;
 
 LIBBPF_API struct bpf_link *bpf_map__attach_struct_ops(const struct bpf_map *map);
diff --git a/tools/lib/bpf/libbpf.map b/tools/lib/bpf/libbpf.map
index 7521a2fb7626..e13d60608bf3 100644
--- a/tools/lib/bpf/libbpf.map
+++ b/tools/lib/bpf/libbpf.map
@@ -395,4 +395,5 @@ LIBBPF_1.2.0 {
 LIBBPF_1.3.0 {
 	global:
 		bpf_obj_pin_opts;
+		bpf_program__attach_netfilter_opts;
 } LIBBPF_1.2.0;
-- 
2.39.3

[PATCH bpf-next 2/2] selftests/bpf: Add bpf_program__attach_netfilter_opts helper test

[Florian Westphal]

Call bpf_program__attach_netfilter_opts() with different
protocol/hook/priority combinations.

Test fails if supposedly-illegal attachments work
(e.g., bogus protocol family, illegal priority and so on)
or if a should-work attachment fails.

Signed-off-by: Florian Westphal <fw@strlen.de>
---
 .../bpf/prog_tests/netfilter_basic.c          | 87 +++++++++++++++++++
 .../bpf/progs/test_netfilter_link_attach.c    | 14 +++
 2 files changed, 101 insertions(+)
 create mode 100644 tools/testing/selftests/bpf/prog_tests/netfilter_basic.c
 create mode 100644 tools/testing/selftests/bpf/progs/test_netfilter_link_attach.c

diff --git a/tools/testing/selftests/bpf/prog_tests/netfilter_basic.c b/tools/testing/selftests/bpf/prog_tests/netfilter_basic.c
new file mode 100644
index 000000000000..a64b5feaaca4
--- /dev/null
+++ b/tools/testing/selftests/bpf/prog_tests/netfilter_basic.c
@@ -0,0 +1,87 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+
+#include <netinet/in.h>
+#include <linux/netfilter.h>
+
+#include "test_progs.h"
+#include "test_netfilter_link_attach.skel.h"
+
+struct nf_hook_options {
+	__u32 pf;
+	__u32 hooknum;
+	__s32 priority;
+	__u32 flags;
+
+	bool expect_success;
+};
+
+struct nf_hook_options nf_hook_attach_tests[] = {
+	{  },
+	{ .pf = NFPROTO_NUMPROTO, },
+	{ .pf = NFPROTO_IPV4, .hooknum = 42, },
+	{ .pf = NFPROTO_IPV4, .priority = INT_MIN },
+	{ .pf = NFPROTO_IPV4, .priority = INT_MAX },
+	{ .pf = NFPROTO_IPV4, .flags = UINT_MAX },
+
+	{ .pf = NFPROTO_INET, .priority = 1, },
+
+	{ .pf = NFPROTO_IPV4, .priority = -10000, .expect_success = true },
+	{ .pf = NFPROTO_IPV6, .priority = 10001, .expect_success = true },
+};
+
+static void __test_netfilter_link_attach(struct bpf_program *prog)
+{
+	LIBBPF_OPTS(bpf_netfilter_opts, opts);
+	int i;
+
+	for (i = 0; i < ARRAY_SIZE(nf_hook_attach_tests); i++) {
+		struct bpf_link *link;
+
+#define X(opts, m, i)	opts.m = nf_hook_attach_tests[(i)].m
+		X(opts, pf, i);
+		X(opts, hooknum, i);
+		X(opts, priority, i);
+		X(opts, flags, i);
+#undef X
+		link = bpf_program__attach_netfilter_opts(prog, &opts);
+		if (nf_hook_attach_tests[i].expect_success) {
+			struct bpf_link *link2;
+
+			if (!ASSERT_OK_PTR(link, "program attach successful"))
+				continue;
+
+			link2 = bpf_program__attach_netfilter_opts(prog, &opts);
+			ASSERT_NULL(link2, "attach program with same pf/hook/priority");
+
+			if (!ASSERT_EQ(bpf_link__destroy(link), 0, "link destroy"))
+				break;
+
+			link2 = bpf_program__attach_netfilter_opts(prog, &opts);
+			if (!ASSERT_OK_PTR(link2, "program reattach successful"))
+				continue;
+			if (!ASSERT_EQ(bpf_link__destroy(link2), 0, "link destroy"))
+				break;
+		} else {
+			ASSERT_NULL(link, "program load failure");
+		}
+	}
+}
+
+static void test_netfilter_link_attach(void)
+{
+	struct test_netfilter_link_attach *skel;
+
+	skel = test_netfilter_link_attach__open_and_load();
+	if (!ASSERT_OK_PTR(skel, "test_netfilter_link_attach__open_and_load"))
+		goto out;
+
+	__test_netfilter_link_attach(skel->progs.nf_link_attach_test);
+out:
+	test_netfilter_link_attach__destroy(skel);
+}
+
+void test_netfilter_basic(void)
+{
+	if (test__start_subtest("netfilter link attach"))
+		test_netfilter_link_attach();
+}
diff --git a/tools/testing/selftests/bpf/progs/test_netfilter_link_attach.c b/tools/testing/selftests/bpf/progs/test_netfilter_link_attach.c
new file mode 100644
index 000000000000..03a475160abe
--- /dev/null
+++ b/tools/testing/selftests/bpf/progs/test_netfilter_link_attach.c
@@ -0,0 +1,14 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+
+#include "vmlinux.h"
+#include <bpf/bpf_helpers.h>
+
+#define NF_ACCEPT 1
+
+SEC("netfilter")
+int nf_link_attach_test(struct bpf_nf_ctx *ctx)
+{
+	return NF_ACCEPT;
+}
+
+char _license[] SEC("license") = "GPL";
-- 
2.39.3

Re: [PATCH bpf-next 1/2] tools: libbpf: add netfilter link attach helper

[Andrii Nakryiko]

On Thu, May 25, 2023 at 4:01 AM Florian Westphal <fw@strlen.de> wrote:
>
> Add new api function: bpf_program__attach_netfilter_opts.
>
> It takes a bpf program (netfilter type), and a pointer to a option struct
> that contains the desired attachment (protocol family, priority, hook
> location, ...).
>
> It returns a pointer to a 'bpf_link' structure or NULL on error.
>
> Next patch adds new netfilter_basic test that uses this function to
> attach a program to a few pf/hook/priority combinations.
>
> Suggested-by: Andrii Nakryiko <andrii.nakryiko@gmail.com>
> Signed-off-by: Florian Westphal <fw@strlen.de>
> ---
>  tools/lib/bpf/libbpf.c   | 51 ++++++++++++++++++++++++++++++++++++++++
>  tools/lib/bpf/libbpf.h   | 15 ++++++++++++
>  tools/lib/bpf/libbpf.map |  1 +
>  3 files changed, 67 insertions(+)
>
> diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
> index 5cca00979aae..033447aa0773 100644
> --- a/tools/lib/bpf/libbpf.c
> +++ b/tools/lib/bpf/libbpf.c
> @@ -11811,6 +11811,57 @@ static int attach_iter(const struct bpf_program *prog, long cookie, struct bpf_l
>         return libbpf_get_error(*link);
>  }
>
> +struct bpf_link *bpf_program__attach_netfilter_opts(const struct bpf_program *prog,
> +                                                   const struct bpf_netfilter_opts *opts)

let's just call it `bpf_program__attach_netfilter`. We add "_opts" if
we had variant without opts. This doesn't apply here, so a shorter
name is preferable.

> +{
> +       const size_t attr_sz = offsetofend(union bpf_attr, link_create);
> +       struct bpf_link *link;
> +       int prog_fd, link_fd;
> +       union bpf_attr attr;
> +
> +       if (!OPTS_VALID(opts, bpf_netfilter_opts))
> +               return libbpf_err_ptr(-EINVAL);
> +
> +       prog_fd = bpf_program__fd(prog);
> +       if (prog_fd < 0) {
> +               pr_warn("prog '%s': can't attach before loaded\n", prog->name);
> +               return libbpf_err_ptr(-EINVAL);
> +       }
> +
> +       link = calloc(1, sizeof(*link));
> +       if (!link)
> +               return libbpf_err_ptr(-ENOMEM);
> +       link->detach = &bpf_link__detach_fd;
> +
> +       memset(&attr, 0, attr_sz);
> +
> +       attr.link_create.prog_fd = prog_fd;
> +       attr.link_create.netfilter.pf = OPTS_GET(opts, pf, 0);
> +       attr.link_create.netfilter.hooknum = OPTS_GET(opts, hooknum, 0);
> +       attr.link_create.netfilter.priority = OPTS_GET(opts, priority, 0);
> +       attr.link_create.netfilter.flags = OPTS_GET(opts, flags, 0);
> +
> +       link_fd = syscall(__NR_bpf, BPF_LINK_CREATE, &attr, attr_sz);

this code shouldn't do direct syscall, these high-level APIs should go
through libbpf low-level API. In this case, you need to call
bpf_link_create().

Except bpf_link_create() doesn't really support NETLINK links yet,
which is what we'll need to fix first. bpf_link_create() determines
what kind of parameters to pass to kernel based on bpf_attach_type.
And we currently don't have an attach type for NETLINK BPF link.
Thankfully it's not too late to add it. I see that link_create() in
kernel/bpf/syscall.c just bypasses attach_type check. We shouldn't
have done that. Instead we need to add BPF_NETLINK attach type to enum
bpf_attach_type. And wire all that properly throughout the kernel and
libbpf itself. Thankfully kernel release is not finalized and we can
still fix that up, but please prioritize it before we get too far into
rc releases.

> +
> +       link->fd = ensure_good_fd(link_fd);
> +
> +       if (link->fd < 0) {
> +               char errmsg[STRERR_BUFSIZE];
> +
> +               link_fd = -errno;
> +               free(link);
> +               pr_warn("prog '%s': failed to attach to pf:%d,hooknum:%d:prio:%d: %s\n",
> +                       prog->name,
> +                       OPTS_GET(opts, pf, 0),
> +                       OPTS_GET(opts, hooknum, 0),
> +                       OPTS_GET(opts, priority, 0),
> +                       libbpf_strerror_r(link_fd, errmsg, sizeof(errmsg)));
> +               return libbpf_err_ptr(link_fd);
> +       }
> +
> +       return link;
> +}
> +
>  struct bpf_link *bpf_program__attach(const struct bpf_program *prog)
>  {
>         struct bpf_link *link = NULL;
> diff --git a/tools/lib/bpf/libbpf.h b/tools/lib/bpf/libbpf.h
> index 754da73c643b..081beb95a097 100644
> --- a/tools/lib/bpf/libbpf.h
> +++ b/tools/lib/bpf/libbpf.h
> @@ -718,6 +718,21 @@ LIBBPF_API struct bpf_link *
>  bpf_program__attach_freplace(const struct bpf_program *prog,
>                              int target_fd, const char *attach_func_name);
>
> +struct bpf_netfilter_opts {
> +       /* size of this struct, for forward/backward compatibility */
> +       size_t sz;
> +
> +       __u32 pf;
> +       __u32 hooknum;
> +       __s32 priority;
> +       __u32 flags;
> +};
> +#define bpf_netfilter_opts__last_field flags
> +
> +LIBBPF_API struct bpf_link *
> +bpf_program__attach_netfilter_opts(const struct bpf_program *prog,
> +                                  const struct bpf_netfilter_opts *opts);
> +
>  struct bpf_map;
>
>  LIBBPF_API struct bpf_link *bpf_map__attach_struct_ops(const struct bpf_map *map);
> diff --git a/tools/lib/bpf/libbpf.map b/tools/lib/bpf/libbpf.map
> index 7521a2fb7626..e13d60608bf3 100644
> --- a/tools/lib/bpf/libbpf.map
> +++ b/tools/lib/bpf/libbpf.map
> @@ -395,4 +395,5 @@ LIBBPF_1.2.0 {
>  LIBBPF_1.3.0 {
>         global:
>                 bpf_obj_pin_opts;
> +               bpf_program__attach_netfilter_opts;

opts and the rest looks good, thanks

>  } LIBBPF_1.2.0;
> --
> 2.39.3
>

Re: [PATCH bpf-next 2/2] selftests/bpf: Add bpf_program__attach_netfilter_opts helper test

[Andrii Nakryiko]

On Thu, May 25, 2023 at 4:01 AM Florian Westphal <fw@strlen.de> wrote:
>
> Call bpf_program__attach_netfilter_opts() with different
> protocol/hook/priority combinations.
>
> Test fails if supposedly-illegal attachments work
> (e.g., bogus protocol family, illegal priority and so on)
> or if a should-work attachment fails.
>
> Signed-off-by: Florian Westphal <fw@strlen.de>
> ---
>  .../bpf/prog_tests/netfilter_basic.c          | 87 +++++++++++++++++++
>  .../bpf/progs/test_netfilter_link_attach.c    | 14 +++
>  2 files changed, 101 insertions(+)
>  create mode 100644 tools/testing/selftests/bpf/prog_tests/netfilter_basic.c
>  create mode 100644 tools/testing/selftests/bpf/progs/test_netfilter_link_attach.c
>
> diff --git a/tools/testing/selftests/bpf/prog_tests/netfilter_basic.c b/tools/testing/selftests/bpf/prog_tests/netfilter_basic.c
> new file mode 100644
> index 000000000000..a64b5feaaca4
> --- /dev/null
> +++ b/tools/testing/selftests/bpf/prog_tests/netfilter_basic.c
> @@ -0,0 +1,87 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +
> +#include <netinet/in.h>
> +#include <linux/netfilter.h>
> +
> +#include "test_progs.h"
> +#include "test_netfilter_link_attach.skel.h"
> +
> +struct nf_hook_options {
> +       __u32 pf;
> +       __u32 hooknum;
> +       __s32 priority;
> +       __u32 flags;
> +
> +       bool expect_success;
> +};
> +
> +struct nf_hook_options nf_hook_attach_tests[] = {
> +       {  },
> +       { .pf = NFPROTO_NUMPROTO, },
> +       { .pf = NFPROTO_IPV4, .hooknum = 42, },
> +       { .pf = NFPROTO_IPV4, .priority = INT_MIN },
> +       { .pf = NFPROTO_IPV4, .priority = INT_MAX },
> +       { .pf = NFPROTO_IPV4, .flags = UINT_MAX },
> +
> +       { .pf = NFPROTO_INET, .priority = 1, },
> +
> +       { .pf = NFPROTO_IPV4, .priority = -10000, .expect_success = true },
> +       { .pf = NFPROTO_IPV6, .priority = 10001, .expect_success = true },
> +};
> +
> +static void __test_netfilter_link_attach(struct bpf_program *prog)
> +{
> +       LIBBPF_OPTS(bpf_netfilter_opts, opts);
> +       int i;
> +
> +       for (i = 0; i < ARRAY_SIZE(nf_hook_attach_tests); i++) {
> +               struct bpf_link *link;
> +
> +#define X(opts, m, i)  opts.m = nf_hook_attach_tests[(i)].m
> +               X(opts, pf, i);
> +               X(opts, hooknum, i);
> +               X(opts, priority, i);
> +               X(opts, flags, i);
> +#undef X
> +               link = bpf_program__attach_netfilter_opts(prog, &opts);
> +               if (nf_hook_attach_tests[i].expect_success) {
> +                       struct bpf_link *link2;
> +
> +                       if (!ASSERT_OK_PTR(link, "program attach successful"))
> +                               continue;
> +
> +                       link2 = bpf_program__attach_netfilter_opts(prog, &opts);
> +                       ASSERT_NULL(link2, "attach program with same pf/hook/priority");

we have ASSERT_ERR_PTR(), which semantically is a bit more explicit,
let's use it here and below for !expect_success case

> +
> +                       if (!ASSERT_EQ(bpf_link__destroy(link), 0, "link destroy"))

ASSERT_OK()

> +                               break;
> +
> +                       link2 = bpf_program__attach_netfilter_opts(prog, &opts);
> +                       if (!ASSERT_OK_PTR(link2, "program reattach successful"))
> +                               continue;
> +                       if (!ASSERT_EQ(bpf_link__destroy(link2), 0, "link destroy"))

same, ASSERT_OK()

> +                               break;
> +               } else {
> +                       ASSERT_NULL(link, "program load failure");
> +               }
> +       }
> +}
> +
> +static void test_netfilter_link_attach(void)
> +{
> +       struct test_netfilter_link_attach *skel;
> +
> +       skel = test_netfilter_link_attach__open_and_load();
> +       if (!ASSERT_OK_PTR(skel, "test_netfilter_link_attach__open_and_load"))
> +               goto out;
> +
> +       __test_netfilter_link_attach(skel->progs.nf_link_attach_test);

nit: I'd just inline that function here instead of having
double-underscored helper function

> +out:
> +       test_netfilter_link_attach__destroy(skel);
> +}
> +
> +void test_netfilter_basic(void)
> +{
> +       if (test__start_subtest("netfilter link attach"))
> +               test_netfilter_link_attach();

Do you plan to add more subtests? If not, then this should be just a
test. Single subtest per test doesn't make much sense. Alternatively
(and perhaps better) is to treat each combination in
nf_hook_attach_tests as its own subtest.

> +}
> diff --git a/tools/testing/selftests/bpf/progs/test_netfilter_link_attach.c b/tools/testing/selftests/bpf/progs/test_netfilter_link_attach.c
> new file mode 100644
> index 000000000000..03a475160abe
> --- /dev/null
> +++ b/tools/testing/selftests/bpf/progs/test_netfilter_link_attach.c
> @@ -0,0 +1,14 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +
> +#include "vmlinux.h"
> +#include <bpf/bpf_helpers.h>
> +
> +#define NF_ACCEPT 1
> +
> +SEC("netfilter")
> +int nf_link_attach_test(struct bpf_nf_ctx *ctx)
> +{
> +       return NF_ACCEPT;
> +}
> +
> +char _license[] SEC("license") = "GPL";
> --
> 2.39.3
>

[PATCH bpf] bpf: netfilter: add BPF_NETFILTER bpf_attach_type

[Florian Westphal]

Andrii Nakryiko writes:

 And we currently don't have an attach type for NETLINK BPF link.
 Thankfully it's not too late to add it. I see that link_create() in
 kernel/bpf/syscall.c just bypasses attach_type check. We shouldn't
 have done that. Instead we need to add BPF_NETLINK attach type to enum
 bpf_attach_type. And wire all that properly throughout the kernel and
 libbpf itself.

This adds BPF_NETFILTER and uses it.  This breaks uabi but this
wasn't in any non-rc release yet, so it should be fine.

Fixes: 84601d6ee68a ("bpf: add bpf_link support for BPF_NETFILTER programs")
Suggested-by: Andrii Nakryiko <andrii.nakryiko@gmail.com>
Link: https://lore.kernel.org/bpf/CAEf4BzZ69YgrQW7DHCJUT_X+GqMq_ZQQPBwopaJJVGFD5=d5Vg@mail.gmail.com/
Signed-off-by: Florian Westphal <fw@strlen.de>
---
 include/uapi/linux/bpf.h       | 1 +
 kernel/bpf/syscall.c           | 4 ++++
 tools/include/uapi/linux/bpf.h | 1 +
 tools/lib/bpf/libbpf.c         | 2 +-
 4 files changed, 7 insertions(+), 1 deletion(-)

diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
index 1bb11a6ee667..c994ff5b157c 100644
--- a/include/uapi/linux/bpf.h
+++ b/include/uapi/linux/bpf.h
@@ -1035,6 +1035,7 @@ enum bpf_attach_type {
 	BPF_TRACE_KPROBE_MULTI,
 	BPF_LSM_CGROUP,
 	BPF_STRUCT_OPS,
+	BPF_NETFILTER,
 	__MAX_BPF_ATTACH_TYPE
 };
 
diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c
index 14f39c1e573e..cc1fc2404406 100644
--- a/kernel/bpf/syscall.c
+++ b/kernel/bpf/syscall.c
@@ -2433,6 +2433,10 @@ bpf_prog_load_check_attach(enum bpf_prog_type prog_type,
 		default:
 			return -EINVAL;
 		}
+	case BPF_PROG_TYPE_NETFILTER:
+		if (expected_attach_type == BPF_NETFILTER)
+			return 0;
+		return -EINVAL;
 	case BPF_PROG_TYPE_SYSCALL:
 	case BPF_PROG_TYPE_EXT:
 		if (expected_attach_type)
diff --git a/tools/include/uapi/linux/bpf.h b/tools/include/uapi/linux/bpf.h
index 1bb11a6ee667..c994ff5b157c 100644
--- a/tools/include/uapi/linux/bpf.h
+++ b/tools/include/uapi/linux/bpf.h
@@ -1035,6 +1035,7 @@ enum bpf_attach_type {
 	BPF_TRACE_KPROBE_MULTI,
 	BPF_LSM_CGROUP,
 	BPF_STRUCT_OPS,
+	BPF_NETFILTER,
 	__MAX_BPF_ATTACH_TYPE
 };
 
diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
index ad1ec893b41b..532a97cf1cc1 100644
--- a/tools/lib/bpf/libbpf.c
+++ b/tools/lib/bpf/libbpf.c
@@ -8712,7 +8712,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),
+	SEC_DEF("netfilter",		NETFILTER, BPF_NETFILTER, SEC_NONE),
 };
 
 static size_t custom_sec_def_cnt;
-- 
2.39.3

Re: [PATCH bpf] bpf: netfilter: add BPF_NETFILTER bpf_attach_type

[Andrii Nakryiko]

On Fri, May 26, 2023 at 5:11 AM Florian Westphal <fw@strlen.de> wrote:
>
> Andrii Nakryiko writes:
>
>  And we currently don't have an attach type for NETLINK BPF link.
>  Thankfully it's not too late to add it. I see that link_create() in
>  kernel/bpf/syscall.c just bypasses attach_type check. We shouldn't
>  have done that. Instead we need to add BPF_NETLINK attach type to enum
>  bpf_attach_type. And wire all that properly throughout the kernel and
>  libbpf itself.
>
> This adds BPF_NETFILTER and uses it.  This breaks uabi but this
> wasn't in any non-rc release yet, so it should be fine.
>
> Fixes: 84601d6ee68a ("bpf: add bpf_link support for BPF_NETFILTER programs")
> Suggested-by: Andrii Nakryiko <andrii.nakryiko@gmail.com>
> Link: https://lore.kernel.org/bpf/CAEf4BzZ69YgrQW7DHCJUT_X+GqMq_ZQQPBwopaJJVGFD5=d5Vg@mail.gmail.com/
> Signed-off-by: Florian Westphal <fw@strlen.de>
> ---
>  include/uapi/linux/bpf.h       | 1 +
>  kernel/bpf/syscall.c           | 4 ++++
>  tools/include/uapi/linux/bpf.h | 1 +
>  tools/lib/bpf/libbpf.c         | 2 +-
>  4 files changed, 7 insertions(+), 1 deletion(-)
>
> diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
> index 1bb11a6ee667..c994ff5b157c 100644
> --- a/include/uapi/linux/bpf.h
> +++ b/include/uapi/linux/bpf.h
> @@ -1035,6 +1035,7 @@ enum bpf_attach_type {
>         BPF_TRACE_KPROBE_MULTI,
>         BPF_LSM_CGROUP,
>         BPF_STRUCT_OPS,
> +       BPF_NETFILTER,
>         __MAX_BPF_ATTACH_TYPE
>  };
>
> diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c
> index 14f39c1e573e..cc1fc2404406 100644
> --- a/kernel/bpf/syscall.c
> +++ b/kernel/bpf/syscall.c
> @@ -2433,6 +2433,10 @@ bpf_prog_load_check_attach(enum bpf_prog_type prog_type,
>                 default:
>                         return -EINVAL;
>                 }
> +       case BPF_PROG_TYPE_NETFILTER:
> +               if (expected_attach_type == BPF_NETFILTER)
> +                       return 0;
> +               return -EINVAL;
>         case BPF_PROG_TYPE_SYSCALL:
>         case BPF_PROG_TYPE_EXT:
>                 if (expected_attach_type)

You've missed updating link_create() as well, there is a

case BPF_PROG_TYPE_NETFILTER:
    break;

switch case, which should validate that attr->link_create.attach_type
is BPF_NETFILTER


Other than that, it looks good to me, thanks!

> diff --git a/tools/include/uapi/linux/bpf.h b/tools/include/uapi/linux/bpf.h
> index 1bb11a6ee667..c994ff5b157c 100644
> --- a/tools/include/uapi/linux/bpf.h
> +++ b/tools/include/uapi/linux/bpf.h
> @@ -1035,6 +1035,7 @@ enum bpf_attach_type {
>         BPF_TRACE_KPROBE_MULTI,
>         BPF_LSM_CGROUP,
>         BPF_STRUCT_OPS,
> +       BPF_NETFILTER,
>         __MAX_BPF_ATTACH_TYPE
>  };
>
> diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
> index ad1ec893b41b..532a97cf1cc1 100644
> --- a/tools/lib/bpf/libbpf.c
> +++ b/tools/lib/bpf/libbpf.c
> @@ -8712,7 +8712,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),
> +       SEC_DEF("netfilter",            NETFILTER, BPF_NETFILTER, SEC_NONE),
>  };
>
>  static size_t custom_sec_def_cnt;
> --
> 2.39.3
>