netfilter: nat: don't register device notifier twice
authorFlorian Westphal <fw@strlen.de>
Wed, 6 Mar 2019 11:44:17 +0000 (12:44 +0100)
committerPablo Neira Ayuso <pablo@netfilter.org>
Fri, 8 Mar 2019 15:41:09 +0000 (16:41 +0100)
Otherwise, we get notifier list corruption.

This is the most simple fix: remove the device notifier call chain
from the ipv6 masquerade register function and handle it only
in the ipv4 version.

The better fix is merge
nf_nat_masquerade_ipv4/6_(un)register_notifier
  into a single
nf_nat_masquerade_(un)register_notifiers

but to do this its needed to first merge the two masquerade modules
into a single xt_MASQUERADE.

Furthermore, we need to use different refcounts for ipv4/ipv6
until we can merge MASQUERADE.

Fixes: d1aca8ab3104a ("netfilter: nat: merge ipv4 and ipv6 masquerade functionality")
Signed-off-by: Florian Westphal <fw@strlen.de>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
net/netfilter/nf_nat_masquerade.c

index 86fa4dcc63c555f3a5ed8a71dc78d5e47df27fb9..d85c4d902e7b1deca67975abdf13b3bf8326351f 100644 (file)
@@ -11,7 +11,8 @@
 #include <net/netfilter/ipv6/nf_nat_masquerade.h>
 
 static DEFINE_MUTEX(masq_mutex);
-static unsigned int masq_refcnt __read_mostly;
+static unsigned int masq_refcnt4 __read_mostly;
+static unsigned int masq_refcnt6 __read_mostly;
 
 unsigned int
 nf_nat_masquerade_ipv4(struct sk_buff *skb, unsigned int hooknum,
@@ -141,8 +142,13 @@ int nf_nat_masquerade_ipv4_register_notifier(void)
        int ret = 0;
 
        mutex_lock(&masq_mutex);
+       if (WARN_ON_ONCE(masq_refcnt4 == UINT_MAX)) {
+               ret = -EOVERFLOW;
+               goto out_unlock;
+       }
+
        /* check if the notifier was already set */
-       if (++masq_refcnt > 1)
+       if (++masq_refcnt4 > 1)
                goto out_unlock;
 
        /* Register for device down reports */
@@ -160,7 +166,7 @@ int nf_nat_masquerade_ipv4_register_notifier(void)
 err_unregister:
        unregister_netdevice_notifier(&masq_dev_notifier);
 err_dec:
-       masq_refcnt--;
+       masq_refcnt4--;
 out_unlock:
        mutex_unlock(&masq_mutex);
        return ret;
@@ -171,7 +177,7 @@ void nf_nat_masquerade_ipv4_unregister_notifier(void)
 {
        mutex_lock(&masq_mutex);
        /* check if the notifier still has clients */
-       if (--masq_refcnt > 0)
+       if (--masq_refcnt4 > 0)
                goto out_unlock;
 
        unregister_netdevice_notifier(&masq_dev_notifier);
@@ -321,25 +327,23 @@ int nf_nat_masquerade_ipv6_register_notifier(void)
        int ret = 0;
 
        mutex_lock(&masq_mutex);
-       /* check if the notifier is already set */
-       if (++masq_refcnt > 1)
+       if (WARN_ON_ONCE(masq_refcnt6 == UINT_MAX)) {
+               ret = -EOVERFLOW;
                goto out_unlock;
+       }
 
-       ret = register_netdevice_notifier(&masq_dev_notifier);
-       if (ret)
-               goto err_dec;
+       /* check if the notifier is already set */
+       if (++masq_refcnt6 > 1)
+               goto out_unlock;
 
        ret = register_inet6addr_notifier(&masq_inet6_notifier);
        if (ret)
-               goto err_unregister;
+               goto err_dec;
 
        mutex_unlock(&masq_mutex);
        return ret;
-
-err_unregister:
-       unregister_netdevice_notifier(&masq_dev_notifier);
 err_dec:
-       masq_refcnt--;
+       masq_refcnt6--;
 out_unlock:
        mutex_unlock(&masq_mutex);
        return ret;
@@ -350,11 +354,10 @@ void nf_nat_masquerade_ipv6_unregister_notifier(void)
 {
        mutex_lock(&masq_mutex);
        /* check if the notifier still has clients */
-       if (--masq_refcnt > 0)
+       if (--masq_refcnt6 > 0)
                goto out_unlock;
 
        unregister_inet6addr_notifier(&masq_inet6_notifier);
-       unregister_netdevice_notifier(&masq_dev_notifier);
 out_unlock:
        mutex_unlock(&masq_mutex);
 }