[PATCH 0/2] xt_bpf: fix handling of pinned objects

[PATCH 0/2] xt_bpf: fix handling of pinned objects

[Rafael Buchbinder]

Following set of commits fixes xt_bpf extension to correctly handle
pinned eBPF programs.

The origin of the bug lies in the fact that xt_bpf_info_v1 structure
requires an open file descriptor to create an eBPF match. 
This file descriptor is checked on every replace. However, as this file
descriptor is valid only for the iptables invocation which loads the
eBPF for the first time, all subsequent iptables invocations fail in
bpf_mt_check (kernel) function.

See discussion in [1] for more details.

The following patches add a hook in extensions which is called
immediately after TC_INIT to fixup whatever needs to be fixed up.
In case of xt_bpf, the fixup function gets the eBPF object by path to
populate xt_bpf_info_v1 structure with a valid file descriptor.

[1] https://marc.info/?l=netfilter-devel&m=150530909630143&w=2

Rafael Buchbinder (2):
  iptables: support match info fixup after tc_init
  extensions: xt_bpf: get the pinned ebpf object when match is
    initialized

 extensions/libxt_bpf.c |  9 +++++++++
 include/xtables.h      |  3 +++
 iptables/ip6tables.c   | 35 +++++++++++++++++++++++++++++++++++
 iptables/iptables.c    | 34 ++++++++++++++++++++++++++++++++++
 4 files changed, 81 insertions(+)

-- 
2.14.1

[PATCH 1/2] iptables: support match info fixup after tc_init

[Rafael Buchbinder]

This commit introduces a framework to fixup match info,
which may be required by an extension.

Signed-off-by: Rafael Buchbinder <rafi@rbk.ms>
Signed-off-by: Shmulik Ladkani <shmulik@nsof.io>
---
 include/xtables.h    |  3 +++
 iptables/ip6tables.c | 35 +++++++++++++++++++++++++++++++++++
 iptables/iptables.c  | 34 ++++++++++++++++++++++++++++++++++
 3 files changed, 72 insertions(+)

diff --git a/include/xtables.h b/include/xtables.h
index e9bc3b7d..687cfe9f 100644
--- a/include/xtables.h
+++ b/include/xtables.h
@@ -273,6 +273,9 @@ struct xtables_match {
 	/* ip is struct ipt_ip * for example */
 	void (*save)(const void *ip, const struct xt_entry_match *match);
 
+	/* Fixes the match info after init. */
+	void (*tc_init_fixup)(struct xt_entry_match *match);
+
 	/* Print match name or alias */
 	const char *(*alias)(const struct xt_entry_match *match);
 
diff --git a/iptables/ip6tables.c b/iptables/ip6tables.c
index 49bd006f..0a6afa77 100644
--- a/iptables/ip6tables.c
+++ b/iptables/ip6tables.c
@@ -925,6 +925,39 @@ delete_chain6(const xt_chainlabel chain, int verbose,
 	return ip6tc_delete_chain(chain, handle);
 }
 
+
+static int
+tc_init_fixup_match(struct xt_entry_match *m)
+{
+	const struct xtables_match *match =
+		xtables_find_match(m->u.user.name, XTF_TRY_LOAD, NULL);
+
+	if (match) {
+		if (match->tc_init_fixup && m->u.user.revision == match->revision)
+			match->tc_init_fixup(m);
+	}
+
+	/* Don't stop iterating. */
+	return 0;
+}
+
+static void
+tc_init_fixup(struct xtc_handle *handle)
+{
+	const char *chain;
+
+	for (chain = ip6tc_first_chain(handle);
+	     chain;
+	     chain = ip6tc_next_chain(handle)) {
+		const struct ip6t_entry *entry = ip6tc_first_rule(chain, handle);
+
+		while (entry) {
+			IP6T_MATCH_ITERATE(entry, tc_init_fixup_match);
+			entry = ip6tc_next_rule(entry, handle);
+		}
+	}
+}
+
 static int
 list_entries(const xt_chainlabel chain, int rulenum, int verbose, int numeric,
 	     int expanded, int linenumbers, struct xtc_handle *handle)
@@ -1795,6 +1828,8 @@ int do_command6(int argc, char *argv[], char **table,
 			"can't initialize ip6tables table `%s': %s",
 			*table, ip6tc_strerror(errno));
 
+	tc_init_fixup(*handle);
+
 	if (command == CMD_APPEND
 	    || command == CMD_DELETE
 	    || command == CMD_CHECK
diff --git a/iptables/iptables.c b/iptables/iptables.c
index 69d19fec..f220a8e4 100644
--- a/iptables/iptables.c
+++ b/iptables/iptables.c
@@ -909,6 +909,38 @@ delete_chain4(const xt_chainlabel chain, int verbose,
 	return iptc_delete_chain(chain, handle);
 }
 
+static int
+tc_init_fixup_match(struct xt_entry_match *m)
+{
+	const struct xtables_match *match =
+		xtables_find_match(m->u.user.name, XTF_TRY_LOAD, NULL);
+
+	if (match) {
+		if (match->tc_init_fixup && m->u.user.revision == match->revision)
+			match->tc_init_fixup(m);
+	}
+
+	/* Don't stop iterating. */
+	return 0;
+}
+
+static void
+tc_init_fixup(struct xtc_handle *handle)
+{
+	const char *chain;
+
+	for (chain = iptc_first_chain(handle);
+	     chain;
+	     chain = iptc_next_chain(handle)) {
+		const struct ipt_entry *entry = iptc_first_rule(chain, handle);
+
+		while (entry) {
+			IPT_MATCH_ITERATE(entry, tc_init_fixup_match);
+			entry = iptc_next_rule(entry, handle);
+		}
+	}
+}
+
 static int
 list_entries(const xt_chainlabel chain, int rulenum, int verbose, int numeric,
 	     int expanded, int linenumbers, struct xtc_handle *handle)
@@ -1781,6 +1813,8 @@ int do_command4(int argc, char *argv[], char **table,
 			   "can't initialize iptables table `%s': %s",
 			   *table, iptc_strerror(errno));
 
+	tc_init_fixup(*handle);
+
 	if (command == CMD_APPEND
 	    || command == CMD_DELETE
 	    || command == CMD_CHECK
-- 
2.14.1

[PATCH 2/2] extensions: xt_bpf: get the pinned ebpf object when match is initialized

[Rafael Buchbinder]

xt_bpf_info_v1 structure requires an open file descriptor to create an
eBPF match. This file descriptor is checked on every replace. However,
as this file descriptor is valid only for the iptables invocation which
loads the eBPF for the first time, all subsequent iptables invocations
fail in bpf_mt_check (kernel) function.

This commit fixes handling of pinned ebpf objects.

The file descriptor saved in xt_bpf_info_v1 structure is being re-open
in tc_init_fixup which is invoked immediately after tc_init.

Signed-off-by: Rafael Buchbinder <rafi@rbk.ms>
Signed-off-by: Shmulik Ladkani <shmulik@nsof.io>
---
 extensions/libxt_bpf.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/extensions/libxt_bpf.c b/extensions/libxt_bpf.c
index 9510c190..16d6bc25 100644
--- a/extensions/libxt_bpf.c
+++ b/extensions/libxt_bpf.c
@@ -247,6 +247,14 @@ static void bpf_print_v1(const void *ip, const struct xt_entry_match *match,
 		printf("unknown");
 }
 
+static void bpf_tc_init_fixup_v1(struct xt_entry_match *match)
+{
+	struct xt_bpf_info_v1 *info = (void *) match->data;
+
+	if (info->mode == XT_BPF_MODE_FD_PINNED)
+		bpf_parse_obj_pinned(info, info->path);
+}
+
 static struct xtables_match bpf_matches[] = {
 	{
 		.family		= NFPROTO_UNSPEC,
@@ -272,6 +280,7 @@ static struct xtables_match bpf_matches[] = {
 		.help		= bpf_help_v1,
 		.print		= bpf_print_v1,
 		.save		= bpf_save_v1,
+		.tc_init_fixup	= bpf_tc_init_fixup_v1,
 		.x6_parse	= bpf_parse_v1,
 		.x6_fcheck	= bpf_fcheck_v1,
 		.x6_options	= bpf_opts_v1,
-- 
2.14.1

Re: [PATCH 0/2] xt_bpf: fix handling of pinned objects

[Shmulik Ladkani]

please drop, wrong 'From:' field, will resend v2

On Sun, 17 Sep 2017 14:07:49 +0300
Rafael Buchbinder <shmulik@nsof.io> wrote:

> Following set of commits fixes xt_bpf extension to correctly handle
> pinned eBPF programs.
> 
> The origin of the bug lies in the fact that xt_bpf_info_v1 structure
> requires an open file descriptor to create an eBPF match. 
> This file descriptor is checked on every replace. However, as this file
> descriptor is valid only for the iptables invocation which loads the
> eBPF for the first time, all subsequent iptables invocations fail in
> bpf_mt_check (kernel) function.
> 
> See discussion in [1] for more details.
> 
> The following patches add a hook in extensions which is called
> immediately after TC_INIT to fixup whatever needs to be fixed up.
> In case of xt_bpf, the fixup function gets the eBPF object by path to
> populate xt_bpf_info_v1 structure with a valid file descriptor.
> 
> [1] https://marc.info/?l=netfilter-devel&m=150530909630143&w=2
> 
> Rafael Buchbinder (2):
>   iptables: support match info fixup after tc_init
>   extensions: xt_bpf: get the pinned ebpf object when match is
>     initialized
> 
>  extensions/libxt_bpf.c |  9 +++++++++
>  include/xtables.h      |  3 +++
>  iptables/ip6tables.c   | 35 +++++++++++++++++++++++++++++++++++
>  iptables/iptables.c    | 34 ++++++++++++++++++++++++++++++++++
>  4 files changed, 81 insertions(+)
>