lib/bgpd: re-fix bgp_info_extra_free()

Make the wart slightly less bad... also there is still a possible write
after free here.  This needs to be fixed again, properly, by some
structure changes.

Signed-off-by: David Lamparter <equinox@diac24.net>
This commit is contained in:
David Lamparter 2018-08-18 04:47:27 +02:00 committed by David Lamparter
parent a2dc7057e0
commit 0e70e6c89d
3 changed files with 22 additions and 10 deletions

View File

@ -188,8 +188,24 @@ static void bgp_info_extra_free(struct bgp_info_extra **extra)
if (e->parent) {
struct bgp_info *bi = (struct bgp_info *)e->parent;
if (bi->net)
bi->net = bgp_unlock_node((struct bgp_node *)bi->net);
if (bi->net) {
/* FIXME: since multiple e may have the same e->parent
* and e->parent->net is holding a refcount for each
* of them, we need to do some fudging here.
*
* WARNING: if bi->net->lock drops to 0, bi may be
* freed as well (because bi->net was holding the
* last reference to bi) => write after free!
*/
unsigned refcount;
bi = bgp_info_lock(bi);
refcount = bi->net->lock - 1;
bgp_unlock_node((struct bgp_node *)bi->net);
if (!refcount)
bi->net = NULL;
bgp_info_unlock(bi);
}
bgp_info_unlock(e->parent);
e->parent = NULL;
}

View File

@ -128,9 +128,9 @@ static inline struct bgp_node *bgp_node_parent_nolock(struct bgp_node *node)
/*
* bgp_unlock_node
*/
static inline struct bgp_node *bgp_unlock_node(struct bgp_node *node)
static inline void bgp_unlock_node(struct bgp_node *node)
{
return (struct bgp_node *)route_unlock_node(bgp_node_to_rnode(node));
route_unlock_node(bgp_node_to_rnode(node));
}
/*

View File

@ -235,17 +235,13 @@ static inline struct route_node *route_lock_node(struct route_node *node)
}
/* Unlock node. */
static inline struct route_node *route_unlock_node(struct route_node *node)
static inline void 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;
}
/*