Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]


Groups > linux.kernel > #1429628

Re: [PATCH 3.14 21/29] netfilter: x_tables: validate targets of jumps

From Florian Westphal <fw@strlen.de>
Newsgroups linux.kernel
Subject Re: [PATCH 3.14 21/29] netfilter: x_tables: validate targets of jumps
Date 2016-06-23 11:20 +0200
Message-ID <rN8XM-7PN-35@gated-at.bofh.it> (permalink)
References <rMYYp-11q-3@gated-at.bofh.it> <rMZKP-1BV-57@gated-at.bofh.it> <rN8Eq-7sH-37@gated-at.bofh.it>
Organization linux.* mail to news gateway

Show all headers | View raw


Florian Westphal <fw@strlen.de> wrote:
> Greg Kroah-Hartman <gregkh@linuxfoundation.org> 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 <fw@strlen.de>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
 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

Back to linux.kernel | Previous | NextPrevious in thread | Next in thread | Find similar | Unroll thread


Thread

[PATCH 3.14 00/29] 3.14.73-stable review Greg Kroah-Hartman <gregkh@linuxfoundation.org> - 2016-06-23 01:30 +0200
  [PATCH 3.14 12/29] fix d_walk()/non-delayed __d_free() race Greg Kroah-Hartman <gregkh@linuxfoundation.org> - 2016-06-23 01:30 +0200
  [PATCH 3.14 14/29] powerpc/pseries/eeh: Handle RTAS delay requests in configure_bridge Greg Kroah-Hartman <gregkh@linuxfoundation.org> - 2016-06-23 01:30 +0200
  [PATCH 3.14 21/29] netfilter: x_tables: validate targets of jumps Greg Kroah-Hartman <gregkh@linuxfoundation.org> - 2016-06-23 01:30 +0200
    Re: [PATCH 3.14 21/29] netfilter: x_tables: validate targets of jumps Florian Westphal <fw@strlen.de> - 2016-06-23 11:00 +0200
      Re: [PATCH 3.14 21/29] netfilter: x_tables: validate targets of jumps Florian Westphal <fw@strlen.de> - 2016-06-23 11:20 +0200
        Re: [PATCH 3.14 21/29] netfilter: x_tables: validate targets of jumps Greg Kroah-Hartman <gregkh@linuxfoundation.org> - 2016-06-24 04:50 +0200
  [PATCH 3.14 02/29] sfc: on MC reset, clear PIO buffer linkage in TXQs Greg Kroah-Hartman <gregkh@linuxfoundation.org> - 2016-06-23 01:30 +0200
  [PATCH 3.14 15/29] netfilter: x_tables: validate e->target_offset early Greg Kroah-Hartman <gregkh@linuxfoundation.org> - 2016-06-23 01:30 +0200
  [PATCH 3.14 16/29] netfilter: x_tables: make sure e->next_offset covers remaining blob size Greg Kroah-Hartman <gregkh@linuxfoundation.org> - 2016-06-23 01:30 +0200
  [PATCH 3.14 08/29] powerpc: Use privileged SPR number for MMCR2 Greg Kroah-Hartman <gregkh@linuxfoundation.org> - 2016-06-23 01:30 +0200
  [PATCH 3.14 09/29] parisc: Fix pagefault crash in unaligned __get_user() call Greg Kroah-Hartman <gregkh@linuxfoundation.org> - 2016-06-23 01:30 +0200
  [PATCH 3.14 11/29] wext: Fix 32 bit iwpriv compatibility issue with 64 bit Kernel Greg Kroah-Hartman <gregkh@linuxfoundation.org> - 2016-06-23 01:40 +0200
  Re: [PATCH 3.14 00/29] 3.14.73-stable review -rc2 Greg Kroah-Hartman <gregkh@linuxfoundation.org> - 2016-06-23 07:00 +0200
    Re: [PATCH 3.14 00/29] 3.14.73-stable review -rc2 Guenter Roeck <linux@roeck-us.net> - 2016-06-23 21:50 +0200
    Re: [PATCH 3.14 00/29] 3.14.73-stable review -rc2 Shuah Khan <shuahkh@osg.samsung.com> - 2016-06-24 00:00 +0200

csiph-web