Skip to contentSkip to conversation

xt_bpf: fix handling of pinned objects

PATCH Superseded Rafael Buchbinder 1 participant 4 messages Sep 17, 2017 netfilter

v1 of 2 Superseded Latest is v2 →

lore
Share & tools
lore

Rafael Buchbinder

author
#0 lore raw

cover xt_bpf: fix handling of pinned objects

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(+)
Signature · 1 line
2.14.1
author
#1 lore raw

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

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

Signed-off-by Rafael Buchbinder <[email protected]>
Signed-off-by Shmulik Ladkani <[email protected]>
---
 include/xtables.h    |  3 +++
 iptables/ip6tables.c | 35 +++++++++++++++++++++++++++++++++++
 iptables/iptables.c  | 34 ++++++++++++++++++++++++++++++++++
 3 files changed, 72 insertions(+)
include/xtables.h +3 -0
index e9bc3b7d..687cfe9f 100644
@@ -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);
iptables/ip6tables.c +35 -0
index 49bd006f..0a6afa77 100644
@@ -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
iptables/iptables.c +34 -0
index 69d19fec..f220a8e4 100644
@@ -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
Signature · 1 line
2.14.1
author
#2 lore raw

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

In reply to 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 <[email protected]>
Signed-off-by Shmulik Ladkani <[email protected]>
---
 extensions/libxt_bpf.c | 9 +++++++++
 1 file changed, 9 insertions(+)
extensions/libxt_bpf.c +9 -0
index 9510c190..16d6bc25 100644
@@ -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,
Signature · 1 line
2.14.1
author
#3 lore raw
In reply to Rafael Buchbinder

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

Quoted 30 lines · On Sun, 17 Sep 2017 14:07:49 +0300 Rafael Buchbinder <[email protected]> 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(+)