Skip to contentSkip to conversation

datalen variable unused in ftp helper.

PATCH In review Pablo Neira 3 participants 5 messages Mar 9, 2004 – Mar 17, 2004

Subsystems (2)
Networking netfilter
lore
Share & tools
lore

Pablo Neira

author
#0 lore raw

PATCH 1/1 datalen variable unused in ftp helper.

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

net/ipv4/netfilter/ip_conntrack_ftp.c +2 -2
@@ -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,
#1 lore raw
Quoted 13 lines · Pablo Neira

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

------------------------------------------------------------------------

net/ipv4/netfilter/ip_conntrack_ftp.c line 281

Quoted 1 line
net/ipv4/netfilter/ip_conntrack_ftp.c +0 -0
@@ -281,7 +281,7 @@
 	datalen = skb->len - dataoff;

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

Regards
Patrick

Quoted 16 lines
net/ipv4/netfilter/ip_conntrack_ftp.c +2 -2
 	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,
author
#2 lore raw

Hi Patrick,

Quoted 9 lines · Patrick McHardy

net/ipv4/netfilter/ip_conntrack_ftp.c line 281

Quoted 6 lines

--- 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
net/ipv4/netfilter/ip_conntrack_ftp.c +0 -0
@@ -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

#3 lore raw
Quoted 17 lines · Pablo Neira

Hi Patrick,

Quoted 10 lines · Patrick McHardy

net/ipv4/netfilter/ip_conntrack_ftp.c line 281

Quoted 6 lines

--- 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
net/ipv4/netfilter/ip_conntrack_ftp.c +0 -0
@@ -281,7 +281,7 @@
     datalen = skb->len - dataoff;

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

Quoted 3 lines

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

Quoted 4 lines

regards,
Pablo

#4 lore raw
In reply to Pablo Neira
Quoted 7 lines · Pablo Neira

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.

Signature · 5 lines
- Harald Welte <[email protected]>             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