From 1e2530a0ed46dc6d9a50654097d247d05ef3e9cb Mon Sep 17 00:00:00 2001 From: Ben Hutchings Date: Mon, 14 Dec 2009 02:03:57 +0000 Subject: [PATCH] netfilter: xtables: fix conntrack match v1 ipt-save output (Closes: #556587) svn path=/dists/trunk/linux-2.6/; revision=14785 --- debian/changelog | 2 + ...x-conntrack-match-v1-ipt-save-output.patch | 175 ++++++++++++++++++ debian/patches/series/2 | 1 + 3 files changed, 178 insertions(+) create mode 100644 debian/patches/bugfix/all/netfilter-xtables-fix-conntrack-match-v1-ipt-save-output.patch diff --git a/debian/changelog b/debian/changelog index edd57959d..0e811e7fc 100644 --- a/debian/changelog +++ b/debian/changelog @@ -17,6 +17,8 @@ linux-2.6 (2.6.32-2) UNRELEASED; urgency=low * postinst: Fix failure paths in check for missing firmware (Closes: #560263) * atl1c: Fix system hang when link drops (Closes: #559577) + * netfilter: xtables: fix conntrack match v1 ipt-save output + (Closes: #556587) [ Aurelien Jarno ] * Add support for the sparc64 architecture. diff --git a/debian/patches/bugfix/all/netfilter-xtables-fix-conntrack-match-v1-ipt-save-output.patch b/debian/patches/bugfix/all/netfilter-xtables-fix-conntrack-match-v1-ipt-save-output.patch new file mode 100644 index 000000000..080a72263 --- /dev/null +++ b/debian/patches/bugfix/all/netfilter-xtables-fix-conntrack-match-v1-ipt-save-output.patch @@ -0,0 +1,175 @@ +From 3a0429292daa0e1ec848bd26479f5e48b0d54a42 Mon Sep 17 00:00:00 2001 +From: Florian Westphal +Date: Mon, 23 Nov 2009 10:43:57 +0100 +Subject: [PATCH] netfilter: xtables: fix conntrack match v1 ipt-save output + +commit d6d3f08b0fd998b647a05540cedd11a067b72867 +(netfilter: xtables: conntrack match revision 2) does break the +v1 conntrack match iptables-save output in a subtle way. + +Problem is as follows: + + up = kmalloc(sizeof(*up), GFP_KERNEL); +[..] + /* + * The strategy here is to minimize the overhead of v1 matching, + * by prebuilding a v2 struct and putting the pointer into the + * v1 dataspace. + */ + memcpy(up, info, offsetof(typeof(*info), state_mask)); +[..] + *(void **)info = up; + +As the v2 struct pointer is saved in the match data space, +it clobbers the first structure member (->origsrc_addr). + +Because the _v1 match function grabs this pointer and does not actually +look at the v1 origsrc, run time functionality does not break. +But iptables -nvL (or iptables-save) cannot know that v1 origsrc_addr +has been overloaded in this way: + +$ iptables -p tcp -A OUTPUT -m conntrack --ctorigsrc 10.0.0.1 -j ACCEPT +$ iptables-save +-A OUTPUT -p tcp -m conntrack --ctorigsrc 128.173.134.206 -j ACCEPT + +(128.173... is the address to the v2 match structure). + +To fix this, we take advantage of the fact that the v1 and v2 structures +are identical with exception of the last two structure members (u8 in v1, +u16 in v2). + +We extract them as early as possible and prevent the v2 matching function +from looking at those two members directly. + +Previously reported by Michel Messerschmidt via Ben Hutchings, also +see Debian Bug tracker #556587. + +Signed-off-by: Florian Westphal +Signed-off-by: Patrick McHardy +--- + net/netfilter/xt_conntrack.c | 61 +++++++++++------------------------------ + 1 files changed, 17 insertions(+), 44 deletions(-) + +diff --git a/net/netfilter/xt_conntrack.c b/net/netfilter/xt_conntrack.c +index 6dc4652..ae66305 100644 +--- a/net/netfilter/xt_conntrack.c ++++ b/net/netfilter/xt_conntrack.c +@@ -113,7 +113,8 @@ ct_proto_port_check(const struct xt_conntrack_mtinfo2 *info, + } + + static bool +-conntrack_mt(const struct sk_buff *skb, const struct xt_match_param *par) ++conntrack_mt(const struct sk_buff *skb, const struct xt_match_param *par, ++ u16 state_mask, u16 status_mask) + { + const struct xt_conntrack_mtinfo2 *info = par->matchinfo; + enum ip_conntrack_info ctinfo; +@@ -136,7 +137,7 @@ conntrack_mt(const struct sk_buff *skb, const struct xt_match_param *par) + if (test_bit(IPS_DST_NAT_BIT, &ct->status)) + statebit |= XT_CONNTRACK_STATE_DNAT; + } +- if (!!(info->state_mask & statebit) ^ ++ if (!!(state_mask & statebit) ^ + !(info->invert_flags & XT_CONNTRACK_STATE)) + return false; + } +@@ -172,7 +173,7 @@ conntrack_mt(const struct sk_buff *skb, const struct xt_match_param *par) + return false; + + if ((info->match_flags & XT_CONNTRACK_STATUS) && +- (!!(info->status_mask & ct->status) ^ ++ (!!(status_mask & ct->status) ^ + !(info->invert_flags & XT_CONNTRACK_STATUS))) + return false; + +@@ -192,11 +193,17 @@ conntrack_mt(const struct sk_buff *skb, const struct xt_match_param *par) + static bool + conntrack_mt_v1(const struct sk_buff *skb, const struct xt_match_param *par) + { +- const struct xt_conntrack_mtinfo2 *const *info = par->matchinfo; +- struct xt_match_param newpar = *par; ++ const struct xt_conntrack_mtinfo1 *info = par->matchinfo; + +- newpar.matchinfo = *info; +- return conntrack_mt(skb, &newpar); ++ return conntrack_mt(skb, par, info->state_mask, info->status_mask); ++} ++ ++static bool ++conntrack_mt_v2(const struct sk_buff *skb, const struct xt_match_param *par) ++{ ++ const struct xt_conntrack_mtinfo2 *info = par->matchinfo; ++ ++ return conntrack_mt(skb, par, info->state_mask, info->status_mask); + } + + static bool conntrack_mt_check(const struct xt_mtchk_param *par) +@@ -209,45 +216,11 @@ static bool conntrack_mt_check(const struct xt_mtchk_param *par) + return true; + } + +-static bool conntrack_mt_check_v1(const struct xt_mtchk_param *par) +-{ +- struct xt_conntrack_mtinfo1 *info = par->matchinfo; +- struct xt_conntrack_mtinfo2 *up; +- int ret = conntrack_mt_check(par); +- +- if (ret < 0) +- return ret; +- +- up = kmalloc(sizeof(*up), GFP_KERNEL); +- if (up == NULL) { +- nf_ct_l3proto_module_put(par->family); +- return -ENOMEM; +- } +- +- /* +- * The strategy here is to minimize the overhead of v1 matching, +- * by prebuilding a v2 struct and putting the pointer into the +- * v1 dataspace. +- */ +- memcpy(up, info, offsetof(typeof(*info), state_mask)); +- up->state_mask = info->state_mask; +- up->status_mask = info->status_mask; +- *(void **)info = up; +- return true; +-} +- + static void conntrack_mt_destroy(const struct xt_mtdtor_param *par) + { + nf_ct_l3proto_module_put(par->family); + } + +-static void conntrack_mt_destroy_v1(const struct xt_mtdtor_param *par) +-{ +- struct xt_conntrack_mtinfo2 **info = par->matchinfo; +- kfree(*info); +- conntrack_mt_destroy(par); +-} +- + static struct xt_match conntrack_mt_reg[] __read_mostly = { + { + .name = "conntrack", +@@ -255,8 +228,8 @@ static struct xt_match conntrack_mt_reg[] __read_mostly = { + .family = NFPROTO_UNSPEC, + .matchsize = sizeof(struct xt_conntrack_mtinfo1), + .match = conntrack_mt_v1, +- .checkentry = conntrack_mt_check_v1, +- .destroy = conntrack_mt_destroy_v1, ++ .checkentry = conntrack_mt_check, ++ .destroy = conntrack_mt_destroy, + .me = THIS_MODULE, + }, + { +@@ -264,7 +237,7 @@ static struct xt_match conntrack_mt_reg[] __read_mostly = { + .revision = 2, + .family = NFPROTO_UNSPEC, + .matchsize = sizeof(struct xt_conntrack_mtinfo2), +- .match = conntrack_mt, ++ .match = conntrack_mt_v2, + .checkentry = conntrack_mt_check, + .destroy = conntrack_mt_destroy, + .me = THIS_MODULE, +-- +1.6.5.4 + diff --git a/debian/patches/series/2 b/debian/patches/series/2 index 17a858ecc..205fb762c 100644 --- a/debian/patches/series/2 +++ b/debian/patches/series/2 @@ -1,2 +1,3 @@ + features/all/aufs2/aufs2-20091205.patch + bugfix/all/atl1c-use-common_task-instead-of-reset_task-and-link.patch ++ bugfix/all/netfilter-xtables-fix-conntrack-match-v1-ipt-save-output.patch