[NET]: Fully plug netigh_create/inetdev_destroy race.

So here is a patch to make sure that there is a barrier between the
reading of dev->*_ptr and *dev->neigh_parms.

With these barriers in place, it's clear that *dev->neigh_parms can no
longer be NULL since once the parms are allocated, that pointer is never
reset to NULL again.  Therefore I've also removed the parms check in
these paths.

They were bogus to begin with since if they ever triggered then we'll
have dead neigh entries stuck in the hash table.

Unfortunately I couldn't arrange for this to happen with DECnet due
to the dn_db->parms.up() call that's sandwiched between the assignment
of dev->dn_ptr and dn_db->neigh_parms.  So I've kept the parms check
there but it will now fail instead of continuing.  I've also added an
smp_wmb() there so that at least we won't be reading garbage from
dn_db->neigh_parms.

DECnet is also buggy since there is no locking at all in the destruction
path.  It either needs locking or RCU like IPv4.

Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
Signed-off-by: David S. Miller <davem@davemloft.net>

diff --git a/drivers/s390/net/qeth_main.c b/drivers/s390/net/qeth_main.c
index d5285d105c650598beb1b4485699124f7c134ab3..a8e034b156cfdd6c265b232d44ea4c3cb715339c 100644 (file)
--- a/drivers/s390/net/qeth_main.c
+++ b/drivers/s390/net/qeth_main.c
@@ -6718,17 +6718,15 @@ qeth_arp_constructor(struct neighbour *neigh)
        }
 
        rcu_read_lock();
-       in_dev = __in_dev_get(dev);
+       in_dev = rcu_dereference(__in_dev_get(dev));
        if (in_dev == NULL) {
                rcu_read_unlock();
                return -EINVAL;
        }
 
        parms = in_dev->arp_parms;
-       if (parms) {
-               __neigh_parms_put(neigh->parms);
-               neigh->parms = neigh_parms_clone(parms);
-       }
+       __neigh_parms_put(neigh->parms);
+       neigh->parms = neigh_parms_clone(parms);
        rcu_read_unlock();
 
        neigh->type = inet_addr_type(*(u32 *) neigh->primary_key);

diff --git a/net/atm/clip.c b/net/atm/clip.c
index f7756e1f93ce943266a516c95801fc0e05a1afae..104dd4d19da496ca36065679d44542c5d0093902 100644 (file)
--- a/net/atm/clip.c
+++ b/net/atm/clip.c
@@ -320,17 +320,15 @@ static int clip_constructor(struct neighbour *neigh)
        if (neigh->type != RTN_UNICAST) return -EINVAL;
 
        rcu_read_lock();
-       in_dev = __in_dev_get(dev);
+       in_dev = rcu_dereference(__in_dev_get(dev));
        if (!in_dev) {
                rcu_read_unlock();
                return -EINVAL;
        }
 
        parms = in_dev->arp_parms;
-       if (parms) {
-               __neigh_parms_put(neigh->parms);
-               neigh->parms = neigh_parms_clone(parms);
-       }
+       __neigh_parms_put(neigh->parms);
+       neigh->parms = neigh_parms_clone(parms);
        rcu_read_unlock();
 
        neigh->ops = &clip_neigh_ops;

diff --git a/net/decnet/dn_dev.c b/net/decnet/dn_dev.c
index 733b1cf6c4408d3a2e6a23c979d434a9ac72423d..a21a326808b45865c3b368beff027009a005fbfa 100644 (file)
--- a/net/decnet/dn_dev.c
+++ b/net/decnet/dn_dev.c
@@ -41,6 +41,7 @@
 #include <linux/sysctl.h>
 #include <linux/notifier.h>
 #include <asm/uaccess.h>
+#include <asm/system.h>
 #include <net/neighbour.h>
 #include <net/dst.h>
 #include <net/flow.h>
@@ -1108,6 +1109,7 @@ struct dn_dev *dn_dev_create(struct net_device *dev, int *err)
 
        memset(dn_db, 0, sizeof(struct dn_dev));
        memcpy(&dn_db->parms, p, sizeof(struct dn_dev_parms));
+       smp_wmb();
        dev->dn_ptr = dn_db;
        dn_db->dev = dev;
        init_timer(&dn_db->timer);

diff --git a/net/decnet/dn_neigh.c b/net/decnet/dn_neigh.c
index e874232ec54bb0dc24fc89444b4a14977295749f..d3d6c592a5cbef0edb48b06f11cdd2d2ab546113 100644 (file)
--- a/net/decnet/dn_neigh.c
+++ b/net/decnet/dn_neigh.c
@@ -139,17 +139,20 @@ static int dn_neigh_construct(struct neighbour *neigh)
        struct neigh_parms *parms;
 
        rcu_read_lock();
-       dn_db = dev->dn_ptr;
+       dn_db = rcu_dereference(dev->dn_ptr);
        if (dn_db == NULL) {
                rcu_read_unlock();
                return -EINVAL;
        }
 
        parms = dn_db->neigh_parms;
-       if (parms) {
-               __neigh_parms_put(neigh->parms);
-               neigh->parms = neigh_parms_clone(parms);
+       if (!parms) {
+               rcu_read_unlock();
+               return -EINVAL;
        }
+
+       __neigh_parms_put(neigh->parms);
+       neigh->parms = neigh_parms_clone(parms);
        rcu_read_unlock();
 
        if (dn_db->use_long)

diff --git a/net/ipv4/arp.c b/net/ipv4/arp.c
index f4e6a4a368ec78935f9a73f779a196da0e9952d4..41e726ac3337d1432890057db6b0173f20813fbf 100644 (file)
--- a/net/ipv4/arp.c
+++ b/net/ipv4/arp.c
@@ -244,17 +244,15 @@ static int arp_constructor(struct neighbour *neigh)
        neigh->type = inet_addr_type(addr);
 
        rcu_read_lock();
-       in_dev = __in_dev_get(dev);
+       in_dev = rcu_dereference(__in_dev_get(dev));
        if (in_dev == NULL) {
                rcu_read_unlock();
                return -EINVAL;
        }
 
        parms = in_dev->arp_parms;
-       if (parms) {
-               __neigh_parms_put(neigh->parms);
-               neigh->parms = neigh_parms_clone(parms);
-       }
+       __neigh_parms_put(neigh->parms);
+       neigh->parms = neigh_parms_clone(parms);
        rcu_read_unlock();
 
        if (dev->hard_header == NULL) {

diff --git a/net/ipv6/ndisc.c b/net/ipv6/ndisc.c
index 6d23ea909aca2236dda3dd90907d5a1ddebc0e06..e1f5aeb792588e535d5ea6b00f74ad1edf91b29f 100644 (file)
--- a/net/ipv6/ndisc.c
+++ b/net/ipv6/ndisc.c
@@ -297,10 +297,8 @@ static int ndisc_constructor(struct neighbour *neigh)
        }
 
        parms = in6_dev->nd_parms;
-       if (parms) {
-               __neigh_parms_put(neigh->parms);
-               neigh->parms = neigh_parms_clone(parms);
-       }
+       __neigh_parms_put(neigh->parms);
+       neigh->parms = neigh_parms_clone(parms);
        rcu_read_unlock();
 
        neigh->type = is_multicast ? RTN_MULTICAST : RTN_UNICAST;