[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>

6 files changed, 20 insertions, 23 deletions

diff --git a/drivers/s390/net/qeth_main.c b/drivers/s390/net/qeth_main.c
index d5285d105c65..a8e034b156cf 100644
--- 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 f7756e1f93ce..104dd4d19da4 100644
--- 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 733b1cf6c440..a21a326808b4 100644
--- 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 e874232ec54b..d3d6c592a5cb 100644
--- 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 f4e6a4a368ec..41e726ac3337 100644
--- 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 6d23ea909aca..e1f5aeb79258 100644
--- 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;