From 4d6b80762b9384db3281f792a71746b388f395be Mon Sep 17 00:00:00 2001 From: Kirill Tkhai Date: Mon, 19 Feb 2018 11:50:45 +0300 Subject: net: Convert ip_tables_net_ops, udplite6_net_ops and xt_net_ops ip_tables_net_ops and udplite6_net_ops create and destroy /proc entries. xt_net_ops does nothing. So, we are able to mark them async. Signed-off-by: Kirill Tkhai Signed-off-by: David S. Miller --- net/ipv4/netfilter/ip_tables.c | 1 + 1 file changed, 1 insertion(+) (limited to 'net/ipv4/netfilter/ip_tables.c') diff --git a/net/ipv4/netfilter/ip_tables.c b/net/ipv4/netfilter/ip_tables.c index 9a71f31..39a7cf9 100644 --- a/net/ipv4/netfilter/ip_tables.c +++ b/net/ipv4/netfilter/ip_tables.c @@ -1911,6 +1911,7 @@ static void __net_exit ip_tables_net_exit(struct net *net) static struct pernet_operations ip_tables_net_ops = { .init = ip_tables_net_init, .exit = ip_tables_net_exit, + .async = true, }; static int __init ip_tables_init(void) -- cgit v1.1 From f31e5f1a891f989f107e8caa6b49dd4df0e12265 Mon Sep 17 00:00:00 2001 From: Xin Long Date: Fri, 16 Feb 2018 18:04:56 +0800 Subject: netfilter: unlock xt_table earlier in __do_replace Now it's doing cleanup_entry for oldinfo under the xt_table lock, but it's not really necessary. After the replacement job is done in xt_replace_table, oldinfo is not used elsewhere any more, and it can be freed without xt_table lock safely. The important thing is that rtnl_lock is called in some xt_target destroy, which means rtnl_lock, a big lock is used in xt_table lock, a smaller one. It usually could be the reason why a dead lock may happen. Besides, all xt_target/match checkentry is called out of xt_table lock. It's better also to move all cleanup_entry calling out of xt_table lock, just as do_replace_finish does for ebtables. Signed-off-by: Xin Long Signed-off-by: Pablo Neira Ayuso --- net/ipv4/netfilter/ip_tables.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) (limited to 'net/ipv4/netfilter/ip_tables.c') diff --git a/net/ipv4/netfilter/ip_tables.c b/net/ipv4/netfilter/ip_tables.c index d4f7584..4f7153e 100644 --- a/net/ipv4/netfilter/ip_tables.c +++ b/net/ipv4/netfilter/ip_tables.c @@ -1087,6 +1087,8 @@ __do_replace(struct net *net, const char *name, unsigned int valid_hooks, (newinfo->number <= oldinfo->initial_entries)) module_put(t->me); + xt_table_unlock(t); + get_old_counters(oldinfo, counters); /* Decrease module usage counts and free resource */ @@ -1100,7 +1102,6 @@ __do_replace(struct net *net, const char *name, unsigned int valid_hooks, net_warn_ratelimited("iptables: counters copy to user failed while replacing table\n"); } vfree(counters); - xt_table_unlock(t); return ret; put_module: -- cgit v1.1 From 07a9da51b4b6aece8bc71e0b1b601fc4c3eb8b64 Mon Sep 17 00:00:00 2001 From: Florian Westphal Date: Tue, 27 Feb 2018 19:42:27 +0100 Subject: netfilter: x_tables: check standard verdicts in core Userspace must provide a valid verdict to the standard target. The verdict can be either a jump (signed int > 0), or a return code. Allowed return codes are either RETURN (pop from stack), NF_ACCEPT, DROP and QUEUE (latter is allowed for legacy reasons). Jump offsets (verdict > 0) are checked in more detail later on when loop-detection is performed. Signed-off-by: Florian Westphal Signed-off-by: Pablo Neira Ayuso --- net/ipv4/netfilter/ip_tables.c | 5 ----- 1 file changed, 5 deletions(-) (limited to 'net/ipv4/netfilter/ip_tables.c') diff --git a/net/ipv4/netfilter/ip_tables.c b/net/ipv4/netfilter/ip_tables.c index 4f7153e..c9b57a6 100644 --- a/net/ipv4/netfilter/ip_tables.c +++ b/net/ipv4/netfilter/ip_tables.c @@ -402,11 +402,6 @@ mark_source_chains(const struct xt_table_info *newinfo, t->verdict < 0) || visited) { unsigned int oldpos, size; - if ((strcmp(t->target.u.user.name, - XT_STANDARD_TARGET) == 0) && - t->verdict < -NF_MAX_VERDICT - 1) - return 0; - /* Return: backtrack through the last big jump. */ do { -- cgit v1.1 From 1b293e30f759b03f246baae862bdf35e57b2c39e Mon Sep 17 00:00:00 2001 From: Florian Westphal Date: Tue, 27 Feb 2018 19:42:29 +0100 Subject: netfilter: x_tables: move hook entry checks into core Allow followup patch to change on location instead of three. Signed-off-by: Florian Westphal Signed-off-by: Pablo Neira Ayuso --- net/ipv4/netfilter/ip_tables.c | 13 +++---------- 1 file changed, 3 insertions(+), 10 deletions(-) (limited to 'net/ipv4/netfilter/ip_tables.c') diff --git a/net/ipv4/netfilter/ip_tables.c b/net/ipv4/netfilter/ip_tables.c index c9b57a6..29bda94 100644 --- a/net/ipv4/netfilter/ip_tables.c +++ b/net/ipv4/netfilter/ip_tables.c @@ -702,16 +702,9 @@ translate_table(struct net *net, struct xt_table_info *newinfo, void *entry0, if (i != repl->num_entries) goto out_free; - /* Check hooks all assigned */ - for (i = 0; i < NF_INET_NUMHOOKS; i++) { - /* Only hooks which are valid */ - if (!(repl->valid_hooks & (1 << i))) - continue; - if (newinfo->hook_entry[i] == 0xFFFFFFFF) - goto out_free; - if (newinfo->underflow[i] == 0xFFFFFFFF) - goto out_free; - } + ret = xt_check_table_hooks(newinfo, repl->valid_hooks); + if (ret) + goto out_free; if (!mark_source_chains(newinfo, repl->valid_hooks, entry0, offsets)) { ret = -ELOOP; -- cgit v1.1 From c84ca954ac9fa67a6ce27f91f01e4451c74fd8f6 Mon Sep 17 00:00:00 2001 From: Florian Westphal Date: Tue, 27 Feb 2018 19:42:33 +0100 Subject: netfilter: x_tables: add counters allocation wrapper allows to have size checks in a single spot. This is supposed to reduce oom situations when fuzz-testing xtables. Signed-off-by: Florian Westphal Signed-off-by: Pablo Neira Ayuso --- net/ipv4/netfilter/ip_tables.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'net/ipv4/netfilter/ip_tables.c') diff --git a/net/ipv4/netfilter/ip_tables.c b/net/ipv4/netfilter/ip_tables.c index 29bda94..4901ca6 100644 --- a/net/ipv4/netfilter/ip_tables.c +++ b/net/ipv4/netfilter/ip_tables.c @@ -1045,7 +1045,7 @@ __do_replace(struct net *net, const char *name, unsigned int valid_hooks, struct ipt_entry *iter; ret = 0; - counters = vzalloc(num_counters * sizeof(struct xt_counters)); + counters = xt_counters_alloc(num_counters); if (!counters) { ret = -ENOMEM; goto out; -- cgit v1.1 From 9782a11efc072faaf91d4aa60e9d23553f918029 Mon Sep 17 00:00:00 2001 From: Florian Westphal Date: Tue, 27 Feb 2018 19:42:34 +0100 Subject: netfilter: compat: prepare xt_compat_init_offsets to return errors should have no impact, function still always returns 0. This patch is only to ease review. Signed-off-by: Florian Westphal Signed-off-by: Pablo Neira Ayuso --- net/ipv4/netfilter/ip_tables.c | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) (limited to 'net/ipv4/netfilter/ip_tables.c') diff --git a/net/ipv4/netfilter/ip_tables.c b/net/ipv4/netfilter/ip_tables.c index 4901ca6..f906351 100644 --- a/net/ipv4/netfilter/ip_tables.c +++ b/net/ipv4/netfilter/ip_tables.c @@ -933,7 +933,9 @@ static int compat_table_info(const struct xt_table_info *info, memcpy(newinfo, info, offsetof(struct xt_table_info, entries)); newinfo->initial_entries = 0; loc_cpu_entry = info->entries; - xt_compat_init_offsets(AF_INET, info->number); + ret = xt_compat_init_offsets(AF_INET, info->number); + if (ret) + return ret; xt_entry_foreach(iter, loc_cpu_entry, info->size) { ret = compat_calc_entry(iter, info, loc_cpu_entry, newinfo); if (ret != 0) @@ -1407,7 +1409,9 @@ translate_compat_table(struct net *net, j = 0; xt_compat_lock(AF_INET); - xt_compat_init_offsets(AF_INET, compatr->num_entries); + ret = xt_compat_init_offsets(AF_INET, compatr->num_entries); + if (ret) + goto out_unlock; /* Walk through entries, checking offsets. */ xt_entry_foreach(iter0, entry0, compatr->size) { ret = check_compat_entry_size_and_hooks(iter0, info, &size, -- cgit v1.1 From 0d7df906a0e78079a02108b06d32c3ef2238ad25 Mon Sep 17 00:00:00 2001 From: Florian Westphal Date: Tue, 27 Feb 2018 19:42:37 +0100 Subject: netfilter: x_tables: ensure last rule in base chain matches underflow/policy Harmless from kernel point of view, but again iptables assumes that this is true when decoding ruleset coming from kernel. If a (syzkaller generated) ruleset doesn't have the underflow/policy stored as the last rule in the base chain, then iptables will abort() because it doesn't find the chain policy. libiptc assumes that the policy is the last rule in the basechain, which is only true for iptables-generated rulesets. Unfortunately this needs code duplication -- the functions need the struct layout of the rule head, but that is different for ip/ip6/arptables. NB: pr_warn could be pr_debug but in case this break rulesets somehow its useful to know why blob was rejected. Signed-off-by: Florian Westphal Signed-off-by: Pablo Neira Ayuso --- net/ipv4/netfilter/ip_tables.c | 17 ++++++++++++++++- 1 file changed, 16 insertions(+), 1 deletion(-) (limited to 'net/ipv4/netfilter/ip_tables.c') diff --git a/net/ipv4/netfilter/ip_tables.c b/net/ipv4/netfilter/ip_tables.c index f906351..2362ca2 100644 --- a/net/ipv4/netfilter/ip_tables.c +++ b/net/ipv4/netfilter/ip_tables.c @@ -378,10 +378,13 @@ mark_source_chains(const struct xt_table_info *newinfo, for (hook = 0; hook < NF_INET_NUMHOOKS; hook++) { unsigned int pos = newinfo->hook_entry[hook]; struct ipt_entry *e = entry0 + pos; + unsigned int last_pos, depth; if (!(valid_hooks & (1 << hook))) continue; + depth = 0; + last_pos = pos; /* Set initial back pointer. */ e->counters.pcnt = pos; @@ -410,6 +413,8 @@ mark_source_chains(const struct xt_table_info *newinfo, pos = e->counters.pcnt; e->counters.pcnt = 0; + if (depth) + --depth; /* We're at the start. */ if (pos == oldpos) goto next; @@ -434,6 +439,9 @@ mark_source_chains(const struct xt_table_info *newinfo, if (!xt_find_jump_offset(offsets, newpos, newinfo->number)) return 0; + + if (entry0 + newpos != ipt_next_entry(e)) + ++depth; } else { /* ... this is a fallthru */ newpos = pos + e->next_offset; @@ -444,8 +452,15 @@ mark_source_chains(const struct xt_table_info *newinfo, e->counters.pcnt = pos; pos = newpos; } + if (depth == 0) + last_pos = pos; + } +next: + if (last_pos != newinfo->underflow[hook]) { + pr_err_ratelimited("last base chain position %u doesn't match underflow %u (hook %u)\n", + last_pos, newinfo->underflow[hook], hook); + return 0; } -next: ; } return 1; } -- cgit v1.1 From 2f635ceeb22ba13c307236d69795fbb29cfa3e7c Mon Sep 17 00:00:00 2001 From: Kirill Tkhai Date: Tue, 27 Mar 2018 18:02:13 +0300 Subject: net: Drop pernet_operations::async Synchronous pernet_operations are not allowed anymore. All are asynchronous. So, drop the structure member. Signed-off-by: Kirill Tkhai Signed-off-by: David S. Miller --- net/ipv4/netfilter/ip_tables.c | 1 - 1 file changed, 1 deletion(-) (limited to 'net/ipv4/netfilter/ip_tables.c') diff --git a/net/ipv4/netfilter/ip_tables.c b/net/ipv4/netfilter/ip_tables.c index d4f7584..e38395a 100644 --- a/net/ipv4/netfilter/ip_tables.c +++ b/net/ipv4/netfilter/ip_tables.c @@ -1916,7 +1916,6 @@ static void __net_exit ip_tables_net_exit(struct net *net) static struct pernet_operations ip_tables_net_ops = { .init = ip_tables_net_init, .exit = ip_tables_net_exit, - .async = true, }; static int __init ip_tables_init(void) -- cgit v1.1 From e3b5e1ec75234fb6b27708a316cdf69f9fb176a8 Mon Sep 17 00:00:00 2001 From: Florian Westphal Date: Fri, 30 Mar 2018 11:39:12 +0200 Subject: Revert "netfilter: x_tables: ensure last rule in base chain matches underflow/policy" This reverts commit 0d7df906a0e78079a02108b06d32c3ef2238ad25. Valdis Kletnieks reported that xtables is broken in linux-next since 0d7df906a0e78 ("netfilter: x_tables: ensure last rule in base chain matches underflow/policy"), as kernel rejects the (well-formed) ruleset: [ 64.402790] ip6_tables: last base chain position 1136 doesn't match underflow 1344 (hook 1) mark_source_chains is not the correct place for such a check, as it terminates evaluation of a chain once it sees an unconditional verdict (following rules are known to be unreachable). It seems preferrable to fix libiptc instead, so remove this check again. Fixes: 0d7df906a0e78 ("netfilter: x_tables: ensure last rule in base chain matches underflow/policy") Reported-by: Valdis Kletnieks Signed-off-by: Florian Westphal Signed-off-by: Pablo Neira Ayuso --- net/ipv4/netfilter/ip_tables.c | 17 +---------------- 1 file changed, 1 insertion(+), 16 deletions(-) (limited to 'net/ipv4/netfilter/ip_tables.c') diff --git a/net/ipv4/netfilter/ip_tables.c b/net/ipv4/netfilter/ip_tables.c index 2362ca2..f906351 100644 --- a/net/ipv4/netfilter/ip_tables.c +++ b/net/ipv4/netfilter/ip_tables.c @@ -378,13 +378,10 @@ mark_source_chains(const struct xt_table_info *newinfo, for (hook = 0; hook < NF_INET_NUMHOOKS; hook++) { unsigned int pos = newinfo->hook_entry[hook]; struct ipt_entry *e = entry0 + pos; - unsigned int last_pos, depth; if (!(valid_hooks & (1 << hook))) continue; - depth = 0; - last_pos = pos; /* Set initial back pointer. */ e->counters.pcnt = pos; @@ -413,8 +410,6 @@ mark_source_chains(const struct xt_table_info *newinfo, pos = e->counters.pcnt; e->counters.pcnt = 0; - if (depth) - --depth; /* We're at the start. */ if (pos == oldpos) goto next; @@ -439,9 +434,6 @@ mark_source_chains(const struct xt_table_info *newinfo, if (!xt_find_jump_offset(offsets, newpos, newinfo->number)) return 0; - - if (entry0 + newpos != ipt_next_entry(e)) - ++depth; } else { /* ... this is a fallthru */ newpos = pos + e->next_offset; @@ -452,15 +444,8 @@ mark_source_chains(const struct xt_table_info *newinfo, e->counters.pcnt = pos; pos = newpos; } - if (depth == 0) - last_pos = pos; - } -next: - if (last_pos != newinfo->underflow[hook]) { - pr_err_ratelimited("last base chain position %u doesn't match underflow %u (hook %u)\n", - last_pos, newinfo->underflow[hook], hook); - return 0; } +next: ; } return 1; } -- cgit v1.1