From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751449AbcGUGgY (ORCPT ); Thu, 21 Jul 2016 02:36:24 -0400 Received: from mail-wm0-f67.google.com ([74.125.82.67]:34333 "EHLO mail-wm0-f67.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750884AbcGUGgW (ORCPT ); Thu, 21 Jul 2016 02:36:22 -0400 From: Jiri Slaby X-Google-Original-From: Jiri Slaby Subject: Re: [PATCH 3.12 28/88] netfilter: x_tables: validate targets of jumps To: Jiri Slaby , stable@vger.kernel.org References: <3d4036cb9b963cdd270c02856a888183da0623db.1468483951.git.jslaby@suse.cz> <42b891251b3b9f241fb5f0a90e87b15d2fef0a74.1468483951.git.jslaby@suse.cz> Cc: linux-kernel@vger.kernel.org, Florian Westphal , Pablo Neira Ayuso Message-ID: <8d66c06f-7633-357e-f091-c182d78fb219@gmail.com> Date: Thu, 21 Jul 2016 08:36:18 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.1.1 MIME-Version: 1.0 In-Reply-To: <42b891251b3b9f241fb5f0a90e87b15d2fef0a74.1468483951.git.jslaby@suse.cz> Content-Type: text/plain; charset=iso-8859-2 Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 07/14/2016, 10:15 AM, Jiri Slaby wrote: > From: Florian Westphal > > 3.12-stable review patch. If anyone has any objections, please let me know. > > =============== > > commit 36472341017529e2b12573093cc0f68719300997 upstream. I am now dropping this one. 3.12.62 will be released without that patch. After the performance issue is resolved, it will be requeued. > 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: Jiri Slaby > --- > 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 7460b7bef3ab..473ec559ce7b 100644 > --- a/net/ipv4/netfilter/arp_tables.c > +++ b/net/ipv4/netfilter/arp_tables.c > @@ -358,6 +358,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. > */ > @@ -451,6 +464,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 8fc22eed9603..dba9d8070d07 100644 > --- a/net/ipv4/netfilter/ip_tables.c > +++ b/net/ipv4/netfilter/ip_tables.c > @@ -434,6 +434,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 > @@ -531,6 +544,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 63f7876c4f29..97a8d2525c26 100644 > --- a/net/ipv6/netfilter/ip6_tables.c > +++ b/net/ipv6/netfilter/ip6_tables.c > @@ -444,6 +444,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 > @@ -541,6 +554,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; >