[PATCH] pptp conntrack broken when non-modular

[PATCH] pptp conntrack broken when non-modular

[Phil Oester]

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

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 <kernel@linuxace.com>



[-- Attachment #2: patch-pptp --]
[-- Type: text/plain, Size: 3175 bytes --]

diff -ru linux-orig/include/linux/netfilter_ipv4/ip_nat_protocol.h linux-po/include/linux/netfilter_ipv4/ip_nat_protocol.h
--- linux-orig/include/linux/netfilter_ipv4/ip_nat_protocol.h	2005-10-27 20:02:08.000000000 -0400
+++ linux-po/include/linux/netfilter_ipv4/ip_nat_protocol.h	2005-12-18 14:06:48.000000000 -0500
@@ -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);
diff -ru linux-orig/net/ipv4/netfilter/ip_nat_core.c linux-po/net/ipv4/netfilter/ip_nat_core.c
--- linux-orig/net/ipv4/netfilter/ip_nat_core.c	2005-12-18 14:15:42.000000000 -0500
+++ linux-po/net/ipv4/netfilter/ip_nat_core.c	2005-12-18 14:44:31.000000000 -0500
@@ -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;
diff -ru linux-orig/net/ipv4/netfilter/ip_nat_helper_pptp.c linux-po/net/ipv4/netfilter/ip_nat_helper_pptp.c
--- linux-orig/net/ipv4/netfilter/ip_nat_helper_pptp.c	2005-12-18 14:15:42.000000000 -0500
+++ linux-po/net/ipv4/netfilter/ip_nat_helper_pptp.c	2005-12-18 14:19:52.000000000 -0500
@@ -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;
diff -ru linux-orig/net/ipv4/netfilter/ip_nat_proto_gre.c linux-po/net/ipv4/netfilter/ip_nat_proto_gre.c
--- linux-orig/net/ipv4/netfilter/ip_nat_proto_gre.c	2005-12-18 14:15:42.000000000 -0500
+++ linux-po/net/ipv4/netfilter/ip_nat_proto_gre.c	2005-12-18 14:21:18.000000000 -0500
@@ -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)

Re: [PATCH] pptp conntrack broken when non-modular

[Patrick McHardy]

Phil Oester wrote:
> 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.

Re: [PATCH] pptp conntrack broken when non-modular

[Patrick McHardy]

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

Patrick McHardy wrote:
> Phil Oester wrote:
> 
>> 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.

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

[-- Attachment #2: x --]
[-- Type: text/plain, Size: 1429 bytes --]

[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 <kaber@trash.net>

---
commit 87704c86b3406255a2b68b1d1a68ff72baa6177e
tree 349f73319574ccb3a02acaefab7cec8edaa798f6
parent afe1ec2b866d310f47db2f368f1f4a7b4961ffed
author Patrick McHardy <kaber@trash.net> Mon, 19 Dec 2005 11:17:20 +0100
committer Patrick McHardy <kaber@trash.net> Mon, 19 Dec 2005 11:17:20 +0100

 net/ipv4/netfilter/Makefile |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/net/ipv4/netfilter/Makefile b/net/ipv4/netfilter/Makefile
index 058c48e..d0a447e 100644
--- a/net/ipv4/netfilter/Makefile
+++ b/net/ipv4/netfilter/Makefile
@@ -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