[NETFILTER 00/06]: Netfilter fixes for 2.6.17

[NETFILTER 00/06]: Netfilter fixes for 2.6.17

[Patrick McHardy]

Hi Dave,

following are my netfilter patches for 2.6.17, which fix a couple of bad bugs:

- The SCTP conntrack loop noticed by Ingo
- The unchecked __copy_from/to in the compat code noticed by Linus
- Two H.323 bugs found by the PROTOS c07-h2250v4 testsuite

I'll push the SCTP patch to -stable as well. There is still at least one open
bug in the H.323 helper, but I can't really keep up with the pace new bugs are
discovered at currently.

Please apply.


 include/linux/netfilter_ipv4/ip_conntrack_helper_h323_asn1.h |    2 +-
 net/ipv4/netfilter/ip_conntrack_helper_h323.c                |    2 ++
 net/ipv4/netfilter/ip_conntrack_helper_h323_asn1.c           |   10 +++++-----
 net/ipv4/netfilter/ip_conntrack_proto_sctp.c                 |   11 +++++++----
 net/ipv4/netfilter/ip_nat_standalone.c                       |    2 ++
 net/ipv4/netfilter/ip_tables.c                               |    6 +++---
 net/netfilter/nf_conntrack_proto_sctp.c                      |   11 +++++++----
 net/netfilter/x_tables.c                                     |    4 ++--
 8 files changed, 29 insertions(+), 19 deletions(-)

Jing Min Zhao:
      [NETFILTER]: H.323 helper: Change author's email address

Patrick McHardy:
      [NETFILTER]: H.323 helper: fix endless loop caused by invalid TPKT len
      [NETFILTER]: H.323 helper: fix use of uninitialized data
      [NETFILTER]: SCTP conntrack: fix infinite loop
      [NETFILTER]: NAT: silence unused variable warnings with CONFIG_XFRM=n
      [NETFILTER]: x_tables: don't use __copy_{from,to}_user on unchecked memory in compat layer

[NETFILTER 01/06]: H.323 helper: fix endless loop caused by invalid TPKT len

[Patrick McHardy]

[NETFILTER]: H.323 helper: fix endless loop caused by invalid TPKT len

When the TPKT len included in the packet is below the lowest valid value
of 4 an underflow occurs which results in an endless loop.

Found by testcase 0000058 from the PROTOS c07-h2250v4 testsuite.

Signed-off-by: Patrick McHardy <kaber@trash.net>

---
commit ce641a7a27c17eaffbc769ef81d29c3925214655
tree 224c05dcc5fddb2bb8bc08f3cf01394d35605ca9
parent 532f57da408c5a5710075d17047e2d97bdfd22f3
author Patrick McHardy <kaber@trash.net> Tue, 02 May 2006 21:57:27 +0200
committer Patrick McHardy <kaber@trash.net> Tue, 02 May 2006 21:57:27 +0200

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

diff --git a/net/ipv4/netfilter/ip_conntrack_helper_h323.c b/net/ipv4/netfilter/ip_conntrack_helper_h323.c
index 2c2fb70..518f581 100644
--- a/net/ipv4/netfilter/ip_conntrack_helper_h323.c
+++ b/net/ipv4/netfilter/ip_conntrack_helper_h323.c
@@ -162,6 +162,8 @@ static int get_tpkt_data(struct sk_buff 
 
 	/* Validate TPKT length */
 	tpktlen = tpkt[2] * 256 + tpkt[3];
+	if (tpktlen < 4)
+		goto clear_out;
 	if (tpktlen > tcpdatalen) {
 		if (tcpdatalen == 4) {	/* Separate TPKT header */
 			/* Netmeeting sends TPKT header and data separately */

[NETFILTER 02/06]: H.323 helper: fix use of uninitialized data

[Patrick McHardy]

[NETFILTER]: H.323 helper: fix use of uninitialized data

When a Choice element contains an unsupported choice no error is returned
and parsing continues normally, but the choice value is not set and
contains data from the last parsed message. This may in turn lead to
parsing of more stale data and following crashes.

Fixes a crash triggered by testcase 0003243 from the PROTOS c07-h2250v4
testsuite following random other testcases:

CPU:    0
EIP:    0060:[<c01a9554>]    Not tainted VLI
EFLAGS: 00210646   (2.6.17-rc2 #3)
EIP is at memmove+0x19/0x22
eax: d7be0307   ebx: d7be0307   ecx: e841fcf9   edx: d7be0307
esi: bfffffff   edi: bfffffff   ebp: da5eb980   esp: c0347e2c
ds: 007b   es: 007b   ss: 0068
Process events/0 (pid: 4, threadinfo=c0347000 task=dff86a90)
Stack: <0>00000006 c0347ea6 d7be0301 e09a6b2c 00000006 da5eb980 d7be003e d7be0052
       c0347f6c e09a6d9c 00000006 c0347ea6 00000006 00000000 d7b9a548 00000000
       c0347f6c d7b9a548 00000004 e0a1a119 0000028f 00000006 c0347ea6 00000006
Call Trace:
 [<e09a6b2c>] mangle_contents+0x40/0xd8 [ip_nat]
 [<e09a6d9c>] ip_nat_mangle_tcp_packet+0xa1/0x191 [ip_nat]
 [<e0a1a119>] set_addr+0x60/0x14d [ip_nat_h323]
 [<e0ab6e66>] q931_help+0x2da/0x71a [ip_conntrack_h323]
 [<e0ab6e98>] q931_help+0x30c/0x71a [ip_conntrack_h323]
 [<e09af242>] ip_conntrack_help+0x22/0x2f [ip_conntrack]
 [<c022934a>] nf_iterate+0x2e/0x5f
 [<c025d357>] xfrm4_output_finish+0x0/0x39f
 [<c02294ce>] nf_hook_slow+0x42/0xb0
 [<c025d357>] xfrm4_output_finish+0x0/0x39f
 [<c025d732>] xfrm4_output+0x3c/0x4e
 [<c025d357>] xfrm4_output_finish+0x0/0x39f
 [<c0230370>] ip_forward+0x1c2/0x1fa
 [<c022f417>] ip_rcv+0x388/0x3b5
 [<c02188f9>] netif_receive_skb+0x2bc/0x2ec
 [<c0218994>] process_backlog+0x6b/0xd0
 [<c021675a>] net_rx_action+0x4b/0xb7
 [<c0115606>] __do_softirq+0x35/0x7d
 [<c0104294>] do_softirq+0x38/0x3f

Signed-off-by: Patrick McHardy <kaber@trash.net>

---
commit 6d7b293e330618cbfa6b4e4dd85dc0072ccd8c7c
tree 1f5b291ac727cc5c4508e588f836ca55dbd73f80
parent ce641a7a27c17eaffbc769ef81d29c3925214655
author Patrick McHardy <kaber@trash.net> Tue, 02 May 2006 21:58:03 +0200
committer Patrick McHardy <kaber@trash.net> Tue, 02 May 2006 21:58:03 +0200

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

diff --git a/net/ipv4/netfilter/ip_conntrack_helper_h323_asn1.c b/net/ipv4/netfilter/ip_conntrack_helper_h323_asn1.c
index 4807800..f52d2c4 100644
--- a/net/ipv4/netfilter/ip_conntrack_helper_h323_asn1.c
+++ b/net/ipv4/netfilter/ip_conntrack_helper_h323_asn1.c
@@ -703,6 +703,10 @@ int decode_choice(bitstr_t * bs, field_t
 		type = get_bits(bs, f->sz);
 	}
 
+	/* Write Type */
+	if (base)
+		*(unsigned *) base = type;
+
 	/* Check Range */
 	if (type >= f->ub) {	/* Newer version? */
 		BYTE_ALIGN(bs);
@@ -712,10 +716,6 @@ int decode_choice(bitstr_t * bs, field_t
 		return H323_ERROR_NONE;
 	}
 
-	/* Write Type */
-	if (base)
-		*(unsigned *) base = type;
-
 	/* Transfer to son level */
 	son = &f->fields[type];
 	if (son->attr & STOP) {

[NETFILTER 03/06]: SCTP conntrack: fix infinite loop

[Patrick McHardy]

[NETFILTER]: SCTP conntrack: fix infinite loop

fix infinite loop in the SCTP-netfilter code: check SCTP chunk size to
guarantee progress of for_each_sctp_chunk(). (all other uses of
for_each_sctp_chunk() are preceded by do_basic_checks(), so this fix
should be complete.)

Based on patch from Ingo Molnar <mingo@elte.hu>

Signed-off-by: Patrick McHardy <kaber@trash.net>

---
commit fcf294628012638bc925b2c158c03efea2decb4c
tree 65284c4c81d889853453017627099a0908650c41
parent 6d7b293e330618cbfa6b4e4dd85dc0072ccd8c7c
author Patrick McHardy <kaber@trash.net> Tue, 02 May 2006 21:58:45 +0200
committer Patrick McHardy <kaber@trash.net> Tue, 02 May 2006 21:58:45 +0200

 net/ipv4/netfilter/ip_conntrack_proto_sctp.c |   11 +++++++----
 net/netfilter/nf_conntrack_proto_sctp.c      |   11 +++++++----
 2 files changed, 14 insertions(+), 8 deletions(-)

diff --git a/net/ipv4/netfilter/ip_conntrack_proto_sctp.c b/net/ipv4/netfilter/ip_conntrack_proto_sctp.c
index 5259abd..0416073 100644
--- a/net/ipv4/netfilter/ip_conntrack_proto_sctp.c
+++ b/net/ipv4/netfilter/ip_conntrack_proto_sctp.c
@@ -235,12 +235,15 @@ static int do_basic_checks(struct ip_con
 			flag = 1;
 		}
 
-		/* Cookie Ack/Echo chunks not the first OR 
-		   Init / Init Ack / Shutdown compl chunks not the only chunks */
-		if ((sch->type == SCTP_CID_COOKIE_ACK 
+		/*
+		 * Cookie Ack/Echo chunks not the first OR
+		 * Init / Init Ack / Shutdown compl chunks not the only chunks
+		 * OR zero-length.
+		 */
+		if (((sch->type == SCTP_CID_COOKIE_ACK
 			|| sch->type == SCTP_CID_COOKIE_ECHO
 			|| flag)
-		     && count !=0 ) {
+		      && count !=0) || !sch->length) {
 			DEBUGP("Basic checks failed\n");
 			return 1;
 		}
diff --git a/net/netfilter/nf_conntrack_proto_sctp.c b/net/netfilter/nf_conntrack_proto_sctp.c
index 9cccc32..0c6da49 100644
--- a/net/netfilter/nf_conntrack_proto_sctp.c
+++ b/net/netfilter/nf_conntrack_proto_sctp.c
@@ -240,12 +240,15 @@ static int do_basic_checks(struct nf_con
 			flag = 1;
 		}
 
-		/* Cookie Ack/Echo chunks not the first OR 
-		   Init / Init Ack / Shutdown compl chunks not the only chunks */
-		if ((sch->type == SCTP_CID_COOKIE_ACK 
+		/*
+		 * Cookie Ack/Echo chunks not the first OR
+		 * Init / Init Ack / Shutdown compl chunks not the only chunks
+		 * OR zero-length.
+		 */
+		if (((sch->type == SCTP_CID_COOKIE_ACK
 			|| sch->type == SCTP_CID_COOKIE_ECHO
 			|| flag)
-		     && count !=0 ) {
+		      && count !=0) || !sch->length) {
 			DEBUGP("Basic checks failed\n");
 			return 1;
 		}

[NETFILTER 04/06]: NAT: silence unused variable warnings with CONFIG_XFRM=n

[Patrick McHardy]

[NETFILTER]: NAT: silence unused variable warnings with CONFIG_XFRM=n

net/ipv4/netfilter/ip_nat_standalone.c: In function 'ip_nat_out':
net/ipv4/netfilter/ip_nat_standalone.c:223: warning: unused variable 'ctinfo'
net/ipv4/netfilter/ip_nat_standalone.c:222: warning: unused variable 'ct'

Surprisingly no complaints so far ..

Signed-off-by: Patrick McHardy <kaber@trash.net>

---
commit 1a8e196d1f5fbb3c43de5ce4e11c29c66188443c
tree 0474880ecfd42616f11fa8cf41ee6c7022746d53
parent fcf294628012638bc925b2c158c03efea2decb4c
author Patrick McHardy <kaber@trash.net> Tue, 02 May 2006 22:02:49 +0200
committer Patrick McHardy <kaber@trash.net> Tue, 02 May 2006 22:02:49 +0200

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

diff --git a/net/ipv4/netfilter/ip_nat_standalone.c b/net/ipv4/netfilter/ip_nat_standalone.c
index 8f760b2..67e6767 100644
--- a/net/ipv4/netfilter/ip_nat_standalone.c
+++ b/net/ipv4/netfilter/ip_nat_standalone.c
@@ -219,8 +219,10 @@ ip_nat_out(unsigned int hooknum,
 	   const struct net_device *out,
 	   int (*okfn)(struct sk_buff *))
 {
+#ifdef CONFIG_XFRM
 	struct ip_conntrack *ct;
 	enum ip_conntrack_info ctinfo;
+#endif
 	unsigned int ret;
 
 	/* root is playing with raw sockets. */

[NETFILTER 05/06]: H.323 helper: Change author's email address

[Patrick McHardy]

[NETFILTER]: H.323 helper: Change author's email address

Signed-off-by: Jing Min Zhao <zhaojingmin@users.sourceforge.net>
Signed-off-by: Patrick McHardy <kaber@trash.net>

---
commit b29ae226e0f4b00546194dae9263928e1abae468
tree de2ee40b3a1cdc92bcef238c3bc81a39a6d85a99
parent 1a8e196d1f5fbb3c43de5ce4e11c29c66188443c
author Jing Min Zhao <zhaojingmin@users.sourceforge.net> Tue, 02 May 2006 22:03:37 +0200
committer Patrick McHardy <kaber@trash.net> Tue, 02 May 2006 22:03:37 +0200

 .../netfilter_ipv4/ip_conntrack_helper_h323_asn1.h |    2 +-
 net/ipv4/netfilter/ip_conntrack_helper_h323_asn1.c |    2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/include/linux/netfilter_ipv4/ip_conntrack_helper_h323_asn1.h b/include/linux/netfilter_ipv4/ip_conntrack_helper_h323_asn1.h
index 0bd8280..c6e9a0b 100644
--- a/include/linux/netfilter_ipv4/ip_conntrack_helper_h323_asn1.h
+++ b/include/linux/netfilter_ipv4/ip_conntrack_helper_h323_asn1.h
@@ -2,7 +2,7 @@
  * ip_conntrack_helper_h323_asn1.h - BER and PER decoding library for H.323
  * 			      	     conntrack/NAT module.
  *
- * Copyright (c) 2006 by Jing Min Zhao <zhaojingmin@hotmail.com>
+ * Copyright (c) 2006 by Jing Min Zhao <zhaojingmin@users.sourceforge.net>
  *
  * This source code is licensed under General Public License version 2.
  *
diff --git a/net/ipv4/netfilter/ip_conntrack_helper_h323_asn1.c b/net/ipv4/netfilter/ip_conntrack_helper_h323_asn1.c
index f52d2c4..355a53a 100644
--- a/net/ipv4/netfilter/ip_conntrack_helper_h323_asn1.c
+++ b/net/ipv4/netfilter/ip_conntrack_helper_h323_asn1.c
@@ -2,7 +2,7 @@
  * ip_conntrack_helper_h323_asn1.c - BER and PER decoding library for H.323
  * 			      	     conntrack/NAT module.
  *
- * Copyright (c) 2006 by Jing Min Zhao <zhaojingmin@hotmail.com>
+ * Copyright (c) 2006 by Jing Min Zhao <zhaojingmin@users.sourceforge.net>
  *
  * This source code is licensed under General Public License version 2.
  *

[NETFILTER 06/06]: x_tables: don't use __copy_{from, to}_user on unchecked memory in compat layer

[Patrick McHardy]

[NETFILTER]: x_tables: don't use __copy_{from,to}_user on unchecked memory in compat layer

Noticed by Linus Torvalds <torvalds@osdl.org>

Signed-off-by: Patrick McHardy <kaber@trash.net>

---
commit c0a6597ffa6dce9261565319327fa9d060ee8740
tree de902d88c478f87a15325f805f4325100514a5d7
parent b29ae226e0f4b00546194dae9263928e1abae468
author Patrick McHardy <kaber@trash.net> Tue, 02 May 2006 22:28:08 +0200
committer Patrick McHardy <kaber@trash.net> Tue, 02 May 2006 22:28:08 +0200

 net/ipv4/netfilter/ip_tables.c |    6 +++---
 net/netfilter/x_tables.c       |    4 ++--
 2 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/net/ipv4/netfilter/ip_tables.c b/net/ipv4/netfilter/ip_tables.c
index 6d1c115..cee3397 100644
--- a/net/ipv4/netfilter/ip_tables.c
+++ b/net/ipv4/netfilter/ip_tables.c
@@ -1441,7 +1441,7 @@ static int compat_copy_entry_to_user(str
 	ret = -EFAULT;
 	origsize = *size;
 	ce = (struct compat_ipt_entry __user *)*dstptr;
-	if (__copy_to_user(ce, e, sizeof(struct ipt_entry)))
+	if (copy_to_user(ce, e, sizeof(struct ipt_entry)))
 		goto out;
 
 	*dstptr += sizeof(struct compat_ipt_entry);
@@ -1459,9 +1459,9 @@ static int compat_copy_entry_to_user(str
 		goto out;
 	ret = -EFAULT;
 	next_offset = e->next_offset - (origsize - *size);
-	if (__put_user(target_offset, &ce->target_offset))
+	if (put_user(target_offset, &ce->target_offset))
 		goto out;
-	if (__put_user(next_offset, &ce->next_offset))
+	if (put_user(next_offset, &ce->next_offset))
 		goto out;
 	return 0;
 out:
diff --git a/net/netfilter/x_tables.c b/net/netfilter/x_tables.c
index 17abf60..99293c6 100644
--- a/net/netfilter/x_tables.c
+++ b/net/netfilter/x_tables.c
@@ -289,7 +289,7 @@ int xt_compat_match(void *match, void **
 		case COMPAT_TO_USER:
 			pm = (struct xt_entry_match *)match;
 			msize = pm->u.user.match_size;
-			if (__copy_to_user(*dstptr, pm, msize)) {
+			if (copy_to_user(*dstptr, pm, msize)) {
 				ret = -EFAULT;
 				break;
 			}
@@ -366,7 +366,7 @@ int xt_compat_target(void *target, void 
 		case COMPAT_TO_USER:
 			pt = (struct xt_entry_target *)target;
 			tsize = pt->u.user.target_size;
-			if (__copy_to_user(*dstptr, pt, tsize)) {
+			if (copy_to_user(*dstptr, pt, tsize)) {
 				ret = -EFAULT;
 				break;
 			}

Re: [NETFILTER 00/06]: Netfilter fixes for 2.6.17

[David S. Miller]

From: Patrick McHardy <kaber@trash.net>
Date: Tue,  2 May 2006 22:48:03 +0200 (MEST)

> following are my netfilter patches for 2.6.17, which fix a couple of bad bugs:
> 
> - The SCTP conntrack loop noticed by Ingo
> - The unchecked __copy_from/to in the compat code noticed by Linus
> - Two H.323 bugs found by the PROTOS c07-h2250v4 testsuite
> 
> I'll push the SCTP patch to -stable as well. There is still at least one open
> bug in the H.323 helper, but I can't really keep up with the pace new bugs are
> discovered at currently.
> 
> Please apply.

Thanks a lot for all of your great work killing off these
bugs.

All applied, excep the SCTP loop since Linus put it in already.

Thanks again!