Re: patch for conntrack expectations

Re: patch for conntrack expectations

[Harald Welte]

[-- Attachment #1: Type: text/plain, Size: 1573 bytes --]

On Mon, Feb 16, 2004 at 01:54:29AM +0100, Pablo Neira wrote:
> Hi harald!
> So I decided to implement the  function:
> 
> int ip_conntrack_alloc_expect(struct ip_conntrack_expect **new)
> 
> This function is exported. It's called from the help(...) function when 
> the pattern is found.
> It creates an expectation which will be manipulated by the helper and 
> which will be inserted by ip_conntrack_expect_related(...).
> 
> This way we don't need to work with a fake expectation and pass it to 
> ip_conntrack_expect_related which will copy the information to the real 
> ip_conntrack_expect_related.

This is indeed a true observation and good suggestion. 

Hoever, I'm not sure whether it would make any measurable difference.
The expect was allocated on the stack [which is cheap], and now we have
one additional (inter-module) function call instead of the allocation of
a couple of bytes on the stack.  I'm not sure which one is really worse.

Also, 2.4.x kernel series are heading for a freeze pretty soon.
So if at all, such a patch would be accepted for 2.6.x.  But then I
would like to ask you to change all the helpers (esp. the ones in
patch-o-matic-ng), too.

-- 
- Harald Welte <laforge@netfilter.org>             http://www.netfilter.org/
============================================================================
  "Fragmentation is like classful addressing -- an interesting early
   architectural error that shows how much experimentation was going
   on while IP was being designed."                    -- Paul Vixie

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

Re: patch for conntrack expectations

[Pablo Neira]

Harald Welte wrote:

>This is indeed a true observation and good suggestion. 
>
>Hoever, I'm not sure whether it would make any measurable difference.
>The expect was allocated on the stack [which is cheap], and now we have
>one additional (inter-module) function call instead of the allocation of
>a couple of bytes on the stack.  I'm not sure which one is really worse.
>  
>
well, sorry because I'm going to be a bit repetitive but this way 
everyone's going to understand my patch clearly.

in some steps:

1) helper function is called
2) a pointer to a ip_conntrack_expect structure is allocated on the stack
3) if the pattern which we are looking for is *not* found
    -> return NF_ACCEPT
4) else
    -> a ip_conntrack_expect structure is allocated in memory
5) all the fields of the expectation are fulfilled
6) helper calls ip_conntrack_expect_related to insert the expectation
but it passes a pointer to a structure allocated in memory.

what's the different between this approach and the current one?

If the pattern which we are looking for is not found, nothing will 
change, we just allocated less memory on the stack but of course this 
doesn't improve the performance, so it's the same.

By other hand, in the current approach if the pattern is found, the 
function ip_conntrack_expect_related does a memcpy of a structure of 116 
bytes (expectation) from the stack to memory. This is not needed anymore 
since with the new approach the helper works with the future expectation 
already allocated in memory.

I also think that this will result in a cleaner API to manage the 
expectations. well, maybe you could also use this to allocate an 
expectation for the connection tracking replication (well... i just 
started talking so much).

>Also, 2.4.x kernel series are heading for a freeze pretty soon.
>  
>
yes it's true

>So if at all, such a patch would be accepted for 2.6.x.  But then I
>would like to ask you to change all the helpers (esp. the ones in
>patch-o-matic-ng), too.
>  
>
I had a look at almost all the helpers which are in patch-o-matic-ng 
(even the pptp helper). I have no problem about modifying them. BTW, the 
documentation should be updated as well if this change is applied.

Pablo

Re: patch for conntrack expectations

[Harald Welte]

[-- Attachment #1: Type: text/plain, Size: 1461 bytes --]

On Wed, Feb 18, 2004 at 06:15:10AM +0100, Pablo Neira wrote:

> By other hand, in the current approach if the pattern is found, the 
> function ip_conntrack_expect_related does a memcpy of a structure of 116 
> bytes (expectation) from the stack to memory. This is not needed anymore 
> since with the new approach the helper works with the future expectation 
> already allocated in memory.

Ok, I missed that part.

> I also think that this will result in a cleaner API to manage the 
> expectations. well, maybe you could also use this to allocate an 
> expectation for the connection tracking replication (well... i just 
> started talking so much).

Yes, ct_sync would also need an allocation function anyway.

> I had a look at almost all the helpers which are in patch-o-matic-ng 
> (even the pptp helper). I have no problem about modifying them. BTW, the 
> documentation should be updated as well if this change is applied.

Yes, indeed.

Would you be willing to submit a 2.6.x merge, together with updates to
the existing helpers?

Thanks.

> Pablo

-- 
- Harald Welte <laforge@netfilter.org>             http://www.netfilter.org/
============================================================================
  "Fragmentation is like classful addressing -- an interesting early
   architectural error that shows how much experimentation was going
   on while IP was being designed."                    -- Paul Vixie

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

Re: patch for conntrack expectations

[Pablo Neira]

Harald Welte wrote:

>Would you be willing to submit a 2.6.x merge, together with updates to
>the existing helpers?
>  
>
yes, no problem. I'll work on those patches this weekend. More news soon.

best regards,
Pablo

Re: patch for conntrack expectations

[Pablo Neira]

[-- Attachment #1: Type: text/plain, Size: 1928 bytes --]

Hi Harald and list!

I finished my patch to add an ip_conntrack_expect_alloc(...) function. I 
also patched all the current conntrack helpers available in the stable 
branch and patch-o-matic-ng.

Some considerations:

- I tested the irc, tftp and ftp helpers, they work fine for me. I 
haven't tested the others since I don't have access to all those 
protocols (I mean, I would need more time to install some programs and 
do some testing), but I have no problem about fixing problems related to 
those patches.

- I saw that Patrick submitted recently a patch to clean up the amanda 
helper (BTW, nice work!), I tried to patch my kernel with his patch 
available in patch-o-matic but it failed. So I patched the current. I 
also have no problem about patching the new one.

BTW, I know that there's a skeleton of a helper in the documention, due 
to they mostly look the same but I think that I could add it more details.

- There's a memset in expect_alloc (see expect.patch) after the 
allocation is done, I don't like it anyway but I just added to keep 
backward compatibility because some helper usually do it. I think that 
it's better setting all the fields of the new expectation in the helper 
(this implies being a bit more restrictive with the programmer but well, 
this is kernel programming anyway, it's restrictive :-)).

I could delete that memset and have a look at all the helpers to make 
sure that all the fields are correctly set.

- I didn't exported the expect_insert function, but you could do it 
later to use it in the ct_sync code.

- I found a condition where expect_related could succesfully insert an 
expectaction when removing an old one and return EPERM (see 
expect_related-newnat14.patch). I think that this doesn't make sense. Am 
I forgetting something? That patch fix that problem.

- I also have no problem about updating the documentation to add info 
about expect_alloc.

cheers,
Pablo

[-- Attachment #2: expect.patch --]
[-- Type: text/plain, Size: 6010 bytes --]

diff -Nru --exclude .depend --exclude '*.o' --exclude '*.ko' --exclude '*.ver' --exclude '.*.flags' --exclude '*.orig' --exclude '*.rej' --exclude '*.cmd' --exclude '*.mod.c' --exclude '*~' linux-2.6.3-old/net/ipv4/netfilter/ip_conntrack_core.c linux-2.6.3/net/ipv4/netfilter/ip_conntrack_core.c
--- linux-2.6.3-old/net/ipv4/netfilter/ip_conntrack_core.c	2004-02-22 13:29:30.000000000 +0100
+++ linux-2.6.3/net/ipv4/netfilter/ip_conntrack_core.c	2004-02-22 13:29:12.000000000 +0100
@@ -917,11 +917,53 @@
 	WRITE_UNLOCK(&ip_conntrack_lock);
 }
 
+int
+ip_conntrack_expect_alloc(struct ip_conntrack_expect **new)
+{
+        *new = (struct ip_conntrack_expect *)
+              kmalloc(sizeof(struct ip_conntrack_expect), GFP_ATOMIC);
+        if (!*new) {
+                DEBUGP("expect_related: OOM allocating expect\n");
+                return -ENOMEM;
+        }
+
+	/* is this important? */
+	memset(*new, 0, sizeof(struct ip_conntrack_expect));
+
+        return 1;
+}
+
+void
+ip_conntrack_expect_insert(struct ip_conntrack_expect *new,
+               struct ip_conntrack *related_to)
+{
+       DEBUGP("new expectation %p of conntrack %p\n", new, related_to);
+       new->expectant = related_to;
+       new->sibling = NULL;
+       atomic_set(&new->use, 1);
+
+       /* add to expected list for this connection */
+       list_add(&new->expected_list, &related_to->sibling_list);
+       /* add to global list of expectations */
+
+       list_prepend(&ip_conntrack_expect_list, &new->list);
+       /* add and start timer if required */
+       if (related_to->helper->timeout) {
+               init_timer(&new->timeout);
+               new->timeout.data = (unsigned long)new;
+               new->timeout.function = expectation_timed_out;
+               new->timeout.expires = jiffies +
+                                       related_to->helper->timeout * HZ;
+               add_timer(&new->timeout);
+       }
+       related_to->expecting++;
+}
+
 /* Add a related connection. */
 int ip_conntrack_expect_related(struct ip_conntrack *related_to,
 				struct ip_conntrack_expect *expect)
 {
-	struct ip_conntrack_expect *old, *new;
+	struct ip_conntrack_expect *old;
 	int ret = 0;
 
 	WRITE_LOCK(&ip_conntrack_lock);
@@ -953,6 +995,7 @@
 
 		if (old) {
 			WRITE_UNLOCK(&ip_conntrack_lock);
+			kfree(expect);
 			return -EEXIST;
 		}
 	} else if (related_to->helper->max_expected && 
@@ -971,6 +1014,7 @@
 				       related_to->helper->name,
  		    	       	       NIPQUAD(related_to->tuplehash[IP_CT_DIR_ORIGINAL].tuple.src.ip),
  		    	       	       NIPQUAD(related_to->tuplehash[IP_CT_DIR_ORIGINAL].tuple.dst.ip));
+			kfree(expect);
 			return -EPERM;
 		}
 		DEBUGP("ip_conntrack: max number of expected "
@@ -1004,43 +1048,17 @@
 		 * related_to->expecting. 
 		 */
 		unexpect_related(old);
-		ret = -EPERM;
 	} else if (LIST_FIND(&ip_conntrack_expect_list, expect_clash,
 			     struct ip_conntrack_expect *, &expect->tuple, 
 			     &expect->mask)) {
 		WRITE_UNLOCK(&ip_conntrack_lock);
 		DEBUGP("expect_related: busy!\n");
+
+		kfree(expect);
 		return -EBUSY;
 	}
-	
-	new = (struct ip_conntrack_expect *) 
-	      kmalloc(sizeof(struct ip_conntrack_expect), GFP_ATOMIC);
-	if (!new) {
-		WRITE_UNLOCK(&ip_conntrack_lock);
-		DEBUGP("expect_relaed: OOM allocating expect\n");
-		return -ENOMEM;
-	}
-	
-	DEBUGP("new expectation %p of conntrack %p\n", new, related_to);
-	memcpy(new, expect, sizeof(*expect));
-	new->expectant = related_to;
-	new->sibling = NULL;
-	atomic_set(&new->use, 1);
-	
-	/* add to expected list for this connection */	
-	list_add(&new->expected_list, &related_to->sibling_list);
-	/* add to global list of expectations */
-	list_prepend(&ip_conntrack_expect_list, &new->list);
-	/* add and start timer if required */
-	if (related_to->helper->timeout) {
-		init_timer(&new->timeout);
-		new->timeout.data = (unsigned long)new;
-		new->timeout.function = expectation_timed_out;
-		new->timeout.expires = jiffies + 
-					related_to->helper->timeout * HZ;
-		add_timer(&new->timeout);
-	}
-	related_to->expecting++;
+
+	ip_conntrack_expect_insert(expect, related_to);
 
 	WRITE_UNLOCK(&ip_conntrack_lock);
 
diff -Nru --exclude .depend --exclude '*.o' --exclude '*.ko' --exclude '*.ver' --exclude '.*.flags' --exclude '*.orig' --exclude '*.rej' --exclude '*.cmd' --exclude '*.mod.c' --exclude '*~' linux-2.6.3-old/net/ipv4/netfilter/ip_conntrack_standalone.c linux-2.6.3/net/ipv4/netfilter/ip_conntrack_standalone.c
--- linux-2.6.3-old/net/ipv4/netfilter/ip_conntrack_standalone.c	2004-02-18 04:57:46.000000000 +0100
+++ linux-2.6.3/net/ipv4/netfilter/ip_conntrack_standalone.c	2004-02-22 12:50:49.000000000 +0100
@@ -497,6 +497,7 @@
 EXPORT_SYMBOL(ip_ct_find_proto);
 EXPORT_SYMBOL(__ip_ct_find_proto);
 EXPORT_SYMBOL(ip_ct_find_helper);
+EXPORT_SYMBOL_GPL(ip_conntrack_expect_alloc);
 EXPORT_SYMBOL(ip_conntrack_expect_related);
 EXPORT_SYMBOL(ip_conntrack_change_expect);
 EXPORT_SYMBOL(ip_conntrack_unexpect_related);
diff -Nru --exclude .depend --exclude '*.o' --exclude '*.ko' --exclude '*.ver' --exclude '.*.flags' --exclude '*.orig' --exclude '*.rej' --exclude '*.cmd' --exclude '*.mod.c' --exclude '*~' linux-2.6.3-old/include/linux/netfilter_ipv4/ip_conntrack_helper.h linux-2.6.3/include/linux/netfilter_ipv4/ip_conntrack_helper.h
--- linux-2.6.3-old/include/linux/netfilter_ipv4/ip_conntrack_helper.h	2004-02-18 04:57:16.000000000 +0100
+++ linux-2.6.3/include/linux/netfilter_ipv4/ip_conntrack_helper.h	2004-02-22 12:52:20.000000000 +0100
@@ -35,6 +35,10 @@
 
 extern struct ip_conntrack_helper *ip_ct_find_helper(const struct ip_conntrack_tuple *tuple);
 
+
+/* Allocate space for an expectation: this is mandatory before calling 
+   ip_conntrack_expect_related. */
+extern int ip_conntrack_expect_alloc(struct ip_conntrack_expect **new);
 /* Add an expected connection: can have more than one per connection */
 extern int ip_conntrack_expect_related(struct ip_conntrack *related_to,
 				       struct ip_conntrack_expect *exp);

[-- Attachment #3: expect_related-newnat14.patch --]
[-- Type: text/plain, Size: 371 bytes --]

--- ip_conntrack_core.old	2004-02-22 12:43:27.000000000 +0100
+++ ip_conntrack_core.c	2004-02-22 12:44:14.000000000 +0100
@@ -1004,7 +1004,6 @@
 		 * related_to->expecting. 
 		 */
 		unexpect_related(old);
-		ret = -EPERM;
 	} else if (LIST_FIND(&ip_conntrack_expect_list, expect_clash,
 			     struct ip_conntrack_expect *, &expect->tuple, 
 			     &expect->mask)) {

[-- Attachment #4: ip_conntrack_amanda.patch --]
[-- Type: text/plain, Size: 2738 bytes --]

diff -Nru --exclude .depend --exclude '*.o' --exclude '*.ko' --exclude '*.ver' --exclude '.*.flags' --exclude '*.orig' --exclude '*.rej' --exclude '*.cmd' --exclude '*.mod.c' --exclude '*~' linux-2.6.3-old/net/ipv4/netfilter/ip_conntrack_amanda.c linux-2.6.3/net/ipv4/netfilter/ip_conntrack_amanda.c
--- linux-2.6.3-old/net/ipv4/netfilter/ip_conntrack_amanda.c	2004-02-18 04:58:39.000000000 +0100
+++ linux-2.6.3/net/ipv4/netfilter/ip_conntrack_amanda.c	2004-02-22 12:51:29.000000000 +0100
@@ -104,18 +104,20 @@
 		char *match = strstr(data, conns[i]);
 		if (match) {
 			char *portchr;
-			struct ip_conntrack_expect expect;
-			struct ip_ct_amanda_expect *exp_amanda_info =
-				&expect.help.exp_amanda_info;
+			struct ip_conntrack_expect *expect;
+			struct ip_ct_amanda_expect *exp_amanda_info;
 
-			memset(&expect, 0, sizeof(expect));
+			if (ip_conntrack_expect_alloc(&expect) < 0)
+				return -ENOMEM;
 
+			exp_amanda_info = &expect->help.exp_amanda_info;
+			
 			data += strlen(conns[i]);
 			/* this is not really tcp, but let's steal an
 			 * idea from a tcp stream helper :-) */
 			// XXX expect.seq = data - amanda_buffer;
 			exp_amanda_info->offset = data - amanda_buffer;
-// XXX DEBUGP("expect.seq = %p - %p = %d\n", data, amanda_buffer, expect.seq);
+// XXX DEBUGP("expect->seq = %p - %p = %d\n", data, amanda_buffer, expect->seq);
 DEBUGP("exp_amanda_info->offset = %p - %p = %d\n", data, amanda_buffer, exp_amanda_info->offset);
 			portchr = data;
 			exp_amanda_info->port = simple_strtoul(data, &data,10);
@@ -129,26 +131,26 @@
 			       "%u found\n", conns[i],
 			       exp_amanda_info->port);
 
-			expect.tuple = ((struct ip_conntrack_tuple)
+			expect->tuple = ((struct ip_conntrack_tuple)
 				{ { ct->tuplehash[IP_CT_DIR_ORIGINAL].tuple.src.ip,
 				    { 0 } },
 				  { ct->tuplehash[IP_CT_DIR_ORIGINAL].tuple.dst.ip,
 				    { htons(exp_amanda_info->port) },
 				    IPPROTO_TCP }});
-			expect.mask = ((struct ip_conntrack_tuple)
+			expect->mask = ((struct ip_conntrack_tuple)
 				{ { 0, { 0 } },
 				  { 0xFFFFFFFF, { 0xFFFF }, 0xFFFF }});
 
-			expect.expectfn = NULL;
+			expect->expectfn = NULL;
 
 			DEBUGP ("ip_conntrack_amanda_help: "
 				"expect_related: %u.%u.%u.%u:%u - "
 				"%u.%u.%u.%u:%u\n",
-				NIPQUAD(expect.tuple.src.ip),
-				ntohs(expect.tuple.src.u.tcp.port),
-				NIPQUAD(expect.tuple.dst.ip),
-				ntohs(expect.tuple.dst.u.tcp.port));
-			if (ip_conntrack_expect_related(ct, &expect)
+				NIPQUAD(expect->tuple.src.ip),
+				ntohs(expect->tuple.src.u.tcp.port),
+				NIPQUAD(expect->tuple.dst.ip),
+				ntohs(expect->tuple.dst.u.tcp.port));
+			if (ip_conntrack_expect_related(ct, expect)
 			    == -EEXIST) {
 				;
 				/* this must be a packet being resent */

[-- Attachment #5: ip_conntrack_egg-expect.patch --]
[-- Type: text/plain, Size: 1455 bytes --]

--- ip_conntrack_egg.old	2004-02-22 02:24:10.000000000 +0100
+++ ip_conntrack_egg.c	2004-02-22 02:25:19.000000000 +0100
@@ -87,7 +87,7 @@
 	u_int32_t datalen = tcplen - tcph->doff * 4;
 	int dir = CTINFO2DIR(ctinfo);
 	int bytes_scanned = 0;
-	struct ip_conntrack_expect exp;
+	struct ip_conntrack_expect *exp;
 
 	u_int32_t egg_ip;
 	u_int16_t egg_port;
@@ -142,7 +142,8 @@
 			return NF_ACCEPT;
 		}
 
-		memset(&exp, 0, sizeof(exp));
+		if (ip_conntrack_expect_alloc(&exp) < 0)
+			return -ENOMEM;
 
 		if (ct->tuplehash[dir].tuple.src.ip != htonl(egg_ip)) {
 			if (net_ratelimit())
@@ -153,20 +154,20 @@
 			return NF_ACCEPT;
 		}
 
-		exp.tuple.src.ip = iph->daddr;
-		exp.tuple.src.u.tcp.port = 0;
-		exp.tuple.dst.ip = htonl(egg_ip);
-		exp.tuple.dst.u.tcp.port = htons(egg_port);
-		exp.tuple.dst.protonum = IPPROTO_TCP;
+		exp->tuple.src.ip = iph->daddr;
+		exp->tuple.src.u.tcp.port = 0;
+		exp->tuple.dst.ip = htonl(egg_ip);
+		exp->tuple.dst.u.tcp.port = htons(egg_port);
+		exp->tuple.dst.protonum = IPPROTO_TCP;
 
-		exp.mask.dst.u.tcp.port = 0xffff;
-		exp.mask.dst.protonum = 0xffff;
+		exp->mask.dst.u.tcp.port = 0xffff;
+		exp->mask.dst.protonum = 0xffff;
 
 		DEBUGP("expect_related %u.%u.%u.%u:%u - %u.%u.%u.%u:%u\n",
 			NIPQUAD(t.src.ip), ntohs(t.src.u.tcp.port),
 			NIPQUAD(t.dst.ip), ntohs(t.dst.u.tcp.port));
 
-		ip_conntrack_expect_related(ct, &exp);
+		ip_conntrack_expect_related(ct, exp);
 		break;
 	}
 	return NF_ACCEPT;

[-- Attachment #6: ip_conntrack_ftp.patch --]
[-- Type: text/plain, Size: 1582 bytes --]

diff -Nru --exclude .depend --exclude '*.o' --exclude '*.ko' --exclude '*.ver' --exclude '.*.flags' --exclude '*.orig' --exclude '*.rej' --exclude '*.cmd' --exclude '*.mod.c' --exclude '*~' linux-2.6.3-old/net/ipv4/netfilter/ip_conntrack_ftp.c linux-2.6.3/net/ipv4/netfilter/ip_conntrack_ftp.c
--- linux-2.6.3-old/net/ipv4/netfilter/ip_conntrack_ftp.c	2004-02-18 04:59:06.000000000 +0100
+++ linux-2.6.3/net/ipv4/netfilter/ip_conntrack_ftp.c	2004-02-22 12:51:05.000000000 +0100
@@ -256,8 +256,8 @@
 	int dir = CTINFO2DIR(ctinfo);
 	unsigned int matchlen, matchoff;
 	struct ip_ct_ftp_master *ct_ftp_info = &ct->help.ct_ftp_info;
-	struct ip_conntrack_expect expect, *exp = &expect;
-	struct ip_ct_ftp_expect *exp_ftp_info = &exp->help.exp_ftp_info;
+	struct ip_conntrack_expect *exp;
+	struct ip_ct_ftp_expect *exp_ftp_info;
 
 	unsigned int i;
 	int found = 0;
@@ -346,8 +346,12 @@
 	DEBUGP("conntrack_ftp: match `%.*s' (%u bytes at %u)\n",
 	       (int)matchlen, data + matchoff,
 	       matchlen, ntohl(tcph.seq) + matchoff);
-	       
-	memset(&expect, 0, sizeof(expect));
+
+	/* Allocate expectation which will be inserted */
+	if (ip_conntrack_expect_alloc(&exp) < 0)
+		return -ENOMEM;
+
+	exp_ftp_info = &exp->help.exp_ftp_info;
 
 	/* Update the ftp info */
 	if (htonl((array[0] << 24) | (array[1] << 16) | (array[2] << 8) | array[3])
@@ -389,7 +393,7 @@
 	exp->expectfn = NULL;
 
 	/* Ignore failure; should only happen with NAT */
-	ip_conntrack_expect_related(ct, &expect);
+	ip_conntrack_expect_related(ct, exp);
 	ret = NF_ACCEPT;
  out:
 	UNLOCK_BH(&ip_ftp_lock);

[-- Attachment #7: ip_conntrack_h323-expect.patch --]
[-- Type: text/plain, Size: 1694 bytes --]

--- ip_conntrack_h323.old	2004-02-22 02:27:17.000000000 +0100
+++ ip_conntrack_h323.c	2004-02-22 02:31:05.000000000 +0100
@@ -47,8 +47,8 @@
 	u_int32_t datalen = tcplen - tcph->doff * 4;
 	int dir = CTINFO2DIR(ctinfo);
 	struct ip_ct_h225_master *info = &ct->help.ct_h225_info;
-	struct ip_conntrack_expect expect, *exp = &expect;
-	struct ip_ct_h225_expect *exp_info = &exp->help.exp_h225_info;
+	struct ip_conntrack_expect *exp;
+	struct ip_ct_h225_expect *exp_info = NULL;
 	u_int16_t data_port;
 	u_int32_t data_ip;
 	unsigned int i;
@@ -91,7 +91,12 @@
 		data_ip = *((u_int32_t *)data);
 		if (data_ip == iph->saddr) {
 			data_port = *((u_int16_t *)(data + 4));
-			memset(&expect, 0, sizeof(expect));
+
+			if (ip_conntrack_expect_alloc(&exp) < 0)
+				return -ENOMEM;
+
+			exp_info = &exp->help.exp_h225_info;
+			
 			/* update the H.225 info */
 			DEBUGP("ct_h245_help: new RTCP/RTP requested %u.%u.%u.%u:->%u.%u.%u.%u:%u\n",
 				NIPQUAD(ct->tuplehash[!dir].tuple.src.ip),
@@ -164,8 +169,8 @@
 	u_int32_t datalen = tcplen - tcph->doff * 4;
 	int dir = CTINFO2DIR(ctinfo);
 	struct ip_ct_h225_master *info = &ct->help.ct_h225_info;
-	struct ip_conntrack_expect expect, *exp = &expect;
-	struct ip_ct_h225_expect *exp_info = &exp->help.exp_h225_info;
+	struct ip_conntrack_expect *exp;
+	struct ip_ct_h225_expect *exp_info = NULL;
 	u_int16_t data_port;
 	u_int32_t data_ip;
 	unsigned int i;
@@ -222,7 +227,10 @@
 				UNLOCK_BH(&ip_h323_lock);
 #endif
 			} else {
-				memset(&expect, 0, sizeof(expect));
+				if (ip_conntrack_expect_alloc(&exp) < 0)
+					return -ENOMEM;
+
+				exp_info = &exp->help.exp_h225_info;
 
 				/* update the H.225 info */
 				LOCK_BH(&ip_h323_lock);

[-- Attachment #8: ip_conntrack_irc.patch --]
[-- Type: text/plain, Size: 1336 bytes --]

diff -Nru --exclude .depend --exclude '*.o' --exclude '*.ko' --exclude '*.ver' --exclude '.*.flags' --exclude '*.orig' --exclude '*.rej' --exclude '*.cmd' --exclude '*.mod.c' --exclude '*~' linux-2.6.3-old/net/ipv4/netfilter/ip_conntrack_irc.c linux-2.6.3/net/ipv4/netfilter/ip_conntrack_irc.c
--- linux-2.6.3-old/net/ipv4/netfilter/ip_conntrack_irc.c	2004-02-18 04:59:06.000000000 +0100
+++ linux-2.6.3/net/ipv4/netfilter/ip_conntrack_irc.c	2004-02-22 12:51:19.000000000 +0100
@@ -106,8 +106,8 @@
 	struct tcphdr tcph;
 	char *data, *data_limit;
 	int dir = CTINFO2DIR(ctinfo);
-	struct ip_conntrack_expect expect, *exp = &expect;
-	struct ip_ct_irc_expect *exp_irc_info = &exp->help.exp_irc_info;
+	struct ip_conntrack_expect *exp;
+	struct ip_ct_irc_expect *exp_irc_info = NULL;
 
 	u_int32_t dcc_ip;
 	u_int16_t dcc_port;
@@ -190,8 +190,11 @@
 
 				continue;
 			}
-			
-			memset(&expect, 0, sizeof(expect));
+
+			if (ip_conntrack_expect_alloc(&exp) < 0)
+				return -ENOMEM;
+
+			exp_irc_info = &exp->help.exp_irc_info;
 
 			/* save position of address in dcc string,
 			 * necessary for NAT */
@@ -218,7 +221,7 @@
 				NIPQUAD(exp->tuple.dst.ip),
 				ntohs(exp->tuple.dst.u.tcp.port));
 
-			ip_conntrack_expect_related(ct, &expect);
+			ip_conntrack_expect_related(ct, exp);
 
 			goto out;
 		} /* for .. NUM_DCCPROTO */

[-- Attachment #9: ip_conntrack_mms-expect.patch --]
[-- Type: text/plain, Size: 1187 bytes --]

--- ip_conntrack_mms.old	2004-02-22 02:32:36.000000000 +0100
+++ ip_conntrack_mms.c	2004-02-22 02:34:25.000000000 +0100
@@ -150,8 +150,8 @@
 	unsigned int tcplen = len - iph->ihl * 4;
 	unsigned int datalen = tcplen - tcph->doff * 4;
 	int dir = CTINFO2DIR(ctinfo);
-	struct ip_conntrack_expect expect, *exp = &expect; 
-	struct ip_ct_mms_expect *exp_mms_info = &exp->help.exp_mms_info;
+	struct ip_conntrack_expect *exp; 
+	struct ip_ct_mms_expect *exp_mms_info = NULL;
 	
 	u_int32_t mms_ip;
 	u_int16_t mms_proto;
@@ -207,7 +207,10 @@
 				printk(KERN_WARNING
 				       "ip_conntrack_mms: Unable to parse data payload\n");
 
-		memset(&expect, 0, sizeof(expect));
+		if (ip_conntrack_expect_alloc(&exp) < 0)
+			return -ENOMEM;
+
+		exp_mms_info = &exp->help.exp_mms_info;
 
 		sprintf(mms_proto_string, "(%u)", mms_proto);
 		DEBUGP("ip_conntrack_mms: adding %s expectation %u.%u.%u.%u -> %u.%u.%u.%u:%u\n",
@@ -243,7 +246,7 @@
 		             { { 0xFFFFFFFF, { 0 } },
 		               { 0xFFFFFFFF, { .tcp = { 0xFFFF } }, 0xFFFF }});
 		exp->expectfn = NULL;
-		ip_conntrack_expect_related(ct, &expect);
+		ip_conntrack_expect_related(ct, exp);
 		UNLOCK_BH(&ip_mms_lock);
 	}
 

[-- Attachment #10: ip_conntrack_pptp.patch --]
[-- Type: text/plain, Size: 4245 bytes --]

--- ip_conntrack_pptp.old	2004-02-22 12:18:11.000000000 +0100
+++ ip_conntrack_pptp.c	2004-02-22 12:57:42.000000000 +0100
@@ -169,72 +169,74 @@
 	u_int16_t callid,
 	u_int16_t peer_callid)
 {
-	struct ip_conntrack_expect exp;
+	struct ip_conntrack_expect *exp;
 	struct ip_conntrack_tuple inv_tuple;
 
-	memset(&exp, 0, sizeof(exp));
+	if (ip_conntrack_expect_alloc(&exp) < 0)
+		return -ENOMEM;
+	
 	/* tuple in original direction, PNS->PAC */
-	exp.tuple.src.ip = master->tuplehash[IP_CT_DIR_ORIGINAL].tuple.src.ip;
-	exp.tuple.src.u.gre.key = htonl(ntohs(peer_callid));
-	exp.tuple.dst.ip = master->tuplehash[IP_CT_DIR_ORIGINAL].tuple.dst.ip;
-	exp.tuple.dst.u.gre.key = htonl(ntohs(callid));
-	exp.tuple.dst.u.gre.protocol = __constant_htons(GRE_PROTOCOL_PPTP);
-	exp.tuple.dst.u.gre.version = GRE_VERSION_PPTP;
-	exp.tuple.dst.protonum = IPPROTO_GRE;
-
-	exp.mask.src.ip = 0xffffffff;
-	exp.mask.src.u.all = 0;
-	exp.mask.dst.u.all = 0;
-	exp.mask.dst.u.gre.key = 0xffffffff;
-	exp.mask.dst.u.gre.version = 0xff;
-	exp.mask.dst.u.gre.protocol = 0xffff;
-	exp.mask.dst.ip = 0xffffffff;
-	exp.mask.dst.protonum = 0xffff;
+	exp->tuple.src.ip = master->tuplehash[IP_CT_DIR_ORIGINAL].tuple.src.ip;
+	exp->tuple.src.u.gre.key = htonl(ntohs(peer_callid));
+	exp->tuple.dst.ip = master->tuplehash[IP_CT_DIR_ORIGINAL].tuple.dst.ip;
+	exp->tuple.dst.u.gre.key = htonl(ntohs(callid));
+	exp->tuple.dst.u.gre.protocol = __constant_htons(GRE_PROTOCOL_PPTP);
+	exp->tuple.dst.u.gre.version = GRE_VERSION_PPTP;
+	exp->tuple.dst.protonum = IPPROTO_GRE;
+
+	exp->mask.src.ip = 0xffffffff;
+	exp->mask.src.u.all = 0;
+	exp->mask.dst.u.all = 0;
+	exp->mask.dst.u.gre.key = 0xffffffff;
+	exp->mask.dst.u.gre.version = 0xff;
+	exp->mask.dst.u.gre.protocol = 0xffff;
+	exp->mask.dst.ip = 0xffffffff;
+	exp->mask.dst.protonum = 0xffff;
 			
-	exp.seq = seq;
-	exp.expectfn = pptp_expectfn;
+	exp->seq = seq;
+	exp->expectfn = pptp_expectfn;
 
-	exp.help.exp_pptp_info.pac_call_id = ntohs(callid);
-	exp.help.exp_pptp_info.pns_call_id = ntohs(peer_callid);
+	exp->help.exp_pptp_info.pac_call_id = ntohs(callid);
+	exp->help.exp_pptp_info.pns_call_id = ntohs(peer_callid);
 
 	DEBUGP("calling expect_related ");
-	DUMP_TUPLE_RAW(&exp.tuple);
+	DUMP_TUPLE_RAW(&exp->tuple);
 	
 	/* Add GRE keymap entries */
-	if (ip_ct_gre_keymap_add(&exp, &exp.tuple, 0) != 0)
+	if (ip_ct_gre_keymap_add(exp, &exp->tuple, 0) != 0)
 		return 1;
 
-	invert_tuplepr(&inv_tuple, &exp.tuple);
-	if (ip_ct_gre_keymap_add(&exp, &inv_tuple, 1) != 0) {
-		ip_ct_gre_keymap_destroy(&exp);
+	invert_tuplepr(&inv_tuple, &exp->tuple);
+	if (ip_ct_gre_keymap_add(exp, &inv_tuple, 1) != 0) {
+		ip_ct_gre_keymap_destroy(exp);
 		return 1;
 	}
 	
-	if (ip_conntrack_expect_related(master, &exp) != 0) {
-		ip_ct_gre_keymap_destroy(&exp);
+	if (ip_conntrack_expect_related(master, exp) != 0) {
+		ip_ct_gre_keymap_destroy(exp);
 		DEBUGP("cannot expect_related()\n");
 		return 1;
 	}
 
 	/* tuple in reply direction, PAC->PNS */
-	exp.tuple.src.ip = master->tuplehash[IP_CT_DIR_REPLY].tuple.src.ip;
-	exp.tuple.src.u.gre.key = htonl(ntohs(callid));
-	exp.tuple.dst.ip = master->tuplehash[IP_CT_DIR_REPLY].tuple.dst.ip;
-	exp.tuple.dst.u.gre.key = htonl(ntohs(peer_callid));
+	exp->tuple.src.ip = master->tuplehash[IP_CT_DIR_REPLY].tuple.src.ip;
+	exp->tuple.src.u.gre.key = htonl(ntohs(callid));
+	exp->tuple.dst.ip = master->tuplehash[IP_CT_DIR_REPLY].tuple.dst.ip;
+	exp->tuple.dst.u.gre.key = htonl(ntohs(peer_callid));
 
 	DEBUGP("calling expect_related ");
-	DUMP_TUPLE_RAW(&exp.tuple);
+	DUMP_TUPLE_RAW(&exp->tuple);
 	
 	/* Add GRE keymap entries */
-	ip_ct_gre_keymap_add(&exp, &exp.tuple, 0);
-	invert_tuplepr(&inv_tuple, &exp.tuple);
-	ip_ct_gre_keymap_add(&exp, &inv_tuple, 1);
+	ip_ct_gre_keymap_add(exp, &exp->tuple, 0);
+	invert_tuplepr(&inv_tuple, &exp->tuple);
+	ip_ct_gre_keymap_add(exp, &inv_tuple, 1);
 	/* FIXME: cannot handle error correctly, since we need to free
 	 * the above keymap :( */
 	
-	if (ip_conntrack_expect_related(master, &exp) != 0) {
+	if (ip_conntrack_expect_related(master, exp) != 0) {
 		/* free the second pair of keypmaps */
-		ip_ct_gre_keymap_destroy(&exp);
+		ip_ct_gre_keymap_destroy(exp);
 		DEBUGP("cannot expect_related():\n");
 		return 1;
 	}

[-- Attachment #11: ip_conntrack_quake3-expect.patch --]
[-- Type: text/plain, Size: 1374 bytes --]

--- ip_conntrack_quake3.old	2004-02-22 02:37:23.000000000 +0100
+++ ip_conntrack_quake3.c	2004-02-22 02:38:18.000000000 +0100
@@ -58,7 +58,7 @@
 {
 	struct udphdr *udph = (void *)iph + iph->ihl * 4;
 	int dir = CTINFO2DIR(ctinfo);
-	struct ip_conntrack_expect exp;
+	struct ip_conntrack_expect *exp;
 	int i;
 	
         /* Until there's been traffic both ways, don't look in packets. note: it's UDP ! */
@@ -77,20 +77,21 @@
 			       NIPQUAD( (u_int32_t) *( (u_int32_t *)( (int)udph + i ) ) ), 
 			       ntohs((__u16) *( (__u16 *)( (int)udph + i + 4 ) ) ) );
 
-			memset(&exp, 0, sizeof(exp));
+			if (ip_conntrack_expect_alloc(&exp) < 0)
+				return -ENOMEM;
 
-			exp.tuple = ((struct ip_conntrack_tuple)
+			exp->tuple = ((struct ip_conntrack_tuple)
 			             { { ct->tuplehash[!dir].tuple.src.ip, { 0 } },
 			               { (u_int32_t) *((u_int32_t *)((int)udph + i)), 
 			               { .udp = { (__u16) *((__u16 *)((int)udph+i+4)) } }, 
 			                 IPPROTO_UDP } }
 			            );
-			exp.mask  = ((struct ip_conntrack_tuple)
+			exp->mask  = ((struct ip_conntrack_tuple)
 			             { { 0xFFFFFFFF, { 0 } },
 		                       { 0xFFFFFFFF, { .udp = { 0xFFFF } }, 0xFFFF }});
-			exp.expectfn = NULL;
+			exp->expectfn = NULL;
 
-			ip_conntrack_expect_related(ct, &exp);
+			ip_conntrack_expect_related(ct, exp);
 		}
 
 	}

[-- Attachment #12: ip_conntrack_rtsp-expect.patch --]
[-- Type: text/plain, Size: 3403 bytes --]

--- ip_conntrack_rtsp.c	2004-02-22 02:43:27.000000000 +0100
+++ ip_conntrack_rtsp.old	2004-02-22 02:42:50.000000000 +0100
@@ -269,7 +269,7 @@
     uint    datalen = tcplen - tcph->doff * 4;
     uint    dataoff = 0;
 
-    struct ip_conntrack_expect exp;
+    struct ip_conntrack_expect *exp;
 
     while (dataoff < datalen)
     {
@@ -296,7 +296,8 @@
         }
         DEBUGP("found a setup message\n");
 
-        memset(&exp, 0, sizeof(exp));
+	if (ip_conntrack_expect_alloc(&exp) < 0)
+		return -ENOMEM;
 
         off = 0;
         while (nf_mime_nextline(pdata+hdrsoff, hdrslen, &off,
@@ -315,42 +316,42 @@
             if (nf_strncasecmp(pdata+hdrsoff+lineoff, "Transport:", 10) == 0)
             {
                 rtsp_parse_transport(pdata+hdrsoff+lineoff, linelen,
-                                     &exp.help.exp_rtsp_info);
+                                     &exp->help.exp_rtsp_info);
             }
         }
 
-        if (exp.help.exp_rtsp_info.loport == 0)
+        if (exp->help.exp_rtsp_info.loport == 0)
         {
             DEBUGP("no udp transports found\n");
             continue;   /* no udp transports found */
         }
 
         DEBUGP("udp transport found, ports=(%d,%hu,%hu)\n",
-              (int)exp.help.exp_rtsp_info.pbtype,
-              exp.help.exp_rtsp_info.loport,
-              exp.help.exp_rtsp_info.hiport);
+              (int)exp->help.exp_rtsp_info.pbtype,
+              exp->help.exp_rtsp_info.loport,
+              exp->help.exp_rtsp_info.hiport);
 
         LOCK_BH(&ip_rtsp_lock);
-        exp.seq = ntohl(tcph->seq) + hdrsoff; /* mark all the headers */
-        exp.help.exp_rtsp_info.len = hdrslen;
+        exp->seq = ntohl(tcph->seq) + hdrsoff; /* mark all the headers */
+        exp->help.exp_rtsp_info.len = hdrslen;
 
-        exp.tuple.src.ip = ct->tuplehash[!dir].tuple.src.ip;
-        exp.mask.src.ip  = 0xffffffff;
-        exp.tuple.dst.ip = ct->tuplehash[dir].tuple.src.ip;
-        exp.mask.dst.ip  = 0xffffffff;
-        exp.tuple.dst.u.udp.port = exp.help.exp_rtsp_info.loport;
-        exp.mask.dst.u.udp.port  = (exp.help.exp_rtsp_info.pbtype == pb_range) ? 0xfffe : 0xffff;
-        exp.tuple.dst.protonum = IPPROTO_UDP;
-        exp.mask.dst.protonum  = 0xffff;
+        exp->tuple.src.ip = ct->tuplehash[!dir].tuple.src.ip;
+        exp->mask.src.ip  = 0xffffffff;
+        exp->tuple.dst.ip = ct->tuplehash[dir].tuple.src.ip;
+        exp->mask.dst.ip  = 0xffffffff;
+        exp->tuple.dst.u.udp.port = exp->help.exp_rtsp_info.loport;
+        exp->mask.dst.u.udp.port  = (exp->help.exp_rtsp_info.pbtype == pb_range) ? 0xfffe : 0xffff;
+        exp->tuple.dst.protonum = IPPROTO_UDP;
+        exp->mask.dst.protonum  = 0xffff;
 
         DEBUGP("expect_related %u.%u.%u.%u:%u-%u.%u.%u.%u:%u\n",
-                NIPQUAD(exp.tuple.src.ip),
-                ntohs(exp.tuple.src.u.tcp.port),
-                NIPQUAD(exp.tuple.dst.ip),
-                ntohs(exp.tuple.dst.u.tcp.port));
+                NIPQUAD(exp->tuple.src.ip),
+                ntohs(exp->tuple.src.u.tcp.port),
+                NIPQUAD(exp->tuple.dst.ip),
+                ntohs(exp->tuple.dst.u.tcp.port));
 
         /* pass the request off to the nat helper */
-        rc = ip_conntrack_expect_related(ct, &exp);
+        rc = ip_conntrack_expect_related(ct, exp);
         UNLOCK_BH(&ip_rtsp_lock);
         if (rc == 0)
         {

[-- Attachment #13: ip_conntrack_talk-expect.patch --]
[-- Type: text/plain, Size: 1111 bytes --]

--- ip_conntrack_talk.old	2004-02-22 02:46:10.000000000 +0100
+++ ip_conntrack_talk.c	2004-02-22 02:48:15.000000000 +0100
@@ -94,8 +94,8 @@
 		              struct talk_addr *addr)
 {
 	int dir = CTINFO2DIR(ctinfo);
-	struct ip_conntrack_expect expect, *exp = &expect;
-	struct ip_ct_talk_expect *exp_talk_info = &exp->help.exp_talk_info;
+	struct ip_conntrack_expect *exp;
+	struct ip_ct_talk_expect *exp_talk_info = NULL;
 
 	DEBUGP("ip_ct_talk_help_response: %u.%u.%u.%u:%u, type %d answer %d\n",
 		NIPQUAD(addr->ta_addr), ntohs(addr->ta_port),
@@ -103,8 +103,11 @@
 
 	if (!(answer == SUCCESS && type == mode))
 		return NF_ACCEPT;
-	
-	memset(&expect, 0, sizeof(expect));
+
+	if (ip_conntrack_expect_alloc(&exp) < 0)
+		return -ENOMEM;
+
+	exp_talk_info = &exp->help.exp_talk_info;
 	
 	if (type == ANNOUNCE) {
 
@@ -132,7 +135,7 @@
 		       NIPQUAD(exp->tuple.dst.ip), ntohs(exp->tuple.dst.u.udp.port));
 
 		/* Ignore failure; should only happen with NAT */
-		ip_conntrack_expect_related(ct, &expect);
+		ip_conntrack_expect_related(ct, exp);
 		UNLOCK_BH(&ip_talk_lock);
 	}
 	if (type == LOOK_UP) {

[-- Attachment #14: ip_conntrack_tftp.patch --]
[-- Type: text/plain, Size: 1695 bytes --]

diff -Nru --exclude .depend --exclude '*.o' --exclude '*.ko' --exclude '*.ver' --exclude '.*.flags' --exclude '*.orig' --exclude '*.rej' --exclude '*.cmd' --exclude '*.mod.c' --exclude '*~' linux-2.6.3-old/net/ipv4/netfilter/ip_conntrack_tftp.c linux-2.6.3/net/ipv4/netfilter/ip_conntrack_tftp.c
--- linux-2.6.3-old/net/ipv4/netfilter/ip_conntrack_tftp.c	2004-02-18 04:57:22.000000000 +0100
+++ linux-2.6.3/net/ipv4/netfilter/ip_conntrack_tftp.c	2004-02-22 12:51:42.000000000 +0100
@@ -44,7 +44,7 @@
 		     enum ip_conntrack_info ctinfo)
 {
 	struct tftphdr tftph;
-	struct ip_conntrack_expect exp;
+	struct ip_conntrack_expect *exp;
 
 	if (skb_copy_bits(skb, skb->nh.iph->ihl * 4 + sizeof(struct udphdr),
 			  &tftph, sizeof(tftph)) != 0)
@@ -57,19 +57,21 @@
 		DEBUGP("");
 		DUMP_TUPLE(&ct->tuplehash[IP_CT_DIR_ORIGINAL].tuple);
 		DUMP_TUPLE(&ct->tuplehash[IP_CT_DIR_REPLY].tuple);
-		memset(&exp, 0, sizeof(exp));
 
-		exp.tuple = ct->tuplehash[IP_CT_DIR_REPLY].tuple;
-		exp.mask.src.ip = 0xffffffff;
-		exp.mask.dst.ip = 0xffffffff;
-		exp.mask.dst.u.udp.port = 0xffff;
-		exp.mask.dst.protonum = 0xffff;
-		exp.expectfn = NULL;
+		if (ip_conntrack_expect_alloc(&exp) < 0)
+			return -ENOMEM;
+
+		exp->tuple = ct->tuplehash[IP_CT_DIR_REPLY].tuple;
+		exp->mask.src.ip = 0xffffffff;
+		exp->mask.dst.ip = 0xffffffff;
+		exp->mask.dst.u.udp.port = 0xffff;
+		exp->mask.dst.protonum = 0xffff;
+		exp->expectfn = NULL;
 
 		DEBUGP("expect: ");
-		DUMP_TUPLE(&exp.tuple);
-		DUMP_TUPLE(&exp.mask);
-		ip_conntrack_expect_related(ct, &exp);
+		DUMP_TUPLE(&exp->tuple);
+		DUMP_TUPLE(&exp->mask);
+		ip_conntrack_expect_related(ct, exp);
 		break;
 	default:
 		DEBUGP("Unknown opcode\n");

Re: patch for conntrack expectations

[Harald Welte]

[-- Attachment #1: Type: text/plain, Size: 4229 bytes --]

On Sun, Feb 22, 2004 at 02:40:28PM +0100, Pablo Neira wrote:
> Hi Harald and list!

Thanks pablo.

I am fine with that patch, and I'd like to put it in patch-o-matic for
now.  As the changes are fairly obvious, I hope we won't get too much
trouble with it ;)  However, please see my following comments.

> I finished my patch to add an ip_conntrack_expect_alloc(...) function. I 
> also patched all the current conntrack helpers available in the stable 
> branch and patch-o-matic-ng.

thanks. This means that once we get it into the core kernel, we should
definitely have  the first pom-ng release out, otherwise people will
still use the old API.

This makes me think of another issue.  We should either rename the
function, reorder arguments, do whatever.   We cannot just make all old
(unpatched) code compile without any warning, and still have it crash :(
I think it's best to exchange the order of the two arguments in
ip_conntrack_expect_related().  This way a compiler would complain if
some old helper was compiled with the new core.

> - I saw that Patrick submitted recently a patch to clean up the amanda 
> helper (BTW, nice work!), I tried to patch my kernel with his patch 
> available in patch-o-matic but it failed. So I patched the current. I 
> also have no problem about patching the new one.

I thought this was included already in the stock kernel. At least in
2.4.x the patch went in.  Patrick, can you comment on this?

> BTW, I know that there's a skeleton of a helper in the documention, due 
> to they mostly look the same but I think that I could add it more details.

yes, please.  Use the SGML master file, don't edit the HTML

> - There's a memset in expect_alloc (see expect.patch) after the 
> allocation is done, I don't like it anyway but I just added to keep 
> backward compatibility because some helper usually do it. I think that 
> it's better setting all the fields of the new expectation in the helper 
> (this implies being a bit more restrictive with the programmer but well, 
> this is kernel programming anyway, it's restrictive :-)).

yes, sometimes code like this is included mainly for debugging purpose
and then left behind.

> I could delete that memset and have a look at all the helpers to make 
> sure that all the fields are correctly set.

The issue is not very easy in this case.  Assume that tuple->dst.u
is 32bits wide (like after applying the pptp patch).  This in turn means
that every helper has to explicitly take care not to initialize
tuple->dst.u with a 16bit tcp/udp port number, since it could end
up in the 'other' 16bit half.  Rather, they have to assign
tuple->dst.u.tcp.port.  Those cases should all be fixed by now.

Now look what tuple_cmp does.  It memcmp's the whole union, not just the
individual fields.  This means that a tuple always has to be cleanly
initialized with zero, even in those parts that don't contain any
useful information.

So for now, I'd like to keep that memset until all the code is audited
with that regard.

> - I didn't exported the expect_insert function, but you could do it 
> later to use it in the ct_sync code.

that's fine.

> - I found a condition where expect_related could succesfully insert an 
> expectaction when removing an old one and return EPERM (see 
> expect_related-newnat14.patch). I think that this doesn't make sense. Am 
> I forgetting something? That patch fix that problem.

 
> - I also have no problem about updating the documentation to add info 
> about expect_alloc.

ok, please do so.

One issue with your patch:  It doesn't seem to use tab's for
indentation, but rather 8 spaces. Please clean this up.

Also, we cannot export that symbol GPL only, since it doesn't add a new
API but rather replaces an old one.
 
> cheers,
> Pablo

-- 
- Harald Welte <laforge@netfilter.org>             http://www.netfilter.org/
============================================================================
  "Fragmentation is like classful addressing -- an interesting early
   architectural error that shows how much experimentation was going
   on while IP was being designed."                    -- Paul Vixie

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

Re: patch for conntrack expectations

[Patrick McHardy]

Harald Welte wrote:
> On Sun, Feb 22, 2004 at 02:40:28PM +0100, Pablo Neira wrote:
> 
>>- I saw that Patrick submitted recently a patch to clean up the amanda 
>>helper (BTW, nice work!), I tried to patch my kernel with his patch 
>>available in patch-o-matic but it failed. So I patched the current. I 
>>also have no problem about patching the new one.
> 
> 
> I thought this was included already in the stock kernel. At least in
> 2.4.x the patch went in.  Patrick, can you comment on this?

The 2.4 patch went in, the 2.6 didn't. Some of the problems got fixed
in 2.6 when removing skb_linearize, but some didn't. Do you want me to
resubmit the patch to you ?

Regards
Patrick

Re: patch for conntrack expectations

[Harald Welte]

[-- Attachment #1: Type: text/plain, Size: 659 bytes --]

On Tue, Feb 24, 2004 at 10:54:39AM +0100, Patrick McHardy wrote:

> The 2.4 patch went in, the 2.6 didn't. Some of the problems got fixed
> in 2.6 when removing skb_linearize, but some didn't. Do you want me to
> resubmit the patch to you ?

yes, please.

> Regards
> Patrick

-- 
- Harald Welte <laforge@netfilter.org>             http://www.netfilter.org/
============================================================================
  "Fragmentation is like classful addressing -- an interesting early
   architectural error that shows how much experimentation was going
   on while IP was being designed."                    -- Paul Vixie

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

Re: patch for conntrack expectations

[Patrick McHardy]

[-- Attachment #1: Type: text/plain, Size: 367 bytes --]

Harald Welte wrote:
> On Tue, Feb 24, 2004 at 10:54:39AM +0100, Patrick McHardy wrote:
> 
> 
>>The 2.4 patch went in, the 2.6 didn't. Some of the problems got fixed
>>in 2.6 when removing skb_linearize, but some didn't. Do you want me to
>>resubmit the patch to you ?
> 
> 
> yes, please.

Old patch attached, it still applies cleanly.

> 
>>Regards
>>Patrick
> 
> 


[-- Attachment #2: 2.6-amanda-helpers.diff --]
[-- Type: text/x-patch, Size: 16339 bytes --]

# This is a BitKeeper generated patch for the following project:
# Project Name: Linux kernel tree
# This patch format is intended for GNU patch command version 2.5 or higher.
# This patch includes the following deltas:
#	           ChangeSet	1.1356  -> 1.1357 
#	include/linux/netfilter_ipv4/ip_conntrack_amanda.h	1.2     -> 1.3    
#	net/ipv4/netfilter/ip_nat_amanda.c	1.5     -> 1.6    
#	net/ipv4/netfilter/ip_conntrack_amanda.c	1.4     -> 1.5    
#
# The following is the BitKeeper ChangeSet Log
# --------------------------------------------
# 03/09/28	kaber@trash.net	1.1357
# [NETFILTER]: Fix amanda helpers
# --------------------------------------------
#
diff -Nru a/include/linux/netfilter_ipv4/ip_conntrack_amanda.h b/include/linux/netfilter_ipv4/ip_conntrack_amanda.h
--- a/include/linux/netfilter_ipv4/ip_conntrack_amanda.h	Sun Sep 28 04:38:42 2003
+++ b/include/linux/netfilter_ipv4/ip_conntrack_amanda.h	Sun Sep 28 04:38:42 2003
@@ -2,20 +2,11 @@
 #define _IP_CONNTRACK_AMANDA_H
 /* AMANDA tracking. */
 
-#ifdef __KERNEL__
-
-#include <linux/netfilter_ipv4/lockhelp.h>
-
-/* Protects amanda part of conntracks */
-DECLARE_LOCK_EXTERN(ip_amanda_lock);
-
-#endif
-
 struct ip_ct_amanda_expect
 {
 	u_int16_t port;		/* port number of this expectation */
-	u_int16_t offset;	/* offset of the port specification in ctrl packet */
-	u_int16_t len;		/* the length of the port number specification */
+	u_int16_t offset;	/* offset of port in ctrl packet */
+	u_int16_t len;		/* length of the port number string */
 };
 
 #endif /* _IP_CONNTRACK_AMANDA_H */
diff -Nru a/net/ipv4/netfilter/ip_conntrack_amanda.c b/net/ipv4/netfilter/ip_conntrack_amanda.c
--- a/net/ipv4/netfilter/ip_conntrack_amanda.c	Sun Sep 28 04:38:42 2003
+++ b/net/ipv4/netfilter/ip_conntrack_amanda.c	Sun Sep 28 04:38:42 2003
@@ -18,6 +18,7 @@
  *
  */
 
+#include <linux/kernel.h>
 #include <linux/module.h>
 #include <linux/netfilter.h>
 #include <linux/ip.h>
@@ -36,50 +37,37 @@
 MODULE_PARM(master_timeout, "i");
 MODULE_PARM_DESC(master_timeout, "timeout for the master connection");
 
-DECLARE_LOCK(ip_amanda_lock);
-
-char *conns[] = { "DATA ", "MESG ", "INDEX " };
-
-#if 0
-#define DEBUGP printk
-#else
-#define DEBUGP(format, args...)
-#endif
+static char *conns[] = { "DATA ", "MESG ", "INDEX " };
 
 /* This is slow, but it's simple. --RR */
 static char amanda_buffer[65536];
+static DECLARE_LOCK(amanda_buffer_lock);
 
 static int help(struct sk_buff *skb,
-		struct ip_conntrack *ct, enum ip_conntrack_info ctinfo)
+                struct ip_conntrack *ct, enum ip_conntrack_info ctinfo)
 {
-	char *data, *data_limit;
-	int dir = CTINFO2DIR(ctinfo);
+	struct ip_conntrack_expect exp;
+	struct ip_ct_amanda_expect *exp_amanda_info;
+	char *data, *data_limit, *tmp;
 	unsigned int dataoff, i;
-	struct ip_ct_amanda *info =
-				(struct ip_ct_amanda *)&ct->help.ct_ftp_info;
 
-	/* Can't track connections formed before we registered */
-	if (!info)
+	/* Only look at packets from the Amanda server */
+	if (CTINFO2DIR(ctinfo) == IP_CT_DIR_ORIGINAL)
 		return NF_ACCEPT;
 
 	/* increase the UDP timeout of the master connection as replies from
 	 * Amanda clients to the server can be quite delayed */
 	ip_ct_refresh(ct, master_timeout * HZ);
 
-	/* If packet is coming from Amanda server */
-	if (dir == IP_CT_DIR_ORIGINAL)
-		return NF_ACCEPT;
-
 	/* No data? */
 	dataoff = skb->nh.iph->ihl*4 + sizeof(struct udphdr);
 	if (dataoff >= skb->len) {
 		if (net_ratelimit())
-			printk("ip_conntrack_amanda_help: skblen = %u\n",
-			       (unsigned)skb->len);
+			printk("amanda_help: skblen = %u\n", skb->len);
 		return NF_ACCEPT;
 	}
 
-	LOCK_BH(&ip_amanda_lock);
+	LOCK_BH(&amanda_buffer_lock);
 	skb_copy_bits(skb, dataoff, amanda_buffer, skb->len - dataoff);
 	data = amanda_buffer;
 	data_limit = amanda_buffer + skb->len - dataoff;
@@ -89,84 +77,39 @@
 	data = strstr(data, "CONNECT ");
 	if (!data)
 		goto out;
-
-	DEBUGP("ip_conntrack_amanda_help: CONNECT found in connection "
-		   "%u.%u.%u.%u:%u %u.%u.%u.%u:%u\n",
-		   NIPQUAD(iph->saddr), htons(udph->source),
-		   NIPQUAD(iph->daddr), htons(udph->dest));
 	data += strlen("CONNECT ");
 
+	memset(&exp, 0, sizeof(exp));
+	exp.tuple.src.ip = ct->tuplehash[IP_CT_DIR_ORIGINAL].tuple.src.ip;
+	exp.tuple.dst.ip = ct->tuplehash[IP_CT_DIR_ORIGINAL].tuple.dst.ip;
+	exp.tuple.dst.protonum = IPPROTO_TCP;
+	exp.mask.src.ip = 0xFFFFFFFF;
+	exp.mask.dst.ip = 0xFFFFFFFF;
+	exp.mask.dst.protonum = 0xFFFF;
+	exp.mask.dst.u.tcp.port = 0xFFFF;
+
 	/* Only search first line. */	
-	if (strchr(data, '\n'))
-		*strchr(data, '\n') = '\0';
+	if ((tmp = strchr(data, '\n')))
+		*tmp = '\0';
 
+	exp_amanda_info = &exp.help.exp_amanda_info;
 	for (i = 0; i < ARRAY_SIZE(conns); i++) {
 		char *match = strstr(data, conns[i]);
-		if (match) {
-			char *portchr;
-			struct ip_conntrack_expect expect;
-			struct ip_ct_amanda_expect *exp_amanda_info =
-				&expect.help.exp_amanda_info;
-
-			memset(&expect, 0, sizeof(expect));
-
-			data += strlen(conns[i]);
-			/* this is not really tcp, but let's steal an
-			 * idea from a tcp stream helper :-) */
-			// XXX expect.seq = data - amanda_buffer;
-			exp_amanda_info->offset = data - amanda_buffer;
-// XXX DEBUGP("expect.seq = %p - %p = %d\n", data, amanda_buffer, expect.seq);
-DEBUGP("exp_amanda_info->offset = %p - %p = %d\n", data, amanda_buffer, exp_amanda_info->offset);
-			portchr = data;
-			exp_amanda_info->port = simple_strtoul(data, &data,10);
-			exp_amanda_info->len = data - portchr;
-
-			/* eat whitespace */
-			while (*data == ' ')
-				data++;
-			DEBUGP("ip_conntrack_amanda_help: "
-			       "CONNECT %s request with port "
-			       "%u found\n", conns[i],
-			       exp_amanda_info->port);
-
-			expect.tuple = ((struct ip_conntrack_tuple)
-				{ { ct->tuplehash[IP_CT_DIR_ORIGINAL].tuple.src.ip,
-				    { 0 } },
-				  { ct->tuplehash[IP_CT_DIR_ORIGINAL].tuple.dst.ip,
-				    { htons(exp_amanda_info->port) },
-				    IPPROTO_TCP }});
-			expect.mask = ((struct ip_conntrack_tuple)
-				{ { 0, { 0 } },
-				  { 0xFFFFFFFF, { 0xFFFF }, 0xFFFF }});
-
-			expect.expectfn = NULL;
-
-			DEBUGP ("ip_conntrack_amanda_help: "
-				"expect_related: %u.%u.%u.%u:%u - "
-				"%u.%u.%u.%u:%u\n",
-				NIPQUAD(expect.tuple.src.ip),
-				ntohs(expect.tuple.src.u.tcp.port),
-				NIPQUAD(expect.tuple.dst.ip),
-				ntohs(expect.tuple.dst.u.tcp.port));
-			if (ip_conntrack_expect_related(ct, &expect)
-			    == -EEXIST) {
-				;
-				/* this must be a packet being resent */
-				/* XXX - how do I get the
-				 *       ip_conntrack_expect that
-				 *       already exists so that I can
-				 *       update the .seq so that the
-				 *       nat module rewrites the port
-				 *       numbers?
-				 *       Perhaps I should use the
-				 *       exp_amanda_info instead of
-				 *       .seq.
-				 */
-			}
-		}
+		if (!match)
+			continue;
+		tmp = data = match + strlen(conns[i]);
+		exp_amanda_info->offset = data - amanda_buffer;
+		exp_amanda_info->port   = simple_strtoul(data, &data, 10);
+		exp_amanda_info->len    = data - tmp;
+		if (exp_amanda_info->port == 0 || exp_amanda_info->len > 5)
+			break;
+
+		exp.tuple.dst.u.tcp.port = htons(exp_amanda_info->port);
+		ip_conntrack_expect_related(ct, &exp);
 	}
- out:
-	UNLOCK_BH(&ip_amanda_lock);
+
+out:
+	UNLOCK_BH(&amanda_buffer_lock);
 	return NF_ACCEPT;
 }
 
@@ -186,29 +129,16 @@
 	},
 };
 
-static void fini(void)
+static void __exit fini(void)
 {
-	DEBUGP("ip_ct_amanda: unregistering helper for port 10080\n");
 	ip_conntrack_helper_unregister(&amanda_helper);
 }
 
 static int __init init(void)
 {
-	int ret;
-
-	DEBUGP("ip_ct_amanda: registering helper for port 10080\n");
-	ret = ip_conntrack_helper_register(&amanda_helper);
-
-	if (ret) {
-		printk("ip_ct_amanda: ERROR registering helper\n");
-		fini();
-		return -EBUSY;
-	}
-	return 0;
+	return ip_conntrack_helper_register(&amanda_helper);
 }
 
 PROVIDES_CONNTRACK(amanda);
-EXPORT_SYMBOL(ip_amanda_lock);
-
 module_init(init);
 module_exit(fini);
diff -Nru a/net/ipv4/netfilter/ip_nat_amanda.c b/net/ipv4/netfilter/ip_nat_amanda.c
--- a/net/ipv4/netfilter/ip_nat_amanda.c	Sun Sep 28 04:38:42 2003
+++ b/net/ipv4/netfilter/ip_nat_amanda.c	Sun Sep 28 04:38:42 2003
@@ -11,69 +11,45 @@
  * 	insmod ip_nat_amanda.o
  */
 
+#include <linux/kernel.h>
 #include <linux/module.h>
-#include <linux/netfilter_ipv4.h>
+#include <linux/netfilter.h>
+#include <linux/skbuff.h>
 #include <linux/ip.h>
 #include <linux/udp.h>
-#include <linux/kernel.h>
 #include <net/tcp.h>
 #include <net/udp.h>
 
+#include <linux/netfilter_ipv4.h>
 #include <linux/netfilter_ipv4/ip_nat.h>
 #include <linux/netfilter_ipv4/ip_nat_helper.h>
-#include <linux/netfilter_ipv4/ip_nat_rule.h>
 #include <linux/netfilter_ipv4/ip_conntrack_helper.h>
 #include <linux/netfilter_ipv4/ip_conntrack_amanda.h>
 
 
-#if 0
-#define DEBUGP printk
-#define DUMP_OFFSET(x)	printk("offset_before=%d, offset_after=%d, correction_pos=%u\n", x->offset_before, x->offset_after, x->correction_pos);
-#else
-#define DEBUGP(format, args...)
-#define DUMP_OFFSET(x)
-#endif
-
 MODULE_AUTHOR("Brian J. Murrell <netfilter@interlinx.bc.ca>");
 MODULE_DESCRIPTION("Amanda NAT helper");
 MODULE_LICENSE("GPL");
 
-/* protects amanda part of conntracks */
-DECLARE_LOCK_EXTERN(ip_amanda_lock);
-
 static unsigned int
 amanda_nat_expected(struct sk_buff **pskb,
-		 unsigned int hooknum,
-		 struct ip_conntrack *ct,
-		 struct ip_nat_info *info)
+                    unsigned int hooknum,
+                    struct ip_conntrack *ct,
+                    struct ip_nat_info *info)
 {
-	struct ip_nat_multi_range mr;
-	u_int32_t newdstip, newsrcip, newip;
-	u_int16_t port;
-	struct ip_ct_amanda_expect *exp_info;
 	struct ip_conntrack *master = master_ct(ct);
+	struct ip_ct_amanda_expect *exp_amanda_info;
+	struct ip_nat_multi_range mr;
+	u_int32_t newip;
 
 	IP_NF_ASSERT(info);
 	IP_NF_ASSERT(master);
-
 	IP_NF_ASSERT(!(info->initialized & (1 << HOOK2MANIP(hooknum))));
 
-	DEBUGP("nat_expected: We have a connection!\n");
-	exp_info = &ct->master->help.exp_amanda_info;
-
-	newdstip = ct->tuplehash[IP_CT_DIR_REPLY].tuple.src.ip;
-	newsrcip = master->tuplehash[IP_CT_DIR_ORIGINAL].tuple.src.ip;
-	DEBUGP("nat_expected: %u.%u.%u.%u->%u.%u.%u.%u\n",
-	       NIPQUAD(newsrcip), NIPQUAD(newdstip));
-
-	port = exp_info->port;
-
 	if (HOOK2MANIP(hooknum) == IP_NAT_MANIP_SRC)
-		newip = newsrcip;
+		newip = master->tuplehash[IP_CT_DIR_REPLY].tuple.dst.ip;
 	else
-		newip = newdstip;
-
-	DEBUGP("nat_expected: IP to %u.%u.%u.%u\n", NIPQUAD(newip));
+		newip = master->tuplehash[IP_CT_DIR_REPLY].tuple.src.ip;
 
 	mr.rangesize = 1;
 	/* We don't want to manip the per-protocol, just the IPs. */
@@ -81,121 +57,79 @@
 	mr.range[0].min_ip = mr.range[0].max_ip = newip;
 
 	if (HOOK2MANIP(hooknum) == IP_NAT_MANIP_DST) {
+		exp_amanda_info = &ct->master->help.exp_amanda_info;
 		mr.range[0].flags |= IP_NAT_RANGE_PROTO_SPECIFIED;
 		mr.range[0].min = mr.range[0].max
 			= ((union ip_conntrack_manip_proto)
-				{ .udp = { htons(port) } });
+				{ .udp = { htons(exp_amanda_info->port) } });
 	}
 
 	return ip_nat_setup_info(ct, &mr, hooknum);
 }
 
 static int amanda_data_fixup(struct ip_conntrack *ct,
-			  struct sk_buff **pskb,
-			  enum ip_conntrack_info ctinfo,
-			  struct ip_conntrack_expect *expect)
+                             struct sk_buff **pskb,
+                             enum ip_conntrack_info ctinfo,
+                             struct ip_conntrack_expect *exp)
 {
-	u_int32_t newip;
-	/* DATA 99999 MESG 99999 INDEX 99999 */
-	char buffer[6];
-	struct ip_conntrack_expect *exp = expect;
-	struct ip_ct_amanda_expect *ct_amanda_info = &exp->help.exp_amanda_info;
+	struct ip_ct_amanda_expect *exp_amanda_info;
 	struct ip_conntrack_tuple t = exp->tuple;
+	char buffer[sizeof("65535")];
 	u_int16_t port;
 
-	MUST_BE_LOCKED(&ip_amanda_lock);
-
-	newip = ct->tuplehash[IP_CT_DIR_ORIGINAL].tuple.dst.ip;
-	DEBUGP ("ip_nat_amanda_help: newip = %u.%u.%u.%u\n", NIPQUAD(newip));
-
 	/* Alter conntrack's expectations. */
-
-	/* We can read expect here without conntrack lock, since it's
-	   only set in ip_conntrack_amanda, with ip_amanda_lock held
-	   writable */
-
-	t.dst.ip = newip;
-	for (port = ct_amanda_info->port; port != 0; port++) {
+	exp_amanda_info = &exp->help.exp_amanda_info;
+	t.dst.ip = ct->tuplehash[IP_CT_DIR_ORIGINAL].tuple.dst.ip;
+	for (port = exp_amanda_info->port; port != 0; port++) {
 		t.dst.u.tcp.port = htons(port);
 		if (ip_conntrack_change_expect(exp, &t) == 0)
 			break;
 	}
-
 	if (port == 0)
 		return 0;
 
 	sprintf(buffer, "%u", port);
-
-	return ip_nat_mangle_udp_packet(pskb, ct, ctinfo, /* XXX exp->seq */ ct_amanda_info->offset, 
-					ct_amanda_info->len, buffer, strlen(buffer));
+	return ip_nat_mangle_udp_packet(pskb, ct, ctinfo,
+	                                exp_amanda_info->offset,
+	                                exp_amanda_info->len,
+	                                buffer, strlen(buffer));
 }
 
 static unsigned int help(struct ip_conntrack *ct,
-			 struct ip_conntrack_expect *exp,
-			 struct ip_nat_info *info,
-			 enum ip_conntrack_info ctinfo,
-			 unsigned int hooknum,
-			 struct sk_buff **pskb)
+                         struct ip_conntrack_expect *exp,
+                         struct ip_nat_info *info,
+                         enum ip_conntrack_info ctinfo,
+                         unsigned int hooknum,
+                         struct sk_buff **pskb)
 {
-	int dir;
+	int dir = CTINFO2DIR(ctinfo);
+	int ret = NF_ACCEPT;
 
-	if (!exp)
-		DEBUGP("ip_nat_amanda: no exp!!");
-		
 	/* Only mangle things once: original direction in POST_ROUTING
 	   and reply direction on PRE_ROUTING. */
-	dir = CTINFO2DIR(ctinfo);
 	if (!((hooknum == NF_IP_POST_ROUTING && dir == IP_CT_DIR_ORIGINAL)
-	      || (hooknum == NF_IP_PRE_ROUTING && dir == IP_CT_DIR_REPLY))) {
-		DEBUGP("ip_nat_amanda_help: Not touching dir %s at hook %s\n",
-		       dir == IP_CT_DIR_ORIGINAL ? "ORIG" : "REPLY",
-		       hooknum == NF_IP_POST_ROUTING ? "POSTROUTING"
-		       : hooknum == NF_IP_PRE_ROUTING ? "PREROUTING"
-		       : hooknum == NF_IP_LOCAL_OUT ? "OUTPUT"
-		       : hooknum == NF_IP_LOCAL_IN ? "INPUT" : "???");
+	      || (hooknum == NF_IP_PRE_ROUTING && dir == IP_CT_DIR_REPLY)))
 		return NF_ACCEPT;
-	}
-	DEBUGP("ip_nat_amanda_help: got beyond not touching: dir %s at hook %s for expect: ",
-		   dir == IP_CT_DIR_ORIGINAL ? "ORIG" : "REPLY",
-		   hooknum == NF_IP_POST_ROUTING ? "POSTROUTING"
-		   : hooknum == NF_IP_PRE_ROUTING ? "PREROUTING"
-		     : hooknum == NF_IP_LOCAL_OUT ? "OUTPUT"
-		       : hooknum == NF_IP_LOCAL_IN ? "INPUT" : "???");
-	DUMP_TUPLE(&exp->tuple);
 
-	LOCK_BH(&ip_amanda_lock);
-// XXX	if (exp->seq != 0)
+	/* if this exectation has a "offset" the packet needs to be mangled */
 	if (exp->help.exp_amanda_info.offset != 0)
-		/*  if this packet has a "seq" it needs to have it's content mangled */
-		if (!amanda_data_fixup(ct, pskb, ctinfo, exp)) {
-			UNLOCK_BH(&ip_amanda_lock);
-			DEBUGP("ip_nat_amanda: NF_DROP\n");
-			return NF_DROP;
-		}
+		if (!amanda_data_fixup(ct, pskb, ctinfo, exp))
+			ret = NF_DROP;
 	exp->help.exp_amanda_info.offset = 0;
-	UNLOCK_BH(&ip_amanda_lock);
 
-	DEBUGP("ip_nat_amanda: NF_ACCEPT\n");
-	return NF_ACCEPT;
+	return ret;
 }
 
 static struct ip_nat_helper ip_nat_amanda_helper;
 
-/* This function is intentionally _NOT_ defined as  __exit, because
- * it is needed by init() */
-static void fini(void)
+static void __exit fini(void)
 {
-	DEBUGP("ip_nat_amanda: unregistering nat helper\n");
 	ip_nat_helper_unregister(&ip_nat_amanda_helper);
 }
 
 static int __init init(void)
 {
-	int ret = 0;
-	struct ip_nat_helper *hlpr;
-
-	hlpr = &ip_nat_amanda_helper;
-	memset(hlpr, 0, sizeof(struct ip_nat_helper));
+	struct ip_nat_helper *hlpr = &ip_nat_amanda_helper;
 
 	hlpr->tuple.dst.protonum = IPPROTO_UDP;
 	hlpr->tuple.src.u.udp.port = htons(10080);
@@ -205,20 +139,9 @@
 	hlpr->flags = 0;
 	hlpr->me = THIS_MODULE;
 	hlpr->expect = amanda_nat_expected;
-
 	hlpr->name = "amanda";
 
-	DEBUGP
-	    ("ip_nat_amanda: Trying to register nat helper\n");
-	ret = ip_nat_helper_register(hlpr);
-
-	if (ret) {
-		printk
-		    ("ip_nat_amanda: error registering nat helper\n");
-		fini();
-		return 1;
-	}
-	return ret;
+	return ip_nat_helper_register(hlpr);
 }
 
 NEEDS_CONNTRACK(amanda);

Re: patch for conntrack expectations

[Pablo Neira]

[-- Attachment #1: Type: text/plain, Size: 17447 bytes --]

Attached my patch for the patrick's amanda helper to use the new 
expect_alloc api.

Pablo

Patrick McHardy wrote:

> Harald Welte wrote:
>
>> On Tue, Feb 24, 2004 at 10:54:39AM +0100, Patrick McHardy wrote:
>>
>>
>>> The 2.4 patch went in, the 2.6 didn't. Some of the problems got fixed
>>> in 2.6 when removing skb_linearize, but some didn't. Do you want me to
>>> resubmit the patch to you ?
>>
>>
>>
>> yes, please.
>
>
> Old patch attached, it still applies cleanly.
>
>>
>>> Regards
>>> Patrick
>>
>>
>>
>
>------------------------------------------------------------------------
>
># This is a BitKeeper generated patch for the following project:
># Project Name: Linux kernel tree
># This patch format is intended for GNU patch command version 2.5 or higher.
># This patch includes the following deltas:
>#	           ChangeSet	1.1356  -> 1.1357 
>#	include/linux/netfilter_ipv4/ip_conntrack_amanda.h	1.2     -> 1.3    
>#	net/ipv4/netfilter/ip_nat_amanda.c	1.5     -> 1.6    
>#	net/ipv4/netfilter/ip_conntrack_amanda.c	1.4     -> 1.5    
>#
># The following is the BitKeeper ChangeSet Log
># --------------------------------------------
># 03/09/28	kaber@trash.net	1.1357
># [NETFILTER]: Fix amanda helpers
># --------------------------------------------
>#
>diff -Nru a/include/linux/netfilter_ipv4/ip_conntrack_amanda.h b/include/linux/netfilter_ipv4/ip_conntrack_amanda.h
>--- a/include/linux/netfilter_ipv4/ip_conntrack_amanda.h	Sun Sep 28 04:38:42 2003
>+++ b/include/linux/netfilter_ipv4/ip_conntrack_amanda.h	Sun Sep 28 04:38:42 2003
>@@ -2,20 +2,11 @@
> #define _IP_CONNTRACK_AMANDA_H
> /* AMANDA tracking. */
> 
>-#ifdef __KERNEL__
>-
>-#include <linux/netfilter_ipv4/lockhelp.h>
>-
>-/* Protects amanda part of conntracks */
>-DECLARE_LOCK_EXTERN(ip_amanda_lock);
>-
>-#endif
>-
> struct ip_ct_amanda_expect
> {
> 	u_int16_t port;		/* port number of this expectation */
>-	u_int16_t offset;	/* offset of the port specification in ctrl packet */
>-	u_int16_t len;		/* the length of the port number specification */
>+	u_int16_t offset;	/* offset of port in ctrl packet */
>+	u_int16_t len;		/* length of the port number string */
> };
> 
> #endif /* _IP_CONNTRACK_AMANDA_H */
>diff -Nru a/net/ipv4/netfilter/ip_conntrack_amanda.c b/net/ipv4/netfilter/ip_conntrack_amanda.c
>--- a/net/ipv4/netfilter/ip_conntrack_amanda.c	Sun Sep 28 04:38:42 2003
>+++ b/net/ipv4/netfilter/ip_conntrack_amanda.c	Sun Sep 28 04:38:42 2003
>@@ -18,6 +18,7 @@
>  *
>  */
> 
>+#include <linux/kernel.h>
> #include <linux/module.h>
> #include <linux/netfilter.h>
> #include <linux/ip.h>
>@@ -36,50 +37,37 @@
> MODULE_PARM(master_timeout, "i");
> MODULE_PARM_DESC(master_timeout, "timeout for the master connection");
> 
>-DECLARE_LOCK(ip_amanda_lock);
>-
>-char *conns[] = { "DATA ", "MESG ", "INDEX " };
>-
>-#if 0
>-#define DEBUGP printk
>-#else
>-#define DEBUGP(format, args...)
>-#endif
>+static char *conns[] = { "DATA ", "MESG ", "INDEX " };
> 
> /* This is slow, but it's simple. --RR */
> static char amanda_buffer[65536];
>+static DECLARE_LOCK(amanda_buffer_lock);
> 
> static int help(struct sk_buff *skb,
>-		struct ip_conntrack *ct, enum ip_conntrack_info ctinfo)
>+                struct ip_conntrack *ct, enum ip_conntrack_info ctinfo)
> {
>-	char *data, *data_limit;
>-	int dir = CTINFO2DIR(ctinfo);
>+	struct ip_conntrack_expect exp;
>+	struct ip_ct_amanda_expect *exp_amanda_info;
>+	char *data, *data_limit, *tmp;
> 	unsigned int dataoff, i;
>-	struct ip_ct_amanda *info =
>-				(struct ip_ct_amanda *)&ct->help.ct_ftp_info;
> 
>-	/* Can't track connections formed before we registered */
>-	if (!info)
>+	/* Only look at packets from the Amanda server */
>+	if (CTINFO2DIR(ctinfo) == IP_CT_DIR_ORIGINAL)
> 		return NF_ACCEPT;
> 
> 	/* increase the UDP timeout of the master connection as replies from
> 	 * Amanda clients to the server can be quite delayed */
> 	ip_ct_refresh(ct, master_timeout * HZ);
> 
>-	/* If packet is coming from Amanda server */
>-	if (dir == IP_CT_DIR_ORIGINAL)
>-		return NF_ACCEPT;
>-
> 	/* No data? */
> 	dataoff = skb->nh.iph->ihl*4 + sizeof(struct udphdr);
> 	if (dataoff >= skb->len) {
> 		if (net_ratelimit())
>-			printk("ip_conntrack_amanda_help: skblen = %u\n",
>-			       (unsigned)skb->len);
>+			printk("amanda_help: skblen = %u\n", skb->len);
> 		return NF_ACCEPT;
> 	}
> 
>-	LOCK_BH(&ip_amanda_lock);
>+	LOCK_BH(&amanda_buffer_lock);
> 	skb_copy_bits(skb, dataoff, amanda_buffer, skb->len - dataoff);
> 	data = amanda_buffer;
> 	data_limit = amanda_buffer + skb->len - dataoff;
>@@ -89,84 +77,39 @@
> 	data = strstr(data, "CONNECT ");
> 	if (!data)
> 		goto out;
>-
>-	DEBUGP("ip_conntrack_amanda_help: CONNECT found in connection "
>-		   "%u.%u.%u.%u:%u %u.%u.%u.%u:%u\n",
>-		   NIPQUAD(iph->saddr), htons(udph->source),
>-		   NIPQUAD(iph->daddr), htons(udph->dest));
> 	data += strlen("CONNECT ");
> 
>+	memset(&exp, 0, sizeof(exp));
>+	exp.tuple.src.ip = ct->tuplehash[IP_CT_DIR_ORIGINAL].tuple.src.ip;
>+	exp.tuple.dst.ip = ct->tuplehash[IP_CT_DIR_ORIGINAL].tuple.dst.ip;
>+	exp.tuple.dst.protonum = IPPROTO_TCP;
>+	exp.mask.src.ip = 0xFFFFFFFF;
>+	exp.mask.dst.ip = 0xFFFFFFFF;
>+	exp.mask.dst.protonum = 0xFFFF;
>+	exp.mask.dst.u.tcp.port = 0xFFFF;
>+
> 	/* Only search first line. */	
>-	if (strchr(data, '\n'))
>-		*strchr(data, '\n') = '\0';
>+	if ((tmp = strchr(data, '\n')))
>+		*tmp = '\0';
> 
>+	exp_amanda_info = &exp.help.exp_amanda_info;
> 	for (i = 0; i < ARRAY_SIZE(conns); i++) {
> 		char *match = strstr(data, conns[i]);
>-		if (match) {
>-			char *portchr;
>-			struct ip_conntrack_expect expect;
>-			struct ip_ct_amanda_expect *exp_amanda_info =
>-				&expect.help.exp_amanda_info;
>-
>-			memset(&expect, 0, sizeof(expect));
>-
>-			data += strlen(conns[i]);
>-			/* this is not really tcp, but let's steal an
>-			 * idea from a tcp stream helper :-) */
>-			// XXX expect.seq = data - amanda_buffer;
>-			exp_amanda_info->offset = data - amanda_buffer;
>-// XXX DEBUGP("expect.seq = %p - %p = %d\n", data, amanda_buffer, expect.seq);
>-DEBUGP("exp_amanda_info->offset = %p - %p = %d\n", data, amanda_buffer, exp_amanda_info->offset);
>-			portchr = data;
>-			exp_amanda_info->port = simple_strtoul(data, &data,10);
>-			exp_amanda_info->len = data - portchr;
>-
>-			/* eat whitespace */
>-			while (*data == ' ')
>-				data++;
>-			DEBUGP("ip_conntrack_amanda_help: "
>-			       "CONNECT %s request with port "
>-			       "%u found\n", conns[i],
>-			       exp_amanda_info->port);
>-
>-			expect.tuple = ((struct ip_conntrack_tuple)
>-				{ { ct->tuplehash[IP_CT_DIR_ORIGINAL].tuple.src.ip,
>-				    { 0 } },
>-				  { ct->tuplehash[IP_CT_DIR_ORIGINAL].tuple.dst.ip,
>-				    { htons(exp_amanda_info->port) },
>-				    IPPROTO_TCP }});
>-			expect.mask = ((struct ip_conntrack_tuple)
>-				{ { 0, { 0 } },
>-				  { 0xFFFFFFFF, { 0xFFFF }, 0xFFFF }});
>-
>-			expect.expectfn = NULL;
>-
>-			DEBUGP ("ip_conntrack_amanda_help: "
>-				"expect_related: %u.%u.%u.%u:%u - "
>-				"%u.%u.%u.%u:%u\n",
>-				NIPQUAD(expect.tuple.src.ip),
>-				ntohs(expect.tuple.src.u.tcp.port),
>-				NIPQUAD(expect.tuple.dst.ip),
>-				ntohs(expect.tuple.dst.u.tcp.port));
>-			if (ip_conntrack_expect_related(ct, &expect)
>-			    == -EEXIST) {
>-				;
>-				/* this must be a packet being resent */
>-				/* XXX - how do I get the
>-				 *       ip_conntrack_expect that
>-				 *       already exists so that I can
>-				 *       update the .seq so that the
>-				 *       nat module rewrites the port
>-				 *       numbers?
>-				 *       Perhaps I should use the
>-				 *       exp_amanda_info instead of
>-				 *       .seq.
>-				 */
>-			}
>-		}
>+		if (!match)
>+			continue;
>+		tmp = data = match + strlen(conns[i]);
>+		exp_amanda_info->offset = data - amanda_buffer;
>+		exp_amanda_info->port   = simple_strtoul(data, &data, 10);
>+		exp_amanda_info->len    = data - tmp;
>+		if (exp_amanda_info->port == 0 || exp_amanda_info->len > 5)
>+			break;
>+
>+		exp.tuple.dst.u.tcp.port = htons(exp_amanda_info->port);
>+		ip_conntrack_expect_related(ct, &exp);
> 	}
>- out:
>-	UNLOCK_BH(&ip_amanda_lock);
>+
>+out:
>+	UNLOCK_BH(&amanda_buffer_lock);
> 	return NF_ACCEPT;
> }
> 
>@@ -186,29 +129,16 @@
> 	},
> };
> 
>-static void fini(void)
>+static void __exit fini(void)
> {
>-	DEBUGP("ip_ct_amanda: unregistering helper for port 10080\n");
> 	ip_conntrack_helper_unregister(&amanda_helper);
> }
> 
> static int __init init(void)
> {
>-	int ret;
>-
>-	DEBUGP("ip_ct_amanda: registering helper for port 10080\n");
>-	ret = ip_conntrack_helper_register(&amanda_helper);
>-
>-	if (ret) {
>-		printk("ip_ct_amanda: ERROR registering helper\n");
>-		fini();
>-		return -EBUSY;
>-	}
>-	return 0;
>+	return ip_conntrack_helper_register(&amanda_helper);
> }
> 
> PROVIDES_CONNTRACK(amanda);
>-EXPORT_SYMBOL(ip_amanda_lock);
>-
> module_init(init);
> module_exit(fini);
>diff -Nru a/net/ipv4/netfilter/ip_nat_amanda.c b/net/ipv4/netfilter/ip_nat_amanda.c
>--- a/net/ipv4/netfilter/ip_nat_amanda.c	Sun Sep 28 04:38:42 2003
>+++ b/net/ipv4/netfilter/ip_nat_amanda.c	Sun Sep 28 04:38:42 2003
>@@ -11,69 +11,45 @@
>  * 	insmod ip_nat_amanda.o
>  */
> 
>+#include <linux/kernel.h>
> #include <linux/module.h>
>-#include <linux/netfilter_ipv4.h>
>+#include <linux/netfilter.h>
>+#include <linux/skbuff.h>
> #include <linux/ip.h>
> #include <linux/udp.h>
>-#include <linux/kernel.h>
> #include <net/tcp.h>
> #include <net/udp.h>
> 
>+#include <linux/netfilter_ipv4.h>
> #include <linux/netfilter_ipv4/ip_nat.h>
> #include <linux/netfilter_ipv4/ip_nat_helper.h>
>-#include <linux/netfilter_ipv4/ip_nat_rule.h>
> #include <linux/netfilter_ipv4/ip_conntrack_helper.h>
> #include <linux/netfilter_ipv4/ip_conntrack_amanda.h>
> 
> 
>-#if 0
>-#define DEBUGP printk
>-#define DUMP_OFFSET(x)	printk("offset_before=%d, offset_after=%d, correction_pos=%u\n", x->offset_before, x->offset_after, x->correction_pos);
>-#else
>-#define DEBUGP(format, args...)
>-#define DUMP_OFFSET(x)
>-#endif
>-
> MODULE_AUTHOR("Brian J. Murrell <netfilter@interlinx.bc.ca>");
> MODULE_DESCRIPTION("Amanda NAT helper");
> MODULE_LICENSE("GPL");
> 
>-/* protects amanda part of conntracks */
>-DECLARE_LOCK_EXTERN(ip_amanda_lock);
>-
> static unsigned int
> amanda_nat_expected(struct sk_buff **pskb,
>-		 unsigned int hooknum,
>-		 struct ip_conntrack *ct,
>-		 struct ip_nat_info *info)
>+                    unsigned int hooknum,
>+                    struct ip_conntrack *ct,
>+                    struct ip_nat_info *info)
> {
>-	struct ip_nat_multi_range mr;
>-	u_int32_t newdstip, newsrcip, newip;
>-	u_int16_t port;
>-	struct ip_ct_amanda_expect *exp_info;
> 	struct ip_conntrack *master = master_ct(ct);
>+	struct ip_ct_amanda_expect *exp_amanda_info;
>+	struct ip_nat_multi_range mr;
>+	u_int32_t newip;
> 
> 	IP_NF_ASSERT(info);
> 	IP_NF_ASSERT(master);
>-
> 	IP_NF_ASSERT(!(info->initialized & (1 << HOOK2MANIP(hooknum))));
> 
>-	DEBUGP("nat_expected: We have a connection!\n");
>-	exp_info = &ct->master->help.exp_amanda_info;
>-
>-	newdstip = ct->tuplehash[IP_CT_DIR_REPLY].tuple.src.ip;
>-	newsrcip = master->tuplehash[IP_CT_DIR_ORIGINAL].tuple.src.ip;
>-	DEBUGP("nat_expected: %u.%u.%u.%u->%u.%u.%u.%u\n",
>-	       NIPQUAD(newsrcip), NIPQUAD(newdstip));
>-
>-	port = exp_info->port;
>-
> 	if (HOOK2MANIP(hooknum) == IP_NAT_MANIP_SRC)
>-		newip = newsrcip;
>+		newip = master->tuplehash[IP_CT_DIR_REPLY].tuple.dst.ip;
> 	else
>-		newip = newdstip;
>-
>-	DEBUGP("nat_expected: IP to %u.%u.%u.%u\n", NIPQUAD(newip));
>+		newip = master->tuplehash[IP_CT_DIR_REPLY].tuple.src.ip;
> 
> 	mr.rangesize = 1;
> 	/* We don't want to manip the per-protocol, just the IPs. */
>@@ -81,121 +57,79 @@
> 	mr.range[0].min_ip = mr.range[0].max_ip = newip;
> 
> 	if (HOOK2MANIP(hooknum) == IP_NAT_MANIP_DST) {
>+		exp_amanda_info = &ct->master->help.exp_amanda_info;
> 		mr.range[0].flags |= IP_NAT_RANGE_PROTO_SPECIFIED;
> 		mr.range[0].min = mr.range[0].max
> 			= ((union ip_conntrack_manip_proto)
>-				{ .udp = { htons(port) } });
>+				{ .udp = { htons(exp_amanda_info->port) } });
> 	}
> 
> 	return ip_nat_setup_info(ct, &mr, hooknum);
> }
> 
> static int amanda_data_fixup(struct ip_conntrack *ct,
>-			  struct sk_buff **pskb,
>-			  enum ip_conntrack_info ctinfo,
>-			  struct ip_conntrack_expect *expect)
>+                             struct sk_buff **pskb,
>+                             enum ip_conntrack_info ctinfo,
>+                             struct ip_conntrack_expect *exp)
> {
>-	u_int32_t newip;
>-	/* DATA 99999 MESG 99999 INDEX 99999 */
>-	char buffer[6];
>-	struct ip_conntrack_expect *exp = expect;
>-	struct ip_ct_amanda_expect *ct_amanda_info = &exp->help.exp_amanda_info;
>+	struct ip_ct_amanda_expect *exp_amanda_info;
> 	struct ip_conntrack_tuple t = exp->tuple;
>+	char buffer[sizeof("65535")];
> 	u_int16_t port;
> 
>-	MUST_BE_LOCKED(&ip_amanda_lock);
>-
>-	newip = ct->tuplehash[IP_CT_DIR_ORIGINAL].tuple.dst.ip;
>-	DEBUGP ("ip_nat_amanda_help: newip = %u.%u.%u.%u\n", NIPQUAD(newip));
>-
> 	/* Alter conntrack's expectations. */
>-
>-	/* We can read expect here without conntrack lock, since it's
>-	   only set in ip_conntrack_amanda, with ip_amanda_lock held
>-	   writable */
>-
>-	t.dst.ip = newip;
>-	for (port = ct_amanda_info->port; port != 0; port++) {
>+	exp_amanda_info = &exp->help.exp_amanda_info;
>+	t.dst.ip = ct->tuplehash[IP_CT_DIR_ORIGINAL].tuple.dst.ip;
>+	for (port = exp_amanda_info->port; port != 0; port++) {
> 		t.dst.u.tcp.port = htons(port);
> 		if (ip_conntrack_change_expect(exp, &t) == 0)
> 			break;
> 	}
>-
> 	if (port == 0)
> 		return 0;
> 
> 	sprintf(buffer, "%u", port);
>-
>-	return ip_nat_mangle_udp_packet(pskb, ct, ctinfo, /* XXX exp->seq */ ct_amanda_info->offset, 
>-					ct_amanda_info->len, buffer, strlen(buffer));
>+	return ip_nat_mangle_udp_packet(pskb, ct, ctinfo,
>+	                                exp_amanda_info->offset,
>+	                                exp_amanda_info->len,
>+	                                buffer, strlen(buffer));
> }
> 
> static unsigned int help(struct ip_conntrack *ct,
>-			 struct ip_conntrack_expect *exp,
>-			 struct ip_nat_info *info,
>-			 enum ip_conntrack_info ctinfo,
>-			 unsigned int hooknum,
>-			 struct sk_buff **pskb)
>+                         struct ip_conntrack_expect *exp,
>+                         struct ip_nat_info *info,
>+                         enum ip_conntrack_info ctinfo,
>+                         unsigned int hooknum,
>+                         struct sk_buff **pskb)
> {
>-	int dir;
>+	int dir = CTINFO2DIR(ctinfo);
>+	int ret = NF_ACCEPT;
> 
>-	if (!exp)
>-		DEBUGP("ip_nat_amanda: no exp!!");
>-		
> 	/* Only mangle things once: original direction in POST_ROUTING
> 	   and reply direction on PRE_ROUTING. */
>-	dir = CTINFO2DIR(ctinfo);
> 	if (!((hooknum == NF_IP_POST_ROUTING && dir == IP_CT_DIR_ORIGINAL)
>-	      || (hooknum == NF_IP_PRE_ROUTING && dir == IP_CT_DIR_REPLY))) {
>-		DEBUGP("ip_nat_amanda_help: Not touching dir %s at hook %s\n",
>-		       dir == IP_CT_DIR_ORIGINAL ? "ORIG" : "REPLY",
>-		       hooknum == NF_IP_POST_ROUTING ? "POSTROUTING"
>-		       : hooknum == NF_IP_PRE_ROUTING ? "PREROUTING"
>-		       : hooknum == NF_IP_LOCAL_OUT ? "OUTPUT"
>-		       : hooknum == NF_IP_LOCAL_IN ? "INPUT" : "???");
>+	      || (hooknum == NF_IP_PRE_ROUTING && dir == IP_CT_DIR_REPLY)))
> 		return NF_ACCEPT;
>-	}
>-	DEBUGP("ip_nat_amanda_help: got beyond not touching: dir %s at hook %s for expect: ",
>-		   dir == IP_CT_DIR_ORIGINAL ? "ORIG" : "REPLY",
>-		   hooknum == NF_IP_POST_ROUTING ? "POSTROUTING"
>-		   : hooknum == NF_IP_PRE_ROUTING ? "PREROUTING"
>-		     : hooknum == NF_IP_LOCAL_OUT ? "OUTPUT"
>-		       : hooknum == NF_IP_LOCAL_IN ? "INPUT" : "???");
>-	DUMP_TUPLE(&exp->tuple);
> 
>-	LOCK_BH(&ip_amanda_lock);
>-// XXX	if (exp->seq != 0)
>+	/* if this exectation has a "offset" the packet needs to be mangled */
> 	if (exp->help.exp_amanda_info.offset != 0)
>-		/*  if this packet has a "seq" it needs to have it's content mangled */
>-		if (!amanda_data_fixup(ct, pskb, ctinfo, exp)) {
>-			UNLOCK_BH(&ip_amanda_lock);
>-			DEBUGP("ip_nat_amanda: NF_DROP\n");
>-			return NF_DROP;
>-		}
>+		if (!amanda_data_fixup(ct, pskb, ctinfo, exp))
>+			ret = NF_DROP;
> 	exp->help.exp_amanda_info.offset = 0;
>-	UNLOCK_BH(&ip_amanda_lock);
> 
>-	DEBUGP("ip_nat_amanda: NF_ACCEPT\n");
>-	return NF_ACCEPT;
>+	return ret;
> }
> 
> static struct ip_nat_helper ip_nat_amanda_helper;
> 
>-/* This function is intentionally _NOT_ defined as  __exit, because
>- * it is needed by init() */
>-static void fini(void)
>+static void __exit fini(void)
> {
>-	DEBUGP("ip_nat_amanda: unregistering nat helper\n");
> 	ip_nat_helper_unregister(&ip_nat_amanda_helper);
> }
> 
> static int __init init(void)
> {
>-	int ret = 0;
>-	struct ip_nat_helper *hlpr;
>-
>-	hlpr = &ip_nat_amanda_helper;
>-	memset(hlpr, 0, sizeof(struct ip_nat_helper));
>+	struct ip_nat_helper *hlpr = &ip_nat_amanda_helper;
> 
> 	hlpr->tuple.dst.protonum = IPPROTO_UDP;
> 	hlpr->tuple.src.u.udp.port = htons(10080);
>@@ -205,20 +139,9 @@
> 	hlpr->flags = 0;
> 	hlpr->me = THIS_MODULE;
> 	hlpr->expect = amanda_nat_expected;
>-
> 	hlpr->name = "amanda";
> 
>-	DEBUGP
>-	    ("ip_nat_amanda: Trying to register nat helper\n");
>-	ret = ip_nat_helper_register(hlpr);
>-
>-	if (ret) {
>-		printk
>-		    ("ip_nat_amanda: error registering nat helper\n");
>-		fini();
>-		return 1;
>-	}
>-	return ret;
>+	return ip_nat_helper_register(hlpr);
> }
> 
> NEEDS_CONNTRACK(amanda);
>  
>


[-- Attachment #2: expect-ip_conntrack_amanda.patch --]
[-- Type: text/plain, Size: 1885 bytes --]

--- linux-2.6.3-old/net/ipv4/netfilter/ip_conntrack_amanda.c	2004-02-25 17:34:05.000000000 +0100
+++ linux-2.6.3/net/ipv4/netfilter/ip_conntrack_amanda.c	2004-02-25 17:39:34.000000000 +0100
@@ -46,7 +46,7 @@
 static int help(struct sk_buff *skb,
                 struct ip_conntrack *ct, enum ip_conntrack_info ctinfo)
 {
-	struct ip_conntrack_expect exp;
+	struct ip_conntrack_expect *exp;
 	struct ip_ct_amanda_expect *exp_amanda_info;
 	char *data, *data_limit, *tmp;
 	unsigned int dataoff, i;
@@ -79,20 +79,22 @@
 		goto out;
 	data += strlen("CONNECT ");
 
-	memset(&exp, 0, sizeof(exp));
-	exp.tuple.src.ip = ct->tuplehash[IP_CT_DIR_ORIGINAL].tuple.src.ip;
-	exp.tuple.dst.ip = ct->tuplehash[IP_CT_DIR_ORIGINAL].tuple.dst.ip;
-	exp.tuple.dst.protonum = IPPROTO_TCP;
-	exp.mask.src.ip = 0xFFFFFFFF;
-	exp.mask.dst.ip = 0xFFFFFFFF;
-	exp.mask.dst.protonum = 0xFFFF;
-	exp.mask.dst.u.tcp.port = 0xFFFF;
+	if (ip_conntrack_expect_alloc(&exp) < 0)
+		return -ENOMEM;
+	
+	exp->tuple.src.ip = ct->tuplehash[IP_CT_DIR_ORIGINAL].tuple.src.ip;
+	exp->tuple.dst.ip = ct->tuplehash[IP_CT_DIR_ORIGINAL].tuple.dst.ip;
+	exp->tuple.dst.protonum = IPPROTO_TCP;
+	exp->mask.src.ip = 0xFFFFFFFF;
+	exp->mask.dst.ip = 0xFFFFFFFF;
+	exp->mask.dst.protonum = 0xFFFF;
+	exp->mask.dst.u.tcp.port = 0xFFFF;
 
 	/* Only search first line. */	
 	if ((tmp = strchr(data, '\n')))
 		*tmp = '\0';
 
-	exp_amanda_info = &exp.help.exp_amanda_info;
+	exp_amanda_info = &exp->help.exp_amanda_info;
 	for (i = 0; i < ARRAY_SIZE(conns); i++) {
 		char *match = strstr(data, conns[i]);
 		if (!match)
@@ -104,8 +106,8 @@
 		if (exp_amanda_info->port == 0 || exp_amanda_info->len > 5)
 			break;
 
-		exp.tuple.dst.u.tcp.port = htons(exp_amanda_info->port);
-		ip_conntrack_expect_related(ct, &exp);
+		exp->tuple.dst.u.tcp.port = htons(exp_amanda_info->port);
+		ip_conntrack_expect_related(exp, ct);
 	}
 
 out:

Re: patch for conntrack expectations

[Patrick McHardy]

Pablo Neira wrote:
> Attached my patch for the patrick's amanda helper to use the new 
> expect_alloc api.

The patch has already been merged by davem, can you send a diff
between the fixed version and your one ?

Thanks
Patrick



> 
> Pablo

Re: patch for conntrack expectations

[Patrick McHardy]

Patrick McHardy wrote:
> Pablo Neira wrote:
> 
>> Attached my patch for the patrick's amanda helper to use the new 
>> expect_alloc api.
> 
> 
> The patch has already been merged by davem, can you send a diff
> between the fixed version and your one ?

Oops, my mistake, I thought the quoted patch was the real one.

Regards,
Patrick

Re: patch for conntrack expectations

[Patrick McHardy]

Hi Pablo,
this patch is not correct as far as I can see. With your new API,
ip_contrack_expect_related owns the memory after beeing called.
The cleaned up amanda helper reuses the same memory for all
expectations and expects ip_conntrack_expect_related to allocate
new memory and copy the data, so this will result in corruption.
It also leaks memory if the loop is left early. One thing that
seems to affect all changed helpers, you return -ENOMEM when
ip_conntrack_expect_alloc() fails, in this case while holding
a lock. You should return NF_DROP instead.

Can you please send a new patch ?

Regards
Patrick

Pablo Neira wrote:
> Attached my patch for the patrick's amanda helper to use the new 
> expect_alloc api.
> 
> Pablo
> 
> --- linux-2.6.3-old/net/ipv4/netfilter/ip_conntrack_amanda.c	2004-02-25 17:34:05.000000000 +0100
> +++ linux-2.6.3/net/ipv4/netfilter/ip_conntrack_amanda.c	2004-02-25 17:39:34.000000000 +0100
> @@ -46,7 +46,7 @@
>  static int help(struct sk_buff *skb,
>                  struct ip_conntrack *ct, enum ip_conntrack_info ctinfo)
>  {
> -	struct ip_conntrack_expect exp;
> +	struct ip_conntrack_expect *exp;
>  	struct ip_ct_amanda_expect *exp_amanda_info;
>  	char *data, *data_limit, *tmp;
>  	unsigned int dataoff, i;
> @@ -79,20 +79,22 @@
>  		goto out;
>  	data += strlen("CONNECT ");
>  
> -	memset(&exp, 0, sizeof(exp));
> -	exp.tuple.src.ip = ct->tuplehash[IP_CT_DIR_ORIGINAL].tuple.src.ip;
> -	exp.tuple.dst.ip = ct->tuplehash[IP_CT_DIR_ORIGINAL].tuple.dst.ip;
> -	exp.tuple.dst.protonum = IPPROTO_TCP;
> -	exp.mask.src.ip = 0xFFFFFFFF;
> -	exp.mask.dst.ip = 0xFFFFFFFF;
> -	exp.mask.dst.protonum = 0xFFFF;
> -	exp.mask.dst.u.tcp.port = 0xFFFF;
> +	if (ip_conntrack_expect_alloc(&exp) < 0)
> +		return -ENOMEM;
> +	
> +	exp->tuple.src.ip = ct->tuplehash[IP_CT_DIR_ORIGINAL].tuple.src.ip;
> +	exp->tuple.dst.ip = ct->tuplehash[IP_CT_DIR_ORIGINAL].tuple.dst.ip;
> +	exp->tuple.dst.protonum = IPPROTO_TCP;
> +	exp->mask.src.ip = 0xFFFFFFFF;
> +	exp->mask.dst.ip = 0xFFFFFFFF;
> +	exp->mask.dst.protonum = 0xFFFF;
> +	exp->mask.dst.u.tcp.port = 0xFFFF;
>  
>  	/* Only search first line. */	
>  	if ((tmp = strchr(data, '\n')))
>  		*tmp = '\0';
>  
> -	exp_amanda_info = &exp.help.exp_amanda_info;
> +	exp_amanda_info = &exp->help.exp_amanda_info;
>  	for (i = 0; i < ARRAY_SIZE(conns); i++) {
>  		char *match = strstr(data, conns[i]);
>  		if (!match)
> @@ -104,8 +106,8 @@
>  		if (exp_amanda_info->port == 0 || exp_amanda_info->len > 5)
>  			break;
>  
> -		exp.tuple.dst.u.tcp.port = htons(exp_amanda_info->port);
> -		ip_conntrack_expect_related(ct, &exp);
> +		exp->tuple.dst.u.tcp.port = htons(exp_amanda_info->port);
> +		ip_conntrack_expect_related(exp, ct);
>  	}
>  
>  out:

Re: patch for conntrack expectations

[Pablo Neira]

Hi Patrick,

Patrick McHardy wrote:

> One thing that
> seems to affect all changed helpers, you return -ENOMEM when
> ip_conntrack_expect_alloc() fails

actually i was thinking about this issue today, since I also posted in a 
recent email (please see: comments about ip_conntrack_in) that i didn't 
find a helper which returns -1, then I realized that my patch returned 
-ENOMEM.

I'll submit a new patch tomorrow.

Thanks,
Pablo

Re: patch for conntrack expectations

[Patrick McHardy]

Pablo Neira wrote:
> Hi Patrick,
> 
> Patrick McHardy wrote:
> 
>> One thing that
>> seems to affect all changed helpers, you return -ENOMEM when
>> ip_conntrack_expect_alloc() fails
> 
> 
> actually i was thinking about this issue today, since I also posted in a 
> recent email (please see: comments about ip_conntrack_in) that i didn't 
> find a helper which returns -1, then I realized that my patch returned 
> -ENOMEM.

Sorry, I'm not entirely up-to-date with my mails as you can see from my
late comments ;) I'll read up on it now.

> 
> I'll submit a new patch tomorrow.

Thanks.

Regards,
Patrick

> 
> Thanks,
> Pablo
> 

Re: patch for conntrack expectations

[Patrick McHardy]

One more suggestion, sorry for not bringing this up earlier.
ip_conntrack_expect_alloc() takes a pointer to the pointer
to assign the memory to and returns -ENOMEM/1, why not just
return the pointer ? Besides being what I would expect from
an _alloc-function it helps gcc generate better code because
it assumes comparisons with NULL unlikely to be true.

Regards,
Patrick

Patrick McHardy wrote:
> Hi Pablo,
> this patch is not correct as far as I can see. With your new API,
> ip_contrack_expect_related owns the memory after beeing called.
> The cleaned up amanda helper reuses the same memory for all
> expectations and expects ip_conntrack_expect_related to allocate
> new memory and copy the data, so this will result in corruption.
> It also leaks memory if the loop is left early. One thing that
> seems to affect all changed helpers, you return -ENOMEM when
> ip_conntrack_expect_alloc() fails, in this case while holding
> a lock. You should return NF_DROP instead.
> 
> Can you please send a new patch ?
> 
> Regards
> Patrick
> 
> Pablo Neira wrote:
> 
>> Attached my patch for the patrick's amanda helper to use the new 
>> expect_alloc api.
>>
>> Pablo

Re: patch for conntrack expectations

[Pablo Neira]

Hi again Patrick,

Patrick McHardy wrote:

> Besides being what I would expect from
> an _alloc-function it helps gcc generate better code because
> it assumes comparisons with NULL unlikely to be true.

yes, I agree with you, thanks for the suggestion. I'll modify the 
current prototype.

thanks for the feedback,
Pablo

Re: patch for conntrack expectations

[Pablo Neira]

Hi Patrick,

Patrick McHardy wrote:

> Hi Pablo,
> this patch is not correct as far as I can see. With your new API,
> ip_contrack_expect_related owns the memory after beeing called.
> The cleaned up amanda helper reuses the same memory for all
> expectations and expects ip_conntrack_expect_related to allocate
> new memory and copy the data, so this will result in corruption.
> It also leaks memory if the loop is left early.

oops, i'm sorry, I didn't realize that the structure of the amanda 
helper changed a bit. Yes, my patch for the amanda helper sucks a bit 
!:-). I know how to fix it, I'll submit the patch tomorrow.

thanks,
Pablo

Re: patch for conntrack expectations

[Pablo Neira]

[-- Attachment #1: Type: text/plain, Size: 346 bytes --]

Hi Patrick,

I attached to this email a new patch for the amanda helper with the new 
API to allocate memory for an expectation. As you could see, I also 
modified the API following your suggestion. Please have a look at it, if 
you are fine with it I'll send the updates for expect-optimize patch in 
pom-ng to Harald ASAP.

best regards,
Pablo

[-- Attachment #2: amanda-expect_alloc.patch --]
[-- Type: text/plain, Size: 2427 bytes --]

--- linux-2.6.3-old/net/ipv4/netfilter/ip_conntrack_amanda.c	2004-03-05 03:24:47.000000000 +0100
+++ linux-2.6.3/net/ipv4/netfilter/ip_conntrack_amanda.c	2004-03-06 01:03:45.000000000 +0100
@@ -46,10 +46,11 @@
 static int help(struct sk_buff *skb,
                 struct ip_conntrack *ct, enum ip_conntrack_info ctinfo)
 {
-	struct ip_conntrack_expect exp;
+	struct ip_conntrack_expect *exp;
 	struct ip_ct_amanda_expect *exp_amanda_info;
 	char *data, *data_limit, *tmp;
 	unsigned int dataoff, i;
+	u_int16_t port, len;
 
 	/* Only look at packets from the Amanda server */
 	if (CTINFO2DIR(ctinfo) == IP_CT_DIR_ORIGINAL)
@@ -79,33 +80,40 @@
 		goto out;
 	data += strlen("CONNECT ");
 
-	memset(&exp, 0, sizeof(exp));
-	exp.tuple.src.ip = ct->tuplehash[IP_CT_DIR_ORIGINAL].tuple.src.ip;
-	exp.tuple.dst.ip = ct->tuplehash[IP_CT_DIR_ORIGINAL].tuple.dst.ip;
-	exp.tuple.dst.protonum = IPPROTO_TCP;
-	exp.mask.src.ip = 0xFFFFFFFF;
-	exp.mask.dst.ip = 0xFFFFFFFF;
-	exp.mask.dst.protonum = 0xFFFF;
-	exp.mask.dst.u.tcp.port = 0xFFFF;
-
 	/* Only search first line. */	
 	if ((tmp = strchr(data, '\n')))
 		*tmp = '\0';
 
-	exp_amanda_info = &exp.help.exp_amanda_info;
 	for (i = 0; i < ARRAY_SIZE(conns); i++) {
 		char *match = strstr(data, conns[i]);
 		if (!match)
 			continue;
 		tmp = data = match + strlen(conns[i]);
-		exp_amanda_info->offset = data - amanda_buffer;
-		exp_amanda_info->port   = simple_strtoul(data, &data, 10);
-		exp_amanda_info->len    = data - tmp;
-		if (exp_amanda_info->port == 0 || exp_amanda_info->len > 5)
+		port = simple_strtoul(data, &data, 10);
+		len = data - tmp;
+		if (port == 0 || len > 5)
 			break;
 
-		exp.tuple.dst.u.tcp.port = htons(exp_amanda_info->port);
-		ip_conntrack_expect_related(ct, &exp);
+		exp = ip_conntrack_expect_alloc();
+		if (exp == NULL)
+			return NF_DROP;
+
+		exp->tuple.src.ip = ct->tuplehash[IP_CT_DIR_ORIGINAL].tuple.src.ip;
+		exp->tuple.dst.ip = ct->tuplehash[IP_CT_DIR_ORIGINAL].tuple.dst.ip;
+		exp->tuple.dst.protonum = IPPROTO_TCP;
+		exp->mask.src.ip = 0xFFFFFFFF;
+		exp->mask.dst.ip = 0xFFFFFFFF;
+		exp->mask.dst.protonum = 0xFFFF;
+		exp->mask.dst.u.tcp.port = 0xFFFF;
+
+		exp_amanda_info = &exp->help.exp_amanda_info;
+		exp_amanda_info->offset = data - amanda_buffer;
+		exp_amanda_info->port   = port;
+		exp_amanda_info->len    = len;
+
+		exp->tuple.dst.u.tcp.port = htons(port);
+
+		ip_conntrack_expect_related(exp, ct);
 	}
 
 out:

Re: patch for conntrack expectations

[Patrick McHardy]

Hi Pablo,

Pablo Neira wrote:
> Hi Patrick,
> 
> I attached to this email a new patch for the amanda helper with the new 
> API to allocate memory for an expectation. As you could see, I also 
> modified the API following your suggestion. Please have a look at it, if 
> you are fine with it I'll send the updates for expect-optimize patch in 
> pom-ng to Harald ASAP.

The patch looks fine, except for one thing, you return from the function
after a failed memory allocation without dropping amanda_buffer_lock. My
advice of returning NF_DROP from a helper in case of memory allocation
wasn't right anyway, connection tracking should not drop packets. Just
replace that return with a break and it's fine.

Regards
Patrick

> 
> best regards,
> Pablo
> 
> 
> ------------------------------------------------------------------------
> 
> --- linux-2.6.3-old/net/ipv4/netfilter/ip_conntrack_amanda.c	2004-03-05 03:24:47.000000000 +0100
> +++ linux-2.6.3/net/ipv4/netfilter/ip_conntrack_amanda.c	2004-03-06 01:03:45.000000000 +0100
> @@ -46,10 +46,11 @@
>  static int help(struct sk_buff *skb,
>                  struct ip_conntrack *ct, enum ip_conntrack_info ctinfo)
>  {
> -	struct ip_conntrack_expect exp;
> +	struct ip_conntrack_expect *exp;
>  	struct ip_ct_amanda_expect *exp_amanda_info;
>  	char *data, *data_limit, *tmp;
>  	unsigned int dataoff, i;
> +	u_int16_t port, len;
>  
>  	/* Only look at packets from the Amanda server */
>  	if (CTINFO2DIR(ctinfo) == IP_CT_DIR_ORIGINAL)
> @@ -79,33 +80,40 @@
>  		goto out;
>  	data += strlen("CONNECT ");
>  
> -	memset(&exp, 0, sizeof(exp));
> -	exp.tuple.src.ip = ct->tuplehash[IP_CT_DIR_ORIGINAL].tuple.src.ip;
> -	exp.tuple.dst.ip = ct->tuplehash[IP_CT_DIR_ORIGINAL].tuple.dst.ip;
> -	exp.tuple.dst.protonum = IPPROTO_TCP;
> -	exp.mask.src.ip = 0xFFFFFFFF;
> -	exp.mask.dst.ip = 0xFFFFFFFF;
> -	exp.mask.dst.protonum = 0xFFFF;
> -	exp.mask.dst.u.tcp.port = 0xFFFF;
> -
>  	/* Only search first line. */	
>  	if ((tmp = strchr(data, '\n')))
>  		*tmp = '\0';
>  
> -	exp_amanda_info = &exp.help.exp_amanda_info;
>  	for (i = 0; i < ARRAY_SIZE(conns); i++) {
>  		char *match = strstr(data, conns[i]);
>  		if (!match)
>  			continue;
>  		tmp = data = match + strlen(conns[i]);
> -		exp_amanda_info->offset = data - amanda_buffer;
> -		exp_amanda_info->port   = simple_strtoul(data, &data, 10);
> -		exp_amanda_info->len    = data - tmp;
> -		if (exp_amanda_info->port == 0 || exp_amanda_info->len > 5)
> +		port = simple_strtoul(data, &data, 10);
> +		len = data - tmp;
> +		if (port == 0 || len > 5)
>  			break;
>  
> -		exp.tuple.dst.u.tcp.port = htons(exp_amanda_info->port);
> -		ip_conntrack_expect_related(ct, &exp);
> +		exp = ip_conntrack_expect_alloc();
> +		if (exp == NULL)
> +			return NF_DROP;
> +
> +		exp->tuple.src.ip = ct->tuplehash[IP_CT_DIR_ORIGINAL].tuple.src.ip;
> +		exp->tuple.dst.ip = ct->tuplehash[IP_CT_DIR_ORIGINAL].tuple.dst.ip;
> +		exp->tuple.dst.protonum = IPPROTO_TCP;
> +		exp->mask.src.ip = 0xFFFFFFFF;
> +		exp->mask.dst.ip = 0xFFFFFFFF;
> +		exp->mask.dst.protonum = 0xFFFF;
> +		exp->mask.dst.u.tcp.port = 0xFFFF;
> +
> +		exp_amanda_info = &exp->help.exp_amanda_info;
> +		exp_amanda_info->offset = data - amanda_buffer;
> +		exp_amanda_info->port   = port;
> +		exp_amanda_info->len    = len;
> +
> +		exp->tuple.dst.u.tcp.port = htons(port);
> +
> +		ip_conntrack_expect_related(exp, ct);
>  	}
>  
>  out:

Re: patch for conntrack expectations

[Pablo Neira]

[-- Attachment #1: Type: text/plain, Size: 478 bytes --]

Patrick McHardy wrote:

> The patch looks fine, except for one thing, you return from the function
> after a failed memory allocation without dropping amanda_buffer_lock.

ok, I fixed this problem.

> My
> advice of returning NF_DROP from a helper in case of memory allocation
> wasn't right anyway, connection tracking should not drop packets. Just
> replace that return with a break and it's fine.

Also fixed. Attached last modification. Thanks patrick.

Best regards,
Pablo

[-- Attachment #2: amanda-expect_alloc-2.patch --]
[-- Type: text/plain, Size: 2421 bytes --]

--- linux-2.6.3-old/net/ipv4/netfilter/ip_conntrack_amanda.c	2004-03-05 03:24:47.000000000 +0100
+++ linux-2.6.3/net/ipv4/netfilter/ip_conntrack_amanda.c	2004-03-06 02:22:20.000000000 +0100
@@ -46,10 +46,11 @@
 static int help(struct sk_buff *skb,
                 struct ip_conntrack *ct, enum ip_conntrack_info ctinfo)
 {
-	struct ip_conntrack_expect exp;
+	struct ip_conntrack_expect *exp;
 	struct ip_ct_amanda_expect *exp_amanda_info;
 	char *data, *data_limit, *tmp;
 	unsigned int dataoff, i;
+	u_int16_t port, len;
 
 	/* Only look at packets from the Amanda server */
 	if (CTINFO2DIR(ctinfo) == IP_CT_DIR_ORIGINAL)
@@ -79,33 +80,40 @@
 		goto out;
 	data += strlen("CONNECT ");
 
-	memset(&exp, 0, sizeof(exp));
-	exp.tuple.src.ip = ct->tuplehash[IP_CT_DIR_ORIGINAL].tuple.src.ip;
-	exp.tuple.dst.ip = ct->tuplehash[IP_CT_DIR_ORIGINAL].tuple.dst.ip;
-	exp.tuple.dst.protonum = IPPROTO_TCP;
-	exp.mask.src.ip = 0xFFFFFFFF;
-	exp.mask.dst.ip = 0xFFFFFFFF;
-	exp.mask.dst.protonum = 0xFFFF;
-	exp.mask.dst.u.tcp.port = 0xFFFF;
-
 	/* Only search first line. */	
 	if ((tmp = strchr(data, '\n')))
 		*tmp = '\0';
 
-	exp_amanda_info = &exp.help.exp_amanda_info;
 	for (i = 0; i < ARRAY_SIZE(conns); i++) {
 		char *match = strstr(data, conns[i]);
 		if (!match)
 			continue;
 		tmp = data = match + strlen(conns[i]);
-		exp_amanda_info->offset = data - amanda_buffer;
-		exp_amanda_info->port   = simple_strtoul(data, &data, 10);
-		exp_amanda_info->len    = data - tmp;
-		if (exp_amanda_info->port == 0 || exp_amanda_info->len > 5)
+		port = simple_strtoul(data, &data, 10);
+		len = data - tmp;
+		if (port == 0 || len > 5)
 			break;
 
-		exp.tuple.dst.u.tcp.port = htons(exp_amanda_info->port);
-		ip_conntrack_expect_related(ct, &exp);
+		exp = ip_conntrack_expect_alloc();
+		if (exp == NULL)
+			goto out;
+
+		exp->tuple.src.ip = ct->tuplehash[IP_CT_DIR_ORIGINAL].tuple.src.ip;
+		exp->tuple.dst.ip = ct->tuplehash[IP_CT_DIR_ORIGINAL].tuple.dst.ip;
+		exp->tuple.dst.protonum = IPPROTO_TCP;
+		exp->mask.src.ip = 0xFFFFFFFF;
+		exp->mask.dst.ip = 0xFFFFFFFF;
+		exp->mask.dst.protonum = 0xFFFF;
+		exp->mask.dst.u.tcp.port = 0xFFFF;
+
+		exp_amanda_info = &exp->help.exp_amanda_info;
+		exp_amanda_info->offset = data - amanda_buffer;
+		exp_amanda_info->port   = port;
+		exp_amanda_info->len    = len;
+
+		exp->tuple.dst.u.tcp.port = htons(port);
+
+		ip_conntrack_expect_related(exp, ct);
 	}
 
 out:

Re: patch for conntrack expectations

[Patrick McHardy]

Pablo Neira wrote:
> Also fixed. Attached last modification. Thanks patrick.

Looks good, thanks.

Regards
Patrick

> 
> Best regards,
> Pablo

Re: patch for conntrack expectations

[Pablo Neira]

[-- Attachment #1: Type: text/plain, Size: 4009 bytes --]

Hi harald,

attached with this email last version of patches and some comments.

Harald Welte wrote:

>Thanks pablo.
>
>I am fine with that patch, and I'd like to put it in patch-o-matic for
>now.  As the changes are fairly obvious, I hope we won't get too much
>trouble with it ;)  However, please see my following comments.
>
>  
>
ok, feedback is always welcome. :-)

>>I finished my patch to add an ip_conntrack_expect_alloc(...) function. I 
>>also patched all the current conntrack helpers available in the stable 
>>branch and patch-o-matic-ng.
>>    
>>
>
>thanks. This means that once we get it into the core kernel, we should
>definitely have  the first pom-ng release out, otherwise people will
>still use the old API.
>
>This makes me think of another issue.  We should either rename the
>function, reorder arguments, do whatever.   We cannot just make all old
>(unpatched) code compile without any warning, and still have it crash :(
>I think it's best to exchange the order of the two arguments in
>ip_conntrack_expect_related().  This way a compiler would complain if
>some old helper was compiled with the new core.
>
>  
>

As you suggested, I modified the order of the arguments in expect_related.

>>BTW, I know that there's a skeleton of a helper in the documention, due 
>>to they mostly look the same but I think that I could add it more details.
>>    
>>
>
>yes, please.  Use the SGML master file, don't edit the HTML
>  
>

ok! I will.

>>- There's a memset in expect_alloc (see expect.patch) after the 
>>allocation is done, I don't like it anyway but I just added to keep 
>>backward compatibility because some helper usually do it. I think that 
>>it's better setting all the fields of the new expectation in the helper 
>>(this implies being a bit more restrictive with the programmer but well, 
>>this is kernel programming anyway, it's restrictive :-)).
>>    
>>
>
>yes, sometimes code like this is included mainly for debugging purpose
>and then left behind.
>
>  
>
>>I could delete that memset and have a look at all the helpers to make 
>>sure that all the fields are correctly set.
>>    
>>
>
>The issue is not very easy in this case.  Assume that tuple->dst.u
>is 32bits wide (like after applying the pptp patch).  This in turn means
>that every helper has to explicitly take care not to initialize
>tuple->dst.u with a 16bit tcp/udp port number, since it could end
>up in the 'other' 16bit half.  Rather, they have to assign
>tuple->dst.u.tcp.port.  Those cases should all be fixed by now.
>
>Now look what tuple_cmp does.  It memcmp's the whole union, not just the
>individual fields.  This means that a tuple always has to be cleanly
>initialized with zero, even in those parts that don't contain any
>useful information.
>
>So for now, I'd like to keep that memset until all the code is audited
>with that regard.
>  
>

ok, i'll study this issue calmly.

>>- I found a condition where expect_related could succesfully insert an 
>>expectaction when removing an old one and return EPERM (see 
>>expect_related-newnat14.patch). I think that this doesn't make sense. Am 
>>I forgetting something? That patch fix that problem.
>>    
>>
>
> 
>  
>
>>- I also have no problem about updating the documentation to add info 
>>about expect_alloc.
>>    
>>
>
>ok, please do so.
>  
>

ok! I will.

>One issue with your patch:  It doesn't seem to use tab's for
>indentation, but rather 8 spaces. Please clean this up.
>  
>

it's true, I think that it was because of some copy&paste issues related 
to vim and the use of cat. I'll pay attention to this kind of stuff in 
future since it seems that they are not sensible to indentation. I think 
these patches don't have this problem.

>Also, we cannot export that symbol GPL only, since it doesn't add a new
>API but rather replaces an old one.
> 
>  
>
also fixed!

If these patches are ok, I'll also prepare the patches for the pom-ng 
helpers.

Pablo

P.D: BTW, so should I patch the amanda helper against patrick patch?

[-- Attachment #2: expect-ip_conntrack_amanda.patch --]
[-- Type: text/plain, Size: 2438 bytes --]

--- linux-2.6.3-old/net/ipv4/netfilter/ip_conntrack_amanda.c	2004-02-18 04:58:39.000000000 +0100
+++ linux-2.6.3/net/ipv4/netfilter/ip_conntrack_amanda.c	2004-02-25 15:32:06.000000000 +0100
@@ -104,18 +104,20 @@
 		char *match = strstr(data, conns[i]);
 		if (match) {
 			char *portchr;
-			struct ip_conntrack_expect expect;
-			struct ip_ct_amanda_expect *exp_amanda_info =
-				&expect.help.exp_amanda_info;
+			struct ip_conntrack_expect *expect;
+			struct ip_ct_amanda_expect *exp_amanda_info;
 
-			memset(&expect, 0, sizeof(expect));
+			if (ip_conntrack_expect_alloc(&expect) < 0)
+				return -ENOMEM;
 
+			exp_amanda_info = &expect->help.exp_amanda_info;
+			
 			data += strlen(conns[i]);
 			/* this is not really tcp, but let's steal an
 			 * idea from a tcp stream helper :-) */
 			// XXX expect.seq = data - amanda_buffer;
 			exp_amanda_info->offset = data - amanda_buffer;
-// XXX DEBUGP("expect.seq = %p - %p = %d\n", data, amanda_buffer, expect.seq);
+// XXX DEBUGP("expect->seq = %p - %p = %d\n", data, amanda_buffer, expect->seq);
 DEBUGP("exp_amanda_info->offset = %p - %p = %d\n", data, amanda_buffer, exp_amanda_info->offset);
 			portchr = data;
 			exp_amanda_info->port = simple_strtoul(data, &data,10);
@@ -129,26 +131,26 @@
 			       "%u found\n", conns[i],
 			       exp_amanda_info->port);
 
-			expect.tuple = ((struct ip_conntrack_tuple)
+			expect->tuple = ((struct ip_conntrack_tuple)
 				{ { ct->tuplehash[IP_CT_DIR_ORIGINAL].tuple.src.ip,
 				    { 0 } },
 				  { ct->tuplehash[IP_CT_DIR_ORIGINAL].tuple.dst.ip,
 				    { htons(exp_amanda_info->port) },
 				    IPPROTO_TCP }});
-			expect.mask = ((struct ip_conntrack_tuple)
+			expect->mask = ((struct ip_conntrack_tuple)
 				{ { 0, { 0 } },
 				  { 0xFFFFFFFF, { 0xFFFF }, 0xFFFF }});
 
-			expect.expectfn = NULL;
+			expect->expectfn = NULL;
 
 			DEBUGP ("ip_conntrack_amanda_help: "
 				"expect_related: %u.%u.%u.%u:%u - "
 				"%u.%u.%u.%u:%u\n",
-				NIPQUAD(expect.tuple.src.ip),
-				ntohs(expect.tuple.src.u.tcp.port),
-				NIPQUAD(expect.tuple.dst.ip),
-				ntohs(expect.tuple.dst.u.tcp.port));
-			if (ip_conntrack_expect_related(ct, &expect)
+				NIPQUAD(expect->tuple.src.ip),
+				ntohs(expect->tuple.src.u.tcp.port),
+				NIPQUAD(expect->tuple.dst.ip),
+				ntohs(expect->tuple.dst.u.tcp.port));
+			if (ip_conntrack_expect_related(expect, ct)
 			    == -EEXIST) {
 				;
 				/* this must be a packet being resent */

[-- Attachment #3: expect-ip_conntrack_ftp.patch --]
[-- Type: text/plain, Size: 1288 bytes --]

--- linux-2.6.3-old/net/ipv4/netfilter/ip_conntrack_ftp.c	2004-02-18 04:59:06.000000000 +0100
+++ linux-2.6.3/net/ipv4/netfilter/ip_conntrack_ftp.c	2004-02-25 15:31:41.000000000 +0100
@@ -256,8 +256,8 @@
 	int dir = CTINFO2DIR(ctinfo);
 	unsigned int matchlen, matchoff;
 	struct ip_ct_ftp_master *ct_ftp_info = &ct->help.ct_ftp_info;
-	struct ip_conntrack_expect expect, *exp = &expect;
-	struct ip_ct_ftp_expect *exp_ftp_info = &exp->help.exp_ftp_info;
+	struct ip_conntrack_expect *exp;
+	struct ip_ct_ftp_expect *exp_ftp_info;
 
 	unsigned int i;
 	int found = 0;
@@ -346,8 +346,12 @@
 	DEBUGP("conntrack_ftp: match `%.*s' (%u bytes at %u)\n",
 	       (int)matchlen, data + matchoff,
 	       matchlen, ntohl(tcph.seq) + matchoff);
-	       
-	memset(&expect, 0, sizeof(expect));
+
+	/* Allocate expectation which will be inserted */
+	if (ip_conntrack_expect_alloc(&exp) < 0)
+		return -ENOMEM;
+
+	exp_ftp_info = &exp->help.exp_ftp_info;
 
 	/* Update the ftp info */
 	if (htonl((array[0] << 24) | (array[1] << 16) | (array[2] << 8) | array[3])
@@ -389,7 +393,7 @@
 	exp->expectfn = NULL;
 
 	/* Ignore failure; should only happen with NAT */
-	ip_conntrack_expect_related(ct, &expect);
+	ip_conntrack_expect_related(exp, ct);
 	ret = NF_ACCEPT;
  out:
 	UNLOCK_BH(&ip_ftp_lock);

[-- Attachment #4: expect-ip_conntrack_irc.patch --]
[-- Type: text/plain, Size: 1042 bytes --]

--- linux-2.6.3-old/net/ipv4/netfilter/ip_conntrack_irc.c	2004-02-18 04:59:06.000000000 +0100
+++ linux-2.6.3/net/ipv4/netfilter/ip_conntrack_irc.c	2004-02-25 15:32:27.000000000 +0100
@@ -106,8 +106,8 @@
 	struct tcphdr tcph;
 	char *data, *data_limit;
 	int dir = CTINFO2DIR(ctinfo);
-	struct ip_conntrack_expect expect, *exp = &expect;
-	struct ip_ct_irc_expect *exp_irc_info = &exp->help.exp_irc_info;
+	struct ip_conntrack_expect *exp;
+	struct ip_ct_irc_expect *exp_irc_info = NULL;
 
 	u_int32_t dcc_ip;
 	u_int16_t dcc_port;
@@ -190,8 +190,11 @@
 
 				continue;
 			}
-			
-			memset(&expect, 0, sizeof(expect));
+
+			if (ip_conntrack_expect_alloc(&exp) < 0)
+				return -ENOMEM;
+
+			exp_irc_info = &exp->help.exp_irc_info;
 
 			/* save position of address in dcc string,
 			 * necessary for NAT */
@@ -218,7 +221,7 @@
 				NIPQUAD(exp->tuple.dst.ip),
 				ntohs(exp->tuple.dst.u.tcp.port));
 
-			ip_conntrack_expect_related(ct, &expect);
+			ip_conntrack_expect_related(exp, ct);
 
 			goto out;
 		} /* for .. NUM_DCCPROTO */

[-- Attachment #5: expect-ip_conntrack_tftp.patch --]
[-- Type: text/plain, Size: 1398 bytes --]

--- linux-2.6.3-old/net/ipv4/netfilter/ip_conntrack_tftp.c	2004-02-18 04:57:22.000000000 +0100
+++ linux-2.6.3/net/ipv4/netfilter/ip_conntrack_tftp.c	2004-02-25 15:31:52.000000000 +0100
@@ -44,7 +44,7 @@
 		     enum ip_conntrack_info ctinfo)
 {
 	struct tftphdr tftph;
-	struct ip_conntrack_expect exp;
+	struct ip_conntrack_expect *exp;
 
 	if (skb_copy_bits(skb, skb->nh.iph->ihl * 4 + sizeof(struct udphdr),
 			  &tftph, sizeof(tftph)) != 0)
@@ -57,19 +57,21 @@
 		DEBUGP("");
 		DUMP_TUPLE(&ct->tuplehash[IP_CT_DIR_ORIGINAL].tuple);
 		DUMP_TUPLE(&ct->tuplehash[IP_CT_DIR_REPLY].tuple);
-		memset(&exp, 0, sizeof(exp));
 
-		exp.tuple = ct->tuplehash[IP_CT_DIR_REPLY].tuple;
-		exp.mask.src.ip = 0xffffffff;
-		exp.mask.dst.ip = 0xffffffff;
-		exp.mask.dst.u.udp.port = 0xffff;
-		exp.mask.dst.protonum = 0xffff;
-		exp.expectfn = NULL;
+		if (ip_conntrack_expect_alloc(&exp) < 0)
+			return -ENOMEM;
+
+		exp->tuple = ct->tuplehash[IP_CT_DIR_REPLY].tuple;
+		exp->mask.src.ip = 0xffffffff;
+		exp->mask.dst.ip = 0xffffffff;
+		exp->mask.dst.u.udp.port = 0xffff;
+		exp->mask.dst.protonum = 0xffff;
+		exp->expectfn = NULL;
 
 		DEBUGP("expect: ");
-		DUMP_TUPLE(&exp.tuple);
-		DUMP_TUPLE(&exp.mask);
-		ip_conntrack_expect_related(ct, &exp);
+		DUMP_TUPLE(&exp->tuple);
+		DUMP_TUPLE(&exp->mask);
+		ip_conntrack_expect_related(exp, ct);
 		break;
 	default:
 		DEBUGP("Unknown opcode\n");

[-- Attachment #6: expect.patch --]
[-- Type: text/plain, Size: 5070 bytes --]

--- linux-2.6.3-old/net/ipv4/netfilter/ip_conntrack_core.c	2004-02-22 13:29:30.000000000 +0100
+++ linux-2.6.3/net/ipv4/netfilter/ip_conntrack_core.c	2004-02-25 16:31:09.000000000 +0100
@@ -917,11 +917,53 @@
 	WRITE_UNLOCK(&ip_conntrack_lock);
 }
 
+int
+ip_conntrack_expect_alloc(struct ip_conntrack_expect **new)
+{
+	*new = (struct ip_conntrack_expect *)
+		kmalloc(sizeof(struct ip_conntrack_expect), GFP_ATOMIC);
+	if (!*new) {
+		DEBUGP("expect_related: OOM allocating expect\n");
+		return -ENOMEM;
+	}
+
+	/* is this important? */
+	memset(*new, 0, sizeof(struct ip_conntrack_expect));
+
+	return 1;
+}
+
+void
+ip_conntrack_expect_insert(struct ip_conntrack_expect *new,
+               struct ip_conntrack *related_to)
+{
+	DEBUGP("new expectation %p of conntrack %p\n", new, related_to);
+	new->expectant = related_to;
+	new->sibling = NULL;
+	atomic_set(&new->use, 1);
+
+	/* add to expected list for this connection */
+	list_add(&new->expected_list, &related_to->sibling_list);
+	/* add to global list of expectations */
+
+	list_prepend(&ip_conntrack_expect_list, &new->list);
+	/* add and start timer if required */
+	if (related_to->helper->timeout) {
+		init_timer(&new->timeout);
+		new->timeout.data = (unsigned long)new;
+		new->timeout.function = expectation_timed_out;
+		new->timeout.expires = jiffies +
+					related_to->helper->timeout * HZ;
+		add_timer(&new->timeout);
+	}
+	related_to->expecting++;
+}
+
 /* Add a related connection. */
-int ip_conntrack_expect_related(struct ip_conntrack *related_to,
-				struct ip_conntrack_expect *expect)
+int ip_conntrack_expect_related(struct ip_conntrack_expect *expect,
+				struct ip_conntrack *related_to)
 {
-	struct ip_conntrack_expect *old, *new;
+	struct ip_conntrack_expect *old;
 	int ret = 0;
 
 	WRITE_LOCK(&ip_conntrack_lock);
@@ -953,6 +995,7 @@
 
 		if (old) {
 			WRITE_UNLOCK(&ip_conntrack_lock);
+			kfree(expect);
 			return -EEXIST;
 		}
 	} else if (related_to->helper->max_expected && 
@@ -971,6 +1014,7 @@
 				       related_to->helper->name,
  		    	       	       NIPQUAD(related_to->tuplehash[IP_CT_DIR_ORIGINAL].tuple.src.ip),
  		    	       	       NIPQUAD(related_to->tuplehash[IP_CT_DIR_ORIGINAL].tuple.dst.ip));
+			kfree(expect);
 			return -EPERM;
 		}
 		DEBUGP("ip_conntrack: max number of expected "
@@ -1010,37 +1054,12 @@
 			     &expect->mask)) {
 		WRITE_UNLOCK(&ip_conntrack_lock);
 		DEBUGP("expect_related: busy!\n");
+
+		kfree(expect);
 		return -EBUSY;
 	}
-	
-	new = (struct ip_conntrack_expect *) 
-	      kmalloc(sizeof(struct ip_conntrack_expect), GFP_ATOMIC);
-	if (!new) {
-		WRITE_UNLOCK(&ip_conntrack_lock);
-		DEBUGP("expect_relaed: OOM allocating expect\n");
-		return -ENOMEM;
-	}
-	
-	DEBUGP("new expectation %p of conntrack %p\n", new, related_to);
-	memcpy(new, expect, sizeof(*expect));
-	new->expectant = related_to;
-	new->sibling = NULL;
-	atomic_set(&new->use, 1);
-	
-	/* add to expected list for this connection */	
-	list_add(&new->expected_list, &related_to->sibling_list);
-	/* add to global list of expectations */
-	list_prepend(&ip_conntrack_expect_list, &new->list);
-	/* add and start timer if required */
-	if (related_to->helper->timeout) {
-		init_timer(&new->timeout);
-		new->timeout.data = (unsigned long)new;
-		new->timeout.function = expectation_timed_out;
-		new->timeout.expires = jiffies + 
-					related_to->helper->timeout * HZ;
-		add_timer(&new->timeout);
-	}
-	related_to->expecting++;
+
+	ip_conntrack_expect_insert(expect, related_to);
 
 	WRITE_UNLOCK(&ip_conntrack_lock);
 
--- linux-2.6.3-old/net/ipv4/netfilter/ip_conntrack_standalone.c	2004-02-18 04:57:46.000000000 +0100
+++ linux-2.6.3/net/ipv4/netfilter/ip_conntrack_standalone.c	2004-02-25 15:16:53.000000000 +0100
@@ -497,6 +497,7 @@
 EXPORT_SYMBOL(ip_ct_find_proto);
 EXPORT_SYMBOL(__ip_ct_find_proto);
 EXPORT_SYMBOL(ip_ct_find_helper);
+EXPORT_SYMBOL(ip_conntrack_expect_alloc);
 EXPORT_SYMBOL(ip_conntrack_expect_related);
 EXPORT_SYMBOL(ip_conntrack_change_expect);
 EXPORT_SYMBOL(ip_conntrack_unexpect_related);
--- linux-2.6.3-old/include/linux/netfilter_ipv4/ip_conntrack_helper.h	2004-02-18 04:57:16.000000000 +0100
+++ linux-2.6.3/include/linux/netfilter_ipv4/ip_conntrack_helper.h	2004-02-25 15:38:27.000000000 +0100
@@ -35,9 +35,13 @@
 
 extern struct ip_conntrack_helper *ip_ct_find_helper(const struct ip_conntrack_tuple *tuple);
 
+
+/* Allocate space for an expectation: this is mandatory before calling 
+   ip_conntrack_expect_related. */
+extern int ip_conntrack_expect_alloc(struct ip_conntrack_expect **new);
 /* Add an expected connection: can have more than one per connection */
-extern int ip_conntrack_expect_related(struct ip_conntrack *related_to,
-				       struct ip_conntrack_expect *exp);
+extern int ip_conntrack_expect_related(struct ip_conntrack_expect *exp,
+				       struct ip_conntrack *related_to);
 extern int ip_conntrack_change_expect(struct ip_conntrack_expect *expect,
 				      struct ip_conntrack_tuple *newtuple);
 extern void ip_conntrack_unexpect_related(struct ip_conntrack_expect *exp);

Re: patch for conntrack expectations

[Pablo Neira]

[-- Attachment #1: Type: text/plain, Size: 333 bytes --]

Hi Harald and list,

I modified the handling of old expectations in the expect_related 
function, some clean ups. Please see line 64 of this patch.

I merged this change to my lastest patch since I think that it's related 
stuff. anyway if you want me to keep this different patch, i won't have 
any problem to do it.

cheers,
Pablo

[-- Attachment #2: expect.patch --]
[-- Type: text/plain, Size: 5507 bytes --]

--- linux-2.6.3-old/net/ipv4/netfilter/ip_conntrack_core.c	2004-02-22 13:29:30.000000000 +0100
+++ linux-2.6.3/net/ipv4/netfilter/ip_conntrack_core.c	2004-02-28 12:10:50.000000000 +0100
@@ -917,11 +917,53 @@
 	WRITE_UNLOCK(&ip_conntrack_lock);
 }
 
+int
+ip_conntrack_expect_alloc(struct ip_conntrack_expect **new)
+{
+	*new = (struct ip_conntrack_expect *)
+		kmalloc(sizeof(struct ip_conntrack_expect), GFP_ATOMIC);
+	if (!*new) {
+		DEBUGP("expect_related: OOM allocating expect\n");
+		return -ENOMEM;
+	}
+
+	/* is this important? */
+	memset(*new, 0, sizeof(struct ip_conntrack_expect));
+
+	return 1;
+}
+
+void
+ip_conntrack_expect_insert(struct ip_conntrack_expect *new,
+               struct ip_conntrack *related_to)
+{
+	DEBUGP("new expectation %p of conntrack %p\n", new, related_to);
+	new->expectant = related_to;
+	new->sibling = NULL;
+	atomic_set(&new->use, 1);
+
+	/* add to expected list for this connection */
+	list_add(&new->expected_list, &related_to->sibling_list);
+	/* add to global list of expectations */
+
+	list_prepend(&ip_conntrack_expect_list, &new->list);
+	/* add and start timer if required */
+	if (related_to->helper->timeout) {
+		init_timer(&new->timeout);
+		new->timeout.data = (unsigned long)new;
+		new->timeout.function = expectation_timed_out;
+		new->timeout.expires = jiffies +
+					related_to->helper->timeout * HZ;
+		add_timer(&new->timeout);
+	}
+	related_to->expecting++;
+}
+
 /* Add a related connection. */
-int ip_conntrack_expect_related(struct ip_conntrack *related_to,
-				struct ip_conntrack_expect *expect)
+int ip_conntrack_expect_related(struct ip_conntrack_expect *expect,
+				struct ip_conntrack *related_to)
 {
-	struct ip_conntrack_expect *old, *new;
+	struct ip_conntrack_expect *old;
 	int ret = 0;
 
 	WRITE_LOCK(&ip_conntrack_lock);
@@ -943,7 +985,7 @@
 		if (related_to->helper->timeout) {
 			if (!del_timer(&old->timeout)) {
 				/* expectation is dying. Fall through */
-				old = NULL;
+				goto out;
 			} else {
 				old->timeout.expires = jiffies + 
 					related_to->helper->timeout * HZ;
@@ -951,10 +993,10 @@
 			}
 		}
 
-		if (old) {
-			WRITE_UNLOCK(&ip_conntrack_lock);
-			return -EEXIST;
-		}
+		WRITE_UNLOCK(&ip_conntrack_lock);
+		kfree(expect);
+		return -EEXIST;
+
 	} else if (related_to->helper->max_expected && 
 		   related_to->expecting >= related_to->helper->max_expected) {
 		struct list_head *cur_item;
@@ -971,6 +1013,7 @@
 				       related_to->helper->name,
  		    	       	       NIPQUAD(related_to->tuplehash[IP_CT_DIR_ORIGINAL].tuple.src.ip),
  		    	       	       NIPQUAD(related_to->tuplehash[IP_CT_DIR_ORIGINAL].tuple.dst.ip));
+			kfree(expect);
 			return -EPERM;
 		}
 		DEBUGP("ip_conntrack: max number of expected "
@@ -1010,37 +1053,12 @@
 			     &expect->mask)) {
 		WRITE_UNLOCK(&ip_conntrack_lock);
 		DEBUGP("expect_related: busy!\n");
+
+		kfree(expect);
 		return -EBUSY;
 	}
-	
-	new = (struct ip_conntrack_expect *) 
-	      kmalloc(sizeof(struct ip_conntrack_expect), GFP_ATOMIC);
-	if (!new) {
-		WRITE_UNLOCK(&ip_conntrack_lock);
-		DEBUGP("expect_relaed: OOM allocating expect\n");
-		return -ENOMEM;
-	}
-	
-	DEBUGP("new expectation %p of conntrack %p\n", new, related_to);
-	memcpy(new, expect, sizeof(*expect));
-	new->expectant = related_to;
-	new->sibling = NULL;
-	atomic_set(&new->use, 1);
-	
-	/* add to expected list for this connection */	
-	list_add(&new->expected_list, &related_to->sibling_list);
-	/* add to global list of expectations */
-	list_prepend(&ip_conntrack_expect_list, &new->list);
-	/* add and start timer if required */
-	if (related_to->helper->timeout) {
-		init_timer(&new->timeout);
-		new->timeout.data = (unsigned long)new;
-		new->timeout.function = expectation_timed_out;
-		new->timeout.expires = jiffies + 
-					related_to->helper->timeout * HZ;
-		add_timer(&new->timeout);
-	}
-	related_to->expecting++;
+
+out:	ip_conntrack_expect_insert(expect, related_to);
 
 	WRITE_UNLOCK(&ip_conntrack_lock);
 
--- linux-2.6.3-old/net/ipv4/netfilter/ip_conntrack_standalone.c	2004-02-18 04:57:46.000000000 +0100
+++ linux-2.6.3/net/ipv4/netfilter/ip_conntrack_standalone.c	2004-02-25 15:16:53.000000000 +0100
@@ -497,6 +497,7 @@
 EXPORT_SYMBOL(ip_ct_find_proto);
 EXPORT_SYMBOL(__ip_ct_find_proto);
 EXPORT_SYMBOL(ip_ct_find_helper);
+EXPORT_SYMBOL(ip_conntrack_expect_alloc);
 EXPORT_SYMBOL(ip_conntrack_expect_related);
 EXPORT_SYMBOL(ip_conntrack_change_expect);
 EXPORT_SYMBOL(ip_conntrack_unexpect_related);
--- linux-2.6.3-old/include/linux/netfilter_ipv4/ip_conntrack_helper.h	2004-02-18 04:57:16.000000000 +0100
+++ linux-2.6.3/include/linux/netfilter_ipv4/ip_conntrack_helper.h	2004-02-25 15:38:27.000000000 +0100
@@ -35,9 +35,13 @@
 
 extern struct ip_conntrack_helper *ip_ct_find_helper(const struct ip_conntrack_tuple *tuple);
 
+
+/* Allocate space for an expectation: this is mandatory before calling 
+   ip_conntrack_expect_related. */
+extern int ip_conntrack_expect_alloc(struct ip_conntrack_expect **new);
 /* Add an expected connection: can have more than one per connection */
-extern int ip_conntrack_expect_related(struct ip_conntrack *related_to,
-				       struct ip_conntrack_expect *exp);
+extern int ip_conntrack_expect_related(struct ip_conntrack_expect *exp,
+				       struct ip_conntrack *related_to);
 extern int ip_conntrack_change_expect(struct ip_conntrack_expect *expect,
 				      struct ip_conntrack_tuple *newtuple);
 extern void ip_conntrack_unexpect_related(struct ip_conntrack_expect *exp);

Re: patch for conntrack expectations

[Pablo Neira]

[-- Attachment #1: Type: text/plain, Size: 208 bytes --]

Hi,

I already sent the updates patches for expect-optimize in pom-ng to 
Harald but I forgot redirecting them to the maillist.

Please use these ones instead since they are the last version.

regards,
Pablo

[-- Attachment #2: expect-06032004.patch --]
[-- Type: text/plain, Size: 11779 bytes --]

--- linux-2.6.3-old/net/ipv4/netfilter/ip_conntrack_core.c	2004-02-18 04:57:11.000000000 +0100
+++ linux-2.6.3/net/ipv4/netfilter/ip_conntrack_core.c	2004-03-05 03:08:30.000000000 +0100
@@ -917,11 +917,55 @@
 	WRITE_UNLOCK(&ip_conntrack_lock);
 }
 
+struct ip_conntrack_expect *
+ip_conntrack_expect_alloc()
+{
+	struct ip_conntrack_expect *new;
+	
+	new = (struct ip_conntrack_expect *)
+		kmalloc(sizeof(struct ip_conntrack_expect), GFP_ATOMIC);
+	if (!new) {
+		DEBUGP("expect_related: OOM allocating expect\n");
+		return NULL;
+	}
+
+	/* tuple_cmp compares whole union, we have to initialized cleanly */
+	memset(new, 0, sizeof(struct ip_conntrack_expect));
+
+	return new;
+}
+
+static void
+ip_conntrack_expect_insert(struct ip_conntrack_expect *new,
+			   struct ip_conntrack *related_to)
+{
+	DEBUGP("new expectation %p of conntrack %p\n", new, related_to);
+	new->expectant = related_to;
+	new->sibling = NULL;
+	atomic_set(&new->use, 1);
+
+	/* add to expected list for this connection */
+	list_add(&new->expected_list, &related_to->sibling_list);
+	/* add to global list of expectations */
+
+	list_prepend(&ip_conntrack_expect_list, &new->list);
+	/* add and start timer if required */
+	if (related_to->helper->timeout) {
+		init_timer(&new->timeout);
+		new->timeout.data = (unsigned long)new;
+		new->timeout.function = expectation_timed_out;
+		new->timeout.expires = jiffies +
+					related_to->helper->timeout * HZ;
+		add_timer(&new->timeout);
+	}
+	related_to->expecting++;
+}
+
 /* Add a related connection. */
-int ip_conntrack_expect_related(struct ip_conntrack *related_to,
-				struct ip_conntrack_expect *expect)
+int ip_conntrack_expect_related(struct ip_conntrack_expect *expect,
+				struct ip_conntrack *related_to)
 {
-	struct ip_conntrack_expect *old, *new;
+	struct ip_conntrack_expect *old;
 	int ret = 0;
 
 	WRITE_LOCK(&ip_conntrack_lock);
@@ -943,7 +987,7 @@
 		if (related_to->helper->timeout) {
 			if (!del_timer(&old->timeout)) {
 				/* expectation is dying. Fall through */
-				old = NULL;
+				goto out;
 			} else {
 				old->timeout.expires = jiffies + 
 					related_to->helper->timeout * HZ;
@@ -951,10 +995,10 @@
 			}
 		}
 
-		if (old) {
-			WRITE_UNLOCK(&ip_conntrack_lock);
-			return -EEXIST;
-		}
+		WRITE_UNLOCK(&ip_conntrack_lock);
+		kfree(expect);
+		return -EEXIST;
+
 	} else if (related_to->helper->max_expected && 
 		   related_to->expecting >= related_to->helper->max_expected) {
 		struct list_head *cur_item;
@@ -971,6 +1015,7 @@
 				       related_to->helper->name,
  		    	       	       NIPQUAD(related_to->tuplehash[IP_CT_DIR_ORIGINAL].tuple.src.ip),
  		    	       	       NIPQUAD(related_to->tuplehash[IP_CT_DIR_ORIGINAL].tuple.dst.ip));
+			kfree(expect);
 			return -EPERM;
 		}
 		DEBUGP("ip_conntrack: max number of expected "
@@ -1010,37 +1055,12 @@
 			     &expect->mask)) {
 		WRITE_UNLOCK(&ip_conntrack_lock);
 		DEBUGP("expect_related: busy!\n");
+
+		kfree(expect);
 		return -EBUSY;
 	}
-	
-	new = (struct ip_conntrack_expect *) 
-	      kmalloc(sizeof(struct ip_conntrack_expect), GFP_ATOMIC);
-	if (!new) {
-		WRITE_UNLOCK(&ip_conntrack_lock);
-		DEBUGP("expect_relaed: OOM allocating expect\n");
-		return -ENOMEM;
-	}
-	
-	DEBUGP("new expectation %p of conntrack %p\n", new, related_to);
-	memcpy(new, expect, sizeof(*expect));
-	new->expectant = related_to;
-	new->sibling = NULL;
-	atomic_set(&new->use, 1);
-	
-	/* add to expected list for this connection */	
-	list_add(&new->expected_list, &related_to->sibling_list);
-	/* add to global list of expectations */
-	list_prepend(&ip_conntrack_expect_list, &new->list);
-	/* add and start timer if required */
-	if (related_to->helper->timeout) {
-		init_timer(&new->timeout);
-		new->timeout.data = (unsigned long)new;
-		new->timeout.function = expectation_timed_out;
-		new->timeout.expires = jiffies + 
-					related_to->helper->timeout * HZ;
-		add_timer(&new->timeout);
-	}
-	related_to->expecting++;
+
+out:	ip_conntrack_expect_insert(expect, related_to);
 
 	WRITE_UNLOCK(&ip_conntrack_lock);
 
--- linux-2.6.3-old/net/ipv4/netfilter/ip_conntrack_standalone.c	2004-02-18 04:57:46.000000000 +0100
+++ linux-2.6.3/net/ipv4/netfilter/ip_conntrack_standalone.c	2004-03-05 03:06:54.000000000 +0100
@@ -497,6 +497,7 @@
 EXPORT_SYMBOL(ip_ct_find_proto);
 EXPORT_SYMBOL(__ip_ct_find_proto);
 EXPORT_SYMBOL(ip_ct_find_helper);
+EXPORT_SYMBOL(ip_conntrack_expect_alloc);
 EXPORT_SYMBOL(ip_conntrack_expect_related);
 EXPORT_SYMBOL(ip_conntrack_change_expect);
 EXPORT_SYMBOL(ip_conntrack_unexpect_related);
--- linux-2.6.3-old/include/linux/netfilter_ipv4/ip_conntrack_helper.h	2004-02-18 04:57:16.000000000 +0100
+++ linux-2.6.3/include/linux/netfilter_ipv4/ip_conntrack_helper.h	2004-03-05 15:11:02.000000000 +0100
@@ -35,9 +35,13 @@
 
 extern struct ip_conntrack_helper *ip_ct_find_helper(const struct ip_conntrack_tuple *tuple);
 
+
+/* Allocate space for an expectation: this is mandatory before calling 
+   ip_conntrack_expect_related. */
+extern struct ip_conntrack_expect *ip_conntrack_expect_alloc(void);
 /* Add an expected connection: can have more than one per connection */
-extern int ip_conntrack_expect_related(struct ip_conntrack *related_to,
-				       struct ip_conntrack_expect *exp);
+extern int ip_conntrack_expect_related(struct ip_conntrack_expect *exp,
+				       struct ip_conntrack *related_to);
 extern int ip_conntrack_change_expect(struct ip_conntrack_expect *expect,
 				      struct ip_conntrack_tuple *newtuple);
 extern void ip_conntrack_unexpect_related(struct ip_conntrack_expect *exp);
--- linux-2.6.3-old/net/ipv4/netfilter/ip_conntrack_tftp.c	2004-02-18 04:57:22.000000000 +0100
+++ linux-2.6.3/net/ipv4/netfilter/ip_conntrack_tftp.c	2004-03-06 02:54:05.000000000 +0100
@@ -44,7 +44,7 @@
 		     enum ip_conntrack_info ctinfo)
 {
 	struct tftphdr tftph;
-	struct ip_conntrack_expect exp;
+	struct ip_conntrack_expect *exp;
 
 	if (skb_copy_bits(skb, skb->nh.iph->ihl * 4 + sizeof(struct udphdr),
 			  &tftph, sizeof(tftph)) != 0)
@@ -57,19 +57,22 @@
 		DEBUGP("");
 		DUMP_TUPLE(&ct->tuplehash[IP_CT_DIR_ORIGINAL].tuple);
 		DUMP_TUPLE(&ct->tuplehash[IP_CT_DIR_REPLY].tuple);
-		memset(&exp, 0, sizeof(exp));
 
-		exp.tuple = ct->tuplehash[IP_CT_DIR_REPLY].tuple;
-		exp.mask.src.ip = 0xffffffff;
-		exp.mask.dst.ip = 0xffffffff;
-		exp.mask.dst.u.udp.port = 0xffff;
-		exp.mask.dst.protonum = 0xffff;
-		exp.expectfn = NULL;
+		exp = ip_conntrack_expect_alloc();
+		if (exp == NULL)
+			return NF_ACCEPT;
+
+		exp->tuple = ct->tuplehash[IP_CT_DIR_REPLY].tuple;
+		exp->mask.src.ip = 0xffffffff;
+		exp->mask.dst.ip = 0xffffffff;
+		exp->mask.dst.u.udp.port = 0xffff;
+		exp->mask.dst.protonum = 0xffff;
+		exp->expectfn = NULL;
 
 		DEBUGP("expect: ");
-		DUMP_TUPLE(&exp.tuple);
-		DUMP_TUPLE(&exp.mask);
-		ip_conntrack_expect_related(ct, &exp);
+		DUMP_TUPLE(&exp->tuple);
+		DUMP_TUPLE(&exp->mask);
+		ip_conntrack_expect_related(exp, ct);
 		break;
 	default:
 		DEBUGP("Unknown opcode\n");
--- linux-2.6.3-old/net/ipv4/netfilter/ip_conntrack_irc.c	2004-02-18 04:59:06.000000000 +0100
+++ linux-2.6.3/net/ipv4/netfilter/ip_conntrack_irc.c	2004-03-06 02:55:26.000000000 +0100
@@ -106,8 +106,8 @@
 	struct tcphdr tcph;
 	char *data, *data_limit;
 	int dir = CTINFO2DIR(ctinfo);
-	struct ip_conntrack_expect expect, *exp = &expect;
-	struct ip_ct_irc_expect *exp_irc_info = &exp->help.exp_irc_info;
+	struct ip_conntrack_expect *exp;
+	struct ip_ct_irc_expect *exp_irc_info = NULL;
 
 	u_int32_t dcc_ip;
 	u_int16_t dcc_port;
@@ -190,8 +190,12 @@
 
 				continue;
 			}
-			
-			memset(&expect, 0, sizeof(expect));
+
+			exp = ip_conntrack_expect_alloc();
+			if (exp == NULL)
+				goto out;
+
+			exp_irc_info = &exp->help.exp_irc_info;
 
 			/* save position of address in dcc string,
 			 * necessary for NAT */
@@ -218,7 +222,7 @@
 				NIPQUAD(exp->tuple.dst.ip),
 				ntohs(exp->tuple.dst.u.tcp.port));
 
-			ip_conntrack_expect_related(ct, &expect);
+			ip_conntrack_expect_related(exp, ct);
 
 			goto out;
 		} /* for .. NUM_DCCPROTO */
--- linux-2.6.3-old/net/ipv4/netfilter/ip_conntrack_ftp.c	2004-02-18 04:59:06.000000000 +0100
+++ linux-2.6.3/net/ipv4/netfilter/ip_conntrack_ftp.c	2004-03-06 02:56:56.000000000 +0100
@@ -256,8 +256,8 @@
 	int dir = CTINFO2DIR(ctinfo);
 	unsigned int matchlen, matchoff;
 	struct ip_ct_ftp_master *ct_ftp_info = &ct->help.ct_ftp_info;
-	struct ip_conntrack_expect expect, *exp = &expect;
-	struct ip_ct_ftp_expect *exp_ftp_info = &exp->help.exp_ftp_info;
+	struct ip_conntrack_expect *exp;
+	struct ip_ct_ftp_expect *exp_ftp_info;
 
 	unsigned int i;
 	int found = 0;
@@ -346,8 +346,15 @@
 	DEBUGP("conntrack_ftp: match `%.*s' (%u bytes at %u)\n",
 	       (int)matchlen, data + matchoff,
 	       matchlen, ntohl(tcph.seq) + matchoff);
-	       
-	memset(&expect, 0, sizeof(expect));
+
+	/* Allocate expectation which will be inserted */
+	exp = ip_conntrack_expect_alloc();
+	if (exp == NULL) {
+		ret = NF_ACCEPT;
+		goto out;
+	}
+
+	exp_ftp_info = &exp->help.exp_ftp_info;
 
 	/* Update the ftp info */
 	if (htonl((array[0] << 24) | (array[1] << 16) | (array[2] << 8) | array[3])
@@ -389,7 +396,7 @@
 	exp->expectfn = NULL;
 
 	/* Ignore failure; should only happen with NAT */
-	ip_conntrack_expect_related(ct, &expect);
+	ip_conntrack_expect_related(exp, ct);
 	ret = NF_ACCEPT;
  out:
 	UNLOCK_BH(&ip_ftp_lock);
--- linux-2.6.3-old/net/ipv4/netfilter/ip_conntrack_amanda.c	2004-03-05 03:24:47.000000000 +0100
+++ linux-2.6.3/net/ipv4/netfilter/ip_conntrack_amanda.c	2004-03-06 02:22:20.000000000 +0100
@@ -46,10 +46,11 @@
 static int help(struct sk_buff *skb,
                 struct ip_conntrack *ct, enum ip_conntrack_info ctinfo)
 {
-	struct ip_conntrack_expect exp;
+	struct ip_conntrack_expect *exp;
 	struct ip_ct_amanda_expect *exp_amanda_info;
 	char *data, *data_limit, *tmp;
 	unsigned int dataoff, i;
+	u_int16_t port, len;
 
 	/* Only look at packets from the Amanda server */
 	if (CTINFO2DIR(ctinfo) == IP_CT_DIR_ORIGINAL)
@@ -79,33 +80,40 @@
 		goto out;
 	data += strlen("CONNECT ");
 
-	memset(&exp, 0, sizeof(exp));
-	exp.tuple.src.ip = ct->tuplehash[IP_CT_DIR_ORIGINAL].tuple.src.ip;
-	exp.tuple.dst.ip = ct->tuplehash[IP_CT_DIR_ORIGINAL].tuple.dst.ip;
-	exp.tuple.dst.protonum = IPPROTO_TCP;
-	exp.mask.src.ip = 0xFFFFFFFF;
-	exp.mask.dst.ip = 0xFFFFFFFF;
-	exp.mask.dst.protonum = 0xFFFF;
-	exp.mask.dst.u.tcp.port = 0xFFFF;
-
 	/* Only search first line. */	
 	if ((tmp = strchr(data, '\n')))
 		*tmp = '\0';
 
-	exp_amanda_info = &exp.help.exp_amanda_info;
 	for (i = 0; i < ARRAY_SIZE(conns); i++) {
 		char *match = strstr(data, conns[i]);
 		if (!match)
 			continue;
 		tmp = data = match + strlen(conns[i]);
-		exp_amanda_info->offset = data - amanda_buffer;
-		exp_amanda_info->port   = simple_strtoul(data, &data, 10);
-		exp_amanda_info->len    = data - tmp;
-		if (exp_amanda_info->port == 0 || exp_amanda_info->len > 5)
+		port = simple_strtoul(data, &data, 10);
+		len = data - tmp;
+		if (port == 0 || len > 5)
 			break;
 
-		exp.tuple.dst.u.tcp.port = htons(exp_amanda_info->port);
-		ip_conntrack_expect_related(ct, &exp);
+		exp = ip_conntrack_expect_alloc();
+		if (exp == NULL)
+			goto out;
+
+		exp->tuple.src.ip = ct->tuplehash[IP_CT_DIR_ORIGINAL].tuple.src.ip;
+		exp->tuple.dst.ip = ct->tuplehash[IP_CT_DIR_ORIGINAL].tuple.dst.ip;
+		exp->tuple.dst.protonum = IPPROTO_TCP;
+		exp->mask.src.ip = 0xFFFFFFFF;
+		exp->mask.dst.ip = 0xFFFFFFFF;
+		exp->mask.dst.protonum = 0xFFFF;
+		exp->mask.dst.u.tcp.port = 0xFFFF;
+
+		exp_amanda_info = &exp->help.exp_amanda_info;
+		exp_amanda_info->offset = data - amanda_buffer;
+		exp_amanda_info->port   = port;
+		exp_amanda_info->len    = len;
+
+		exp->tuple.dst.u.tcp.port = htons(port);
+
+		ip_conntrack_expect_related(exp, ct);
 	}
 
 out:

[-- Attachment #3: expect-06032004-CVS.patch --]
[-- Type: text/plain, Size: 10440 bytes --]

--- /home/pablo/CVS/patch-o-matic-ng/expect-optimize/linux-2.6.patch	2004-03-05 12:56:01.000000000 +0100
+++ expect-06032004.patch	2004-03-06 03:08:10.000000000 +0100
@@ -1,23 +1,25 @@
---- linux-2.6.3-old/net/ipv4/netfilter/ip_conntrack_core.c	2004-02-22 13:29:30.000000000 +0100
-+++ linux-2.6.3-new/net/ipv4/netfilter/ip_conntrack_core.c	2004-03-02 00:39:35.000000000 +0100
-@@ -917,11 +917,53 @@
+--- linux-2.6.3-old/net/ipv4/netfilter/ip_conntrack_core.c	2004-02-18 04:57:11.000000000 +0100
++++ linux-2.6.3/net/ipv4/netfilter/ip_conntrack_core.c	2004-03-05 03:08:30.000000000 +0100
+@@ -917,11 +917,55 @@
  	WRITE_UNLOCK(&ip_conntrack_lock);
  }
  
-+int
-+ip_conntrack_expect_alloc(struct ip_conntrack_expect **new)
++struct ip_conntrack_expect *
++ip_conntrack_expect_alloc()
 +{
-+	*new = (struct ip_conntrack_expect *)
++	struct ip_conntrack_expect *new;
++	
++	new = (struct ip_conntrack_expect *)
 +		kmalloc(sizeof(struct ip_conntrack_expect), GFP_ATOMIC);
-+	if (!*new) {
++	if (!new) {
 +		DEBUGP("expect_related: OOM allocating expect\n");
-+		return -ENOMEM;
++		return NULL;
 +	}
 +
 +	/* tuple_cmp compares whole union, we have to initialized cleanly */
-+	memset(*new, 0, sizeof(struct ip_conntrack_expect));
++	memset(new, 0, sizeof(struct ip_conntrack_expect));
 +
-+	return 1;
++	return new;
 +}
 +
 +static void
@@ -57,7 +59,7 @@
  	int ret = 0;
  
  	WRITE_LOCK(&ip_conntrack_lock);
-@@ -943,7 +985,7 @@
+@@ -943,7 +987,7 @@
  		if (related_to->helper->timeout) {
  			if (!del_timer(&old->timeout)) {
  				/* expectation is dying. Fall through */
@@ -66,7 +68,7 @@
  			} else {
  				old->timeout.expires = jiffies + 
  					related_to->helper->timeout * HZ;
-@@ -951,10 +993,10 @@
+@@ -951,10 +995,10 @@
  			}
  		}
  
@@ -81,7 +83,7 @@
  	} else if (related_to->helper->max_expected && 
  		   related_to->expecting >= related_to->helper->max_expected) {
  		struct list_head *cur_item;
-@@ -971,6 +1013,7 @@
+@@ -971,6 +1015,7 @@
  				       related_to->helper->name,
   		    	       	       NIPQUAD(related_to->tuplehash[IP_CT_DIR_ORIGINAL].tuple.src.ip),
   		    	       	       NIPQUAD(related_to->tuplehash[IP_CT_DIR_ORIGINAL].tuple.dst.ip));
@@ -89,7 +91,7 @@
  			return -EPERM;
  		}
  		DEBUGP("ip_conntrack: max number of expected "
-@@ -1010,37 +1053,12 @@
+@@ -1010,37 +1055,12 @@
  			     &expect->mask)) {
  		WRITE_UNLOCK(&ip_conntrack_lock);
  		DEBUGP("expect_related: busy!\n");
@@ -132,7 +134,7 @@
  	WRITE_UNLOCK(&ip_conntrack_lock);
  
 --- linux-2.6.3-old/net/ipv4/netfilter/ip_conntrack_standalone.c	2004-02-18 04:57:46.000000000 +0100
-+++ linux-2.6.3-new/net/ipv4/netfilter/ip_conntrack_standalone.c	2004-03-02 00:31:00.000000000 +0100
++++ linux-2.6.3/net/ipv4/netfilter/ip_conntrack_standalone.c	2004-03-05 03:06:54.000000000 +0100
 @@ -497,6 +497,7 @@
  EXPORT_SYMBOL(ip_ct_find_proto);
  EXPORT_SYMBOL(__ip_ct_find_proto);
@@ -142,7 +144,7 @@
  EXPORT_SYMBOL(ip_conntrack_change_expect);
  EXPORT_SYMBOL(ip_conntrack_unexpect_related);
 --- linux-2.6.3-old/include/linux/netfilter_ipv4/ip_conntrack_helper.h	2004-02-18 04:57:16.000000000 +0100
-+++ linux-2.6.3-new/include/linux/netfilter_ipv4/ip_conntrack_helper.h	2004-03-02 00:31:00.000000000 +0100
++++ linux-2.6.3/include/linux/netfilter_ipv4/ip_conntrack_helper.h	2004-03-05 15:11:02.000000000 +0100
 @@ -35,9 +35,13 @@
  
  extern struct ip_conntrack_helper *ip_ct_find_helper(const struct ip_conntrack_tuple *tuple);
@@ -150,7 +152,7 @@
 +
 +/* Allocate space for an expectation: this is mandatory before calling 
 +   ip_conntrack_expect_related. */
-+extern int ip_conntrack_expect_alloc(struct ip_conntrack_expect **new);
++extern struct ip_conntrack_expect *ip_conntrack_expect_alloc(void);
  /* Add an expected connection: can have more than one per connection */
 -extern int ip_conntrack_expect_related(struct ip_conntrack *related_to,
 -				       struct ip_conntrack_expect *exp);
@@ -160,7 +162,7 @@
  				      struct ip_conntrack_tuple *newtuple);
  extern void ip_conntrack_unexpect_related(struct ip_conntrack_expect *exp);
 --- linux-2.6.3-old/net/ipv4/netfilter/ip_conntrack_tftp.c	2004-02-18 04:57:22.000000000 +0100
-+++ linux-2.6.3-new/net/ipv4/netfilter/ip_conntrack_tftp.c	2004-02-25 15:31:52.000000000 +0100
++++ linux-2.6.3/net/ipv4/netfilter/ip_conntrack_tftp.c	2004-03-06 02:54:05.000000000 +0100
 @@ -44,7 +44,7 @@
  		     enum ip_conntrack_info ctinfo)
  {
@@ -170,7 +172,7 @@
  
  	if (skb_copy_bits(skb, skb->nh.iph->ihl * 4 + sizeof(struct udphdr),
  			  &tftph, sizeof(tftph)) != 0)
-@@ -57,19 +57,21 @@
+@@ -57,19 +57,22 @@
  		DEBUGP("");
  		DUMP_TUPLE(&ct->tuplehash[IP_CT_DIR_ORIGINAL].tuple);
  		DUMP_TUPLE(&ct->tuplehash[IP_CT_DIR_REPLY].tuple);
@@ -182,8 +184,9 @@
 -		exp.mask.dst.u.udp.port = 0xffff;
 -		exp.mask.dst.protonum = 0xffff;
 -		exp.expectfn = NULL;
-+		if (ip_conntrack_expect_alloc(&exp) < 0)
-+			return -ENOMEM;
++		exp = ip_conntrack_expect_alloc();
++		if (exp == NULL)
++			return NF_ACCEPT;
 +
 +		exp->tuple = ct->tuplehash[IP_CT_DIR_REPLY].tuple;
 +		exp->mask.src.ip = 0xffffffff;
@@ -203,7 +206,7 @@
  	default:
  		DEBUGP("Unknown opcode\n");
 --- linux-2.6.3-old/net/ipv4/netfilter/ip_conntrack_irc.c	2004-02-18 04:59:06.000000000 +0100
-+++ linux-2.6.3-new/net/ipv4/netfilter/ip_conntrack_irc.c	2004-02-25 15:32:27.000000000 +0100
++++ linux-2.6.3/net/ipv4/netfilter/ip_conntrack_irc.c	2004-03-06 02:55:26.000000000 +0100
 @@ -106,8 +106,8 @@
  	struct tcphdr tcph;
  	char *data, *data_limit;
@@ -215,21 +218,22 @@
  
  	u_int32_t dcc_ip;
  	u_int16_t dcc_port;
-@@ -190,8 +190,11 @@
+@@ -190,8 +190,12 @@
  
  				continue;
  			}
 -			
 -			memset(&expect, 0, sizeof(expect));
 +
-+			if (ip_conntrack_expect_alloc(&exp) < 0)
-+				return -ENOMEM;
++			exp = ip_conntrack_expect_alloc();
++			if (exp == NULL)
++				goto out;
 +
 +			exp_irc_info = &exp->help.exp_irc_info;
  
  			/* save position of address in dcc string,
  			 * necessary for NAT */
-@@ -218,7 +221,7 @@
+@@ -218,7 +222,7 @@
  				NIPQUAD(exp->tuple.dst.ip),
  				ntohs(exp->tuple.dst.u.tcp.port));
  
@@ -239,7 +243,7 @@
  			goto out;
  		} /* for .. NUM_DCCPROTO */
 --- linux-2.6.3-old/net/ipv4/netfilter/ip_conntrack_ftp.c	2004-02-18 04:59:06.000000000 +0100
-+++ linux-2.6.3-new/net/ipv4/netfilter/ip_conntrack_ftp.c	2004-02-25 15:31:41.000000000 +0100
++++ linux-2.6.3/net/ipv4/netfilter/ip_conntrack_ftp.c	2004-03-06 02:56:56.000000000 +0100
 @@ -256,8 +256,8 @@
  	int dir = CTINFO2DIR(ctinfo);
  	unsigned int matchlen, matchoff;
@@ -251,7 +255,7 @@
  
  	unsigned int i;
  	int found = 0;
-@@ -346,8 +346,12 @@
+@@ -346,8 +346,15 @@
  	DEBUGP("conntrack_ftp: match `%.*s' (%u bytes at %u)\n",
  	       (int)matchlen, data + matchoff,
  	       matchlen, ntohl(tcph.seq) + matchoff);
@@ -259,14 +263,17 @@
 -	memset(&expect, 0, sizeof(expect));
 +
 +	/* Allocate expectation which will be inserted */
-+	if (ip_conntrack_expect_alloc(&exp) < 0)
-+		return -ENOMEM;
++	exp = ip_conntrack_expect_alloc();
++	if (exp == NULL) {
++		ret = NF_ACCEPT;
++		goto out;
++	}
 +
 +	exp_ftp_info = &exp->help.exp_ftp_info;
  
  	/* Update the ftp info */
  	if (htonl((array[0] << 24) | (array[1] << 16) | (array[2] << 8) | array[3])
-@@ -389,7 +393,7 @@
+@@ -389,7 +396,7 @@
  	exp->expectfn = NULL;
  
  	/* Ignore failure; should only happen with NAT */
@@ -275,9 +282,9 @@
  	ret = NF_ACCEPT;
   out:
  	UNLOCK_BH(&ip_ftp_lock);
---- linux-2.6.3-old/net/ipv4/netfilter/ip_conntrack_amanda.c	2004-02-25 17:34:05.000000000 +0100
-+++ linux-2.6.3-new/net/ipv4/netfilter/ip_conntrack_amanda.c	2004-02-25 17:39:34.000000000 +0100
-@@ -46,7 +46,7 @@
+--- linux-2.6.3-old/net/ipv4/netfilter/ip_conntrack_amanda.c	2004-03-05 03:24:47.000000000 +0100
++++ linux-2.6.3/net/ipv4/netfilter/ip_conntrack_amanda.c	2004-03-06 02:22:20.000000000 +0100
+@@ -46,10 +46,11 @@
  static int help(struct sk_buff *skb,
                  struct ip_conntrack *ct, enum ip_conntrack_info ctinfo)
  {
@@ -286,7 +293,11 @@
  	struct ip_ct_amanda_expect *exp_amanda_info;
  	char *data, *data_limit, *tmp;
  	unsigned int dataoff, i;
-@@ -79,20 +79,22 @@
++	u_int16_t port, len;
+ 
+ 	/* Only look at packets from the Amanda server */
+ 	if (CTINFO2DIR(ctinfo) == IP_CT_DIR_ORIGINAL)
+@@ -79,33 +80,40 @@
  		goto out;
  	data += strlen("CONNECT ");
  
@@ -298,33 +309,47 @@
 -	exp.mask.dst.ip = 0xFFFFFFFF;
 -	exp.mask.dst.protonum = 0xFFFF;
 -	exp.mask.dst.u.tcp.port = 0xFFFF;
-+	if (ip_conntrack_expect_alloc(&exp) < 0)
-+		return -ENOMEM;
-+	
-+	exp->tuple.src.ip = ct->tuplehash[IP_CT_DIR_ORIGINAL].tuple.src.ip;
-+	exp->tuple.dst.ip = ct->tuplehash[IP_CT_DIR_ORIGINAL].tuple.dst.ip;
-+	exp->tuple.dst.protonum = IPPROTO_TCP;
-+	exp->mask.src.ip = 0xFFFFFFFF;
-+	exp->mask.dst.ip = 0xFFFFFFFF;
-+	exp->mask.dst.protonum = 0xFFFF;
-+	exp->mask.dst.u.tcp.port = 0xFFFF;
- 
+-
  	/* Only search first line. */	
  	if ((tmp = strchr(data, '\n')))
  		*tmp = '\0';
  
 -	exp_amanda_info = &exp.help.exp_amanda_info;
-+	exp_amanda_info = &exp->help.exp_amanda_info;
  	for (i = 0; i < ARRAY_SIZE(conns); i++) {
  		char *match = strstr(data, conns[i]);
  		if (!match)
-@@ -104,8 +106,8 @@
- 		if (exp_amanda_info->port == 0 || exp_amanda_info->len > 5)
+ 			continue;
+ 		tmp = data = match + strlen(conns[i]);
+-		exp_amanda_info->offset = data - amanda_buffer;
+-		exp_amanda_info->port   = simple_strtoul(data, &data, 10);
+-		exp_amanda_info->len    = data - tmp;
+-		if (exp_amanda_info->port == 0 || exp_amanda_info->len > 5)
++		port = simple_strtoul(data, &data, 10);
++		len = data - tmp;
++		if (port == 0 || len > 5)
  			break;
  
 -		exp.tuple.dst.u.tcp.port = htons(exp_amanda_info->port);
 -		ip_conntrack_expect_related(ct, &exp);
-+		exp->tuple.dst.u.tcp.port = htons(exp_amanda_info->port);
++		exp = ip_conntrack_expect_alloc();
++		if (exp == NULL)
++			goto out;
++
++		exp->tuple.src.ip = ct->tuplehash[IP_CT_DIR_ORIGINAL].tuple.src.ip;
++		exp->tuple.dst.ip = ct->tuplehash[IP_CT_DIR_ORIGINAL].tuple.dst.ip;
++		exp->tuple.dst.protonum = IPPROTO_TCP;
++		exp->mask.src.ip = 0xFFFFFFFF;
++		exp->mask.dst.ip = 0xFFFFFFFF;
++		exp->mask.dst.protonum = 0xFFFF;
++		exp->mask.dst.u.tcp.port = 0xFFFF;
++
++		exp_amanda_info = &exp->help.exp_amanda_info;
++		exp_amanda_info->offset = data - amanda_buffer;
++		exp_amanda_info->port   = port;
++		exp_amanda_info->len    = len;
++
++		exp->tuple.dst.u.tcp.port = htons(port);
++
 +		ip_conntrack_expect_related(exp, ct);
  	}