From: Donald Sharp <sharpd@nvidia.com>
Date: Tue, 8 Nov 2022 19:38:02 +0000 (-0500)
Subject: bgpd: rpki was decrementing the node lock one time too many
X-Git-Tag: base_8.5~257^2
X-Git-Url: https://git.puffer.fish/?a=commitdiff_plain;h=31d0363ffc3c387204068d9b81c4f281a2116342;p=mirror%2Ffrr.git

bgpd: rpki was decrementing the node lock one time too many

The code was this:

1)        match = bgp_table_subtree_lookup(rrp->bgp->rib[rrp->afi][rrp->safi],
                                         &rrp->prefix);

2)        node = match;

        while (node) {
                if (bgp_dest_has_bgp_path_info_data(node)) {
                        revalidate_bgp_node(node, rrp->afi, rrp->safi);
                }

3)               node = bgp_route_next_until(node, match);
        }

	if (match)
4)              bgp_dest_unlock_node(match);

At 1) match was locked and became +1
At 2) match and node are now equal
At 3) On first iteration, match is decremented( as that node points
at it ) and the next item is locked, if it is found, and returned which becomes node

If 3 is run again because node is non-null then, current node is decremented
and the next node found is incremented and returned which becomes node again.

So if we get to 4) match is unlocked again which is now a double unlock
which, frankly, is not good.  In all code paths that I can see the
test for `if (match) ...` is not needed so let's just remove it.

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

diff --git a/bgpd/bgp_rpki.c b/bgpd/bgp_rpki.c
index d85bb93609..73c6fe0c47 100644
--- a/bgpd/bgp_rpki.c
+++ b/bgpd/bgp_rpki.c
@@ -401,6 +401,7 @@ static void rpki_revalidate_prefix(struct thread *thread)
 
 	match = bgp_table_subtree_lookup(rrp->bgp->rib[rrp->afi][rrp->safi],
 					 &rrp->prefix);
+
 	node = match;
 
 	while (node) {
@@ -411,9 +412,6 @@ static void rpki_revalidate_prefix(struct thread *thread)
 		node = bgp_route_next_until(node, match);
 	}
 
-	if (match)
-		bgp_dest_unlock_node(match);
-
 	XFREE(MTYPE_BGP_RPKI_REVALIDATE, rrp);
 }