From 563f0c2b2de19737e62017d318a7f5cdbbf8c210 Mon Sep 17 00:00:00 2001 From: Donald Sharp Date: Fri, 27 Jul 2018 09:54:39 -0400 Subject: [PATCH 1/2] lib: Modify route unlock code to return appropriate pointer Modify the unlock code for a route_node to return NULL on pointer freed or to return the node itself again. We'll need to go through the code and fix this pattern, but this is a problem for another day. Get this fix in place and we can make it a low hanging problem to fix. Signed-off-by: Donald Sharp --- lib/table.h | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/lib/table.h b/lib/table.h index a9d788b35a..f58a6025e2 100644 --- a/lib/table.h +++ b/lib/table.h @@ -233,13 +233,17 @@ static inline struct route_node *route_lock_node(struct route_node *node) } /* Unlock node. */ -static inline void route_unlock_node(struct route_node *node) +static inline struct route_node *route_unlock_node(struct route_node *node) { assert(node->lock > 0); (*(unsigned *)&node->lock)--; - if (node->lock == 0) + if (node->lock == 0) { route_node_delete(node); + return NULL; + } + + return node; } /* From 239b37bb3c1d1d82423c5934c8615d4bfb392472 Mon Sep 17 00:00:00 2001 From: Donald Sharp Date: Fri, 27 Jul 2018 10:02:34 -0400 Subject: [PATCH 2/2] bgpd: Notice when we unlock if we should NULL pointer The bi->net pointer that is being unlocked had a commit that removed the `bi->net = NULL;` recently. This code was preventing a use after free crash being experienced in other code paths. While commit 37e679629f9 was fixing a different code path crash. Make the parent->net pointer aware it may be locked/freed from multiple places and to not NULL the pointer to it unless we have actually freed the data. Signed-off-by: Donald Sharp --- bgpd/bgp_route.c | 2 +- bgpd/bgp_table.h | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/bgpd/bgp_route.c b/bgpd/bgp_route.c index 7fbc6d9a65..9ba9eb1e59 100644 --- a/bgpd/bgp_route.c +++ b/bgpd/bgp_route.c @@ -188,7 +188,7 @@ static void bgp_info_extra_free(struct bgp_info_extra **extra) struct bgp_info *bi = (struct bgp_info *)e->parent; if (bi->net) - bgp_unlock_node((struct bgp_node *)bi->net); + bi->net = bgp_unlock_node((struct bgp_node *)bi->net); bgp_info_unlock(e->parent); e->parent = NULL; } diff --git a/bgpd/bgp_table.h b/bgpd/bgp_table.h index 60c2cbd4a4..f7eac09546 100644 --- a/bgpd/bgp_table.h +++ b/bgpd/bgp_table.h @@ -128,9 +128,9 @@ static inline struct bgp_node *bgp_node_parent_nolock(struct bgp_node *node) /* * bgp_unlock_node */ -static inline void bgp_unlock_node(struct bgp_node *node) +static inline struct bgp_node *bgp_unlock_node(struct bgp_node *node) { - route_unlock_node(bgp_node_to_rnode(node)); + return (struct bgp_node *)route_unlock_node(bgp_node_to_rnode(node)); } /*