If we register hooks in the order we do today we can have packets passing through the PREROUTING hook before we register the POSTROUTING hook. This can cause unexpected behavior so change it to register hooks in reverse order and leave unregistration as is in forward order. This inverts the behaviour, now we can have packets pass through POSTROUTING without having passed through PREROUTING. This is also a problem but is a lot easier to detect since previous hook functions usually do something that can be detected. This patch also replicates some code between hook functions in order to bail out when previous hook functions hasn't been called. Ipv6 still needs fixing, some code to replicate, mostly the detection of fragmented packets. Signed-off-by: Martin Josefsson --- net/ipv4/netfilter/nf_conntrack_l3proto_ipv4.c | 17 ++++++++++++++++ net/ipv6/netfilter/nf_conntrack_l3proto_ipv6.c | 8 +++---- net/netfilter/core.c | 26 +++++++++++++++++-------- 3 files changed, 39 insertions(+), 12 deletions(-) Index: linux-2.6.17-git22.quilt/net/ipv4/netfilter/nf_conntrack_l3proto_ipv4.c =================================================================== --- linux-2.6.17-git22.quilt.orig/net/ipv4/netfilter/nf_conntrack_l3proto_ipv4.c 2006-08-10 23:48:34.000000000 +0200 +++ linux-2.6.17-git22.quilt/net/ipv4/netfilter/nf_conntrack_l3proto_ipv4.c 2006-08-11 00:08:20.000000000 +0200 @@ -228,8 +228,17 @@ static unsigned int ipv4_conntrack_in(un /* Prevent grace periods during execution of this hook function */ rcu_read_lock(); + /* We shouldn't see fragments here, but it can happen in the middle of + hook registration/unregistration, when ipv4_conntrack_defrag() isn't + registered yet or has just been unregistered. */ + if (unlikely((*pskb)->nh.iph->frag_off & htons(IP_MF|IP_OFFSET))) { + ret = NF_ACCEPT; + goto out; + } + ret = nf_conntrack_in(PF_INET, hooknum, pskb); +out: rcu_read_unlock(); return ret; @@ -255,6 +264,14 @@ static unsigned int ipv4_conntrack_local goto out; } + /* We shouldn't see fragments here, but it can happen in the middle of + hook registration/unregistration, when ipv4_conntrack_defrag() isn't + registered yet or has just been unregistered. */ + if (unlikely((*pskb)->nh.iph->frag_off & htons(IP_MF|IP_OFFSET))) { + ret = NF_ACCEPT; + goto out; + } + ret = nf_conntrack_in(PF_INET, hooknum, pskb); out: Index: linux-2.6.17-git22.quilt/net/ipv6/netfilter/nf_conntrack_l3proto_ipv6.c =================================================================== --- linux-2.6.17-git22.quilt.orig/net/ipv6/netfilter/nf_conntrack_l3proto_ipv6.c 2006-08-10 23:48:34.000000000 +0200 +++ linux-2.6.17-git22.quilt/net/ipv6/netfilter/nf_conntrack_l3proto_ipv6.c 2006-08-11 00:08:20.000000000 +0200 @@ -346,18 +346,18 @@ static struct nf_hook_ops ipv6_conntrack .priority = NF_IP6_PRI_CONNTRACK, }, { - .hook = ipv6_conntrack_local, + .hook = ipv6_defrag, .owner = THIS_MODULE, .pf = PF_INET6, .hooknum = NF_IP6_LOCAL_OUT, - .priority = NF_IP6_PRI_CONNTRACK, + .priority = NF_IP6_PRI_CONNTRACK_DEFRAG, }, { - .hook = ipv6_defrag, + .hook = ipv6_conntrack_local, .owner = THIS_MODULE, .pf = PF_INET6, .hooknum = NF_IP6_LOCAL_OUT, - .priority = NF_IP6_PRI_CONNTRACK_DEFRAG, + .priority = NF_IP6_PRI_CONNTRACK, }, { .hook = ipv6_confirm, Index: linux-2.6.17-git22.quilt/net/netfilter/core.c =================================================================== --- linux-2.6.17-git22.quilt.orig/net/netfilter/core.c 2006-08-10 23:48:34.000000000 +0200 +++ linux-2.6.17-git22.quilt/net/netfilter/core.c 2006-08-11 00:13:22.000000000 +0200 @@ -81,16 +81,29 @@ void nf_unregister_hook(struct nf_hook_o list_del_rcu(®->list); spin_unlock_bh(&nf_hook_lock); + /* This isn't enough if running with the -rt patchset */ synchronize_net(); } EXPORT_SYMBOL(nf_unregister_hook); -int nf_register_hooks(struct nf_hook_ops *reg, unsigned int n) +static void __nf_unregister_hooks(struct nf_hook_ops *reg, unsigned int m, + unsigned int n) { unsigned int i; + + for (i = m; i < n; i++) + nf_unregister_hook(®[i]); +} + +int nf_register_hooks(struct nf_hook_ops *reg, unsigned int n) +{ + int i; int err = 0; - for (i = 0; i < n; i++) { + /* Register in reverse order assuming that the hooks are ordered in + the sequence they are going to be used. This is needed for + inter hook dependencies */ + for (i = n - 1; i >= 0; i--) { err = nf_register_hook(®[i]); if (err) goto err; @@ -98,18 +111,15 @@ int nf_register_hooks(struct nf_hook_ops return err; err: - if (i > 0) - nf_unregister_hooks(reg, i); + if (i < n - 1) + __nf_unregister_hooks(reg, i + 1, n); return err; } EXPORT_SYMBOL(nf_register_hooks); void nf_unregister_hooks(struct nf_hook_ops *reg, unsigned int n) { - unsigned int i; - - for (i = 0; i < n; i++) - nf_unregister_hook(®[i]); + __nf_unregister_hooks(reg, 0, n); } EXPORT_SYMBOL(nf_unregister_hooks);