[linux] 01/01: netfilter: x_tables: speed up jump target validation
debian-kernel at lists.debian.org
debian-kernel at lists.debian.org
Wed Oct 19 04:22:43 UTC 2016
This is an automated email from the git hooks/post-receive script.
carnil pushed a commit to branch jessie-security
in repository linux.
commit f0ef5c2ad7ca39b02c739a45f73bbea89c5d3562
Author: Salvatore Bonaccorso <carnil at debian.org>
Date: Tue Oct 18 08:13:30 2016 +0200
netfilter: x_tables: speed up jump target validation
Closes: #831014
---
debian/changelog | 1 +
...-x_tables-speed-up-jump-target-validation.patch | 493 +++++++++++++++++++++
debian/patches/series | 1 +
3 files changed, 495 insertions(+)
diff --git a/debian/changelog b/debian/changelog
index 903375c..e4507f5 100644
--- a/debian/changelog
+++ b/debian/changelog
@@ -4,6 +4,7 @@ linux (3.16.36-1+deb8u2) UNRELEASED; urgency=medium
* scsi: arcmsr: Buffer overflow in arcmsr_iop_message_xfer() (CVE-2016-7425)
* Bluetooth: Fix potential NULL dereference in RFCOMM bind callback
(CVE-2015-8956)
+ * netfilter: x_tables: speed up jump target validation (Closes: #831014)
-- Salvatore Bonaccorso <carnil at debian.org> Tue, 18 Oct 2016 06:39:49 +0200
diff --git a/debian/patches/bugfix/all/netfilter-x_tables-speed-up-jump-target-validation.patch b/debian/patches/bugfix/all/netfilter-x_tables-speed-up-jump-target-validation.patch
new file mode 100644
index 0000000..e0861f1
--- /dev/null
+++ b/debian/patches/bugfix/all/netfilter-x_tables-speed-up-jump-target-validation.patch
@@ -0,0 +1,493 @@
+From: Florian Westphal <fw at strlen.de>
+Date: Wed, 3 Aug 2016 11:34:46 -0400
+Subject: netfilter: x_tables: speed up jump target validation
+Origin: https://git.kernel.org/cgit/linux/kernel/git/stable/linux-stable.git/patch/?id=f5bba514aff9bb5a7f2ea8e918d8c53684fb6195
+
+[ Upstream commit f4dc77713f8016d2e8a3295e1c9c53a21f296def ]
+
+The dummy ruleset I used to test the original validation change was broken,
+most rules were unreachable and were not tested by mark_source_chains().
+
+In some cases rulesets that used to load in a few seconds now require
+several minutes.
+
+sample ruleset that shows the behaviour:
+
+echo "*filter"
+for i in $(seq 0 100000);do
+ printf ":chain_%06x - [0:0]\n" $i
+done
+for i in $(seq 0 100000);do
+ printf -- "-A INPUT -j chain_%06x\n" $i
+ printf -- "-A INPUT -j chain_%06x\n" $i
+ printf -- "-A INPUT -j chain_%06x\n" $i
+done
+echo COMMIT
+
+[ pipe result into iptables-restore ]
+
+This ruleset will be about 74mbyte in size, with ~500k searches
+though all 500k[1] rule entries. iptables-restore will take forever
+(gave up after 10 minutes)
+
+Instead of always searching the entire blob for a match, fill an
+array with the start offsets of every single ipt_entry struct,
+then do a binary search to check if the jump target is present or not.
+
+After this change ruleset restore times get again close to what one
+gets when reverting 36472341017529e (~3 seconds on my workstation).
+
+[1] every user-defined rule gets an implicit RETURN, so we get
+300k jumps + 100k userchains + 100k returns -> 500k rule entries
+
+Fixes: 36472341017529e ("netfilter: x_tables: validate targets of jumps")
+Reported-by: Jeff Wu <wujiafu at gmail.com>
+Tested-by: Jeff Wu <wujiafu at gmail.com>
+Signed-off-by: Florian Westphal <fw at strlen.de>
+Signed-off-by: Pablo Neira Ayuso <pablo at netfilter.org>
+Signed-off-by: Florian Westphal <fw at strlen.de>
+Signed-off-by: Sasha Levin <alexander.levin at verizon.com>
+[carnil: backport to 3.16, adjust context]
+---
+ include/linux/netfilter/x_tables.h | 4 +++
+ net/ipv4/netfilter/arp_tables.c | 48 ++++++++++++++++++------------------
+ net/ipv4/netfilter/ip_tables.c | 45 ++++++++++++++++++----------------
+ net/ipv6/netfilter/ip6_tables.c | 45 ++++++++++++++++++----------------
+ net/netfilter/x_tables.c | 50 ++++++++++++++++++++++++++++++++++++++
+ 5 files changed, 127 insertions(+), 65 deletions(-)
+
+--- a/include/linux/netfilter/x_tables.h
++++ b/include/linux/netfilter/x_tables.h
+@@ -243,6 +243,10 @@ int xt_check_entry_offsets(const void *b
+ unsigned int target_offset,
+ unsigned int next_offset);
+
++unsigned int *xt_alloc_entry_offsets(unsigned int size);
++bool xt_find_jump_offset(const unsigned int *offsets,
++ unsigned int target, unsigned int size);
++
+ int xt_check_match(struct xt_mtchk_param *, unsigned int size, u_int8_t proto,
+ bool inv_proto);
+ int xt_check_target(struct xt_tgchk_param *, unsigned int size, u_int8_t proto,
+--- a/net/ipv4/netfilter/arp_tables.c
++++ b/net/ipv4/netfilter/arp_tables.c
+@@ -363,24 +363,12 @@ static inline bool unconditional(const s
+ 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.
+ */
+ static int mark_source_chains(const struct xt_table_info *newinfo,
+- unsigned int valid_hooks, void *entry0)
++ unsigned int valid_hooks, void *entry0,
++ unsigned int *offsets)
+ {
+ unsigned int hook;
+
+@@ -469,10 +457,11 @@ static int mark_source_chains(const stru
+ /* This a jump; chase it. */
+ duprintf("Jump rule %u -> %u\n",
+ pos, newpos);
++ if (!xt_find_jump_offset(offsets, newpos,
++ newinfo->number))
++ return 0;
+ 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;
+@@ -632,6 +621,7 @@ static int translate_table(struct xt_tab
+ const struct arpt_replace *repl)
+ {
+ struct arpt_entry *iter;
++ unsigned int *offsets;
+ unsigned int i;
+ int ret = 0;
+
+@@ -645,8 +635,10 @@ static int translate_table(struct xt_tab
+ }
+
+ duprintf("translate_table: size %u\n", newinfo->size);
++ offsets = xt_alloc_entry_offsets(newinfo->number);
++ if (!offsets)
++ return -ENOMEM;
+ i = 0;
+-
+ /* Walk through entries, checking offsets. */
+ xt_entry_foreach(iter, entry0, newinfo->size) {
+ ret = check_entry_size_and_hooks(iter, newinfo, entry0,
+@@ -655,7 +647,9 @@ static int translate_table(struct xt_tab
+ repl->underflow,
+ repl->valid_hooks);
+ if (ret != 0)
+- break;
++ goto out_free;
++ if (i < repl->num_entries)
++ offsets[i] = (void *)iter - entry0;
+ ++i;
+ if (strcmp(arpt_get_target(iter)->u.user.name,
+ XT_ERROR_TARGET) == 0)
+@@ -663,12 +657,13 @@ static int translate_table(struct xt_tab
+ }
+ duprintf("translate_table: ARPT_ENTRY_ITERATE gives %d\n", ret);
+ if (ret != 0)
+- return ret;
++ goto out_free;
+
++ ret = -EINVAL;
+ if (i != repl->num_entries) {
+ duprintf("translate_table: %u not %u entries\n",
+ i, repl->num_entries);
+- return -EINVAL;
++ goto out_free;
+ }
+
+ /* Check hooks all assigned */
+@@ -679,17 +674,20 @@ static int translate_table(struct xt_tab
+ if (newinfo->hook_entry[i] == 0xFFFFFFFF) {
+ duprintf("Invalid hook entry %u %u\n",
+ i, repl->hook_entry[i]);
+- return -EINVAL;
++ goto out_free;
+ }
+ if (newinfo->underflow[i] == 0xFFFFFFFF) {
+ duprintf("Invalid underflow %u %u\n",
+ i, repl->underflow[i]);
+- return -EINVAL;
++ goto out_free;
+ }
+ }
+
+- if (!mark_source_chains(newinfo, repl->valid_hooks, entry0))
+- return -ELOOP;
++ if (!mark_source_chains(newinfo, repl->valid_hooks, entry0, offsets)) {
++ ret = -ELOOP;
++ goto out_free;
++ }
++ kvfree(offsets);
+
+ /* Finally, each sanity check must pass */
+ i = 0;
+@@ -716,6 +714,9 @@ static int translate_table(struct xt_tab
+ }
+
+ return ret;
++ out_free:
++ kvfree(offsets);
++ return ret;
+ }
+
+ static void get_counters(const struct xt_table_info *t,
+--- a/net/ipv4/netfilter/ip_tables.c
++++ b/net/ipv4/netfilter/ip_tables.c
+@@ -439,24 +439,12 @@ 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
+ mark_source_chains(const struct xt_table_info *newinfo,
+- unsigned int valid_hooks, void *entry0)
++ unsigned int valid_hooks, void *entry0,
++ unsigned int *offsets)
+ {
+ unsigned int hook;
+
+@@ -549,10 +537,11 @@ mark_source_chains(const struct xt_table
+ /* This a jump; chase it. */
+ duprintf("Jump rule %u -> %u\n",
+ pos, newpos);
++ if (!xt_find_jump_offset(offsets, newpos,
++ newinfo->number))
++ return 0;
+ 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;
+@@ -799,6 +788,7 @@ translate_table(struct net *net, struct
+ const struct ipt_replace *repl)
+ {
+ struct ipt_entry *iter;
++ unsigned int *offsets;
+ unsigned int i;
+ int ret = 0;
+
+@@ -812,6 +802,9 @@ translate_table(struct net *net, struct
+ }
+
+ duprintf("translate_table: size %u\n", newinfo->size);
++ offsets = xt_alloc_entry_offsets(newinfo->number);
++ if (!offsets)
++ return -ENOMEM;
+ i = 0;
+ /* Walk through entries, checking offsets. */
+ xt_entry_foreach(iter, entry0, newinfo->size) {
+@@ -821,17 +814,20 @@ translate_table(struct net *net, struct
+ repl->underflow,
+ repl->valid_hooks);
+ if (ret != 0)
+- return ret;
++ goto out_free;
++ if (i < repl->num_entries)
++ offsets[i] = (void *)iter - entry0;
+ ++i;
+ if (strcmp(ipt_get_target(iter)->u.user.name,
+ XT_ERROR_TARGET) == 0)
+ ++newinfo->stacksize;
+ }
+
++ ret = -EINVAL;
+ if (i != repl->num_entries) {
+ duprintf("translate_table: %u not %u entries\n",
+ i, repl->num_entries);
+- return -EINVAL;
++ goto out_free;
+ }
+
+ /* Check hooks all assigned */
+@@ -842,17 +838,20 @@ translate_table(struct net *net, struct
+ if (newinfo->hook_entry[i] == 0xFFFFFFFF) {
+ duprintf("Invalid hook entry %u %u\n",
+ i, repl->hook_entry[i]);
+- return -EINVAL;
++ goto out_free;
+ }
+ if (newinfo->underflow[i] == 0xFFFFFFFF) {
+ duprintf("Invalid underflow %u %u\n",
+ i, repl->underflow[i]);
+- return -EINVAL;
++ goto out_free;
+ }
+ }
+
+- if (!mark_source_chains(newinfo, repl->valid_hooks, entry0))
+- return -ELOOP;
++ if (!mark_source_chains(newinfo, repl->valid_hooks, entry0, offsets)) {
++ ret = -ELOOP;
++ goto out_free;
++ }
++ kvfree(offsets);
+
+ /* Finally, each sanity check must pass */
+ i = 0;
+@@ -879,6 +878,9 @@ translate_table(struct net *net, struct
+ }
+
+ return ret;
++ out_free:
++ kvfree(offsets);
++ return ret;
+ }
+
+ static void
+--- a/net/ipv6/netfilter/ip6_tables.c
++++ b/net/ipv6/netfilter/ip6_tables.c
+@@ -449,24 +449,12 @@ 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
+ mark_source_chains(const struct xt_table_info *newinfo,
+- unsigned int valid_hooks, void *entry0)
++ unsigned int valid_hooks, void *entry0,
++ unsigned int *offsets)
+ {
+ unsigned int hook;
+
+@@ -559,10 +547,11 @@ mark_source_chains(const struct xt_table
+ /* This a jump; chase it. */
+ duprintf("Jump rule %u -> %u\n",
+ pos, newpos);
++ if (!xt_find_jump_offset(offsets, newpos,
++ newinfo->number))
++ return 0;
+ 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;
+@@ -809,6 +798,7 @@ translate_table(struct net *net, struct
+ const struct ip6t_replace *repl)
+ {
+ struct ip6t_entry *iter;
++ unsigned int *offsets;
+ unsigned int i;
+ int ret = 0;
+
+@@ -822,6 +812,9 @@ translate_table(struct net *net, struct
+ }
+
+ duprintf("translate_table: size %u\n", newinfo->size);
++ offsets = xt_alloc_entry_offsets(newinfo->number);
++ if (!offsets)
++ return -ENOMEM;
+ i = 0;
+ /* Walk through entries, checking offsets. */
+ xt_entry_foreach(iter, entry0, newinfo->size) {
+@@ -831,17 +824,20 @@ translate_table(struct net *net, struct
+ repl->underflow,
+ repl->valid_hooks);
+ if (ret != 0)
+- return ret;
++ goto out_free;
++ if (i < repl->num_entries)
++ offsets[i] = (void *)iter - entry0;
+ ++i;
+ if (strcmp(ip6t_get_target(iter)->u.user.name,
+ XT_ERROR_TARGET) == 0)
+ ++newinfo->stacksize;
+ }
+
++ ret = -EINVAL;
+ if (i != repl->num_entries) {
+ duprintf("translate_table: %u not %u entries\n",
+ i, repl->num_entries);
+- return -EINVAL;
++ goto out_free;
+ }
+
+ /* Check hooks all assigned */
+@@ -852,17 +848,20 @@ translate_table(struct net *net, struct
+ if (newinfo->hook_entry[i] == 0xFFFFFFFF) {
+ duprintf("Invalid hook entry %u %u\n",
+ i, repl->hook_entry[i]);
+- return -EINVAL;
++ goto out_free;
+ }
+ if (newinfo->underflow[i] == 0xFFFFFFFF) {
+ duprintf("Invalid underflow %u %u\n",
+ i, repl->underflow[i]);
+- return -EINVAL;
++ goto out_free;
+ }
+ }
+
+- if (!mark_source_chains(newinfo, repl->valid_hooks, entry0))
+- return -ELOOP;
++ if (!mark_source_chains(newinfo, repl->valid_hooks, entry0, offsets)) {
++ ret = -ELOOP;
++ goto out_free;
++ }
++ kvfree(offsets);
+
+ /* Finally, each sanity check must pass */
+ i = 0;
+@@ -889,6 +888,9 @@ translate_table(struct net *net, struct
+ }
+
+ return ret;
++ out_free:
++ kvfree(offsets);
++ return ret;
+ }
+
+ static void
+--- a/net/netfilter/x_tables.c
++++ b/net/netfilter/x_tables.c
+@@ -721,6 +721,56 @@ int xt_check_entry_offsets(const void *b
+ }
+ EXPORT_SYMBOL(xt_check_entry_offsets);
+
++/**
++ * xt_alloc_entry_offsets - allocate array to store rule head offsets
++ *
++ * @size: number of entries
++ *
++ * Return: NULL or kmalloc'd or vmalloc'd array
++ */
++unsigned int *xt_alloc_entry_offsets(unsigned int size)
++{
++ unsigned int *off;
++
++ off = kcalloc(size, sizeof(unsigned int), GFP_KERNEL | __GFP_NOWARN);
++
++ if (off)
++ return off;
++
++ if (size < (SIZE_MAX / sizeof(unsigned int)))
++ off = vmalloc(size * sizeof(unsigned int));
++
++ return off;
++}
++EXPORT_SYMBOL(xt_alloc_entry_offsets);
++
++/**
++ * xt_find_jump_offset - check if target is a valid jump offset
++ *
++ * @offsets: array containing all valid rule start offsets of a rule blob
++ * @target: the jump target to search for
++ * @size: entries in @offset
++ */
++bool xt_find_jump_offset(const unsigned int *offsets,
++ unsigned int target, unsigned int size)
++{
++ int m, low = 0, hi = size;
++
++ while (hi > low) {
++ m = (low + hi) / 2u;
++
++ if (offsets[m] > target)
++ hi = m;
++ else if (offsets[m] < target)
++ low = m + 1;
++ else
++ return true;
++ }
++
++ return false;
++}
++EXPORT_SYMBOL(xt_find_jump_offset);
++
+ int xt_check_target(struct xt_tgchk_param *par,
+ unsigned int size, u_int8_t proto, bool inv_proto)
+ {
diff --git a/debian/patches/series b/debian/patches/series
index 6562c67..62fdece 100644
--- a/debian/patches/series
+++ b/debian/patches/series
@@ -680,6 +680,7 @@ bugfix/all/aacraid-Check-size-values-after-double-fetch-from-us.patch
bugfix/all/KEYS-Fix-short-sprintf-buffer-in-proc-keys-show-func.patch
bugfix/all/scsi-arcmsr-Buffer-overflow-in-arcmsr_iop_message_xf.patch
bugfix/all/bluetooth-fix-potential-null-dereference-in-rfcomm-b.patch
+bugfix/all/netfilter-x_tables-speed-up-jump-target-validation.patch
# Fix ABI changes
debian/of-fix-abi-changes.patch
--
Alioth's /usr/local/bin/git-commit-notice on /srv/git.debian.org/git/kernel/linux.git
More information about the Kernel-svn-changes
mailing list