[PATCH] ip_conntrack_ftp segfault

[PATCH] ip_conntrack_ftp segfault

[Nicolas Bouliane]

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


Hi,

This patch fix two bugs (when DEBUGP is defined):
o The var `data` doesn't exist, hence gcc complains.
o It segfault because we print an integer with `%s`.

Cheers! :)

-acidfu

[-- Attachment #2: ip_conntrack_ftp.c.patch --]
[-- Type: text/x-patch, Size: 558 bytes --]

--- /usr/src/old/linux-2.6.10/net/ipv4/netfilter/ip_conntrack_ftp.c	2004-12-24 16:35:59.000000000 -0500
+++ ip_conntrack_ftp.c	2005-01-03 14:09:06.000000000 -0500
@@ -347,9 +347,8 @@
 		goto out;
 	}
 
-	DEBUGP("conntrack_ftp: match `%.*s' (%u bytes at %u)\n",
-	       (int)matchlen, data + matchoff,
-	       matchlen, ntohl(th->seq) + matchoff);
+	DEBUGP("conntrack_ftp: match `%s' (%u bytes at %u)\n",
+			search[i].pattern, matchlen, ntohl(th->seq) + matchoff);
 
 	/* Allocate expectation which will be inserted */
 	exp = ip_conntrack_expect_alloc();

Re: [PATCH] ip_conntrack_ftp segfault

[Pablo Neira]

nice catch, but read the comments below.

Nicolas Bouliane wrote:

> This patch fix two bugs (when DEBUGP is defined):
> o The var `data` doesn't exist, hence gcc complains.
> o It segfault because we print an integer with `%s`.
>
>------------------------------------------------------------------------
>
>--- /usr/src/old/linux-2.6.10/net/ipv4/netfilter/ip_conntrack_ftp.c	2004-12-24 16:35:59.000000000 -0500
>+++ ip_conntrack_ftp.c	2005-01-03 14:09:06.000000000 -0500
>  
>

this header must always look like, for example:

--- linux-2.6.10/net/ipv4/netfilter/ip_conntrack_ftp.c  2004-10-20 
10:12:06 +02:00
+++ linux-2.6.10-nicolas/net/ipv4/netfilter/ip_conntrack_ftp.c        
2005-01-02 16:09:01 +01:00

>@@ -347,9 +347,8 @@
> 		goto out;
> 	}
> 
>-	DEBUGP("conntrack_ftp: match `%.*s' (%u bytes at %u)\n",
>-	       (int)matchlen, data + matchoff,
>-	       matchlen, ntohl(th->seq) + matchoff);
>+	DEBUGP("conntrack_ftp: match `%s' (%u bytes at %u)\n",
>+			search[i].pattern, matchlen, ntohl(th->seq) + matchoff);
>  
>

Since this is a minor fix, try to modify as less things as you can. data 
doesn't exist but look for its natural replacement which is fb_ptr.

Rusty punish me yesterday to read part (11) of 
Documentation/SubmittingPatches in the kernel source tree :). So do it 
you as well and add your sign-off.

Resend a patch which fixes the things above. After that, let wait to see 
if Patrick wants to add this to his changeset 8).

--
Pablo

Re: [PATCH] ip_conntrack_ftp segfault

[Nicolas Bouliane]

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

Pablo Neira wrote:
> nice catch, but read the comments below.

> Since this is a minor fix, try to modify as less things as you can. data 
> doesn't exist but look for its natural replacement which is fb_ptr.

Hey Pablo, thanks for the hints! :)

>> This patch fix two bugs (when DEBUGP is defined):
>> o The var `data` doesn't exist, hence gcc complains.
>> o It segfault because we print an integer with `%s`.

Signed-off-by: Nicolas Bouliane <nib@cookinglinux.org>



[-- Attachment #2: ip_conntrack_ftp.c.patch --]
[-- Type: text/x-patch, Size: 613 bytes --]

--- linux-2.6.10/net/ipv4/netfilter/ip_conntrack_ftp.c	2004-12-24 16:35:59.000000000 -0500
+++ linux-2.6.10NIB/net/ipv4/netfilter/ip_conntrack_ftp.c	2005-01-04 13:24:35.000000000 -0500
@@ -347,10 +347,9 @@
 		goto out;
 	}
 
-	DEBUGP("conntrack_ftp: match `%.*s' (%u bytes at %u)\n",
-	       (int)matchlen, data + matchoff,
-	       matchlen, ntohl(th->seq) + matchoff);
-
+	DEBUGP("conntrack_ftp: match `%s' (%u bytes at %u)\n",
+	       fb_ptr + matchoff, matchlen, ntohl(th->seq) + matchoff);
+			 
 	/* Allocate expectation which will be inserted */
 	exp = ip_conntrack_expect_alloc();
 	if (exp == NULL) {

Re: [PATCH] ip_conntrack_ftp segfault

[Patrick McHardy]

Nicolas Bouliane wrote:

>>> This patch fix two bugs (when DEBUGP is defined):
>>> o The var `data` doesn't exist, hence gcc complains.
>>> o It segfault because we print an integer with `%s`.
>>
>
> Signed-off-by: Nicolas Bouliane <nib@cookinglinux.org>

Applied, thanks.

Regards
Patrick