[NETFILTER 00/02]: Netfilter fixes

[NETFILTER 00/02]: Netfilter fixes

[Patrick McHardy]

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

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

[Patrick McHardy]

[NETFILTER]: nf_conntrack_tcp: fix connection reopening fix

Setting the last_dir parameter was missing from the previous one.

Signed-off-by: Jozsef Kadlecsik <kadlec@blackhole.kfki.hu>
Tested-by: Krzysztof Piotr Oledzki <ole@ans.pl>
Signed-off-by: Patrick McHardy <kaber@trash.net>

---
commit 9768fafed7a5588757d972b2f511cb17a874f496
tree f0a8526f52a503a1cff944c9dc3012e05763310a
parent d85714d81cc0408daddb68c10f7fd69eafe7c213
author Jozsef Kadlecsik <kadlec@blackhole.kfki.hu> Thu, 18 Oct 2007 12:09:51 +0200
committer Patrick McHardy <kaber@trash.net> Thu, 18 Oct 2007 12:09:51 +0200

 net/netfilter/nf_conntrack_proto_tcp.c |   11 +++++++----
 1 files changed, 7 insertions(+), 4 deletions(-)

diff --git a/net/netfilter/nf_conntrack_proto_tcp.c b/net/netfilter/nf_conntrack_proto_tcp.c
index c707534..4dc23ab 100644
--- a/net/netfilter/nf_conntrack_proto_tcp.c
+++ b/net/netfilter/nf_conntrack_proto_tcp.c
@@ -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);

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

[Krzysztof Oledzki]

[-- Attachment #1: Type: TEXT/PLAIN, Size: 529 bytes --]



On Thu, 18 Oct 2007, Patrick McHardy wrote:

> [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

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

[Patrick McHardy]

Krzysztof Oledzki wrote:
> 
> 
> On Thu, 18 Oct 2007, Patrick McHardy wrote:
> 
>> [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."


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

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

[Krzysztof Oledzki]

[-- Attachment #1: Type: TEXT/PLAIN, Size: 1402 bytes --]



On Thu, 18 Oct 2007, Patrick McHardy wrote:

> Krzysztof Oledzki wrote:
>> 
>> 
>> On Thu, 18 Oct 2007, Patrick McHardy wrote:
>> 
>>> [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."
>
>
> 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

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

[David Miller]

From: Krzysztof Oledzki <ole@ans.pl>
Date: Thu, 18 Oct 2007 12:37:42 +0200 (CEST)

> 
> 
> On Thu, 18 Oct 2007, Patrick McHardy wrote:
> 
> > Krzysztof Oledzki wrote:
> >> 
> >> 
> >> On Thu, 18 Oct 2007, Patrick McHardy wrote:
> >> 
> >>> [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."
> >
> >
> > 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

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

Thanks!

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

[Patrick McHardy]

David Miller wrote:
> From: Krzysztof Oledzki <ole@ans.pl>
> Date: Thu, 18 Oct 2007 12:37:42 +0200 (CEST)
> 
>>>> On Thu, 18 Oct 2007, Patrick McHardy wrote:
>>>>
>>>>> [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."
>>>
> Please sort out this changelog message issue so I can apply
> the patch correctly :-)


Please take the one from Krzysztof :)

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

[David Miller]

From: Patrick McHardy <kaber@trash.net>
Date: Thu, 18 Oct 2007 14:16:58 +0200

> David Miller wrote:
> > From: Krzysztof Oledzki <ole@ans.pl>
> > Date: Thu, 18 Oct 2007 12:37:42 +0200 (CEST)
> > 
> >>>> On Thu, 18 Oct 2007, Patrick McHardy wrote:
> >>>>
> >>>>> [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."
> >>>
> > Please sort out this changelog message issue so I can apply
> > the patch correctly :-)
> 
> 
> Please take the one from Krzysztof :)

Done! :-)

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

[Jozsef Kadlecsik]

On Thu, 18 Oct 2007, David Miller wrote:

> > > 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  : kadlec@blackhole.kfki.hu, kadlec@sunserv.kfki.hu
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

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

[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.

Signed-off-by: Li Zefan <lizf@cn.fujitsu.com>
Signed-off-by: Patrick McHardy <kaber@trash.net>

---
commit 49091cc16abab0ee62d22b36371f720f5727537e
tree fbd19a12cad171095c2a0fd9f87c174bbbad0b2f
parent 9768fafed7a5588757d972b2f511cb17a874f496
author Li Zefan <lizf@cn.fujitsu.com> Thu, 18 Oct 2007 12:09:53 +0200
committer Patrick McHardy <kaber@trash.net> 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(-)

diff --git a/include/linux/netfilter/xt_sctp.h b/include/linux/netfilter/xt_sctp.h
index b157897..dd5a4fd 100644
--- a/include/linux/netfilter/xt_sctp.h
+++ b/include/linux/netfilter/xt_sctp.h
@@ -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;			\
diff --git a/net/netfilter/xt_sctp.c b/net/netfilter/xt_sctp.c
index f907770..3358273 100644
--- a/net/netfilter/xt_sctp.c
+++ b/net/netfilter/xt_sctp.c
@@ -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);
 }
 

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

[David Miller]

From: Patrick McHardy <kaber@trash.net>
Date: Thu, 18 Oct 2007 12:12:25 +0200 (MEST)

> [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.
> 
> Signed-off-by: Li Zefan <lizf@cn.fujitsu.com>
> Signed-off-by: Patrick McHardy <kaber@trash.net>

Applied, thanks!