Skip to contentSkip to conversation

: Netfilter fixes

PATCH Applied Patrick McHardy 4 participants 11 messages Oct 18, 2007

Subsystems (2)
lore
Share & tools
lore
#0 lore raw

cover : Netfilter fixes

Hi Dave,

these two patches contain a follow-up fix to the TCP conntrack connection reopening problem and a fix for the sctp match, which uses ARRAY_SIZE on a pointer instead of an array.

Please apply, thanks.

 include/linux/netfilter/xt_sctp.h      |   13 +++++--------
 net/netfilter/nf_conntrack_proto_tcp.c |   11 +++++++----
 net/netfilter/xt_sctp.c                |   18 ++++++++----------
 3 files changed, 20 insertions(+), 22 deletions(-)

Jozsef Kadlecsik (1):
[NETFILTER]: nf_conntrack_tcp: fix connection reopening fix

Li Zefan (1):
[NETFILTER]: xt_sctp: fix mistake to pass a pointer where array is required

#1 lore raw

PATCH 1/2 : nf_conntrack_tcp: fix connection reopening fix

[NETFILTER]: nf_conntrack_tcp: fix connection reopening fix

Setting the last_dir parameter was missing from the previous one.

---
commit 9768fafed7a5588757d972b2f511cb17a874f496
tree f0a8526f52a503a1cff944c9dc3012e05763310a
parent d85714d81cc0408daddb68c10f7fd69eafe7c213
author Jozsef Kadlecsik <[email protected]> Thu, 18 Oct 2007 12:09:51 +0200 committer Patrick McHardy <[email protected]> Thu, 18 Oct 2007 12:09:51 +0200

 net/netfilter/nf_conntrack_proto_tcp.c |   11 +++++++----
 1 files changed, 7 insertions(+), 4 deletions(-)
net/netfilter/nf_conntrack_proto_tcp.c +7 -4
index c707534..4dc23ab 100644
@@ -834,10 +834,12 @@ static int tcp_packet(struct nf_conn *conntrack,
 	case TCP_CONNTRACK_SYN_SENT:
 		if (old_state < TCP_CONNTRACK_TIME_WAIT)
 			break;
-		if (conntrack->proto.tcp.seen[!dir].flags &
-			IP_CT_TCP_FLAG_CLOSE_INIT) {
-			/* Attempt to reopen a closed connection.
-			* Delete this connection and look up again. */
+		if ((conntrack->proto.tcp.seen[!dir].flags &
+			IP_CT_TCP_FLAG_CLOSE_INIT)
+		    || (conntrack->proto.tcp.last_dir == dir
+		        && conntrack->proto.tcp.last_index == TCP_RST_SET)) {
+			/* Attempt to reopen a closed/aborted connection.
+			 * Delete this connection and look up again. */
 			write_unlock_bh(&tcp_lock);
 			if (del_timer(&conntrack->timeout))
 				conntrack->timeout.function((unsigned long)
@@ -925,6 +927,7 @@ static int tcp_packet(struct nf_conn *conntrack,
      in_window:
 	/* From now on we have got in-window packets */
 	conntrack->proto.tcp.last_index = index;
+	conntrack->proto.tcp.last_dir = dir;
 
 	pr_debug("tcp_conntracks: ");
 	NF_CT_DUMP_TUPLE(tuple);
#2 lore raw

Re: [NETFILTER 01/02]: nf_conntrack_tcp: fix connection reopening fix

Quoted 3 lines · Patrick McHardy

[NETFILTER]: nf_conntrack_tcp: fix connection reopening fix

Setting the last_dir parameter was missing from the previous one.

This description is IMHO wrong. It should be as in previous, little incomplete (last_dir), patch:

"If one side aborts an established connection, the entry still lingers for 10s in conntrack for the late packets. Allow to open up the connection again for the party which sent the RST packet."

Best regards,

Krzysztof Olędzki

#3 lore raw

Re: [NETFILTER 01/02]: nf_conntrack_tcp: fix connection reopening fix

Quoted 6 lines · Krzysztof Oledzki
Quoted 2 lines · Patrick McHardy

[NETFILTER]: nf_conntrack_tcp: fix connection reopening fix

Quoted 7 lines
Quoted 1 line

Setting the last_dir parameter was missing from the previous one.

This description is IMHO wrong. It should be as in previous, little incomplete (last_dir), patch:

"If one side aborts an established connection, the entry still lingers for 10s in conntrack for the late packets. Allow to open up the connection again for the party which sent the RST packet."

The first patch was submitted seperately with a somewhat similar description. This is just the incremental fix.

#4 lore raw

Re: [NETFILTER 01/02]: nf_conntrack_tcp: fix connection reopening fix

Quoted 11 lines · Patrick McHardy
Quoted 10 lines · Krzysztof Oledzki
Quoted 3 lines · Patrick McHardy

[NETFILTER]: nf_conntrack_tcp: fix connection reopening fix

Setting the last_dir parameter was missing from the previous one.

This description is IMHO wrong. It should be as in previous, little incomplete (last_dir), patch:

Quoted 7 lines
Quoted 3 lines

"If one side aborts an established connection, the entry still lingers for 10s in conntrack for the late packets. Allow to open up the connection again for the party which sent the RST packet."

The first patch was submitted seperately with a somewhat similar description. This is just the incremental fix.

No, there are two independent fixes. The first one is already in the mainline (nf_conntrack_tcp: fix connection reopening) and it is OK:
http://git.kernel.org/?p=linux/kernel/git/torvalds/linux-2.6.git;a=commitdiff;h=17311393f969090ab060540bd9dbe7dc885a76d5

The second one had been incomplete:
http://marc.info/?l=netfilter-devel&m=119214261727254&w=1
and it was recently fixed:
http://marc.info/?l=netfilter-devel&m=119264302103523&w=1

So, this "Setting the last_dir parameter was missing from the previous one." comment applies to this patch:
http://marc.info/?l=netfilter-devel&m=119214261727254&w=1

Best regards,

Krzysztof Olędzki

maintainer
#5 lore raw

Re: [NETFILTER 01/02]: nf_conntrack_tcp: fix connection reopening fix

From: Krzysztof Oledzki <[email protected]>
Date: Thu, 18 Oct 2007 12:37:42 +0200 (CEST)

Quoted 28 lines · Patrick McHardy
Quoted 13 lines
Quoted 11 lines · Krzysztof Oledzki
Quoted 3 lines · Patrick McHardy

[NETFILTER]: nf_conntrack_tcp: fix connection reopening fix

Setting the last_dir parameter was missing from the previous one.

This description is IMHO wrong. It should be as in previous, little incomplete (last_dir), patch:

Quoted 7 lines
Quoted 3 lines

"If one side aborts an established connection, the entry still lingers for 10s in conntrack for the late packets. Allow to open up the connection again for the party which sent the RST packet."

The first patch was submitted seperately with a somewhat similar description. This is just the incremental fix.

No, there are two independent fixes. The first one is already in the mainline (nf_conntrack_tcp: fix connection reopening) and it is OK:

   http://git.kernel.org/?p=linux/kernel/git/torvalds/linux-2.6.git;a=commitdiff;h=17311393f969090ab060540bd9dbe7dc885a76d5
Quoted 8 lines

The second one had been incomplete:
http://marc.info/?l=netfilter-devel&m=119214261727254&w=1
and it was recently fixed:
http://marc.info/?l=netfilter-devel&m=119264302103523&w=1

So, this "Setting the last_dir parameter was missing from the previous one." comment applies to this patch:
http://marc.info/?l=netfilter-devel&m=119214261727254&w=1

Please sort out this changelog message issue so I can apply the patch correctly :-)

Thanks!

#6 lore raw

Re: [NETFILTER 01/02]: nf_conntrack_tcp: fix connection reopening fix

Quoted 8 lines · David Miller

From: Krzysztof Oledzki <[email protected]>
Date: Thu, 18 Oct 2007 12:37:42 +0200 (CEST)

Quoted 5 lines
Quoted 5 lines
Quoted 5 lines · Patrick McHardy
Quoted 4 lines

[NETFILTER]: nf_conntrack_tcp: fix connection reopening fix

Setting the last_dir parameter was missing from the previous one.

Quoted 8 lines
Quoted 6 lines
Quoted 6 lines
Quoted 5 lines

This description is IMHO wrong. It should be as in previous, little incomplete (last_dir), patch:

"If one side aborts an established connection, the entry still lingers for 10s in conntrack for the late packets. Allow to open up the connection again for the party which sent the RST packet."

Please sort out this changelog message issue so I can apply the patch correctly :-)

Please take the one from Krzysztof :)

maintainer
#7 lore raw

Re: [NETFILTER 01/02]: nf_conntrack_tcp: fix connection reopening fix

From: Patrick McHardy <[email protected]>
Date: Thu, 18 Oct 2007 14:16:58 +0200

Quoted 16 lines
Quoted 15 lines · David Miller

From: Krzysztof Oledzki <[email protected]>
Date: Thu, 18 Oct 2007 12:37:42 +0200 (CEST)

Quoted 12 lines
Quoted 12 lines
Quoted 11 lines · Patrick McHardy
Quoted 4 lines

[NETFILTER]: nf_conntrack_tcp: fix connection reopening fix

Setting the last_dir parameter was missing from the previous one.

This description is IMHO wrong. It should be as in previous, little incomplete (last_dir), patch:

"If one side aborts an established connection, the entry still lingers for 10s in conntrack for the late packets. Allow to open up the connection again for the party which sent the RST packet."

Quoted 5 lines
Quoted 2 lines

Please sort out this changelog message issue so I can apply the patch correctly :-)

Please take the one from Krzysztof :)

Done! :-)

#8 lore raw

Re: [NETFILTER 01/02]: nf_conntrack_tcp: fix connection reopening fix

Quoted 6 lines · David Miller
Quoted 4 lines
Quoted 2 lines

Please sort out this changelog message issue so I can apply the patch correctly :-)

Please take the one from Krzysztof :)

Done! :-)

Sorry for the mess I caused!

Best regards,
Jozsef
-
E-mail : [email protected], [email protected] PGP key : http://www.kfki.hu/~kadlec/pgp_public_key.txt
Address : KFKI Research Institute for Particle and Nuclear Physics

          H-1525 Budapest 114, POB. 49, Hungary
#9 lore raw

PATCH 2/2 : xt_sctp: fix mistake to pass a pointer where array is required

In reply to Patrick McHardy

[NETFILTER]: xt_sctp: fix mistake to pass a pointer where array is required

Macros like SCTP_CHUNKMAP_XXX(chukmap) require chukmap to be an array, but match_packet() passes a pointer to these macros. Also remove the ELEMCOUNT macro and fix a bug in SCTP_CHUNKMAP_COPY.

---
commit 49091cc16abab0ee62d22b36371f720f5727537e
tree fbd19a12cad171095c2a0fd9f87c174bbbad0b2f
parent 9768fafed7a5588757d972b2f511cb17a874f496
author Li Zefan <[email protected]> Thu, 18 Oct 2007 12:09:53 +0200 committer Patrick McHardy <[email protected]> Thu, 18 Oct 2007 12:09:53 +0200

 include/linux/netfilter/xt_sctp.h |   13 +++++--------
 net/netfilter/xt_sctp.c           |   18 ++++++++----------
 2 files changed, 13 insertions(+), 18 deletions(-)
include/linux/netfilter/xt_sctp.h +5 -8
index b157897..dd5a4fd 100644
@@ -7,9 +7,6 @@
 
 #define XT_SCTP_VALID_FLAGS		0x07
 
-#define ELEMCOUNT(x) (sizeof(x)/sizeof(x[0]))
-
-
 struct xt_sctp_flag_info {
 	u_int8_t chunktype;
 	u_int8_t flag;
@@ -59,21 +56,21 @@ struct xt_sctp_info {
 #define SCTP_CHUNKMAP_RESET(chunkmap) 				\
 	do {							\
 		int i; 						\
-		for (i = 0; i < ELEMCOUNT(chunkmap); i++)	\
+		for (i = 0; i < ARRAY_SIZE(chunkmap); i++)	\
 			chunkmap[i] = 0;			\
 	} while (0)
 
 #define SCTP_CHUNKMAP_SET_ALL(chunkmap) 			\
 	do {							\
 		int i; 						\
-		for (i = 0; i < ELEMCOUNT(chunkmap); i++) 	\
+		for (i = 0; i < ARRAY_SIZE(chunkmap); i++) 	\
 			chunkmap[i] = ~0;			\
 	} while (0)
 
 #define SCTP_CHUNKMAP_COPY(destmap, srcmap) 			\
 	do {							\
 		int i; 						\
-		for (i = 0; i < ELEMCOUNT(chunkmap); i++) 	\
+		for (i = 0; i < ARRAY_SIZE(srcmap); i++) 	\
 			destmap[i] = srcmap[i];			\
 	} while (0)
 
@@ -81,7 +78,7 @@ struct xt_sctp_info {
 ({							\
 	int i; 						\
 	int flag = 1;					\
-	for (i = 0; i < ELEMCOUNT(chunkmap); i++) {	\
+	for (i = 0; i < ARRAY_SIZE(chunkmap); i++) {	\
 		if (chunkmap[i]) {			\
 			flag = 0;			\
 			break;				\
@@ -94,7 +91,7 @@ struct xt_sctp_info {
 ({							\
 	int i; 						\
 	int flag = 1;					\
-	for (i = 0; i < ELEMCOUNT(chunkmap); i++) {	\
+	for (i = 0; i < ARRAY_SIZE(chunkmap); i++) {	\
 		if (chunkmap[i] != ~0) {		\
 			flag = 0;			\
 				break;			\
net/netfilter/xt_sctp.c +8 -10
index f907770..3358273 100644
@@ -42,21 +42,21 @@ match_flags(const struct xt_sctp_flag_info *flag_info,
 static inline bool
 match_packet(const struct sk_buff *skb,
 	     unsigned int offset,
-	     const u_int32_t *chunkmap,
-	     int chunk_match_type,
-	     const struct xt_sctp_flag_info *flag_info,
-	     const int flag_count,
+	     const struct xt_sctp_info *info,
 	     bool *hotdrop)
 {
 	u_int32_t chunkmapcopy[256 / sizeof (u_int32_t)];
 	sctp_chunkhdr_t _sch, *sch;
+	int chunk_match_type = info->chunk_match_type;
+	const struct xt_sctp_flag_info *flag_info = info->flag_info;
+	int flag_count = info->flag_count;
 
 #ifdef DEBUG_SCTP
 	int i = 0;
 #endif
 
 	if (chunk_match_type == SCTP_CHUNK_MATCH_ALL)
-		SCTP_CHUNKMAP_COPY(chunkmapcopy, chunkmap);
+		SCTP_CHUNKMAP_COPY(chunkmapcopy, info->chunkmap);
 
 	do {
 		sch = skb_header_pointer(skb, offset, sizeof(_sch), &_sch);
@@ -73,7 +73,7 @@ match_packet(const struct sk_buff *skb,
 
 		duprintf("skb->len: %d\toffset: %d\n", skb->len, offset);
 
-		if (SCTP_CHUNKMAP_IS_SET(chunkmap, sch->type)) {
+		if (SCTP_CHUNKMAP_IS_SET(info->chunkmap, sch->type)) {
 			switch (chunk_match_type) {
 			case SCTP_CHUNK_MATCH_ANY:
 				if (match_flags(flag_info, flag_count,
@@ -104,7 +104,7 @@ match_packet(const struct sk_buff *skb,
 
 	switch (chunk_match_type) {
 	case SCTP_CHUNK_MATCH_ALL:
-		return SCTP_CHUNKMAP_IS_CLEAR(chunkmap);
+		return SCTP_CHUNKMAP_IS_CLEAR(info->chunkmap);
 	case SCTP_CHUNK_MATCH_ANY:
 		return false;
 	case SCTP_CHUNK_MATCH_ONLY:
@@ -148,9 +148,7 @@ match(const struct sk_buff *skb,
 			&& ntohs(sh->dest) <= info->dpts[1],
 			XT_SCTP_DEST_PORTS, info->flags, info->invflags)
 		&& SCCHECK(match_packet(skb, protoff + sizeof (sctp_sctphdr_t),
-					info->chunkmap, info->chunk_match_type,
-					info->flag_info, info->flag_count,
-					hotdrop),
+					info, hotdrop),
 			   XT_SCTP_CHUNK_TYPES, info->flags, info->invflags);
 }
 
maintainer Applied
#10 lore raw

Re: [NETFILTER 02/02]: xt_sctp: fix mistake to pass a pointer where array is required

From: Patrick McHardy <[email protected]>
Date: Thu, 18 Oct 2007 12:12:25 +0200 (MEST)

Quoted 8 lines

[NETFILTER]: xt_sctp: fix mistake to pass a pointer where array is required

Macros like SCTP_CHUNKMAP_XXX(chukmap) require chukmap to be an array, but match_packet() passes a pointer to these macros. Also remove the ELEMCOUNT macro and fix a bug in SCTP_CHUNKMAP_COPY.

Applied, thanks!