bgpd: backpressure - Avoid use after free

raja-rajasekar · raja-rajasekar · commit 40965e599975 · 2024-07-22T10:16:16.000-07:00
Coverity complains there is a use after free (1598495 and 1598496)
At this point, most likely dest-&gt;refcount cannot go 1 and free up
the dest, but there might be some code path where this can happen.

Fixing this with a simple order change (no harm fix).

Ticket :#4001204

Signed-off-by: Rajasekar Raja &lt;rajasekarr@nvidia.com&gt;
diff --git a/bgpd/bgp_evpn.c b/bgpd/bgp_evpn.c
@@ -6339,9 +6339,9 @@ void bgp_evpn_free(struct bgp *bgp, struct bgpevpn *vpn)
 	     dest = dest_next) {
 		dest_next = zebra_announce_next(&bm->zebra_announce_head, dest);
 		if (dest->za_vpn == vpn) {
+			zebra_announce_del(&bm->zebra_announce_head, dest);
 			bgp_path_info_unlock(dest->za_bgp_pi);
 			bgp_dest_unlock_node(dest);
-			zebra_announce_del(&bm->zebra_announce_head, dest);
 		}
 	}
 
diff --git a/bgpd/bgpd.c b/bgpd/bgpd.c
@@ -3954,9 +3954,9 @@ int bgp_delete(struct bgp *bgp)
 		dest_next = zebra_announce_next(&bm->zebra_announce_head, dest);
 		dest_table = bgp_dest_table(dest);
 		if (dest_table->bgp == bgp) {
+			zebra_announce_del(&bm->zebra_announce_head, dest);
 			bgp_path_info_unlock(dest->za_bgp_pi);
 			bgp_dest_unlock_node(dest);
-			zebra_announce_del(&bm->zebra_announce_head, dest);
 		}
 	}
 

Original file line number	Diff line number	Diff line change
`@@ -6339,9 +6339,9 @@ void bgp_evpn_free(struct bgp bgp, struct bgpevpn vpn)`
`6339`	`6339`	`dest = dest_next) {`
`6340`	`6340`	`dest_next = zebra_announce_next(&bm->zebra_announce_head, dest);`
`6341`	`6341`	`if (dest->za_vpn == vpn) {`
	`6342`	`+ zebra_announce_del(&bm->zebra_announce_head, dest);`
`6342`	`6343`	`bgp_path_info_unlock(dest->za_bgp_pi);`
`6343`	`6344`	`bgp_dest_unlock_node(dest);`
`6344`		`- zebra_announce_del(&bm->zebra_announce_head, dest);`
`6345`	`6345`	`}`
`6346`	`6346`	`}`
`6347`	`6347`
Original file line number	Diff line number	Diff line change
`@@ -3954,9 +3954,9 @@ int bgp_delete(struct bgp *bgp)`
`3954`	`3954`	`dest_next = zebra_announce_next(&bm->zebra_announce_head, dest);`
`3955`	`3955`	`dest_table = bgp_dest_table(dest);`
`3956`	`3956`	`if (dest_table->bgp == bgp) {`
	`3957`	`+ zebra_announce_del(&bm->zebra_announce_head, dest);`
`3957`	`3958`	`bgp_path_info_unlock(dest->za_bgp_pi);`
`3958`	`3959`	`bgp_dest_unlock_node(dest);`
`3959`		`- zebra_announce_del(&bm->zebra_announce_head, dest);`
`3960`	`3960`	`}`
`3961`	`3961`	`}`
`3962`	`3962`