]> git.puffer.fish Git - mirror/frr.git/commitdiff
bgpd: rpki was decrementing the node lock one time too many 12287/head
authorDonald Sharp <sharpd@nvidia.com>
Tue, 8 Nov 2022 19:38:02 +0000 (14:38 -0500)
committerDonald Sharp <sharpd@nvidia.com>
Tue, 8 Nov 2022 19:47:00 +0000 (14:47 -0500)
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>
bgpd/bgp_rpki.c

index d85bb93609a4e3353cc12b3e32dbfb04f0bff68d..73c6fe0c4757fa27549e850a7822dfe38eaf17e9 100644 (file)
@@ -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);
 }