netfilter: conntrack: fix possible bug_on with enable_hooks=1

[ Upstream commit e72eeab542dbf4f544e389e64fa13b82a1b6d003 ]

I received a bug report (no reproducer so far) where we trip over

712         rcu_read_lock();
713         ct_hook = rcu_dereference(nf_ct_hook);
714         BUG_ON(ct_hook == NULL);  // here

In nf_conntrack_destroy().

First turn this BUG_ON into a WARN.  I think it was triggered
via enable_hooks=1 flag.

When this flag is turned on, the conntrack hooks are registered
before nf_ct_hook pointer gets assigned.
This opens a short window where packets enter the conntrack machinery,
can have skb->_nfct set up and a subsequent kfree_skb might occur
before nf_ct_hook is set.

Call nf_conntrack_init_end() to set nf_ct_hook before we register the
pernet ops.

Fixes: ae08d40e69a0 ("netfilter: nf_conntrack: provide modparam to always register conntrack hooks")
Signed-off-by: Florian Westphal <fw@strlen.de>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
Signed-off-by: Sasha Levin <sashal@kernel.org>

diff --git a/net/netfilter/core.c b/net/netfilter/core.c
index 451b2df998ea756d5ff988f29e125d5ca75b9f66..c35f45afd394d3a2e6ff8f48dc3fad66325ab5d0 100644 (file)
--- a/net/netfilter/core.c
+++ b/net/netfilter/core.c
@@ -577,9 +577,11 @@ void nf_conntrack_destroy(struct nf_conntrack *nfct)
 
        rcu_read_lock();
        ct_hook = rcu_dereference(nf_ct_hook);
-       BUG_ON(ct_hook == NULL);
-       ct_hook->destroy(nfct);
+       if (ct_hook)
+               ct_hook->destroy(nfct);
        rcu_read_unlock();
+
+       WARN_ON(!ct_hook);
 }
 EXPORT_SYMBOL(nf_conntrack_destroy);
 

diff --git a/net/netfilter/nf_conntrack_standalone.c b/net/netfilter/nf_conntrack_standalone.c
index 43c3c3be6defc439872800c5090359c6195434f5..1e3dbed9d7840f9bb0905ea14daddfa0a51244e2 100644 (file)
--- a/net/netfilter/nf_conntrack_standalone.c
+++ b/net/netfilter/nf_conntrack_standalone.c
@@ -1180,11 +1180,12 @@ static int __init nf_conntrack_standalone_init(void)
        nf_conntrack_htable_size_user = nf_conntrack_htable_size;
 #endif
 
+       nf_conntrack_init_end();
+
        ret = register_pernet_subsys(&nf_conntrack_net_ops);
        if (ret < 0)
                goto out_pernet;
 
-       nf_conntrack_init_end();
        return 0;
 
 out_pernet: