Skip to contentSkip to conversation

pptp conntrack broken when non-modular

PATCH In review Phil Oester 2 participants 3 messages Dec 18, 2005 – Dec 19, 2005

Subsystems (2)
Networking netfilter
lore
Share & tools
lore

Phil Oester

author
#0 lore raw

PATCH 1/1 pptp conntrack broken when non-modular

The GRE protocol helper of PPTP does not get properly registered when it is built in, because ip_nat_proto_gre_init runs prior to ip_nat_init, so ip_nat_protos is unitialized when ip_nat_proto_gre_init tries to register protocol 47.

Changing ip_nat_protocol_register to unconditionally register solves half the problem. But then when ip_nat_init does run, it overwrites the registration with ip_nat_unknown_protocol. So the second part of the fix is to change ip_nat_init not to overwrite previously registered protos.

This fixes netfilter bugzilla #397, and IMHO should go to mainline ASAP to hopefully make 2.6.15.

Phil

Signed-off-by Phil Oester <[email protected]>
include/linux/netfilter_ipv4/ip_nat_protocol.h +1 -1
diff -ru linux-orig/include/linux/netfilter_ipv4/ip_nat_protocol.h linux-po/include/linux/netfilter_ipv4/ip_nat_protocol.h
@@ -57,7 +57,7 @@
 };
 
 /* Protocol registration. */
-extern int ip_nat_protocol_register(struct ip_nat_protocol *proto);
+extern void ip_nat_protocol_register(struct ip_nat_protocol *proto);
 extern void ip_nat_protocol_unregister(struct ip_nat_protocol *proto);
 
 extern struct ip_nat_protocol *ip_nat_proto_find_get(u_int8_t protocol);
net/ipv4/netfilter/ip_nat_core.c +4 -10
diff -ru linux-orig/net/ipv4/netfilter/ip_nat_core.c linux-po/net/ipv4/netfilter/ip_nat_core.c
@@ -516,19 +516,11 @@
 EXPORT_SYMBOL_GPL(ip_nat_icmp_reply_translation);
 
 /* Protocol registration. */
-int ip_nat_protocol_register(struct ip_nat_protocol *proto)
+void ip_nat_protocol_register(struct ip_nat_protocol *proto)
 {
-	int ret = 0;
-
 	write_lock_bh(&ip_nat_lock);
-	if (ip_nat_protos[proto->protonum] != &ip_nat_unknown_protocol) {
-		ret = -EBUSY;
-		goto out;
-	}
 	ip_nat_protos[proto->protonum] = proto;
- out:
 	write_unlock_bh(&ip_nat_lock);
-	return ret;
 }
 EXPORT_SYMBOL(ip_nat_protocol_register);
 

@@ -604,7 +596,9 @@
 	/* Sew in builtin protocols. */
 	write_lock_bh(&ip_nat_lock);
 	for (i = 0; i < MAX_IP_NAT_PROTO; i++)
-		ip_nat_protos[i] = &ip_nat_unknown_protocol;
+		/* Don't overwrite protos already registered */
+		if (!ip_nat_protos[i])
+			ip_nat_protos[i] = &ip_nat_unknown_protocol;
 	ip_nat_protos[IPPROTO_TCP] = &ip_nat_protocol_tcp;
 	ip_nat_protos[IPPROTO_UDP] = &ip_nat_protocol_udp;
 	ip_nat_protos[IPPROTO_ICMP] = &ip_nat_protocol_icmp;
net/ipv4/netfilter/ip_nat_helper_pptp.c +2 -6
diff -ru linux-orig/net/ipv4/netfilter/ip_nat_helper_pptp.c linux-po/net/ipv4/netfilter/ip_nat_helper_pptp.c
@@ -378,18 +378,14 @@
 }
 
 
-extern int __init ip_nat_proto_gre_init(void);
+extern void __init ip_nat_proto_gre_init(void);
 extern void __exit ip_nat_proto_gre_fini(void);
 
 static int __init init(void)
 {
-	int ret;
-
 	DEBUGP("%s: registering NAT helper\n", __FILE__);
 
-	ret = ip_nat_proto_gre_init();
-	if (ret < 0)
-		return ret;
+	ip_nat_proto_gre_init();
 
 	BUG_ON(ip_nat_pptp_hook_outbound);
 	ip_nat_pptp_hook_outbound = &pptp_outbound_pkt;
net/ipv4/netfilter/ip_nat_proto_gre.c +2 -2
diff -ru linux-orig/net/ipv4/netfilter/ip_nat_proto_gre.c linux-po/net/ipv4/netfilter/ip_nat_proto_gre.c
@@ -203,9 +203,9 @@
 #endif
 };
 				  
-int __init ip_nat_proto_gre_init(void)
+void __init ip_nat_proto_gre_init(void)
 {
-	return ip_nat_protocol_register(&gre);
+	ip_nat_protocol_register(&gre);
 }
 
 void __exit ip_nat_proto_gre_fini(void)
#1 lore raw
Quoted 10 lines · Phil Oester

The GRE protocol helper of PPTP does not get properly registered when it is built in, because ip_nat_proto_gre_init runs prior to ip_nat_init, so ip_nat_protos is unitialized when ip_nat_proto_gre_init tries to register protocol 47.

Changing ip_nat_protocol_register to unconditionally register solves half the problem. But then when ip_nat_init does run, it overwrites the registration with ip_nat_unknown_protocol. So the second part of the fix is to change ip_nat_init not to overwrite previously registered protos.

I think the correct fix is to change the initialisation order so the NAT core comes before protocol helpers. I'm going to look into this.

#2 lore raw
Quoted 14 lines · Patrick McHardy
Quoted 10 lines · Phil Oester

The GRE protocol helper of PPTP does not get properly registered when it is built in, because ip_nat_proto_gre_init runs prior to ip_nat_init, so ip_nat_protos is unitialized when ip_nat_proto_gre_init tries to register protocol 47.

Changing ip_nat_protocol_register to unconditionally register solves half the problem. But then when ip_nat_init does run, it overwrites the registration with ip_nat_unknown_protocol. So the second part of the fix is to change ip_nat_init not to overwrite previously registered protos.

Quoted 3 lines

I think the correct fix is to change the initialisation order so the NAT core comes before protocol helpers. I'm going to look into this.

This patch fixes the problem by changing the init order. I'll try to get it in 2.6.15.

[NETFILTER]: Fix NAT init order

As noticed by Phil Oester, the GRE NAT protocol helper is initialized before the NAT core, which makes registration fail.

Change the linking order to make NAT be initialized first.

Signed-off-by Patrick McHardy <[email protected]>

---
commit 87704c86b3406255a2b68b1d1a68ff72baa6177e
tree 349f73319574ccb3a02acaefab7cec8edaa798f6
parent afe1ec2b866d310f47db2f368f1f4a7b4961ffed
author Patrick McHardy <[email protected]> Mon, 19 Dec 2005 11:17:20 +0100 committer Patrick McHardy <[email protected]> Mon, 19 Dec 2005 11:17:20 +0100

 net/ipv4/netfilter/Makefile |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)
net/ipv4/netfilter/Makefile +2 -1
index 058c48e..d0a447e 100644
@@ -12,6 +12,7 @@ ip_nat_pptp-objs	:= ip_nat_helper_pptp.o
 
 # connection tracking
 obj-$(CONFIG_IP_NF_CONNTRACK) += ip_conntrack.o
+obj-$(CONFIG_IP_NF_NAT) += ip_nat.o
 
 # conntrack netlink interface
 obj-$(CONFIG_IP_NF_CONNTRACK_NETLINK) += ip_conntrack_netlink.o

@@ -41,7 +42,7 @@ obj-$(CONFIG_IP_NF_IPTABLES) += ip_table
 # the three instances of ip_tables
 obj-$(CONFIG_IP_NF_FILTER) += iptable_filter.o
 obj-$(CONFIG_IP_NF_MANGLE) += iptable_mangle.o
-obj-$(CONFIG_IP_NF_NAT) += iptable_nat.o ip_nat.o
+obj-$(CONFIG_IP_NF_NAT) += iptable_nat.o
 obj-$(CONFIG_IP_NF_RAW) += iptable_raw.o
 
 # matches