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 <[email protected]>

Author: donaldsharp

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;

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);

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);
 		}
 	}
 }

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;
 }
 
 /*

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);