: Netfilter fixes

Hi Dave,

following are a unfortunately quite large number of netfilter fixes for 2.6.15. They're all pretty simple, I hope its still possible to get them in ..

[NETFILTER]: nf_conntrack: Fix missing check for ICMPv6 type [NETFILTER]: nfnetlink: Fix calculation of minimum message length [NETFILTER]: Fix incorrect argument to ip_nat_initialized() in ctnetlink [NETFILTER]: Fix ip_conntrack_flush abuse in ctnetlink [NETFILTER]: Fix CTA_PROTO_NUM attribute size in ctnetlink [NETFILTER]: Mark ctnetlink as EXPERIMENTAL
[NETFILTER]: Wait for untracked references in nf_conntrack module unload [NETFILTER]: Fix unbalanced read_unlock_bh in ctnetlink [NETFILTER]: Don't use conntrack entry after dropping the reference

[NETFILTER]: nf_conntrack: Fix missing check for ICMPv6 type

This makes nf_conntrack_icmpv6 check that ICMPv6 type isn't < 128 to avoid accessing out of array valid_new[] and invmap[].

---
commit e512e47b2238a9e367f05a36b4ac2ba53f5ad12e
tree faa3dc22ceff3549bc211cc48da4d1d63a9fab35
parent 436b0f76f2cee6617f27a649637766628909dd5d
author Yasuyuki Kozakai <[email protected]> Sun, 04 Dec 2005 16:01:47 +0100 committer Patrick McHardy <[email protected]> Sun, 04 Dec 2005 16:01:47 +0100

 net/ipv6/netfilter/nf_conntrack_proto_icmpv6.c |   12 ++++++------
 1 files changed, 6 insertions(+), 6 deletions(-)

index c0f1da5..a7e03cf 100644
@@ -68,8 +68,8 @@ static int icmpv6_invert_tuple(struct nf
 		[ICMPV6_NI_REPLY - 128]		= ICMPV6_NI_REPLY +1
 	};
 
-	__u8 type = orig->dst.u.icmp.type - 128;
-	if (type >= sizeof(invmap) || !invmap[type])
+	int type = orig->dst.u.icmp.type - 128;
+	if (type < 0 || type >= sizeof(invmap) || !invmap[type])
 		return 0;
 
 	tuple->src.u.icmp.id   = orig->src.u.icmp.id;

@@ -129,12 +129,12 @@ static int icmpv6_new(struct nf_conn *co
 		[ICMPV6_ECHO_REQUEST - 128] = 1,
 		[ICMPV6_NI_QUERY - 128] = 1
 	};
+	int type = conntrack->tuplehash[0].tuple.dst.u.icmp.type - 128;
 
-	if (conntrack->tuplehash[0].tuple.dst.u.icmp.type - 128 >= sizeof(valid_new)
-	    || !valid_new[conntrack->tuplehash[0].tuple.dst.u.icmp.type - 128]) {
+	if (type < 0 || type >= sizeof(valid_new) || !valid_new[type]) {
 		/* Can't create a new ICMPv6 `conn' with this. */
-		DEBUGP("icmp: can't create new conn with type %u\n",
-		       conntrack->tuplehash[0].tuple.dst.u.icmp.type);
+		DEBUGP("icmpv6: can't create new conn with type %u\n",
+		       type + 128);
 		NF_CT_DUMP_TUPLE(&conntrack->tuplehash[0].tuple);
 		return 0;
 	}

[NETFILTER]: nfnetlink: Fix calculation of minimum message length

At least, valid nfnetlink message should have nlmsghdr and nfgenmsg.

---
commit 1ec1f9de65434db4bac6141e25c5b8a6b5110e4f
tree 0eaed90532d67c3ba3281628bfbd256f5cc1163d
parent e512e47b2238a9e367f05a36b4ac2ba53f5ad12e
author Yasuyuki Kozakai <[email protected]> Sun, 04 Dec 2005 16:26:36 +0100 committer Patrick McHardy <[email protected]> Sun, 04 Dec 2005 16:26:36 +0100

 net/netfilter/nfnetlink.c |    5 ++---
 1 files changed, 2 insertions(+), 3 deletions(-)

index a60c59b..95fdf04 100644
@@ -162,7 +162,7 @@ nfnetlink_check_attributes(struct nfnetl
 		return -EINVAL;
 	}
 
-	min_len = NLMSG_ALIGN(sizeof(struct nfgenmsg));
+	min_len = NLMSG_SPACE(sizeof(struct nfgenmsg));
 	if (unlikely(nlh->nlmsg_len < min_len))
 		return -EINVAL;
 

@@ -236,8 +236,7 @@ static inline int nfnetlink_rcv_msg(stru
 	}
 
 	/* All the messages must at least contain nfgenmsg */
-	if (nlh->nlmsg_len < 
-			NLMSG_LENGTH(NLMSG_ALIGN(sizeof(struct nfgenmsg)))) {
+	if (nlh->nlmsg_len < NLMSG_SPACE(sizeof(struct nfgenmsg))) {
 		DEBUGP("received message was too short\n");
 		return 0;
 	}

[NETFILTER]: Fix incorrect argument to ip_nat_initialized() in ctnetlink

ip_nat_initialized() takes enum ip_nat_manip_type as it's second argument, not a hook number.

Noticed and initial patch by Marcus Sundberg <[email protected]>.

---
commit 5b33d73e5d4153241c8e2378332f9810f4eca160
tree 4a1ec661e39d1adf96e8621b36236882295b03ff
parent 1ec1f9de65434db4bac6141e25c5b8a6b5110e4f
author Pablo Neira Ayuso <[email protected]> Sun, 04 Dec 2005 16:35:54 +0100 committer Patrick McHardy <[email protected]> Sun, 04 Dec 2005 16:35:54 +0100

 net/ipv4/netfilter/ip_conntrack_netlink.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

index 3fce91b..70402e0 100644
@@ -877,7 +877,7 @@ ctnetlink_change_status(struct ip_conntr
 		DEBUGP("NAT status: %lu\n", 
 		       status & (IPS_NAT_MASK | IPS_NAT_DONE_MASK));
 		
-		if (ip_nat_initialized(ct, hooknum))
+		if (ip_nat_initialized(ct, HOOK2MANIP(hooknum)))
 			return -EEXIST;
 		ip_nat_setup_info(ct, &range, hooknum);
 

[NETFILTER]: Fix ip_conntrack_flush abuse in ctnetlink

ip_conntrack_flush() used to be part of ip_conntrack_cleanup(), which needs to drop _all_ references on module unload. Table flushed using ctnetlink just needs to clean the table and doesn't need to flush the event cache or wait for any references attached to skbs. Move everything but pure table flushing back to ip_conntrack_cleanup().

---
commit 01563e56ad1c48c85e1258e2eaabcc270385e1a5
tree 244d6a9b8cbe0daf3be9f356d9c69018a75de7ad
parent 5b33d73e5d4153241c8e2378332f9810f4eca160
author Patrick McHardy <[email protected]> Sun, 04 Dec 2005 16:54:55 +0100 committer Patrick McHardy <[email protected]> Sun, 04 Dec 2005 16:54:55 +0100

 net/ipv4/netfilter/ip_conntrack_core.c |   20 +++++++++++---------
 1 files changed, 11 insertions(+), 9 deletions(-)

index 7a4ecdd..84c66db 100644
@@ -1345,6 +1345,11 @@ static int kill_all(struct ip_conntrack 
 	return 1;
 }
 
+void ip_conntrack_flush(void)
+{
+	ip_ct_iterate_cleanup(kill_all, NULL);
+}
+
 static void free_conntrack_hash(struct list_head *hash, int vmalloced,int size)
 {
 	if (vmalloced)

@@ -1354,8 +1359,12 @@ static void free_conntrack_hash(struct l
 			   get_order(sizeof(struct list_head) * size));
 }
 
-void ip_conntrack_flush(void)
+/* Mishearing the voices in his head, our hero wonders how he's
+   supposed to kill the mall. */
+void ip_conntrack_cleanup(void)
 {
+	ip_ct_attach = NULL;
+
 	/* This makes sure all current packets have passed through
            netfilter framework.  Roll on, two-stage module
            delete... */

@@ -1363,7 +1372,7 @@ void ip_conntrack_flush(void)
 
 	ip_ct_event_cache_flush();
  i_see_dead_people:
-	ip_ct_iterate_cleanup(kill_all, NULL);
+	ip_conntrack_flush();
 	if (atomic_read(&ip_conntrack_count) != 0) {
 		schedule();
 		goto i_see_dead_people;

@@ -1371,14 +1380,7 @@ void ip_conntrack_flush(void)
 	/* wait until all references to ip_conntrack_untracked are dropped */
 	while (atomic_read(&ip_conntrack_untracked.ct_general.use) > 1)
 		schedule();
-}
 
-/* Mishearing the voices in his head, our hero wonders how he's
-   supposed to kill the mall. */
-void ip_conntrack_cleanup(void)
-{
-	ip_ct_attach = NULL;
-	ip_conntrack_flush();
 	kmem_cache_destroy(ip_conntrack_cachep);
 	kmem_cache_destroy(ip_conntrack_expect_cachep);
 	free_conntrack_hash(ip_conntrack_hash, ip_conntrack_vmalloc,

[NETFILTER]: Fix CTA_PROTO_NUM attribute size in ctnetlink

CTA_PROTO_NUM is a u_int8_t.

---
commit 0fa82f8caa129bb2377e1b593bf2986fc13c5391
tree 0a7479acf48c9de99cb6f9fd1fac3a01dec2e220
parent 01563e56ad1c48c85e1258e2eaabcc270385e1a5
author Patrick McHardy <[email protected]> Sun, 04 Dec 2005 17:00:48 +0100 committer Patrick McHardy <[email protected]> Sun, 04 Dec 2005 17:00:48 +0100

 net/ipv4/netfilter/ip_conntrack_netlink.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

index 70402e0..d058ac4 100644
@@ -503,7 +503,7 @@ ctnetlink_parse_tuple_ip(struct nfattr *
 }
 
 static const size_t cta_min_proto[CTA_PROTO_MAX] = {
-	[CTA_PROTO_NUM-1]	= sizeof(u_int16_t),
+	[CTA_PROTO_NUM-1]	= sizeof(u_int8_t),
 	[CTA_PROTO_SRC_PORT-1]	= sizeof(u_int16_t),
 	[CTA_PROTO_DST_PORT-1]	= sizeof(u_int16_t),
 	[CTA_PROTO_ICMP_TYPE-1]	= sizeof(u_int8_t),

@@ -528,7 +528,7 @@ ctnetlink_parse_tuple_proto(struct nfatt
 
 	if (!tb[CTA_PROTO_NUM-1])
 		return -EINVAL;
-	tuple->dst.protonum = *(u_int16_t *)NFA_DATA(tb[CTA_PROTO_NUM-1]);
+	tuple->dst.protonum = *(u_int8_t *)NFA_DATA(tb[CTA_PROTO_NUM-1]);
 
 	proto = ip_conntrack_proto_find_get(tuple->dst.protonum);
 

[NETFILTER]: Mark ctnetlink as EXPERIMENTAL

Should have been marked EXPERIMENTAL from the beginning, as the current bunch of fixes show.

---
commit aea6c7fa24378c7f38026177c0b524719789b4be
tree e656f511d3fbfaf666828a8fc6803d7579db6dc9
parent 0fa82f8caa129bb2377e1b593bf2986fc13c5391
author Patrick McHardy <[email protected]> Sun, 04 Dec 2005 17:11:34 +0100 committer Patrick McHardy <[email protected]> Sun, 04 Dec 2005 17:11:34 +0100

 net/ipv4/netfilter/Kconfig |    8 ++++----
 1 files changed, 4 insertions(+), 4 deletions(-)

index 0bc0052..88a6065 100644
@@ -56,8 +56,8 @@ config IP_NF_CONNTRACK_MARK
 	  instead of the individual packets.
 	
 config IP_NF_CONNTRACK_EVENTS
-	bool "Connection tracking events"
-	depends on IP_NF_CONNTRACK
+	bool "Connection tracking events (EXPERIMENTAL)"
+	depends on EXPERIMENTAL && IP_NF_CONNTRACK
 	help
 	  If this option is enabled, the connection tracking code will
 	  provide a notifier chain that can be used by other kernel code

@@ -66,8 +66,8 @@ config IP_NF_CONNTRACK_EVENTS
 	  IF unsure, say `N'.
 
 config IP_NF_CONNTRACK_NETLINK
-	tristate 'Connection tracking netlink interface'
-	depends on IP_NF_CONNTRACK && NETFILTER_NETLINK
+	tristate 'Connection tracking netlink interface (EXPERIMENTAL)'
+	depends on EXPERIMENTAL && IP_NF_CONNTRACK && NETFILTER_NETLINK
 	depends on IP_NF_CONNTRACK!=y || NETFILTER_NETLINK!=m
 	help
 	  This option enables support for a netlink-based userspace interface

Hi Patrick,

Pablo

Indeed, thanks. Dave, please use this patch instead.

[NETFILTER]: Mark ctnetlink as EXPERIMENTAL

Should have been marked EXPERIMENTAL from the beginning, as the current bunch of fixes show.

---
commit f77890fcb84e5e96aaa65f6b5344c986400915fb
tree 332c6e3f7940bccb6d019c5f76cd9e112009f674
parent e4f5c82a92c2a546a16af1614114eec19120e40a
author Patrick McHardy <[email protected]> Mon, 05 Dec 2005 16:25:24 +0100 committer Patrick McHardy <[email protected]> Mon, 05 Dec 2005 16:25:24 +0100

 net/ipv4/netfilter/Kconfig |    8 ++++----
 net/netfilter/Kconfig      |    4 ++--
 2 files changed, 6 insertions(+), 6 deletions(-)

index 0bc0052..88a6065 100644
@@ -56,8 +56,8 @@ config IP_NF_CONNTRACK_MARK
 	  instead of the individual packets.
 	
 config IP_NF_CONNTRACK_EVENTS
-	bool "Connection tracking events"
-	depends on IP_NF_CONNTRACK
+	bool "Connection tracking events (EXPERIMENTAL)"
+	depends on EXPERIMENTAL && IP_NF_CONNTRACK
 	help
 	  If this option is enabled, the connection tracking code will
 	  provide a notifier chain that can be used by other kernel code

@@ -66,8 +66,8 @@ config IP_NF_CONNTRACK_EVENTS
 	  IF unsure, say `N'.
 
 config IP_NF_CONNTRACK_NETLINK
-	tristate 'Connection tracking netlink interface'
-	depends on IP_NF_CONNTRACK && NETFILTER_NETLINK
+	tristate 'Connection tracking netlink interface (EXPERIMENTAL)'
+	depends on EXPERIMENTAL && IP_NF_CONNTRACK && NETFILTER_NETLINK
 	depends on IP_NF_CONNTRACK!=y || NETFILTER_NETLINK!=m
 	help
 	  This option enables support for a netlink-based userspace interface

index a84f922..794c41d 100644
@@ -61,8 +61,8 @@ config NF_CONNTRACK_MARK
 	  instead of the individual packets.
 
 config NF_CONNTRACK_EVENTS
-	bool "Connection tracking events"
-	depends on NF_CONNTRACK
+	bool "Connection tracking events (EXPERIMENTAL)"
+	depends on EXPERIMENTAL && NF_CONNTRACK
 	help
 	  If this option is enabled, the connection tracking code will
 	  provide a notifier chain that can be used by other kernel code

From: Patrick McHardy <[email protected]>
Date: Mon, 05 Dec 2005 16:26:54 +0100

Done, all 9 patches (with updated patch 6) applied.

Thanks.

[NETFILTER]: Wait for untracked references in nf_conntrack module unload

Noticed by Pablo Neira <[email protected]>.

---
commit 96c75906027f008ed3a4058a606938901e9c6d99
tree 87b8bb22d35f61383fbb5f4ee6164a16cc256bb3
parent aea6c7fa24378c7f38026177c0b524719789b4be
author Patrick McHardy <[email protected]> Sun, 04 Dec 2005 17:22:02 +0100 committer Patrick McHardy <[email protected]> Sun, 04 Dec 2005 17:22:02 +0100

 net/netfilter/nf_conntrack_core.c |    3 +++
 1 files changed, 3 insertions(+), 0 deletions(-)

index 1da6783..a99285d 100644
@@ -1383,6 +1383,9 @@ void nf_conntrack_cleanup(void)
 		schedule();
 		goto i_see_dead_people;
 	}
+	/* wait until all references to nf_conntrack_untracked are dropped */
+	while (atomic_read(&nf_conntrack_untracked.ct_general.use) > 1)
+		schedule();
 
 	for (i = 0; i < NF_CT_F_NUM; i++) {
 		if (nf_ct_cache[i].use == 0)

[NETFILTER]: Fix unbalanced read_unlock_bh in ctnetlink

NFA_NEST calls NFA_PUT which jumps to nfattr_failure if the skb has no room left. We call read_unlock_bh at nfattr_failure for the NFA_PUT inside the locked section, so move NFA_NEST inside the locked section too.

---
commit cd85228eea7c7ab9d701090e3dc9643397cf271d
tree e3fa7a6a24c5b199d311a9f74c312fee3b18eae7
parent 96c75906027f008ed3a4058a606938901e9c6d99
author Patrick McHardy <[email protected]> Sun, 04 Dec 2005 20:56:05 +0100 committer Patrick McHardy <[email protected]> Sun, 04 Dec 2005 20:56:05 +0100

 net/ipv4/netfilter/ip_conntrack_proto_tcp.c |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

index aeb7353..e7fa29e 100644
@@ -341,9 +341,10 @@ static int tcp_print_conntrack(struct se
 static int tcp_to_nfattr(struct sk_buff *skb, struct nfattr *nfa,
 			 const struct ip_conntrack *ct)
 {
-	struct nfattr *nest_parms = NFA_NEST(skb, CTA_PROTOINFO_TCP);
+	struct nfattr *nest_parms;
 	
 	read_lock_bh(&tcp_lock);
+	nest_parms = NFA_NEST(skb, CTA_PROTOINFO_TCP);
 	NFA_PUT(skb, CTA_PROTOINFO_TCP_STATE, sizeof(u_int8_t),
 		&ct->proto.tcp.state);
 	read_unlock_bh(&tcp_lock);

[NETFILTER]: Don't use conntrack entry after dropping the reference

---
commit 016ada126fa7f3f46ffeefa14d59edd8464317bf
tree c2586a37bc0af665b9932badc9b2b22dd427a078
parent cd85228eea7c7ab9d701090e3dc9643397cf271d
author Patrick McHardy <[email protected]> Sun, 04 Dec 2005 21:30:33 +0100 committer Patrick McHardy <[email protected]> Sun, 04 Dec 2005 21:30:33 +0100

 net/ipv4/netfilter/ip_conntrack_netlink.c |    6 ++----
 1 files changed, 2 insertions(+), 4 deletions(-)

index d058ac4..91fe8f2 100644
@@ -728,11 +728,9 @@ ctnetlink_del_conntrack(struct sock *ctn
 			return -ENOENT;
 		}
 	}	
-	if (del_timer(&ct->timeout)) {
-		ip_conntrack_put(ct);
+	if (del_timer(&ct->timeout))
 		ct->timeout.function((unsigned long)ct);
-		return 0;
-	}
+
 	ip_conntrack_put(ct);
 	DEBUGP("leaving\n");