bgpd: evpn_cleanup_local_non_best_route could free dest

But never really does due to locking, but since it can
we need to treat it like it does and ensure that FRR
is not making a mistake, by using memory after it
has been freed.

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

diff --git a/bgpd/bgp_evpn.c b/bgpd/bgp_evpn.c
index e5d33e6d598049060102ea1b64351920b0639942..ff931e1f071f7ef0f21bedab1c3b0eec5fd4b9d1 100644 (file)
--- a/bgpd/bgp_evpn.c
+++ b/bgpd/bgp_evpn.c
@@ -2035,10 +2035,10 @@ static void evpn_zebra_reinstall_best_route(struct bgp *bgp,
  * additional handling to prevent bgp from injecting and holding on to a
  * non-best local path.
  */
-static void evpn_cleanup_local_non_best_route(struct bgp *bgp,
-                                             struct bgpevpn *vpn,
-                                             struct bgp_dest *dest,
-                                             struct bgp_path_info *local_pi)
+static struct bgp_dest *
+evpn_cleanup_local_non_best_route(struct bgp *bgp, struct bgpevpn *vpn,
+                                 struct bgp_dest *dest,
+                                 struct bgp_path_info *local_pi)
 {
        /* local path was not picked as the winner; kick it out */
        if (bgp_debug_zebra(NULL))
@@ -2046,10 +2046,11 @@ static void evpn_cleanup_local_non_best_route(struct bgp *bgp,
                           dest);
 
        evpn_delete_old_local_route(bgp, vpn, dest, local_pi, NULL);
-       bgp_path_info_reap(dest, local_pi);
 
        /* tell zebra to re-add the best remote path */
        evpn_zebra_reinstall_best_route(bgp, vpn, dest);
+
+       return bgp_path_info_reap(dest, local_pi);
 }
 
 static inline bool bgp_evpn_route_add_l3_ecomm_ok(struct bgpevpn *vpn,
@@ -2186,7 +2187,8 @@ static int update_evpn_route(struct bgp *bgp, struct bgpevpn *vpn,
        } else {
                if (!CHECK_FLAG(pi->flags, BGP_PATH_SELECTED)) {
                        route_change = 0;
-                       evpn_cleanup_local_non_best_route(bgp, vpn, dest, pi);
+                       dest = evpn_cleanup_local_non_best_route(bgp, vpn, dest,
+                                                                pi);
                } else {
                        bool new_is_sync;
 
@@ -2203,7 +2205,8 @@ static int update_evpn_route(struct bgp *bgp, struct bgpevpn *vpn,
        }
        bgp_path_info_unlock(pi);
 
-       bgp_dest_unlock_node(dest);
+       if (dest)
+               bgp_dest_unlock_node(dest);
 
        /* If this is a new route or some attribute has changed, export the
         * route to the global table. The route will be advertised to peers

diff --git a/bgpd/bgp_route.c b/bgpd/bgp_route.c
index 1832d55ec0f1beab44bdc7c8c650d53c4d3782cb..f769071185afc374abdcb60a96e4ea30237fe061 100644 (file)
--- a/bgpd/bgp_route.c
+++ b/bgpd/bgp_route.c
@@ -438,7 +438,8 @@ void bgp_path_info_add_with_caller(const char *name, struct bgp_dest *dest,
 
 /* Do the actual removal of info from RIB, for use by bgp_process
    completion callback *only* */
-void bgp_path_info_reap(struct bgp_dest *dest, struct bgp_path_info *pi)
+struct bgp_dest *bgp_path_info_reap(struct bgp_dest *dest,
+                                   struct bgp_path_info *pi)
 {
        if (pi->next)
                pi->next->prev = pi->prev;
@@ -450,7 +451,8 @@ void bgp_path_info_reap(struct bgp_dest *dest, struct bgp_path_info *pi)
        bgp_path_info_mpath_dequeue(pi);
        bgp_path_info_unlock(pi);
        hook_call(bgp_snmp_update_stats, dest, pi, false);
-       bgp_dest_unlock_node(dest);
+
+       return bgp_dest_unlock_node(dest);
 }
 
 void bgp_path_info_delete(struct bgp_dest *dest, struct bgp_path_info *pi)

diff --git a/bgpd/bgp_route.h b/bgpd/bgp_route.h
index e001bf4f07b2a9cba8483df3d9f6a9c7a95d3ad2..cc13500fa123822f75e265d0abedf2921f84c955 100644 (file)
--- a/bgpd/bgp_route.h
+++ b/bgpd/bgp_route.h
@@ -727,7 +727,8 @@ extern struct bgp_path_info *
 bgp_get_imported_bpi_ultimate(struct bgp_path_info *info);
 extern void bgp_path_info_add(struct bgp_dest *dest, struct bgp_path_info *pi);
 extern void bgp_path_info_extra_free(struct bgp_path_info_extra **extra);
-extern void bgp_path_info_reap(struct bgp_dest *dest, struct bgp_path_info *pi);
+extern struct bgp_dest *bgp_path_info_reap(struct bgp_dest *dest,
+                                          struct bgp_path_info *pi);
 extern void bgp_path_info_delete(struct bgp_dest *dest,
                                 struct bgp_path_info *pi);
 extern struct bgp_path_info_extra *