>From 43c161caa656fb032fd4f12db2bda3e5083efd2b Mon Sep 17 00:00:00 2001 From: Pablo Neira Ayuso Date: Tue, 3 Feb 2015 18:19:30 +0100 Subject: [PATCH] [RFC] netfilter: fix crash in CLUSTERIP and TCPMSS when used from nft_compat Currently, we have two iptables extensions that cannot be used from the xt over nft compat layer. The problem is that both need to real access to the full blown ipt_entry to validate that the rule comes with the right dependencies. This check was introduced to overcome the lack of sufficient userspace dependency validation in iptables. To resolve this problem, this patch introduces a new field to the xt_tgchk_param structure that tell us if the target is executed from nft_compat context. The two affected extensions are: 1) CLUSTERIP, this target has been supersedes by xt_cluster. So just bail out by returning -EOPNOTSUPP. 2) TCPMSS. Relax the checking when used from nft_compat and make sure that we skip !syn packets in case userspace provides a wrong configuration. This allows us to use this feature from nft_compat. Signed-off-by: Pablo Neira Ayuso --- include/linux/netfilter/x_tables.h | 1 + net/ipv4/netfilter/ipt_CLUSTERIP.c | 5 +++++ net/netfilter/nft_compat.c | 1 + net/netfilter/xt_TCPMSS.c | 26 +++++++++++++++++++------- 4 files changed, 26 insertions(+), 7 deletions(-) diff --git a/include/linux/netfilter/x_tables.h b/include/linux/netfilter/x_tables.h index a3e215b..a7bbfc4e 100644 --- a/include/linux/netfilter/x_tables.h +++ b/include/linux/netfilter/x_tables.h @@ -92,6 +92,7 @@ struct xt_tgchk_param { void *targinfo; unsigned int hook_mask; u_int8_t family; + bool nft_compat; }; /* Target destructor parameters */ diff --git a/net/ipv4/netfilter/ipt_CLUSTERIP.c b/net/ipv4/netfilter/ipt_CLUSTERIP.c index e90f83a..f5ee75f 100644 --- a/net/ipv4/netfilter/ipt_CLUSTERIP.c +++ b/net/ipv4/netfilter/ipt_CLUSTERIP.c @@ -367,6 +367,11 @@ static int clusterip_tg_check(const struct xt_tgchk_param *par) struct clusterip_config *config; int ret; + if (par->nft_compat) { + pr_err("cannot use this target from nftables compat\n"); + return -EOPNOTSUPP; + } + if (cipinfo->hash_mode != CLUSTERIP_HASHMODE_SIP && cipinfo->hash_mode != CLUSTERIP_HASHMODE_SIP_SPT && cipinfo->hash_mode != CLUSTERIP_HASHMODE_SIP_SPT_DPT) { diff --git a/net/netfilter/nft_compat.c b/net/netfilter/nft_compat.c index 265e190..f463099 100644 --- a/net/netfilter/nft_compat.c +++ b/net/netfilter/nft_compat.c @@ -114,6 +114,7 @@ nft_target_set_tgchk_param(struct xt_tgchk_param *par, par->hook_mask = 0; } par->family = ctx->afi->family; + par->nft_compat = true; } static void target_compat_from_user(struct xt_target *t, void *in, void *out) diff --git a/net/netfilter/xt_TCPMSS.c b/net/netfilter/xt_TCPMSS.c index e762de5..cb3b5c1 100644 --- a/net/netfilter/xt_TCPMSS.c +++ b/net/netfilter/xt_TCPMSS.c @@ -83,7 +83,7 @@ tcpmss_mangle_packet(struct sk_buff *skb, unsigned int minlen) { const struct xt_tcpmss_info *info = par->targinfo; - struct tcphdr *tcph; + struct tcphdr *tcph, _tcph; int len, tcp_hdrlen; unsigned int i; __be16 oldval; @@ -94,19 +94,22 @@ tcpmss_mangle_packet(struct sk_buff *skb, if (par->fragoff != 0) return 0; - if (!skb_make_writable(skb, skb->len)) - return -1; + tcph = skb_header_pointer(skb, tcphoff, sizeof(_tcph), &_tcph); + if (!tcph) + return 0; - len = skb->len - tcphoff; - if (len < (int)sizeof(struct tcphdr)) - return -1; + if (unlikely(!tcph->syn)) + return 0; - tcph = (struct tcphdr *)(skb_network_header(skb) + tcphoff); tcp_hdrlen = tcph->doff * 4; + len = skb->len - tcphoff; if (len < tcp_hdrlen) return -1; + if (!skb_make_writable(skb, skb->len)) + return -1; + if (info->mss == XT_TCPMSS_CLAMP_PMTU) { struct net *net = dev_net(par->in ? par->in : par->out); unsigned int in_mtu = tcpmss_reverse_mtu(net, skb, family); @@ -277,6 +280,12 @@ static int tcpmss_tg4_check(const struct xt_tgchk_param *par) "FORWARD, OUTPUT and POSTROUTING hooks\n"); return -EINVAL; } + /* Don't care about rules without --syn from nftables compat. The + * packet is safe, it just skips non-syn packets. + */ + if (par->nft_compat) + return 0; + xt_ematch_foreach(ematch, e) if (find_syn_match(ematch)) return 0; @@ -299,6 +308,9 @@ static int tcpmss_tg6_check(const struct xt_tgchk_param *par) "FORWARD, OUTPUT and POSTROUTING hooks\n"); return -EINVAL; } + if (par->nft_compat) + return 0; + xt_ematch_foreach(ematch, e) if (find_syn_match(ematch)) return 0; -- 1.7.10.4