Commit 90f33bff authored by Nikolay Aleksandrov's avatar Nikolay Aleksandrov Committed by David S. Miller

nexthops: don't modify published nexthop groups

We must avoid modifying published nexthop groups while they might be
in use, otherwise we might see NULL ptr dereferences. In order to do
that we allocate 2 nexthoup group structures upon nexthop creation
and swap between them when we have to delete an entry. The reason is
that we can't fail nexthop group removal, so we can't handle allocation
failure thus we move the extra allocation on creation where we can
safely fail and return ENOMEM.

Fixes: 430a0491 ("nexthop: Add support for nexthop groups")
Signed-off-by: default avatarNikolay Aleksandrov <nikolay@cumulusnetworks.com>
Signed-off-by: default avatarDavid Ahern <dsahern@gmail.com>
Signed-off-by: default avatarDavid S. Miller <davem@davemloft.net>
parent ac21753a
...@@ -70,6 +70,7 @@ struct nh_grp_entry { ...@@ -70,6 +70,7 @@ struct nh_grp_entry {
}; };
struct nh_group { struct nh_group {
struct nh_group *spare; /* spare group for removals */
u16 num_nh; u16 num_nh;
bool mpath; bool mpath;
bool has_v4; bool has_v4;
......
...@@ -63,9 +63,16 @@ static void nexthop_free_mpath(struct nexthop *nh) ...@@ -63,9 +63,16 @@ static void nexthop_free_mpath(struct nexthop *nh)
int i; int i;
nhg = rcu_dereference_raw(nh->nh_grp); nhg = rcu_dereference_raw(nh->nh_grp);
for (i = 0; i < nhg->num_nh; ++i) for (i = 0; i < nhg->num_nh; ++i) {
WARN_ON(nhg->nh_entries[i].nh); struct nh_grp_entry *nhge = &nhg->nh_entries[i];
WARN_ON(!list_empty(&nhge->nh_list));
nexthop_put(nhge->nh);
}
WARN_ON(nhg->spare == nhg);
kfree(nhg->spare);
kfree(nhg); kfree(nhg);
} }
...@@ -697,46 +704,53 @@ static void nh_group_rebalance(struct nh_group *nhg) ...@@ -697,46 +704,53 @@ static void nh_group_rebalance(struct nh_group *nhg)
static void remove_nh_grp_entry(struct net *net, struct nh_grp_entry *nhge, static void remove_nh_grp_entry(struct net *net, struct nh_grp_entry *nhge,
struct nl_info *nlinfo) struct nl_info *nlinfo)
{ {
struct nh_grp_entry *nhges, *new_nhges;
struct nexthop *nhp = nhge->nh_parent; struct nexthop *nhp = nhge->nh_parent;
struct nexthop *nh = nhge->nh; struct nexthop *nh = nhge->nh;
struct nh_grp_entry *nhges; struct nh_group *nhg, *newg;
struct nh_group *nhg; int i, j;
bool found = false;
int i;
WARN_ON(!nh); WARN_ON(!nh);
list_del(&nhge->nh_list);
nhg = rtnl_dereference(nhp->nh_grp); nhg = rtnl_dereference(nhp->nh_grp);
nhges = nhg->nh_entries; newg = nhg->spare;
for (i = 0; i < nhg->num_nh; ++i) {
if (found) {
nhges[i-1].nh = nhges[i].nh;
nhges[i-1].weight = nhges[i].weight;
list_del(&nhges[i].nh_list);
list_add(&nhges[i-1].nh_list, &nhges[i-1].nh->grp_list);
} else if (nhg->nh_entries[i].nh == nh) {
found = true;
}
}
if (WARN_ON(!found)) /* last entry, keep it visible and remove the parent */
if (nhg->num_nh == 1) {
remove_nexthop(net, nhp, nlinfo);
return; return;
}
nhg->num_nh--; newg->has_v4 = nhg->has_v4;
nhg->nh_entries[nhg->num_nh].nh = NULL; newg->mpath = nhg->mpath;
newg->num_nh = nhg->num_nh;
nh_group_rebalance(nhg); /* copy old entries to new except the one getting removed */
nhges = nhg->nh_entries;
new_nhges = newg->nh_entries;
for (i = 0, j = 0; i < nhg->num_nh; ++i) {
/* current nexthop getting removed */
if (nhg->nh_entries[i].nh == nh) {
newg->num_nh--;
continue;
}
nexthop_put(nh); list_del(&nhges[i].nh_list);
new_nhges[j].nh_parent = nhges[i].nh_parent;
new_nhges[j].nh = nhges[i].nh;
new_nhges[j].weight = nhges[i].weight;
list_add(&new_nhges[j].nh_list, &new_nhges[j].nh->grp_list);
j++;
}
nh_group_rebalance(newg);
rcu_assign_pointer(nhp->nh_grp, newg);
list_del(&nhge->nh_list);
nexthop_put(nhge->nh);
if (nlinfo) if (nlinfo)
nexthop_notify(RTM_NEWNEXTHOP, nhp, nlinfo); nexthop_notify(RTM_NEWNEXTHOP, nhp, nlinfo);
/* if this group has no more entries then remove it */
if (!nhg->num_nh)
remove_nexthop(net, nhp, nlinfo);
} }
static void remove_nexthop_from_groups(struct net *net, struct nexthop *nh, static void remove_nexthop_from_groups(struct net *net, struct nexthop *nh,
...@@ -746,6 +760,9 @@ static void remove_nexthop_from_groups(struct net *net, struct nexthop *nh, ...@@ -746,6 +760,9 @@ static void remove_nexthop_from_groups(struct net *net, struct nexthop *nh,
list_for_each_entry_safe(nhge, tmp, &nh->grp_list, nh_list) list_for_each_entry_safe(nhge, tmp, &nh->grp_list, nh_list)
remove_nh_grp_entry(net, nhge, nlinfo); remove_nh_grp_entry(net, nhge, nlinfo);
/* make sure all see the newly published array before releasing rtnl */
synchronize_rcu();
} }
static void remove_nexthop_group(struct nexthop *nh, struct nl_info *nlinfo) static void remove_nexthop_group(struct nexthop *nh, struct nl_info *nlinfo)
...@@ -759,10 +776,7 @@ static void remove_nexthop_group(struct nexthop *nh, struct nl_info *nlinfo) ...@@ -759,10 +776,7 @@ static void remove_nexthop_group(struct nexthop *nh, struct nl_info *nlinfo)
if (WARN_ON(!nhge->nh)) if (WARN_ON(!nhge->nh))
continue; continue;
list_del(&nhge->nh_list); list_del_init(&nhge->nh_list);
nexthop_put(nhge->nh);
nhge->nh = NULL;
nhg->num_nh--;
} }
} }
...@@ -1085,6 +1099,7 @@ static struct nexthop *nexthop_create_group(struct net *net, ...@@ -1085,6 +1099,7 @@ static struct nexthop *nexthop_create_group(struct net *net,
{ {
struct nlattr *grps_attr = cfg->nh_grp; struct nlattr *grps_attr = cfg->nh_grp;
struct nexthop_grp *entry = nla_data(grps_attr); struct nexthop_grp *entry = nla_data(grps_attr);
u16 num_nh = nla_len(grps_attr) / sizeof(*entry);
struct nh_group *nhg; struct nh_group *nhg;
struct nexthop *nh; struct nexthop *nh;
int i; int i;
...@@ -1095,12 +1110,21 @@ static struct nexthop *nexthop_create_group(struct net *net, ...@@ -1095,12 +1110,21 @@ static struct nexthop *nexthop_create_group(struct net *net,
nh->is_group = 1; nh->is_group = 1;
nhg = nexthop_grp_alloc(nla_len(grps_attr) / sizeof(*entry)); nhg = nexthop_grp_alloc(num_nh);
if (!nhg) { if (!nhg) {
kfree(nh); kfree(nh);
return ERR_PTR(-ENOMEM); return ERR_PTR(-ENOMEM);
} }
/* spare group used for removals */
nhg->spare = nexthop_grp_alloc(num_nh);
if (!nhg) {
kfree(nhg);
kfree(nh);
return NULL;
}
nhg->spare->spare = nhg;
for (i = 0; i < nhg->num_nh; ++i) { for (i = 0; i < nhg->num_nh; ++i) {
struct nexthop *nhe; struct nexthop *nhe;
struct nh_info *nhi; struct nh_info *nhi;
...@@ -1132,6 +1156,7 @@ static struct nexthop *nexthop_create_group(struct net *net, ...@@ -1132,6 +1156,7 @@ static struct nexthop *nexthop_create_group(struct net *net,
for (; i >= 0; --i) for (; i >= 0; --i)
nexthop_put(nhg->nh_entries[i].nh); nexthop_put(nhg->nh_entries[i].nh);
kfree(nhg->spare);
kfree(nhg); kfree(nhg);
kfree(nh); kfree(nh);
......
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