[PATCH] datalen variable unused in ftp helper.

[PATCH] datalen variable unused in ftp helper.

[Pablo Neira]

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

Hi,

I found that the variable datalen in the ftp helper is sometimes used 
and sometimes not. Attached to this email the patch to fix this.

regards,
Pablo

[-- Attachment #2: ftp_helper-cleanup.patch --]
[-- Type: text/plain, Size: 774 bytes --]

--- linux-2.6.3/net/ipv4/netfilter/ip_conntrack_ftp.c	2004-03-09 18:49:21.000000000 +0100
+++ linux-2.6.3-old/net/ipv4/netfilter/ip_conntrack_ftp.c	2004-02-18 04:59:06.000000000 +0100
@@ -281,7 +281,7 @@
 	datalen = skb->len - dataoff;
 
 	LOCK_BH(&ip_ftp_lock);
-	skb_copy_bits(skb, dataoff, ftp_buffer, datalen);
+	skb_copy_bits(skb, dataoff, ftp_buffer, skb->len - dataoff);
 
 	old_seq_aft_nl_set = ct_ftp_info->seq_aft_nl_set[dir];
 	old_seq_aft_nl = ct_ftp_info->seq_aft_nl[dir];
@@ -317,7 +317,7 @@
 	for (i = 0; i < ARRAY_SIZE(search); i++) {
 		if (search[i].dir != dir) continue;
 
-		found = find_pattern(ftp_buffer, datalen,
+		found = find_pattern(ftp_buffer, skb->len - dataoff,
 				     search[i].pattern,
 				     search[i].plen,
 				     search[i].skip,

Re: [PATCH] datalen variable unused in ftp helper.

[Patrick McHardy]

Pablo Neira wrote:
> Hi,
> 
> I found that the variable datalen in the ftp helper is sometimes used 
> and sometimes not. Attached to this email the patch to fix this.
> 
> regards,
> Pablo
> 
> 
> ------------------------------------------------------------------------
> 
> --- linux-2.6.3/net/ipv4/netfilter/ip_conntrack_ftp.c	2004-03-09 18:49:21.000000000 +0100
> +++ linux-2.6.3-old/net/ipv4/netfilter/ip_conntrack_ftp.c	2004-02-18 04:59:06.000000000 +0100
> @@ -281,7 +281,7 @@
>  	datalen = skb->len - dataoff;

^^ Shouldn't the patch remove the variable entirely then ?

Regards
Patrick



>  
>  	LOCK_BH(&ip_ftp_lock);
> -	skb_copy_bits(skb, dataoff, ftp_buffer, datalen);
> +	skb_copy_bits(skb, dataoff, ftp_buffer, skb->len - dataoff);
>  
>  	old_seq_aft_nl_set = ct_ftp_info->seq_aft_nl_set[dir];
>  	old_seq_aft_nl = ct_ftp_info->seq_aft_nl[dir];
> @@ -317,7 +317,7 @@
>  	for (i = 0; i < ARRAY_SIZE(search); i++) {
>  		if (search[i].dir != dir) continue;
>  
> -		found = find_pattern(ftp_buffer, datalen,
> +		found = find_pattern(ftp_buffer, skb->len - dataoff,
>  				     search[i].pattern,
>  				     search[i].plen,
>  				     search[i].skip,

Re: [PATCH] datalen variable unused in ftp helper.

[Pablo Neira]

Hi Patrick,

Patrick McHardy wrote:

>> --- linux-2.6.3/net/ipv4/netfilter/ip_conntrack_ftp.c    2004-03-09 
>> 18:49:21.000000000 +0100
>> +++ linux-2.6.3-old/net/ipv4/netfilter/ip_conntrack_ftp.c    
>> 2004-02-18 04:59:06.000000000 +0100
>> @@ -281,7 +281,7 @@
>>      datalen = skb->len - dataoff;
>
>
> ^^ Shouldn't the patch remove the variable entirely then ?


I'm not sure what's better. Adding a new parameter to the stack is cheap 
and this way I'm sure that gcc won't generate code to calculate skb->len 
- dataoff over and over again. I'm not a compiler geek, so I could be 
wrong :-). If you think that removing datalen is a better solution, 
please let me know and I'll post the patch.

regards,
Pablo

Re: [PATCH] datalen variable unused in ftp helper.

[Patrick McHardy]

Pablo Neira wrote:
> Hi Patrick,
> 
> Patrick McHardy wrote:
> 
>>> --- linux-2.6.3/net/ipv4/netfilter/ip_conntrack_ftp.c    2004-03-09 
>>> 18:49:21.000000000 +0100
>>> +++ linux-2.6.3-old/net/ipv4/netfilter/ip_conntrack_ftp.c    
>>> 2004-02-18 04:59:06.000000000 +0100
>>> @@ -281,7 +281,7 @@
>>>      datalen = skb->len - dataoff;
>>
>>
>>
>> ^^ Shouldn't the patch remove the variable entirely then ?
> 
> 
> 
> I'm not sure what's better. Adding a new parameter to the stack is cheap 
> and this way I'm sure that gcc won't generate code to calculate skb->len 
> - dataoff over and over again. I'm not a compiler geek, so I could be 
> wrong :-). If you think that removing datalen is a better solution, 
> please let me know and I'll post the patch.

No I don't think so, I was confused by your patch which is reversed,
so I thought you wanted to remove the variable. It is used quite often,
so that doesn't make much sense. In the right order, your patch is
fine ;)

Regards
Patrick

> 
> regards,
> Pablo
> 

Re: [PATCH] datalen variable unused in ftp helper.

[Harald Welte]

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

On Tue, Mar 09, 2004 at 07:17:49PM +0100, Pablo Neira wrote:
> Hi,
> 
> I found that the variable datalen in the ftp helper is sometimes used 
> and sometimes not. Attached to this email the patch to fix this.
> 
> regards,
> Pablo

Pablo, I seem to loose track of your dozens of patches.  Could you
please re-send all patches that do not yet show up in a current 2.6.x
tree?   I'll then review and submit all of them in one 'flush'

Thanks again.

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