Commit be47e41d authored by Jon Maloy's avatar Jon Maloy Committed by David S. Miller

tipc: fix use-after-free in tipc_nametbl_stop

When we delete a service item in tipc_nametbl_stop() we loop over
all service ranges in the service's RB tree, and for each service
range we loop over its pertaining publications while calling
tipc_service_remove_publ() for each of them.

However, tipc_service_remove_publ() has the side effect that it also
removes the comprising service range item when there are no publications
left. This leads to a "use-after-free" access when the inner loop
continues to the next iteration, since the range item holding the list
we are looping no longer exists.

We fix this by moving the delete of the service range item outside
the said function. Instead, we now let the two functions calling it
test if the list is empty and perform the removal when that is the
case.

Reported-by: syzbot+d64b64afc55660106556@syzkaller.appspotmail.com
Signed-off-by: default avatarJon Maloy <jon.maloy@ericsson.com>
Signed-off-by: default avatarDavid S. Miller <davem@davemloft.net>
parent 9c438d7a
...@@ -241,7 +241,8 @@ static struct publication *tipc_service_insert_publ(struct net *net, ...@@ -241,7 +241,8 @@ static struct publication *tipc_service_insert_publ(struct net *net,
static struct publication *tipc_service_remove_publ(struct net *net, static struct publication *tipc_service_remove_publ(struct net *net,
struct tipc_service *sc, struct tipc_service *sc,
u32 lower, u32 upper, u32 lower, u32 upper,
u32 node, u32 key) u32 node, u32 key,
struct service_range **rng)
{ {
struct tipc_subscription *sub, *tmp; struct tipc_subscription *sub, *tmp;
struct service_range *sr; struct service_range *sr;
...@@ -275,19 +276,15 @@ static struct publication *tipc_service_remove_publ(struct net *net, ...@@ -275,19 +276,15 @@ static struct publication *tipc_service_remove_publ(struct net *net,
list_del(&p->all_publ); list_del(&p->all_publ);
list_del(&p->local_publ); list_del(&p->local_publ);
if (list_empty(&sr->all_publ))
/* Remove service range item if this was its last publication */
if (list_empty(&sr->all_publ)) {
last = true; last = true;
rb_erase(&sr->tree_node, &sc->ranges);
kfree(sr);
}
/* Notify any waiting subscriptions */ /* Notify any waiting subscriptions */
list_for_each_entry_safe(sub, tmp, &sc->subscriptions, service_list) { list_for_each_entry_safe(sub, tmp, &sc->subscriptions, service_list) {
tipc_sub_report_overlap(sub, p->lower, p->upper, TIPC_WITHDRAWN, tipc_sub_report_overlap(sub, p->lower, p->upper, TIPC_WITHDRAWN,
p->port, p->node, p->scope, last); p->port, p->node, p->scope, last);
} }
*rng = sr;
return p; return p;
} }
...@@ -379,13 +376,20 @@ struct publication *tipc_nametbl_remove_publ(struct net *net, u32 type, ...@@ -379,13 +376,20 @@ struct publication *tipc_nametbl_remove_publ(struct net *net, u32 type,
u32 node, u32 key) u32 node, u32 key)
{ {
struct tipc_service *sc = tipc_service_find(net, type); struct tipc_service *sc = tipc_service_find(net, type);
struct service_range *sr = NULL;
struct publication *p = NULL; struct publication *p = NULL;
if (!sc) if (!sc)
return NULL; return NULL;
spin_lock_bh(&sc->lock); spin_lock_bh(&sc->lock);
p = tipc_service_remove_publ(net, sc, lower, upper, node, key); p = tipc_service_remove_publ(net, sc, lower, upper, node, key, &sr);
/* Remove service range item if this was its last publication */
if (sr && list_empty(&sr->all_publ)) {
rb_erase(&sr->tree_node, &sc->ranges);
kfree(sr);
}
/* Delete service item if this no more publications and subscriptions */ /* Delete service item if this no more publications and subscriptions */
if (RB_EMPTY_ROOT(&sc->ranges) && list_empty(&sc->subscriptions)) { if (RB_EMPTY_ROOT(&sc->ranges) && list_empty(&sc->subscriptions)) {
...@@ -747,16 +751,17 @@ int tipc_nametbl_init(struct net *net) ...@@ -747,16 +751,17 @@ int tipc_nametbl_init(struct net *net)
static void tipc_service_delete(struct net *net, struct tipc_service *sc) static void tipc_service_delete(struct net *net, struct tipc_service *sc)
{ {
struct service_range *sr, *tmpr; struct service_range *sr, *tmpr;
struct publication *p, *tmpb; struct publication *p, *tmp;
spin_lock_bh(&sc->lock); spin_lock_bh(&sc->lock);
rbtree_postorder_for_each_entry_safe(sr, tmpr, &sc->ranges, tree_node) { rbtree_postorder_for_each_entry_safe(sr, tmpr, &sc->ranges, tree_node) {
list_for_each_entry_safe(p, tmpb, list_for_each_entry_safe(p, tmp, &sr->all_publ, all_publ) {
&sr->all_publ, all_publ) {
tipc_service_remove_publ(net, sc, p->lower, p->upper, tipc_service_remove_publ(net, sc, p->lower, p->upper,
p->node, p->key); p->node, p->key, &sr);
kfree_rcu(p, rcu); kfree_rcu(p, rcu);
} }
rb_erase(&sr->tree_node, &sc->ranges);
kfree(sr);
} }
hlist_del_init_rcu(&sc->service_list); hlist_del_init_rcu(&sc->service_list);
spin_unlock_bh(&sc->lock); spin_unlock_bh(&sc->lock);
......
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