Commit 33b1341c authored by Jiri Pirko's avatar Jiri Pirko Committed by David S. Miller

mlxsw: spectrum_router: Fix handling of neighbour structure

__neigh_create function works in a different way than assumed.
It passes "n" as a parameter to ndo_neigh_construct. But this "n" might
be destroyed right away before __neigh_create() returns in case there is
already another neighbour struct in the hashtable with the same dev and
primary key. That is not expected by mlxsw_sp_router_neigh_construct()
and the stored "n" points to freed memory, eventually leading to crash.

Fix this by doing tight 1:1 coupling between neighbour struct and
internal driver neigh_entry. That allows to narrow down the key in
internal driver hashtable to do lookups by "n" only.

Fixes: 6cf3c971 ("mlxsw: spectrum_router: Add private neigh table")
Signed-off-by: default avatarJiri Pirko <jiri@mellanox.com>
Acked-by: default avatarIdo Schimmel <idosch@mellanox.com>
Signed-off-by: default avatarDavid S. Miller <davem@davemloft.net>
parent 2ce0af8f
...@@ -600,15 +600,13 @@ static void mlxsw_sp_vrs_fini(struct mlxsw_sp *mlxsw_sp) ...@@ -600,15 +600,13 @@ static void mlxsw_sp_vrs_fini(struct mlxsw_sp *mlxsw_sp)
} }
struct mlxsw_sp_neigh_key { struct mlxsw_sp_neigh_key {
unsigned char addr[sizeof(struct in6_addr)]; struct neighbour *n;
struct net_device *dev;
}; };
struct mlxsw_sp_neigh_entry { struct mlxsw_sp_neigh_entry {
struct rhash_head ht_node; struct rhash_head ht_node;
struct mlxsw_sp_neigh_key key; struct mlxsw_sp_neigh_key key;
u16 rif; u16 rif;
struct neighbour *n;
bool offloaded; bool offloaded;
struct delayed_work dw; struct delayed_work dw;
struct mlxsw_sp_port *mlxsw_sp_port; struct mlxsw_sp_port *mlxsw_sp_port;
...@@ -646,19 +644,15 @@ mlxsw_sp_neigh_entry_remove(struct mlxsw_sp *mlxsw_sp, ...@@ -646,19 +644,15 @@ mlxsw_sp_neigh_entry_remove(struct mlxsw_sp *mlxsw_sp,
static void mlxsw_sp_router_neigh_update_hw(struct work_struct *work); static void mlxsw_sp_router_neigh_update_hw(struct work_struct *work);
static struct mlxsw_sp_neigh_entry * static struct mlxsw_sp_neigh_entry *
mlxsw_sp_neigh_entry_create(const void *addr, size_t addr_len, mlxsw_sp_neigh_entry_create(struct neighbour *n, u16 rif)
struct net_device *dev, u16 rif,
struct neighbour *n)
{ {
struct mlxsw_sp_neigh_entry *neigh_entry; struct mlxsw_sp_neigh_entry *neigh_entry;
neigh_entry = kzalloc(sizeof(*neigh_entry), GFP_ATOMIC); neigh_entry = kzalloc(sizeof(*neigh_entry), GFP_ATOMIC);
if (!neigh_entry) if (!neigh_entry)
return NULL; return NULL;
memcpy(neigh_entry->key.addr, addr, addr_len); neigh_entry->key.n = n;
neigh_entry->key.dev = dev;
neigh_entry->rif = rif; neigh_entry->rif = rif;
neigh_entry->n = n;
INIT_DELAYED_WORK(&neigh_entry->dw, mlxsw_sp_router_neigh_update_hw); INIT_DELAYED_WORK(&neigh_entry->dw, mlxsw_sp_router_neigh_update_hw);
INIT_LIST_HEAD(&neigh_entry->nexthop_list); INIT_LIST_HEAD(&neigh_entry->nexthop_list);
return neigh_entry; return neigh_entry;
...@@ -671,13 +665,11 @@ mlxsw_sp_neigh_entry_destroy(struct mlxsw_sp_neigh_entry *neigh_entry) ...@@ -671,13 +665,11 @@ mlxsw_sp_neigh_entry_destroy(struct mlxsw_sp_neigh_entry *neigh_entry)
} }
static struct mlxsw_sp_neigh_entry * static struct mlxsw_sp_neigh_entry *
mlxsw_sp_neigh_entry_lookup(struct mlxsw_sp *mlxsw_sp, const void *addr, mlxsw_sp_neigh_entry_lookup(struct mlxsw_sp *mlxsw_sp, struct neighbour *n)
size_t addr_len, struct net_device *dev)
{ {
struct mlxsw_sp_neigh_key key = {{ 0 } }; struct mlxsw_sp_neigh_key key;
memcpy(key.addr, addr, addr_len); key.n = n;
key.dev = dev;
return rhashtable_lookup_fast(&mlxsw_sp->router.neigh_ht, return rhashtable_lookup_fast(&mlxsw_sp->router.neigh_ht,
&key, mlxsw_sp_neigh_ht_params); &key, mlxsw_sp_neigh_ht_params);
} }
...@@ -689,26 +681,20 @@ int mlxsw_sp_router_neigh_construct(struct net_device *dev, ...@@ -689,26 +681,20 @@ int mlxsw_sp_router_neigh_construct(struct net_device *dev,
struct mlxsw_sp *mlxsw_sp = mlxsw_sp_port->mlxsw_sp; struct mlxsw_sp *mlxsw_sp = mlxsw_sp_port->mlxsw_sp;
struct mlxsw_sp_neigh_entry *neigh_entry; struct mlxsw_sp_neigh_entry *neigh_entry;
struct mlxsw_sp_rif *r; struct mlxsw_sp_rif *r;
u32 dip;
int err; int err;
if (n->tbl != &arp_tbl) if (n->tbl != &arp_tbl)
return 0; return 0;
dip = ntohl(*((__be32 *) n->primary_key)); neigh_entry = mlxsw_sp_neigh_entry_lookup(mlxsw_sp, n);
neigh_entry = mlxsw_sp_neigh_entry_lookup(mlxsw_sp, &dip, sizeof(dip), if (neigh_entry)
n->dev);
if (neigh_entry) {
WARN_ON(neigh_entry->n != n);
return 0; return 0;
}
r = mlxsw_sp_rif_find_by_dev(mlxsw_sp, n->dev); r = mlxsw_sp_rif_find_by_dev(mlxsw_sp, n->dev);
if (WARN_ON(!r)) if (WARN_ON(!r))
return -EINVAL; return -EINVAL;
neigh_entry = mlxsw_sp_neigh_entry_create(&dip, sizeof(dip), n->dev, neigh_entry = mlxsw_sp_neigh_entry_create(n, r->rif);
r->rif, n);
if (!neigh_entry) if (!neigh_entry)
return -ENOMEM; return -ENOMEM;
err = mlxsw_sp_neigh_entry_insert(mlxsw_sp, neigh_entry); err = mlxsw_sp_neigh_entry_insert(mlxsw_sp, neigh_entry);
...@@ -727,14 +713,11 @@ void mlxsw_sp_router_neigh_destroy(struct net_device *dev, ...@@ -727,14 +713,11 @@ void mlxsw_sp_router_neigh_destroy(struct net_device *dev,
struct mlxsw_sp_port *mlxsw_sp_port = netdev_priv(dev); struct mlxsw_sp_port *mlxsw_sp_port = netdev_priv(dev);
struct mlxsw_sp *mlxsw_sp = mlxsw_sp_port->mlxsw_sp; struct mlxsw_sp *mlxsw_sp = mlxsw_sp_port->mlxsw_sp;
struct mlxsw_sp_neigh_entry *neigh_entry; struct mlxsw_sp_neigh_entry *neigh_entry;
u32 dip;
if (n->tbl != &arp_tbl) if (n->tbl != &arp_tbl)
return; return;
dip = ntohl(*((__be32 *) n->primary_key)); neigh_entry = mlxsw_sp_neigh_entry_lookup(mlxsw_sp, n);
neigh_entry = mlxsw_sp_neigh_entry_lookup(mlxsw_sp, &dip, sizeof(dip),
n->dev);
if (!neigh_entry) if (!neigh_entry)
return; return;
mlxsw_sp_neigh_entry_remove(mlxsw_sp, neigh_entry); mlxsw_sp_neigh_entry_remove(mlxsw_sp, neigh_entry);
...@@ -862,7 +845,7 @@ static void mlxsw_sp_router_neighs_update_nh(struct mlxsw_sp *mlxsw_sp) ...@@ -862,7 +845,7 @@ static void mlxsw_sp_router_neighs_update_nh(struct mlxsw_sp *mlxsw_sp)
* is active regardless of the traffic. * is active regardless of the traffic.
*/ */
if (!list_empty(&neigh_entry->nexthop_list)) if (!list_empty(&neigh_entry->nexthop_list))
neigh_event_send(neigh_entry->n, NULL); neigh_event_send(neigh_entry->key.n, NULL);
} }
rtnl_unlock(); rtnl_unlock();
} }
...@@ -908,9 +891,9 @@ static void mlxsw_sp_router_probe_unresolved_nexthops(struct work_struct *work) ...@@ -908,9 +891,9 @@ static void mlxsw_sp_router_probe_unresolved_nexthops(struct work_struct *work)
rtnl_lock(); rtnl_lock();
list_for_each_entry(neigh_entry, &mlxsw_sp->router.nexthop_neighs_list, list_for_each_entry(neigh_entry, &mlxsw_sp->router.nexthop_neighs_list,
nexthop_neighs_list_node) { nexthop_neighs_list_node) {
if (!(neigh_entry->n->nud_state & NUD_VALID) && if (!(neigh_entry->key.n->nud_state & NUD_VALID) &&
!list_empty(&neigh_entry->nexthop_list)) !list_empty(&neigh_entry->nexthop_list))
neigh_event_send(neigh_entry->n, NULL); neigh_event_send(neigh_entry->key.n, NULL);
} }
rtnl_unlock(); rtnl_unlock();
...@@ -927,7 +910,7 @@ static void mlxsw_sp_router_neigh_update_hw(struct work_struct *work) ...@@ -927,7 +910,7 @@ static void mlxsw_sp_router_neigh_update_hw(struct work_struct *work)
{ {
struct mlxsw_sp_neigh_entry *neigh_entry = struct mlxsw_sp_neigh_entry *neigh_entry =
container_of(work, struct mlxsw_sp_neigh_entry, dw.work); container_of(work, struct mlxsw_sp_neigh_entry, dw.work);
struct neighbour *n = neigh_entry->n; struct neighbour *n = neigh_entry->key.n;
struct mlxsw_sp_port *mlxsw_sp_port = neigh_entry->mlxsw_sp_port; struct mlxsw_sp_port *mlxsw_sp_port = neigh_entry->mlxsw_sp_port;
struct mlxsw_sp *mlxsw_sp = mlxsw_sp_port->mlxsw_sp; struct mlxsw_sp *mlxsw_sp = mlxsw_sp_port->mlxsw_sp;
char rauht_pl[MLXSW_REG_RAUHT_LEN]; char rauht_pl[MLXSW_REG_RAUHT_LEN];
...@@ -1030,11 +1013,8 @@ int mlxsw_sp_router_netevent_event(struct notifier_block *unused, ...@@ -1030,11 +1013,8 @@ int mlxsw_sp_router_netevent_event(struct notifier_block *unused,
mlxsw_sp = mlxsw_sp_port->mlxsw_sp; mlxsw_sp = mlxsw_sp_port->mlxsw_sp;
dip = ntohl(*((__be32 *) n->primary_key)); dip = ntohl(*((__be32 *) n->primary_key));
neigh_entry = mlxsw_sp_neigh_entry_lookup(mlxsw_sp, neigh_entry = mlxsw_sp_neigh_entry_lookup(mlxsw_sp, n);
&dip, if (WARN_ON(!neigh_entry)) {
sizeof(__be32),
dev);
if (WARN_ON(!neigh_entry) || WARN_ON(neigh_entry->n != n)) {
mlxsw_sp_port_dev_put(mlxsw_sp_port); mlxsw_sp_port_dev_put(mlxsw_sp_port);
return NOTIFY_DONE; return NOTIFY_DONE;
} }
...@@ -1343,33 +1323,26 @@ static int mlxsw_sp_nexthop_init(struct mlxsw_sp *mlxsw_sp, ...@@ -1343,33 +1323,26 @@ static int mlxsw_sp_nexthop_init(struct mlxsw_sp *mlxsw_sp,
struct fib_nh *fib_nh) struct fib_nh *fib_nh)
{ {
struct mlxsw_sp_neigh_entry *neigh_entry; struct mlxsw_sp_neigh_entry *neigh_entry;
u32 gwip = ntohl(fib_nh->nh_gw);
struct net_device *dev = fib_nh->nh_dev; struct net_device *dev = fib_nh->nh_dev;
struct neighbour *n; struct neighbour *n;
u8 nud_state; u8 nud_state;
neigh_entry = mlxsw_sp_neigh_entry_lookup(mlxsw_sp, &gwip, /* Take a reference of neigh here ensuring that neigh would
sizeof(gwip), dev); * not be detructed before the nexthop entry is finished.
if (!neigh_entry) { * The reference is taken either in neigh_lookup() or
__be32 gwipn = htonl(gwip); * in neith_create() in case n is not found.
*/
n = neigh_create(&arp_tbl, &gwipn, dev); n = neigh_lookup(&arp_tbl, &fib_nh->nh_gw, dev);
if (!n) {
n = neigh_create(&arp_tbl, &fib_nh->nh_gw, dev);
if (IS_ERR(n)) if (IS_ERR(n))
return PTR_ERR(n); return PTR_ERR(n);
neigh_event_send(n, NULL); neigh_event_send(n, NULL);
neigh_entry = mlxsw_sp_neigh_entry_lookup(mlxsw_sp, &gwip, }
sizeof(gwip), dev); neigh_entry = mlxsw_sp_neigh_entry_lookup(mlxsw_sp, n);
if (!neigh_entry) { if (!neigh_entry) {
neigh_release(n); neigh_release(n);
return -EINVAL; return -EINVAL;
}
} else {
/* Take a reference of neigh here ensuring that neigh would
* not be detructed before the nexthop entry is finished.
* The second branch takes the reference in neith_create()
*/
n = neigh_entry->n;
neigh_clone(n);
} }
/* If that is the first nexthop connected to that neigh, add to /* If that is the first nexthop connected to that neigh, add to
...@@ -1403,7 +1376,7 @@ static void mlxsw_sp_nexthop_fini(struct mlxsw_sp *mlxsw_sp, ...@@ -1403,7 +1376,7 @@ static void mlxsw_sp_nexthop_fini(struct mlxsw_sp *mlxsw_sp,
if (list_empty(&nh->neigh_entry->nexthop_list)) if (list_empty(&nh->neigh_entry->nexthop_list))
list_del(&nh->neigh_entry->nexthop_neighs_list_node); list_del(&nh->neigh_entry->nexthop_neighs_list_node);
neigh_release(neigh_entry->n); neigh_release(neigh_entry->key.n);
} }
static struct mlxsw_sp_nexthop_group * static struct mlxsw_sp_nexthop_group *
...@@ -1463,11 +1436,11 @@ static bool mlxsw_sp_nexthop_match(struct mlxsw_sp_nexthop *nh, ...@@ -1463,11 +1436,11 @@ static bool mlxsw_sp_nexthop_match(struct mlxsw_sp_nexthop *nh,
for (i = 0; i < fi->fib_nhs; i++) { for (i = 0; i < fi->fib_nhs; i++) {
struct fib_nh *fib_nh = &fi->fib_nh[i]; struct fib_nh *fib_nh = &fi->fib_nh[i];
u32 gwip = ntohl(fib_nh->nh_gw); struct neighbour *n = nh->neigh_entry->key.n;
if (memcmp(nh->neigh_entry->key.addr, if (memcmp(n->primary_key, &fib_nh->nh_gw,
&gwip, sizeof(u32)) == 0 && sizeof(fib_nh->nh_gw)) == 0 &&
nh->neigh_entry->key.dev == fib_nh->nh_dev) n->dev == fib_nh->nh_dev)
return true; return true;
} }
return false; return false;
......
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