netfilter: Fix slab corruption.

Use the correct pattern for singly linked list insertion and
deletion.  We can also calculate the list head outside of the
mutex.

Fixes: e3b37f11e6e4 ("netfilter: replace list_head with single linked list")
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
Reviewed-by: Aaron Conole <aconole@bytheb.org>
Signed-off-by: David S. Miller <davem@davemloft.net>
net/netfilter/core.c | 108 ++++++++++++++++-----------------------------------
 1 file changed, 33 insertions(+), 75 deletions(-)

diff --git a/net/netfilter/core.c b/net/netfilter/core.c
index c9d90eb..fcb5d1d 100644 (file)
--- a/net/netfilter/core.c
+++ b/net/netfilter/core.c
@@ -65,49 +65,24 @@ static DEFINE_MUTEX(nf_hook_mutex);
 #define nf_entry_dereference(e) \
        rcu_dereference_protected(e, lockdep_is_held(&nf_hook_mutex))
 
-static struct nf_hook_entry *nf_hook_entry_head(struct net *net,
-                                               const struct nf_hook_ops *reg)
+static struct nf_hook_entry __rcu **nf_hook_entry_head(struct net *net, const struct nf_hook_ops *reg)
 {
-       struct nf_hook_entry *hook_head = NULL;
-
        if (reg->pf != NFPROTO_NETDEV)
-               hook_head = nf_entry_dereference(net->nf.hooks[reg->pf]
-                                                [reg->hooknum]);
-       else if (reg->hooknum == NF_NETDEV_INGRESS) {
+               return net->nf.hooks[reg->pf]+reg->hooknum;
+
 #ifdef CONFIG_NETFILTER_INGRESS
+       if (reg->hooknum == NF_NETDEV_INGRESS) {
                if (reg->dev && dev_net(reg->dev) == net)
-                       hook_head =
-                               nf_entry_dereference(
-                                       reg->dev->nf_hooks_ingress);
-#endif
+                       return &reg->dev->nf_hooks_ingress;
        }
-       return hook_head;
-}
-
-/* must hold nf_hook_mutex */
-static void nf_set_hooks_head(struct net *net, const struct nf_hook_ops *reg,
-                             struct nf_hook_entry *entry)
-{
-       switch (reg->pf) {
-       case NFPROTO_NETDEV:
-#ifdef CONFIG_NETFILTER_INGRESS
-               /* We already checked in nf_register_net_hook() that this is
-                * used from ingress.
-                */
-               rcu_assign_pointer(reg->dev->nf_hooks_ingress, entry);
 #endif
-               break;
-       default:
-               rcu_assign_pointer(net->nf.hooks[reg->pf][reg->hooknum],
-                                  entry);
-               break;
-       }
+       return NULL;
 }
 
 int nf_register_net_hook(struct net *net, const struct nf_hook_ops *reg)
 {
-       struct nf_hook_entry *hooks_entry;
-       struct nf_hook_entry *entry;
+       struct nf_hook_entry __rcu **pp;
+       struct nf_hook_entry *entry, *p;
 
        if (reg->pf == NFPROTO_NETDEV) {
 #ifndef CONFIG_NETFILTER_INGRESS
@@ -119,6 +94,10 @@ int nf_register_net_hook(struct net *net, const struct nf_hook_ops *reg)
                        return -EINVAL;
        }
 
+       pp = nf_hook_entry_head(net, reg);
+       if (!pp)
+               return -EINVAL;
+
        entry = kmalloc(sizeof(*entry), GFP_KERNEL);
        if (!entry)
                return -ENOMEM;
@@ -128,26 +107,15 @@ int nf_register_net_hook(struct net *net, const struct nf_hook_ops *reg)
        entry->next     = NULL;
 
        mutex_lock(&nf_hook_mutex);
-       hooks_entry = nf_hook_entry_head(net, reg);
-
-       if (hooks_entry && hooks_entry->orig_ops->priority > reg->priority) {
-               /* This is the case where we need to insert at the head */
-               entry->next = hooks_entry;
-               hooks_entry = NULL;
-       }
-
-       while (hooks_entry &&
-               reg->priority >= hooks_entry->orig_ops->priority &&
-               nf_entry_dereference(hooks_entry->next)) {
-               hooks_entry = nf_entry_dereference(hooks_entry->next);
-       }
 
-       if (hooks_entry) {
-               entry->next = nf_entry_dereference(hooks_entry->next);
-               rcu_assign_pointer(hooks_entry->next, entry);
-       } else {
-               nf_set_hooks_head(net, reg, entry);
+       /* Find the spot in the list */
+       while ((p = nf_entry_dereference(*pp)) != NULL) {
+               if (reg->priority < p->orig_ops->priority)
+                       break;
+               pp = &p->next;
        }
+       rcu_assign_pointer(entry->next, p);
+       rcu_assign_pointer(*pp, entry);
 
        mutex_unlock(&nf_hook_mutex);
 #ifdef CONFIG_NETFILTER_INGRESS
@@ -163,33 +131,23 @@ EXPORT_SYMBOL(nf_register_net_hook);
 
 void nf_unregister_net_hook(struct net *net, const struct nf_hook_ops *reg)
 {
-       struct nf_hook_entry *hooks_entry;
+       struct nf_hook_entry __rcu **pp;
+       struct nf_hook_entry *p;
 
-       mutex_lock(&nf_hook_mutex);
-       hooks_entry = nf_hook_entry_head(net, reg);
-       if (hooks_entry && hooks_entry->orig_ops == reg) {
-               nf_set_hooks_head(net, reg,
-                                 nf_entry_dereference(hooks_entry->next));
-               goto unlock;
-       }
-       while (hooks_entry && nf_entry_dereference(hooks_entry->next)) {
-               struct nf_hook_entry *next =
-                       nf_entry_dereference(hooks_entry->next);
-               struct nf_hook_entry *nnext;
+       pp = nf_hook_entry_head(net, reg);
+       if (WARN_ON_ONCE(!pp))
+               return;
 
-               if (next->orig_ops != reg) {
-                       hooks_entry = next;
-                       continue;
+       mutex_lock(&nf_hook_mutex);
+       while ((p = nf_entry_dereference(*pp)) != NULL) {
+               if (p->orig_ops == reg) {
+                       rcu_assign_pointer(*pp, p->next);
+                       break;
                }
-               nnext = nf_entry_dereference(next->next);
-               rcu_assign_pointer(hooks_entry->next, nnext);
-               hooks_entry = next;
-               break;
+               pp = &p->next;
        }
-
-unlock:
        mutex_unlock(&nf_hook_mutex);
-       if (!hooks_entry) {
+       if (!p) {
                WARN(1, "nf_unregister_net_hook: hook not found!\n");
                return;
        }
@@ -201,10 +159,10 @@ unlock:
        static_key_slow_dec(&nf_hooks_needed[reg->pf][reg->hooknum]);
 #endif
        synchronize_net();
-       nf_queue_nf_hook_drop(net, hooks_entry);
+       nf_queue_nf_hook_drop(net, p);
        /* other cpu might still process nfqueue verdict that used reg */
        synchronize_net();
-       kfree(hooks_entry);
+       kfree(p);
 }
 EXPORT_SYMBOL(nf_unregister_net_hook);