: 2.4 broken helper locking

diff -Nru a/net/ipv4/netfilter/ip_conntrack_ftp.c b/net/ipv4/netfilter/ip_conntrack_ftp.c
@@ -11,7 +11,7 @@
 #include <linux/netfilter_ipv4/ip_conntrack_helper.h>
 #include <linux/netfilter_ipv4/ip_conntrack_ftp.h>
 
-DECLARE_LOCK(ip_ftp_lock);
+static DECLARE_LOCK(ip_ftp_lock);
 struct module *ip_conntrack_ftp = THIS_MODULE;
 
 #define MAX_PORTS 8

@@ -338,7 +338,6 @@
 	memset(&expect, 0, sizeof(expect));
 
 	/* Update the ftp info */
-	LOCK_BH(&ip_ftp_lock);
 	if (htonl((array[0] << 24) | (array[1] << 16) | (array[2] << 8) | array[3])
 	    == ct->tuplehash[dir].tuple.src.ip) {
 		exp->seq = ntohl(tcph->seq) + matchoff;

@@ -358,7 +357,8 @@
 		   <[email protected]> for reporting this potential
 		   problem (DMZ machines opening holes to internal
 		   networks, or the packet filter itself). */
-		if (!loose) goto out;
+		if (!loose)
+			return NF_ACCEPT;
 	}
 
 	exp->tuple = ((struct ip_conntrack_tuple)

@@ -376,8 +376,6 @@
 
 	/* Ignore failure; should only happen with NAT */
 	ip_conntrack_expect_related(ct, &expect);
- out:
-	UNLOCK_BH(&ip_ftp_lock);
 
 	return NF_ACCEPT;
 }

@@ -434,8 +432,6 @@
 	}
 	return 0;
 }
-
-EXPORT_SYMBOL(ip_ftp_lock);
 
 MODULE_LICENSE("GPL");
 module_init(init);

diff -Nru a/net/ipv4/netfilter/ip_conntrack_irc.c b/net/ipv4/netfilter/ip_conntrack_irc.c
@@ -29,7 +29,6 @@
 #include <net/checksum.h>
 #include <net/tcp.h>
 
-#include <linux/netfilter_ipv4/lockhelp.h>
 #include <linux/netfilter_ipv4/ip_conntrack_helper.h>
 #include <linux/netfilter_ipv4/ip_conntrack_irc.h>
 

@@ -61,7 +60,6 @@
 };
 #define MINMATCHLEN	5
 
-DECLARE_LOCK(ip_irc_lock);
 struct module *ip_conntrack_irc = THIS_MODULE;
 
 #if 0

@@ -208,8 +206,6 @@
 			
 			memset(&expect, 0, sizeof(expect));
 
-			LOCK_BH(&ip_irc_lock);
-
 			/* save position of address in dcc string,
 			 * neccessary for NAT */
 			DEBUGP("tcph->seq = %u\n", tcph->seq);

@@ -236,8 +232,6 @@
 				ntohs(exp->tuple.dst.u.tcp.port));
 
 			ip_conntrack_expect_related(ct, &expect);
-			UNLOCK_BH(&ip_irc_lock);
-
 			return NF_ACCEPT;
 		} /* for .. NUM_DCCPROTO */
 	} /* while data < ... */

@@ -314,8 +308,6 @@
 		ip_conntrack_helper_unregister(&irc_helpers[i]);
 	}
 }
-
-EXPORT_SYMBOL(ip_irc_lock);
 
 module_init(init);
 module_exit(fini);

137 lines not on this page · raw message

diff -Nru a/net/ipv4/netfilter/ip_nat_irc.c b/net/ipv4/netfilter/ip_nat_irc.c
@@ -46,9 +46,6 @@
 MODULE_PARM_DESC(ports, "port numbers of IRC servers");
 #endif
 
-/* protects irc part of conntracks */
-DECLARE_LOCK_EXTERN(ip_irc_lock);
-
 /* FIXME: Time out? --RR */
 
 static unsigned int

@@ -89,7 +86,7 @@
 	return ip_nat_setup_info(ct, &mr, hooknum);
 }
 
-static int irc_data_fixup(const struct ip_ct_irc_expect *ct_irc_info,
+static int irc_data_fixup(const struct ip_ct_irc_expect *exp_irc_info,
 			  struct ip_conntrack *ct,
 			  struct sk_buff **pskb,
 			  enum ip_conntrack_info ctinfo,

@@ -104,23 +101,17 @@
 	/* "4294967296 65635 " */
 	char buffer[18];
 
-	MUST_BE_LOCKED(&ip_irc_lock);
-
 	DEBUGP("IRC_NAT: info (seq %u + %u) in %u\n",
-	       expect->seq, ct_irc_info->len,
+	       expect->seq, exp_irc_info->len,
 	       ntohl(tcph->seq));
 
 	newip = ct->tuplehash[IP_CT_DIR_REPLY].tuple.dst.ip;
 
 	/* Alter conntrack's expectations. */
 
-	/* We can read expect here without conntrack lock, since it's
-	   only set in ip_conntrack_irc, with ip_irc_lock held
-	   writable */
-
 	t = expect->tuple;
 	t.dst.ip = newip;
-	for (port = ct_irc_info->port; port != 0; port++) {
+	for (port = exp_irc_info->port; port != 0; port++) {
 		t.dst.u.tcp.port = htons(port);
 		if (ip_conntrack_change_expect(expect, &t) == 0) {
 			DEBUGP("using port %d", port);

@@ -150,7 +141,7 @@
 
 	return ip_nat_mangle_tcp_packet(pskb, ct, ctinfo, 
 					expect->seq - ntohl(tcph->seq),
-					ct_irc_info->len, buffer, 
+					exp_irc_info->len, buffer, 
 					strlen(buffer));
 }
 

@@ -165,12 +156,12 @@
 	struct tcphdr *tcph = (void *) iph + iph->ihl * 4;
 	unsigned int datalen;
 	int dir;
-	struct ip_ct_irc_expect *ct_irc_info;
+	struct ip_ct_irc_expect *exp_irc_info;
 
 	if (!exp)
 		DEBUGP("ip_nat_irc: no exp!!");
 		
-	ct_irc_info = &exp->help.exp_irc_info;
+	exp_irc_info = &exp->help.exp_irc_info;
 
 	/* Only mangle things once: original direction in POST_ROUTING
 	   and reply direction on PRE_ROUTING. */

@@ -187,29 +178,24 @@
 	DEBUGP("got beyond not touching\n");
 
 	datalen = (*pskb)->len - iph->ihl * 4 - tcph->doff * 4;
-	LOCK_BH(&ip_irc_lock);
 	/* Check wether the whole IP/address pattern is carried in the payload */
-	if (between(exp->seq + ct_irc_info->len,
+	if (between(exp->seq + exp_irc_info->len,
 		    ntohl(tcph->seq),
 		    ntohl(tcph->seq) + datalen)) {
-		if (!irc_data_fixup(ct_irc_info, ct, pskb, ctinfo, exp)) {
-			UNLOCK_BH(&ip_irc_lock);
+		if (!irc_data_fixup(exp_irc_info, ct, pskb, ctinfo, exp))
 			return NF_DROP;
-		}
 	} else { 
 		/* Half a match?  This means a partial retransmisison.
 		   It's a cracker being funky. */
 		if (net_ratelimit()) {
 			printk
 			    ("IRC_NAT: partial packet %u/%u in %u/%u\n",
-			     exp->seq, ct_irc_info->len,
+			     exp->seq, exp_irc_info->len,
 			     ntohl(tcph->seq),
 			     ntohl(tcph->seq) + datalen);
 		}
-		UNLOCK_BH(&ip_irc_lock);
 		return NF_DROP;
 	}
-	UNLOCK_BH(&ip_irc_lock);
 
 	return NF_ACCEPT;
 }