Commit a841765cc5 for frr
commit a841765cc55f975a91d461cae53e8b30a1af6f5c
Author: Nageswara Soma <nsoma@cisco.com>
Date: Tue Oct 13 20:26:57 2026 -0700
bgpd: take meta-queue refs only after enqueue
This reworks the fix from PR #22719, which was
reverted. That version took the table and dest
locks and set BGP_NODE_PROCESS_SCHEDULED before
enqueueing, then released the locks and reset the
flag if the enqueue failed. Coverity treats the
rollback bgp_dest_unlock_node() as a possible free
of dest, and so flagged a false use-after-free in
every bgp_process() caller.
Take the locks and set BGP_NODE_PROCESS_SCHEDULED
only after the dest is on the meta-queue. The
failure path no longer touches the dest or table
locks, and nothing leaks when the process queue is
gone. A successful enqueue leaves dest in the same
state as before.
Signed-off-by: Nageswara Soma <nsoma@cisco.com>
diff --git a/bgpd/bgp_route.c b/bgpd/bgp_route.c
index 9e8749ea9d..0e40043ca1 100644
--- a/bgpd/bgp_route.c
+++ b/bgpd/bgp_route.c
@@ -4410,8 +4410,10 @@ void bgp_process_main_one(struct bgp *bgp, struct bgp_dest *dest, afi_t afi, saf
* we ignore the event for the prefix.
*/
if (BGP_INSTANCE_HIDDEN_DELETE_IN_PROGRESS(bgp, afi, safi)) {
- if (dest)
+ if (dest) {
debug = bgp_debug_bestpath(dest);
+ UNSET_FLAG(dest->flags, BGP_NODE_PROCESS_SCHEDULED);
+ }
if (debug) {
dv = bgp_dest_debug_vrf(dest);
zlog_debug(
@@ -4468,6 +4470,7 @@ void bgp_process_main_one(struct bgp *bgp, struct bgp_dest *dest, afi_t afi, saf
if (BGP_DEBUG(update, UPDATE_OUT))
zlog_debug("SELECT_DEFER flag set for route %p(%s)",
dest, bgp->name_pretty);
+ UNSET_FLAG(dest->flags, BGP_NODE_PROCESS_SCHEDULED);
return;
}
@@ -5470,6 +5473,8 @@ static void bgp_process_internal(struct bgp *bgp, struct bgp_dest *dest,
struct bgp_path_info *pi, afi_t afi,
safi_t safi, bool early_process)
{
+ int ret;
+
/*
* Indicate that *this* pi is in an unsorted
* situation, even if the node is already
@@ -5518,21 +5523,25 @@ static void bgp_process_internal(struct bgp *bgp, struct bgp_dest *dest,
early_process = true;
}
+ if (early_process)
+ ret = early_route_process(bgp, dest);
+ else
+ ret = other_route_process(bgp, dest);
+
+ /* Take the queue's references only once dest is actually enqueued. */
+ if (ret < 0)
+ return;
+
/* all unlocked in process_subq_xxx functions */
bgp_table_lock(bgp_dest_table(dest));
SET_FLAG(dest->flags, BGP_NODE_PROCESS_SCHEDULED);
bgp_dest_lock_node(dest);
- if (early_process) {
+ if (early_process)
SET_FLAG(dest->flags, BGP_NODE_ZEBRA_ANNOUNCE_EARLY);
- early_route_process(bgp, dest);
- } else {
+ else
UNSET_FLAG(dest->flags, BGP_NODE_ZEBRA_ANNOUNCE_EARLY);
- other_route_process(bgp, dest);
- }
-
- return;
}
void bgp_process(struct bgp *bgp, struct bgp_dest *dest,