Commit cb402627 authored by Herbert Xu's avatar Herbert Xu Committed by Patrick McHardy

[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: default avatarHerbert Xu <herbert@gondor.apana.org.au>
Signed-off-by: default avatarDavid S. Miller <davem@davemloft.net>
parent 0c5b8d8a
...@@ -6718,17 +6718,15 @@ qeth_arp_constructor(struct neighbour *neigh) ...@@ -6718,17 +6718,15 @@ qeth_arp_constructor(struct neighbour *neigh)
} }
rcu_read_lock(); rcu_read_lock();
in_dev = __in_dev_get(dev); in_dev = rcu_dereference(__in_dev_get(dev));
if (in_dev == NULL) { if (in_dev == NULL) {
rcu_read_unlock(); rcu_read_unlock();
return -EINVAL; return -EINVAL;
} }
parms = in_dev->arp_parms; parms = in_dev->arp_parms;
if (parms) {
__neigh_parms_put(neigh->parms); __neigh_parms_put(neigh->parms);
neigh->parms = neigh_parms_clone(parms); neigh->parms = neigh_parms_clone(parms);
}
rcu_read_unlock(); rcu_read_unlock();
neigh->type = inet_addr_type(*(u32 *) neigh->primary_key); neigh->type = inet_addr_type(*(u32 *) neigh->primary_key);
......
...@@ -320,17 +320,15 @@ static int clip_constructor(struct neighbour *neigh) ...@@ -320,17 +320,15 @@ static int clip_constructor(struct neighbour *neigh)
if (neigh->type != RTN_UNICAST) return -EINVAL; if (neigh->type != RTN_UNICAST) return -EINVAL;
rcu_read_lock(); rcu_read_lock();
in_dev = __in_dev_get(dev); in_dev = rcu_dereference(__in_dev_get(dev));
if (!in_dev) { if (!in_dev) {
rcu_read_unlock(); rcu_read_unlock();
return -EINVAL; return -EINVAL;
} }
parms = in_dev->arp_parms; parms = in_dev->arp_parms;
if (parms) {
__neigh_parms_put(neigh->parms); __neigh_parms_put(neigh->parms);
neigh->parms = neigh_parms_clone(parms); neigh->parms = neigh_parms_clone(parms);
}
rcu_read_unlock(); rcu_read_unlock();
neigh->ops = &clip_neigh_ops; neigh->ops = &clip_neigh_ops;
......
...@@ -41,6 +41,7 @@ ...@@ -41,6 +41,7 @@
#include <linux/sysctl.h> #include <linux/sysctl.h>
#include <linux/notifier.h> #include <linux/notifier.h>
#include <asm/uaccess.h> #include <asm/uaccess.h>
#include <asm/system.h>
#include <net/neighbour.h> #include <net/neighbour.h>
#include <net/dst.h> #include <net/dst.h>
#include <net/flow.h> #include <net/flow.h>
...@@ -1108,6 +1109,7 @@ struct dn_dev *dn_dev_create(struct net_device *dev, int *err) ...@@ -1108,6 +1109,7 @@ struct dn_dev *dn_dev_create(struct net_device *dev, int *err)
memset(dn_db, 0, sizeof(struct dn_dev)); memset(dn_db, 0, sizeof(struct dn_dev));
memcpy(&dn_db->parms, p, sizeof(struct dn_dev_parms)); memcpy(&dn_db->parms, p, sizeof(struct dn_dev_parms));
smp_wmb();
dev->dn_ptr = dn_db; dev->dn_ptr = dn_db;
dn_db->dev = dev; dn_db->dev = dev;
init_timer(&dn_db->timer); init_timer(&dn_db->timer);
......
...@@ -139,17 +139,20 @@ static int dn_neigh_construct(struct neighbour *neigh) ...@@ -139,17 +139,20 @@ static int dn_neigh_construct(struct neighbour *neigh)
struct neigh_parms *parms; struct neigh_parms *parms;
rcu_read_lock(); rcu_read_lock();
dn_db = dev->dn_ptr; dn_db = rcu_dereference(dev->dn_ptr);
if (dn_db == NULL) { if (dn_db == NULL) {
rcu_read_unlock(); rcu_read_unlock();
return -EINVAL; return -EINVAL;
} }
parms = dn_db->neigh_parms; parms = dn_db->neigh_parms;
if (parms) { if (!parms) {
rcu_read_unlock();
return -EINVAL;
}
__neigh_parms_put(neigh->parms); __neigh_parms_put(neigh->parms);
neigh->parms = neigh_parms_clone(parms); neigh->parms = neigh_parms_clone(parms);
}
rcu_read_unlock(); rcu_read_unlock();
if (dn_db->use_long) if (dn_db->use_long)
......
...@@ -244,17 +244,15 @@ static int arp_constructor(struct neighbour *neigh) ...@@ -244,17 +244,15 @@ static int arp_constructor(struct neighbour *neigh)
neigh->type = inet_addr_type(addr); neigh->type = inet_addr_type(addr);
rcu_read_lock(); rcu_read_lock();
in_dev = __in_dev_get(dev); in_dev = rcu_dereference(__in_dev_get(dev));
if (in_dev == NULL) { if (in_dev == NULL) {
rcu_read_unlock(); rcu_read_unlock();
return -EINVAL; return -EINVAL;
} }
parms = in_dev->arp_parms; parms = in_dev->arp_parms;
if (parms) {
__neigh_parms_put(neigh->parms); __neigh_parms_put(neigh->parms);
neigh->parms = neigh_parms_clone(parms); neigh->parms = neigh_parms_clone(parms);
}
rcu_read_unlock(); rcu_read_unlock();
if (dev->hard_header == NULL) { if (dev->hard_header == NULL) {
......
...@@ -297,10 +297,8 @@ static int ndisc_constructor(struct neighbour *neigh) ...@@ -297,10 +297,8 @@ static int ndisc_constructor(struct neighbour *neigh)
} }
parms = in6_dev->nd_parms; parms = in6_dev->nd_parms;
if (parms) {
__neigh_parms_put(neigh->parms); __neigh_parms_put(neigh->parms);
neigh->parms = neigh_parms_clone(parms); neigh->parms = neigh_parms_clone(parms);
}
rcu_read_unlock(); rcu_read_unlock();
neigh->type = is_multicast ? RTN_MULTICAST : RTN_UNICAST; neigh->type = is_multicast ? RTN_MULTICAST : RTN_UNICAST;
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment