netfilter 00/03: netfilter fixes

netfilter 00/03: netfilter fixes

[Patrick McHardy]

Hi Dave,

the following three patches for 2.6.28 fix a couple of netfilter issues:

- a conntrack creation race in ctnetlink that can cause NULL pointer
  dereferences in ctnetlink and duplicate conntrack entries.

- a missing const qualifier that got lost during the encapsulation of
  iptables target parameters

- a crash with bridge netfilter and GRE caused by a missing update_pmtu()
  function for the fake dst_entry.

Please apply, thanks.


 include/linux/netfilter/x_tables.h   |    2 +-
 net/bridge/br_netfilter.c            |   13 +++++++++++++
 net/netfilter/nf_conntrack_core.c    |    2 --
 net/netfilter/nf_conntrack_netlink.c |    5 +++--
 4 files changed, 17 insertions(+), 5 deletions(-)

Herbert Xu (1):
      bridge: netfilter: fix update_pmtu crash with GRE

Jan Engelhardt (1):
      netfilter: xtables: add missing const qualifier to xt_tgchk_param

Patrick McHardy (1):
      netfilter: ctnetlink: fix conntrack creation race

netfilter 01/03: ctnetlink: fix conntrack creation race

[Patrick McHardy]

commit 580a1b74505dae8b650c7acee28cf8d2fa1b1b8a
Author: Patrick McHardy <kaber@trash.net>
Date:   Wed Nov 19 13:42:03 2008 +0100

    netfilter: ctnetlink: fix conntrack creation race
    
    Conntrack creation through ctnetlink has two races:
    
    - the timer may expire and free the conntrack concurrently, causing an
      invalid memory access when attempting to put it in the hash tables
    
    - an identical conntrack entry may be created in the packet processing
      path in the time between the lookup and hash insertion
    
    Hold the conntrack lock between the lookup and insertion to avoid this.
    
    Reported-by: Zoltan Borbely <bozo@andrews.hu>
    Signed-off-by: Patrick McHardy <kaber@trash.net>

diff --git a/net/netfilter/nf_conntrack_core.c b/net/netfilter/nf_conntrack_core.c
index 622d7c6..233fdd2 100644
--- a/net/netfilter/nf_conntrack_core.c
+++ b/net/netfilter/nf_conntrack_core.c
@@ -305,9 +305,7 @@ void nf_conntrack_hash_insert(struct nf_conn *ct)
 	hash = hash_conntrack(&ct->tuplehash[IP_CT_DIR_ORIGINAL].tuple);
 	repl_hash = hash_conntrack(&ct->tuplehash[IP_CT_DIR_REPLY].tuple);
 
-	spin_lock_bh(&nf_conntrack_lock);
 	__nf_conntrack_hash_insert(ct, hash, repl_hash);
-	spin_unlock_bh(&nf_conntrack_lock);
 }
 EXPORT_SYMBOL_GPL(nf_conntrack_hash_insert);
 
diff --git a/net/netfilter/nf_conntrack_netlink.c b/net/netfilter/nf_conntrack_netlink.c
index a040d46..3b009a3 100644
--- a/net/netfilter/nf_conntrack_netlink.c
+++ b/net/netfilter/nf_conntrack_netlink.c
@@ -1090,7 +1090,7 @@ ctnetlink_create_conntrack(struct nlattr *cda[],
 	struct nf_conn_help *help;
 	struct nf_conntrack_helper *helper;
 
-	ct = nf_conntrack_alloc(&init_net, otuple, rtuple, GFP_KERNEL);
+	ct = nf_conntrack_alloc(&init_net, otuple, rtuple, GFP_ATOMIC);
 	if (ct == NULL || IS_ERR(ct))
 		return -ENOMEM;
 
@@ -1212,13 +1212,14 @@ ctnetlink_new_conntrack(struct sock *ctnl, struct sk_buff *skb,
 			atomic_inc(&master_ct->ct_general.use);
 		}
 
-		spin_unlock_bh(&nf_conntrack_lock);
 		err = -ENOENT;
 		if (nlh->nlmsg_flags & NLM_F_CREATE)
 			err = ctnetlink_create_conntrack(cda,
 							 &otuple,
 							 &rtuple,
 							 master_ct);
+		spin_unlock_bh(&nf_conntrack_lock);
+
 		if (err < 0 && master_ct)
 			nf_ct_put(master_ct);
 

netfilter 02/03: xtables: add missing const qualifier to xt_tgchk_param

[Patrick McHardy]

commit 511bb54d5b276711ce08ab7e63f53226890b0e35
Author: Jan Engelhardt <jengelh@medozas.de>
Date:   Wed Nov 19 13:47:24 2008 +0100

    netfilter: xtables: add missing const qualifier to xt_tgchk_param
    
    When entryinfo was a standalone parameter to functions, it used to be
    "const void *". Put the const back in.
    
    Signed-off-by: Jan Engelhardt <jengelh@medozas.de>
    Signed-off-by: Patrick McHardy <kaber@trash.net>

diff --git a/include/linux/netfilter/x_tables.h b/include/linux/netfilter/x_tables.h
index be41b60..e52ce47 100644
--- a/include/linux/netfilter/x_tables.h
+++ b/include/linux/netfilter/x_tables.h
@@ -251,7 +251,7 @@ struct xt_target_param {
  */
 struct xt_tgchk_param {
 	const char *table;
-	void *entryinfo;
+	const void *entryinfo;
 	const struct xt_target *target;
 	void *targinfo;
 	unsigned int hook_mask;

bridge 03/03: netfilter: fix update_pmtu crash with GRE

[Patrick McHardy]

commit 1e8768f064c00a4fbb42e87bc42b371bd9ca01c0
Author: Herbert Xu <herbert@gondor.apana.org.au>
Date:   Mon Nov 24 13:31:14 2008 +0100

    bridge: netfilter: fix update_pmtu crash with GRE
    
    As GRE tries to call the update_pmtu function on skb->dst and
    bridge supplies an skb->dst that has a NULL ops field, all is
    not well.
    
    This patch fixes this by giving the bridge device an ops field
    with an update_pmtu function.  For the moment I've left all
    other fields blank but we can fill them in later should the
    need arise.
    
    Based on report and patch by Philip Craig.
    
    Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
    Signed-off-by: Patrick McHardy <kaber@trash.net>

diff --git a/net/bridge/br_netfilter.c b/net/bridge/br_netfilter.c
index fa5cda4..45f61c3 100644
--- a/net/bridge/br_netfilter.c
+++ b/net/bridge/br_netfilter.c
@@ -101,6 +101,18 @@ static inline __be16 pppoe_proto(const struct sk_buff *skb)
 	 pppoe_proto(skb) == htons(PPP_IPV6) && \
 	 brnf_filter_pppoe_tagged)
 
+static void fake_update_pmtu(struct dst_entry *dst, u32 mtu)
+{
+}
+
+static struct dst_ops fake_dst_ops = {
+	.family =		AF_INET,
+	.protocol =		__constant_htons(ETH_P_IP),
+	.update_pmtu =		fake_update_pmtu,
+	.entry_size =		sizeof(struct rtable),
+	.entries =		ATOMIC_INIT(0),
+};
+
 /*
  * Initialize bogus route table used to keep netfilter happy.
  * Currently, we fill in the PMTU entry because netfilter
@@ -117,6 +129,7 @@ void br_netfilter_rtable_init(struct net_bridge *br)
 	rt->u.dst.path = &rt->u.dst;
 	rt->u.dst.metrics[RTAX_MTU - 1] = 1500;
 	rt->u.dst.flags	= DST_NOXFRM;
+	rt->u.dst.ops = &fake_dst_ops;
 }
 
 static inline struct rtable *bridge_parent_rtable(const struct net_device *dev)

Re: netfilter 01/03: ctnetlink: fix conntrack creation race

[David Miller]

From: Patrick McHardy <kaber@trash.net>
Date: Mon, 24 Nov 2008 14:44:36 +0100 (MET)

>     netfilter: ctnetlink: fix conntrack creation race
>     
>     Conntrack creation through ctnetlink has two races:
>     
>     - the timer may expire and free the conntrack concurrently, causing an
>       invalid memory access when attempting to put it in the hash tables
>     
>     - an identical conntrack entry may be created in the packet processing
>       path in the time between the lookup and hash insertion
>     
>     Hold the conntrack lock between the lookup and insertion to avoid this.
>     
>     Reported-by: Zoltan Borbely <bozo@andrews.hu>
>     Signed-off-by: Patrick McHardy <kaber@trash.net>
> 

Applied.

Re: netfilter 02/03: xtables: add missing const qualifier to xt_tgchk_param

[David Miller]

From: Patrick McHardy <kaber@trash.net>
Date: Mon, 24 Nov 2008 14:44:38 +0100 (MET)

>     netfilter: xtables: add missing const qualifier to xt_tgchk_param
>     
>     When entryinfo was a standalone parameter to functions, it used to be
>     "const void *". Put the const back in.
>     
>     Signed-off-by: Jan Engelhardt <jengelh@medozas.de>
>     Signed-off-by: Patrick McHardy <kaber@trash.net>

Applied.

Re: bridge 03/03: netfilter: fix update_pmtu crash with GRE

[David Miller]

From: Patrick McHardy <kaber@trash.net>
Date: Mon, 24 Nov 2008 14:44:39 +0100 (MET)

>     bridge: netfilter: fix update_pmtu crash with GRE
>     
>     As GRE tries to call the update_pmtu function on skb->dst and
>     bridge supplies an skb->dst that has a NULL ops field, all is
>     not well.
>     
>     This patch fixes this by giving the bridge device an ops field
>     with an update_pmtu function.  For the moment I've left all
>     other fields blank but we can fill them in later should the
>     need arise.
>     
>     Based on report and patch by Philip Craig.
>     
>     Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
>     Signed-off-by: Patrick McHardy <kaber@trash.net>

Applied.