summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorDavid S. Miller <davem@davemloft.net>2013-08-03 02:44:33 +0400
committerDavid S. Miller <davem@davemloft.net>2013-08-03 02:44:33 +0400
commit4b42df5da8c2f394adeaa61b23e9e58994b1c3c8 (patch)
treee7dab36220c48573e4475034c9c68ce083219a93
parent8a849bb7f0ac513c2b7202620f611d94301c4a93 (diff)
parent9918d5bf329d0dc5bb2d9d293bcb772bdb626e65 (diff)
downloadlinux-4b42df5da8c2f394adeaa61b23e9e58994b1c3c8.tar.xz
Merge branch 'bond_neigh_parms'
Veaceslav Falico says: ==================== Recent patches revealed an old bug, which was there for quite awhile. It's related to vlan on top of bonding and ndo_neigh_setup(). When vlan device is initiated, it calls its real_dev->ndo_neigh_setup(), and in case of bonding - it will modify neigh_parms->neigh_setup to point to bond_neigh_init, while neigh_parms are of vlan's dev. This way, when neigh_parms->neigh_setup() of vlan's dev is called, the bonding function will be called, which expects the dev to be struct bonding, but will receive a vlan dev. It was hidden before because of bond->first_slave usage. Now, with Nikolay's conversion to list/RCU, first_slave is gone and we hit a null pointer dereference when working with lists/slave. First patch moves ndo_neigh_setup() in neigh_parms_alloc() to the bottom, so that the ->dev will be available to the caller. It doesn't really change anything, however is needed for the second patch. Second patch makes bond_neigh_setup() (bond->ndo_neigh_setup()) check if the neigh_parms are really from a bonding dev, and only modify the neigh_setup in this case. ==================== Signed-off-by: David S. Miller <davem@davemloft.net>
-rw-r--r--drivers/net/bonding/bond_main.c8
-rw-r--r--net/core/neighbour.c10
2 files changed, 13 insertions, 5 deletions
diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
index 1d37a9657e0d..476df7d2a354 100644
--- a/drivers/net/bonding/bond_main.c
+++ b/drivers/net/bonding/bond_main.c
@@ -3630,11 +3630,17 @@ static int bond_neigh_init(struct neighbour *n)
* The bonding ndo_neigh_setup is called at init time beofre any
* slave exists. So we must declare proxy setup function which will
* be used at run time to resolve the actual slave neigh param setup.
+ *
+ * It's also called by master devices (such as vlans) to setup their
+ * underlying devices. In that case - do nothing, we're already set up from
+ * our init.
*/
static int bond_neigh_setup(struct net_device *dev,
struct neigh_parms *parms)
{
- parms->neigh_setup = bond_neigh_init;
+ /* modify only our neigh_parms */
+ if (parms->dev == dev)
+ parms->neigh_setup = bond_neigh_init;
return 0;
}
diff --git a/net/core/neighbour.c b/net/core/neighbour.c
index b7de821f98df..576d46f4a69e 100644
--- a/net/core/neighbour.c
+++ b/net/core/neighbour.c
@@ -1441,16 +1441,18 @@ struct neigh_parms *neigh_parms_alloc(struct net_device *dev,
atomic_set(&p->refcnt, 1);
p->reachable_time =
neigh_rand_reach_time(p->base_reachable_time);
+ dev_hold(dev);
+ p->dev = dev;
+ write_pnet(&p->net, hold_net(net));
+ p->sysctl_table = NULL;
if (ops->ndo_neigh_setup && ops->ndo_neigh_setup(dev, p)) {
+ release_net(net);
+ dev_put(dev);
kfree(p);
return NULL;
}
- dev_hold(dev);
- p->dev = dev;
- write_pnet(&p->net, hold_net(net));
- p->sysctl_table = NULL;
write_lock_bh(&tbl->lock);
p->next = tbl->parms.next;
tbl->parms.next = p;