From: Chris Wright Date: Wed, 29 Nov 2006 17:52:10 +0000 (-0800) Subject: replace netfilter-missed-and-reordered-checks-in-arp-ip-ip6-_tables.patch X-Git-Tag: v2.6.18.4~2 X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=efe72a68b1b22d87d3446abe854bf475d6363e78;p=thirdparty%2Fkernel%2Fstable-queue.git replace netfilter-missed-and-reordered-checks-in-arp-ip-ip6-_tables.patch with fixed patch from Patrick, and fix order of netfilter patches to match submission order --- diff --git a/queue-2.6.18/netfilter-missed-and-reordered-checks-in-arp-ip-ip6-_tables.patch b/queue-2.6.18/netfilter-missed-and-reordered-checks-in-arp-ip-ip6-_tables.patch index 495e70f4fca..ed7e0b6bf47 100644 --- a/queue-2.6.18/netfilter-missed-and-reordered-checks-in-arp-ip-ip6-_tables.patch +++ b/queue-2.6.18/netfilter-missed-and-reordered-checks-in-arp-ip-ip6-_tables.patch @@ -1,10 +1,13 @@ -From stable-bounces@linux.kernel.org Thu Nov 16 21:40:15 2006 +From stable-bounces@linux.kernel.org Tue Nov 21 02:39:12 2006 +Message-ID: <4562D63F.5060207@trash.net> +Date: Tue, 21 Nov 2006 11:34:39 +0100 From: Patrick McHardy -To: stable@kernel.org -Message-Id: <20061117053545.10231.94652.sendpatchset@localhost.localdomain> -Date: Fri, 17 Nov 2006 06:35:45 +0100 (MET) -Cc: Patrick McHardy , davem@davemloft.net -Subject: NETFILTER: Missed and reordered checks in {arp, ip, ip6}_tables +To: Chris Wright +Cc: dim@openvz.org, dev@openvz.org, davem@davemloft.net, stable@kernel.org +Subject: NETFILTER: Missed and reordered checks in {arp,ip,ip6}_tables + +Backport fix for missing ruleset validation in {arp,ip,ip6}_tables +and a fix on top which fixes a regression in the first patch. There is a number of issues in parsing user-provided table in translate_table(). Malicious user with CAP_NET_ADMIN may crash system by @@ -23,27 +26,93 @@ ends. As a result, we'll have oops or memory disclosure. And the third issue, that there is no check that the target is completely inside entry. Results are the same, as in previous issue. -Signed-off-by: Dmitry Mishin -Acked-by: Kirill Korotaev -Signed-off-by: Patrick McHardy -Signed-off-by: David S. Miller +Upstream commit 590bdf7fd2292b47c428111cb1360e312eff207e introduced a +regression in match/target hook validation. mark_source_chains builds +a bitmask for each rule representing the hooks it can be reached from, +which is then used by the matches and targets to make sure they are +only called from valid hooks. The patch moved the match/target specific +validation before the mark_source_chains call, at which point the mask +is always zero. + +This patch returns back to the old order and moves the standard checks +to mark_source_chains. This allows to get rid of a special case for +standard targets as a nice side-effect. + +Signed-off-by: Patrick McHardy Signed-off-by: Chris Wright --- -commit 0ef4760e162ea44c847cca7393b36e5bcac5414e -tree 7036ce51d75aaf46d5c4abca281956c39caced10 -parent 94a3d63f9ca6cb404f62ee4186d20fec3e8bdc97 -author Patrick McHardy Fri, 17 Nov 2006 06:24:10 +0100 -committer Patrick McHardy Fri, 17 Nov 2006 06:24:10 +0100 +commit 1cfcb663c5a6d8b4b1172ff481af1b597bc8b54e +tree 61c5b135ee292681f38945a3549cb9005aec1d7c +parent b2ab160e1a3a1eb3fcc80132d8d7db5687a7a113 +author Patrick McHardy Tue, 21 Nov 2006 11:17:03 +0100 +committer Patrick McHardy Tue, 21 Nov 2006 11:24:51 +0100 - net/ipv4/netfilter/arp_tables.c | 25 ++++++++++++++++--------- - net/ipv4/netfilter/ip_tables.c | 30 ++++++++++++++++++++++-------- - net/ipv6/netfilter/ip6_tables.c | 24 ++++++++++++++++-------- - 3 files changed, 54 insertions(+), 25 deletions(-) + net/ipv4/netfilter/arp_tables.c | 37 +++++++++++++--------- + net/ipv4/netfilter/ip_tables.c | 65 +++++++++++++++++++--------------------- + net/ipv6/netfilter/ip6_tables.c | 53 ++++++++++++++------------------ + 3 files changed, 77 insertions(+), 78 deletions(-) --- linux-2.6.18.3.orig/net/ipv4/netfilter/arp_tables.c +++ linux-2.6.18.3/net/ipv4/netfilter/arp_tables.c -@@ -471,7 +471,13 @@ static inline int check_entry(struct arp +@@ -380,6 +380,13 @@ static int mark_source_chains(struct xt_ + && unconditional(&e->arp)) { + unsigned int oldpos, size; + ++ if (t->verdict < -NF_MAX_VERDICT - 1) { ++ duprintf("mark_source_chains: bad " ++ "negative verdict (%i)\n", ++ t->verdict); ++ return 0; ++ } ++ + /* Return: backtrack through the last + * big jump. + */ +@@ -409,6 +416,14 @@ static int mark_source_chains(struct xt_ + if (strcmp(t->target.u.user.name, + ARPT_STANDARD_TARGET) == 0 + && newpos >= 0) { ++ if (newpos > newinfo->size - ++ sizeof(struct arpt_entry)) { ++ duprintf("mark_source_chains: " ++ "bad verdict (%i)\n", ++ newpos); ++ return 0; ++ } ++ + /* This a jump; chase it. */ + duprintf("Jump rule %u -> %u\n", + pos, newpos); +@@ -431,8 +446,6 @@ static int mark_source_chains(struct xt_ + static inline int standard_check(const struct arpt_entry_target *t, + unsigned int max_offset) + { +- struct arpt_standard_target *targ = (void *)t; +- + /* Check standard info. */ + if (t->u.target_size + != ARPT_ALIGN(sizeof(struct arpt_standard_target))) { +@@ -442,18 +455,6 @@ static inline int standard_check(const s + return 0; + } + +- if (targ->verdict >= 0 +- && targ->verdict > max_offset - sizeof(struct arpt_entry)) { +- duprintf("arpt_standard_check: bad verdict (%i)\n", +- targ->verdict); +- return 0; +- } +- +- if (targ->verdict < -NF_MAX_VERDICT - 1) { +- duprintf("arpt_standard_check: bad negative verdict (%i)\n", +- targ->verdict); +- return 0; +- } + return 1; + } + +@@ -471,7 +472,13 @@ static inline int check_entry(struct arp return -EINVAL; } @@ -57,47 +126,74 @@ committer Patrick McHardy Fri, 17 Nov 2006 06:24:10 +0100 target = try_then_request_module(xt_find_target(NF_ARP, t->u.user.name, t->u.user.revision), "arpt_%s", t->u.user.name); -@@ -629,20 +635,18 @@ static int translate_table(const char *n - } - } +@@ -641,7 +648,7 @@ static int translate_table(const char *n -- if (!mark_source_chains(newinfo, valid_hooks, entry0)) { -- duprintf("Looping hook\n"); -- return -ELOOP; -- } -- - /* Finally, each sanity check must pass */ - i = 0; - ret = ARPT_ENTRY_ITERATE(entry0, newinfo->size, - check_entry, name, size, &i); - -- if (ret != 0) { -- ARPT_ENTRY_ITERATE(entry0, newinfo->size, + if (ret != 0) { + ARPT_ENTRY_ITERATE(entry0, newinfo->size, - cleanup_entry, &i); -- return ret; -+ if (ret != 0) -+ goto cleanup; -+ -+ ret = -ELOOP; -+ if (!mark_source_chains(newinfo, valid_hooks, entry0)) { -+ duprintf("Looping hook\n"); -+ goto cleanup; ++ cleanup_entry, &i); + return ret; } - /* And one copy for every other CPU */ -@@ -651,6 +655,9 @@ static int translate_table(const char *n - memcpy(newinfo->entries[i], entry0, newinfo->size); - } +--- linux-2.6.18.3.orig/net/ipv4/netfilter/ip_tables.c ++++ linux-2.6.18.3/net/ipv4/netfilter/ip_tables.c +@@ -404,6 +404,13 @@ mark_source_chains(struct xt_table_info + && unconditional(&e->ip)) { + unsigned int oldpos, size; -+ return 0; -+cleanup: -+ ARPT_ENTRY_ITERATE(entry0, newinfo->size, cleanup_entry, &i); - return ret; ++ if (t->verdict < -NF_MAX_VERDICT - 1) { ++ duprintf("mark_source_chains: bad " ++ "negative verdict (%i)\n", ++ t->verdict); ++ return 0; ++ } ++ + /* Return: backtrack through the last + big jump. */ + do { +@@ -441,6 +448,13 @@ mark_source_chains(struct xt_table_info + if (strcmp(t->target.u.user.name, + IPT_STANDARD_TARGET) == 0 + && newpos >= 0) { ++ if (newpos > newinfo->size - ++ sizeof(struct ipt_entry)) { ++ duprintf("mark_source_chains: " ++ "bad verdict (%i)\n", ++ newpos); ++ return 0; ++ } + /* This a jump; chase it. */ + duprintf("Jump rule %u -> %u\n", + pos, newpos); +@@ -474,27 +488,6 @@ cleanup_match(struct ipt_entry_match *m, } ---- linux-2.6.18.3.orig/net/ipv4/netfilter/ip_tables.c -+++ linux-2.6.18.3/net/ipv4/netfilter/ip_tables.c -@@ -552,12 +552,18 @@ check_entry(struct ipt_entry *e, const c + static inline int +-standard_check(const struct ipt_entry_target *t, +- unsigned int max_offset) +-{ +- struct ipt_standard_target *targ = (void *)t; +- +- /* Check standard info. */ +- if (targ->verdict >= 0 +- && targ->verdict > max_offset - sizeof(struct ipt_entry)) { +- duprintf("ipt_standard_check: bad verdict (%i)\n", +- targ->verdict); +- return 0; +- } +- if (targ->verdict < -NF_MAX_VERDICT - 1) { +- duprintf("ipt_standard_check: bad negative verdict (%i)\n", +- targ->verdict); +- return 0; +- } +- return 1; +-} +- +-static inline int + check_match(struct ipt_entry_match *m, + const char *name, + const struct ipt_ip *ip, +@@ -552,12 +545,18 @@ check_entry(struct ipt_entry *e, const c return -EINVAL; } @@ -116,43 +212,30 @@ committer Patrick McHardy Fri, 17 Nov 2006 06:24:10 +0100 target = try_then_request_module(xt_find_target(AF_INET, t->u.user.name, t->u.user.revision), -@@ -720,19 +726,17 @@ translate_table(const char *name, - } - } +@@ -575,12 +574,7 @@ check_entry(struct ipt_entry *e, const c + if (ret) + goto err; -- if (!mark_source_chains(newinfo, valid_hooks, entry0)) -- return -ELOOP; -- - /* Finally, each sanity check must pass */ - i = 0; - ret = IPT_ENTRY_ITERATE(entry0, newinfo->size, - check_entry, name, size, &i); +- if (t->u.kernel.target == &ipt_standard_target) { +- if (!standard_check(t, size)) { +- ret = -EINVAL; +- goto cleanup_matches; +- } +- } else if (t->u.kernel.target->checkentry ++ if (t->u.kernel.target->checkentry + && !t->u.kernel.target->checkentry(name, e, target, t->data, + t->u.target_size + - sizeof(*t), +@@ -730,7 +724,7 @@ translate_table(const char *name, -- if (ret != 0) { -- IPT_ENTRY_ITERATE(entry0, newinfo->size, + if (ret != 0) { + IPT_ENTRY_ITERATE(entry0, newinfo->size, - cleanup_entry, &i); -- return ret; -- } -+ if (ret != 0) -+ goto cleanup; -+ -+ ret = -ELOOP; -+ if (!mark_source_chains(newinfo, valid_hooks, entry0)) -+ goto cleanup; - - /* And one copy for every other CPU */ - for_each_possible_cpu(i) { -@@ -740,6 +744,9 @@ translate_table(const char *name, - memcpy(newinfo->entries[i], entry0, newinfo->size); ++ cleanup_entry, &i); + return ret; } -+ return 0; -+cleanup: -+ IPT_ENTRY_ITERATE(entry0, newinfo->size, cleanup_entry, &i); - return ret; - } - -@@ -1531,6 +1538,10 @@ check_compat_entry_size_and_hooks(struct +@@ -1531,6 +1525,10 @@ check_compat_entry_size_and_hooks(struct return -EINVAL; } @@ -163,7 +246,7 @@ committer Patrick McHardy Fri, 17 Nov 2006 06:24:10 +0100 off = 0; entry_offset = (void *)e - (void *)base; j = 0; -@@ -1540,6 +1551,9 @@ check_compat_entry_size_and_hooks(struct +@@ -1540,6 +1538,9 @@ check_compat_entry_size_and_hooks(struct goto cleanup_matches; t = ipt_get_target(e); @@ -173,9 +256,87 @@ committer Patrick McHardy Fri, 17 Nov 2006 06:24:10 +0100 target = try_then_request_module(xt_find_target(AF_INET, t->u.user.name, t->u.user.revision), +@@ -1656,19 +1657,15 @@ static int compat_copy_entry_from_user(s + if (ret) + goto err; + +- ret = -EINVAL; +- if (t->u.kernel.target == &ipt_standard_target) { +- if (!standard_check(t, *size)) +- goto err; +- } else if (t->u.kernel.target->checkentry ++ if (t->u.kernel.target->checkentry + && !t->u.kernel.target->checkentry(name, de, target, + t->data, t->u.target_size - sizeof(*t), + de->comefrom)) { + duprintf("ip_tables: compat: check failed for `%s'.\n", + t->u.kernel.target->name); ++ ret = -EINVAL; + goto err; + } +- ret = 0; + err: + return ret; + } --- linux-2.6.18.3.orig/net/ipv6/netfilter/ip6_tables.c +++ linux-2.6.18.3/net/ipv6/netfilter/ip6_tables.c -@@ -592,12 +592,19 @@ check_entry(struct ip6t_entry *e, const +@@ -444,6 +444,13 @@ mark_source_chains(struct xt_table_info + && unconditional(&e->ipv6)) { + unsigned int oldpos, size; + ++ if (t->verdict < -NF_MAX_VERDICT - 1) { ++ duprintf("mark_source_chains: bad " ++ "negative verdict (%i)\n", ++ t->verdict); ++ return 0; ++ } ++ + /* Return: backtrack through the last + big jump. */ + do { +@@ -481,6 +488,13 @@ mark_source_chains(struct xt_table_info + if (strcmp(t->target.u.user.name, + IP6T_STANDARD_TARGET) == 0 + && newpos >= 0) { ++ if (newpos > newinfo->size - ++ sizeof(struct ip6t_entry)) { ++ duprintf("mark_source_chains: " ++ "bad verdict (%i)\n", ++ newpos); ++ return 0; ++ } + /* This a jump; chase it. */ + duprintf("Jump rule %u -> %u\n", + pos, newpos); +@@ -514,27 +528,6 @@ cleanup_match(struct ip6t_entry_match *m + } + + static inline int +-standard_check(const struct ip6t_entry_target *t, +- unsigned int max_offset) +-{ +- struct ip6t_standard_target *targ = (void *)t; +- +- /* Check standard info. */ +- if (targ->verdict >= 0 +- && targ->verdict > max_offset - sizeof(struct ip6t_entry)) { +- duprintf("ip6t_standard_check: bad verdict (%i)\n", +- targ->verdict); +- return 0; +- } +- if (targ->verdict < -NF_MAX_VERDICT - 1) { +- duprintf("ip6t_standard_check: bad negative verdict (%i)\n", +- targ->verdict); +- return 0; +- } +- return 1; +-} +- +-static inline int + check_match(struct ip6t_entry_match *m, + const char *name, + const struct ip6t_ip6 *ipv6, +@@ -592,12 +585,19 @@ check_entry(struct ip6t_entry *e, const return -EINVAL; } @@ -195,39 +356,35 @@ committer Patrick McHardy Fri, 17 Nov 2006 06:24:10 +0100 target = try_then_request_module(xt_find_target(AF_INET6, t->u.user.name, t->u.user.revision), -@@ -760,19 +767,17 @@ translate_table(const char *name, - } - } +@@ -615,12 +615,7 @@ check_entry(struct ip6t_entry *e, const + if (ret) + goto err; -- if (!mark_source_chains(newinfo, valid_hooks, entry0)) -- return -ELOOP; -- - /* Finally, each sanity check must pass */ - i = 0; - ret = IP6T_ENTRY_ITERATE(entry0, newinfo->size, - check_entry, name, size, &i); +- if (t->u.kernel.target == &ip6t_standard_target) { +- if (!standard_check(t, size)) { +- ret = -EINVAL; +- goto cleanup_matches; +- } +- } else if (t->u.kernel.target->checkentry ++ if (t->u.kernel.target->checkentry + && !t->u.kernel.target->checkentry(name, e, target, t->data, + t->u.target_size + - sizeof(*t), +@@ -770,7 +765,7 @@ translate_table(const char *name, -- if (ret != 0) { -- IP6T_ENTRY_ITERATE(entry0, newinfo->size, + if (ret != 0) { + IP6T_ENTRY_ITERATE(entry0, newinfo->size, - cleanup_entry, &i); -- return ret; -- } -+ if (ret != 0) -+ goto cleanup; -+ -+ ret = -ELOOP; -+ if (!mark_source_chains(newinfo, valid_hooks, entry0)) -+ goto cleanup; ++ cleanup_entry, &i); + return ret; + } - /* And one copy for every other CPU */ - for_each_possible_cpu(i) { -@@ -780,6 +785,9 @@ translate_table(const char *name, +@@ -780,7 +775,7 @@ translate_table(const char *name, memcpy(newinfo->entries[i], entry0, newinfo->size); } +- return ret; + return 0; -+cleanup: -+ IP6T_ENTRY_ITERATE(entry0, newinfo->size, cleanup_entry, &i); - return ret; } + /* Gets counters. */ diff --git a/queue-2.6.18/series b/queue-2.6.18/series index d920557bf5b..e5ac9853e78 100644 --- a/queue-2.6.18/series +++ b/queue-2.6.18/series @@ -4,7 +4,7 @@ netfilter-ip_tables-compat-error-way-cleanup.patch netfilter-ip_tables-fix-module-refcount-leaks-in-compat-error-paths.patch netfilter-missed-and-reordered-checks-in-arp-ip-ip6-_tables.patch netfilter-arp_tables-missing-unregistration-on-module-unload.patch -netfilter-kconfig-fix-xt_physdev-dependencies.patch netfilter-honour-source-routing-for-lvs-nat.patch +netfilter-kconfig-fix-xt_physdev-dependencies.patch netfilter-xt_connsecmark-fix-kconfig-dependencies.patch bcm43xx-drain-tx-status-before-starting-irqs.patch