From 0e70e6c89d69976cde1b79bba6ac23d233b19566 Mon Sep 17 00:00:00 2001
From: David Lamparter <equinox@diac24.net>
Date: Sat, 18 Aug 2018 04:47:27 +0200
Subject: [PATCH] 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>
---
 bgpd/bgp_route.c | 20 ++++++++++++++++++--
 bgpd/bgp_table.h |  4 ++--
 lib/table.h      |  8 ++------
 3 files changed, 22 insertions(+), 10 deletions(-)

diff --git a/bgpd/bgp_route.c b/bgpd/bgp_route.c
index 041049d05b..bf97e26230 100644
--- a/bgpd/bgp_route.c
+++ b/bgpd/bgp_route.c
@@ -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;
 	}
diff --git a/bgpd/bgp_table.h b/bgpd/bgp_table.h
index f7eac09546..60c2cbd4a4 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 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));
 }
 
 /*
diff --git a/lib/table.h b/lib/table.h
index ac7df3e695..8304abe59b 100644
--- a/lib/table.h
+++ b/lib/table.h
@@ -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;
 }
 
 /*