bgpd: bgp_clear_adj_in|remove dest may be freed

dest will not be freed due to lock but coverity does not know
that.  Give it a hint.  This change includes modifying bgp_dest_unlock_node
to return the dest pointer so that we can determine if we should
continue working on dest or not with an assert.  Since this
is lock based we should be ok.

Signed-off-by: Donald Sharp <sharpd@nvidia.com>

5 files changed, 18 insertions, 9 deletions

diff --git a/bgpd/bgp_advertise.c b/bgpd/bgp_advertise.c
index da7b496d6..c215611ed 100644
--- a/bgpd/bgp_advertise.c
+++ b/bgpd/bgp_advertise.c
@@ -188,11 +188,11 @@ void bgp_adj_in_set(struct bgp_dest *dest, struct peer *peer, struct attr *attr,
 	bgp_dest_lock_node(dest);
 }
 
-void bgp_adj_in_remove(struct bgp_dest *dest, struct bgp_adj_in *bai)
+void bgp_adj_in_remove(struct bgp_dest **dest, struct bgp_adj_in *bai)
 {
 	bgp_attr_unintern(&bai->attr);
-	BGP_ADJ_IN_DEL(dest, bai);
-	bgp_dest_unlock_node(dest);
+	BGP_ADJ_IN_DEL(*dest, bai);
+	*dest = bgp_dest_unlock_node(*dest);
 	peer_unlock(bai->peer); /* adj_in peer reference */
 	XFREE(MTYPE_BGP_ADJ_IN, bai);
 }
@@ -212,9 +212,11 @@ bool bgp_adj_in_unset(struct bgp_dest *dest, struct peer *peer,
 		adj_next = adj->next;
 
 		if (adj->peer == peer && adj->addpath_rx_id == addpath_id)
-			bgp_adj_in_remove(dest, adj);
+			bgp_adj_in_remove(&dest, adj);
 
 		adj = adj_next;
+
+		assert(dest);
 	}
 
 	return true;
diff --git a/bgpd/bgp_advertise.h b/bgpd/bgp_advertise.h
index a063208b6..e597c56d1 100644
--- a/bgpd/bgp_advertise.h
+++ b/bgpd/bgp_advertise.h
@@ -132,7 +132,7 @@ extern void bgp_adj_in_set(struct bgp_dest *dest, struct peer *peer,
 			   struct attr *attr, uint32_t addpath_id);
 extern bool bgp_adj_in_unset(struct bgp_dest *dest, struct peer *peer,
 			     uint32_t addpath_id);
-extern void bgp_adj_in_remove(struct bgp_dest *dest, struct bgp_adj_in *bai);
+extern void bgp_adj_in_remove(struct bgp_dest **dest, struct bgp_adj_in *bai);
 
 extern unsigned int bgp_advertise_attr_hash_key(const void *p);
 extern bool bgp_advertise_attr_hash_cmp(const void *p1, const void *p2);
diff --git a/bgpd/bgp_route.c b/bgpd/bgp_route.c
index c8271bf5c..1832d55ec 100644
--- a/bgpd/bgp_route.c
+++ b/bgpd/bgp_route.c
@@ -5671,9 +5671,11 @@ static void bgp_clear_route_table(struct peer *peer, afi_t afi, safi_t safi,
 			ain_next = ain->next;
 
 			if (ain->peer == peer)
-				bgp_adj_in_remove(dest, ain);
+				bgp_adj_in_remove(&dest, ain);
 
 			ain = ain_next;
+
+			assert(dest);
 		}
 
 		for (pi = bgp_dest_get_bgp_path_info(dest); pi; pi = next) {
@@ -5778,9 +5780,11 @@ void bgp_clear_adj_in(struct peer *peer, afi_t afi, safi_t safi)
 			ain_next = ain->next;
 
 			if (ain->peer == peer)
-				bgp_adj_in_remove(dest, ain);
+				bgp_adj_in_remove(&dest, ain);
 
 			ain = ain_next;
+
+			assert(dest);
 		}
 	}
 }
diff --git a/bgpd/bgp_table.c b/bgpd/bgp_table.c
index 149a5b914..8465ada99 100644
--- a/bgpd/bgp_table.c
+++ b/bgpd/bgp_table.c
@@ -75,7 +75,7 @@ const char *bgp_dest_get_prefix_str(struct bgp_dest *dest)
 /*
  * bgp_dest_unlock_node
  */
-inline void bgp_dest_unlock_node(struct bgp_dest *dest)
+inline struct bgp_dest *bgp_dest_unlock_node(struct bgp_dest *dest)
 {
 	frrtrace(1, frr_bgp, bgp_dest_unlock, dest);
 	bgp_delete_listnode(dest);
@@ -89,9 +89,12 @@ inline void bgp_dest_unlock_node(struct bgp_dest *dest)
 						   rt->safi);
 		}
 		XFREE(MTYPE_BGP_NODE, dest);
+		dest = NULL;
 		rn->info = NULL;
 	}
 	route_unlock_node(rn);
+
+	return dest;
 }
 
 /*
diff --git a/bgpd/bgp_table.h b/bgpd/bgp_table.h
index 1f28624c1..5b4c3be21 100644
--- a/bgpd/bgp_table.h
+++ b/bgpd/bgp_table.h
@@ -112,7 +112,7 @@ extern struct bgp_table *bgp_table_init(struct bgp *bgp, afi_t, safi_t);
 extern void bgp_table_lock(struct bgp_table *);
 extern void bgp_table_unlock(struct bgp_table *);
 extern void bgp_table_finish(struct bgp_table **);
-extern void bgp_dest_unlock_node(struct bgp_dest *dest);
+extern struct bgp_dest *bgp_dest_unlock_node(struct bgp_dest *dest);
 extern struct bgp_dest *bgp_dest_lock_node(struct bgp_dest *dest);
 extern const char *bgp_dest_get_prefix_str(struct bgp_dest *dest);