From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751788AbcFWJNz (ORCPT ); Thu, 23 Jun 2016 05:13:55 -0400 Received: from Chamillionaire.breakpoint.cc ([80.244.247.6]:35788 "EHLO Chamillionaire.breakpoint.cc" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751355AbcFWJNx (ORCPT ); Thu, 23 Jun 2016 05:13:53 -0400 Date: Thu, 23 Jun 2016 11:13:47 +0200 From: Florian Westphal To: Florian Westphal Cc: Greg Kroah-Hartman , linux-kernel@vger.kernel.org, stable@vger.kernel.org, Pablo Neira Ayuso Subject: Re: [PATCH 3.14 21/29] netfilter: x_tables: validate targets of jumps Message-ID: <20160623091347.GB4662@breakpoint.cc> References: <20160622223530.496939726@linuxfoundation.org> <20160622223531.668543902@linuxfoundation.org> <20160623085450.GA4662@breakpoint.cc> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20160623085450.GA4662@breakpoint.cc> User-Agent: Mutt/1.5.23 (2014-03-12) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Florian Westphal wrote: > Greg Kroah-Hartman wrote: > > 3.14-stable review patch. If anyone has any objections, please let me know. > > I have -- this doesn't work in 3.14 as t->entries (the ruleset blob) > is still kept percpu. > > > +static bool find_jump_target(const struct xt_table_info *t, > > + const struct arpt_entry *target) > > +{ > > + struct arpt_entry *iter; > > + > > + xt_entry_foreach(iter, t->entries, t->size) { > > > .. so this causes in kernel soft lockup when I try to insert a rule. > > I will go over the 3.14 stable queue and see if I can amend this to work > with 3.14. This amended patch works for me (iptables-test.py passes except those tests that I expected to fail due to some missing features in 3.14). I also briefly tried 32bit iptables/ip6tables and that seems happy as well. The reproduces for the two bugs fail with -EINVAL. ebtables doesn't work (even ebtables -A INPUT -j ACCEPT fails), but that should be solved by picking up d26e2c9ffa385dd1b646f43c1397ba12af9e, "Revert "netfilter: ensure number of counters is >0 in do_replace()" [ its a PARTIAL revert, so don't drop the original patch ... ] Subject: netfilter: x_tables: validate targets of jumps commit 36472341017529e2b12573093cc0f68719300997 upstream. When we see a jump also check that the offset gets us to beginning of a rule (an ipt_entry). The extra overhead is negible, even with absurd cases. 300k custom rules, 300k jumps to 'next' user chain: [ plus one jump from INPUT to first userchain ]: Before: real 0m24.874s user 0m7.532s sys 0m16.076s After: real 0m27.464s user 0m7.436s sys 0m18.840s Signed-off-by: Florian Westphal Signed-off-by: Pablo Neira Ayuso Signed-off-by: Greg Kroah-Hartman --- Need to pass the start of the ruleset as extra argument as t->entries won't work in 3.14 (its percpu and not even set up for all processors at this point). net/ipv4/netfilter/arp_tables.c | 17 +++++++++++++++++ net/ipv4/netfilter/ip_tables.c | 17 +++++++++++++++++ net/ipv6/netfilter/ip6_tables.c | 17 +++++++++++++++++ 3 files changed, 51 insertions(+) diff --git a/net/ipv4/netfilter/arp_tables.c b/net/ipv4/netfilter/arp_tables.c index 993da4a..5f3e807 100644 --- a/net/ipv4/netfilter/arp_tables.c +++ b/net/ipv4/netfilter/arp_tables.c @@ -363,6 +363,19 @@ static inline bool unconditional(const struct arpt_entry *e) memcmp(&e->arp, &uncond, sizeof(uncond)) == 0; } +static bool find_jump_target(const struct xt_table_info *t, + const void *entry0, + const struct arpt_entry *target) +{ + struct arpt_entry *iter; + + xt_entry_foreach(iter, entry0, t->size) { + if (iter == target) + return true; + } + return false; +} + /* Figures out from what hook each rule can be called: returns 0 if * there are loops. Puts hook bitmask in comefrom. */ @@ -456,6 +469,10 @@ static int mark_source_chains(const struct xt_table_info *newinfo, /* This a jump; chase it. */ duprintf("Jump rule %u -> %u\n", pos, newpos); + e = (struct arpt_entry *) + (entry0 + newpos); + if (!find_jump_target(newinfo, entry0, e)) + return 0; } else { /* ... this is a fallthru */ newpos = pos + e->next_offset; diff --git a/net/ipv4/netfilter/ip_tables.c b/net/ipv4/netfilter/ip_tables.c index b75c5bb..f402317 100644 --- a/net/ipv4/netfilter/ip_tables.c +++ b/net/ipv4/netfilter/ip_tables.c @@ -439,6 +439,19 @@ ipt_do_table(struct sk_buff *skb, #endif } +static bool find_jump_target(const struct xt_table_info *t, + const void *entry0, + const struct ipt_entry *target) +{ + struct ipt_entry *iter; + + xt_entry_foreach(iter, entry0, t->size) { + if (iter == target) + return true; + } + return false; +} + /* Figures out from what hook each rule can be called: returns 0 if there are loops. Puts hook bitmask in comefrom. */ static int @@ -536,6 +549,10 @@ mark_source_chains(const struct xt_table_info *newinfo, /* This a jump; chase it. */ duprintf("Jump rule %u -> %u\n", pos, newpos); + e = (struct ipt_entry *) + (entry0 + newpos); + if (!find_jump_target(newinfo, entry0, e)) + return 0; } else { /* ... this is a fallthru */ newpos = pos + e->next_offset; diff --git a/net/ipv6/netfilter/ip6_tables.c b/net/ipv6/netfilter/ip6_tables.c index 9367bbd..e312639 100644 --- a/net/ipv6/netfilter/ip6_tables.c +++ b/net/ipv6/netfilter/ip6_tables.c @@ -449,6 +449,19 @@ ip6t_do_table(struct sk_buff *skb, #endif } +static bool find_jump_target(const struct xt_table_info *t, + const void *entry0, + const struct ip6t_entry *target) +{ + struct ip6t_entry *iter; + + xt_entry_foreach(iter, entry0, t->size) { + if (iter == target) + return true; + } + return false; +} + /* Figures out from what hook each rule can be called: returns 0 if there are loops. Puts hook bitmask in comefrom. */ static int @@ -546,6 +559,10 @@ mark_source_chains(const struct xt_table_info *newinfo, /* This a jump; chase it. */ duprintf("Jump rule %u -> %u\n", pos, newpos); + e = (struct ip6t_entry *) + (entry0 + newpos); + if (!find_jump_target(newinfo, entry0, e)) + return 0; } else { /* ... this is a fallthru */ newpos = pos + e->next_offset; -- 2.7.3