bgpd : aggregate-address memory leak fix

Memory leaks are observed in the cleanup code. When “no router bgp" is executed,
cleanup in that flow for aggregate-address command is not taken care.

fixes the below leak:
    --
    ./bgp_local_asn_dot.test_bgp_local_asn_dot_agg/r3.bgpd.asan.3410444:Direct leak of 152 byte(s) in 1 object(s) allocated from:
    ./bgp_local_asn_dot.test_bgp_local_asn_dot_agg/r3.bgpd.asan.3410444-    #0 0x7f163e911037 in __interceptor_calloc ../../../../src/libsanitizer/asan/asan_malloc_linux.cpp:154
    ./bgp_local_asn_dot.test_bgp_local_asn_dot_agg/r3.bgpd.asan.3410444-    #1 0x7f163e4b9259 in qcalloc lib/memory.c:105
    ./bgp_local_asn_dot.test_bgp_local_asn_dot_agg/r3.bgpd.asan.3410444-    #2 0x562bf42ebbd5 in bgp_aggregate_new bgpd/bgp_route.c:7239
    ./bgp_local_asn_dot.test_bgp_local_asn_dot_agg/r3.bgpd.asan.3410444-    #3 0x562bf42f14e8 in bgp_aggregate_set bgpd/bgp_route.c:8421
    ./bgp_local_asn_dot.test_bgp_local_asn_dot_agg/r3.bgpd.asan.3410444-    #4 0x562bf42f1e55 in aggregate_addressv6_magic bgpd/bgp_route.c:8592
    ./bgp_local_asn_dot.test_bgp_local_asn_dot_agg/r3.bgpd.asan.3410444-    #5 0x562bf42be3f5 in aggregate_addressv6 bgpd/bgp_route_clippy.c:341
    ./bgp_local_asn_dot.test_bgp_local_asn_dot_agg/r3.bgpd.asan.3410444-    #6 0x7f163e3f1e1b in cmd_execute_command_real lib/command.c:988
    ./bgp_local_asn_dot.test_bgp_local_asn_dot_agg/r3.bgpd.asan.3410444-    #7 0x7f163e3f219c in cmd_execute_command lib/command.c:1048
    ./bgp_local_asn_dot.test_bgp_local_asn_dot_agg/r3.bgpd.asan.3410444-    #8 0x7f163e3f2df4 in cmd_execute lib/command.c:1215
    ./bgp_local_asn_dot.test_bgp_local_asn_dot_agg/r3.bgpd.asan.3410444-    #9 0x7f163e5a2d73 in vty_command lib/vty.c:544
    ./bgp_local_asn_dot.test_bgp_local_asn_dot_agg/r3.bgpd.asan.3410444-    #10 0x7f163e5a79c8 in vty_execute lib/vty.c:1307
    ./bgp_local_asn_dot.test_bgp_local_asn_dot_agg/r3.bgpd.asan.3410444-    #11 0x7f163e5ad299 in vtysh_read lib/vty.c:2216
    ./bgp_local_asn_dot.test_bgp_local_asn_dot_agg/r3.bgpd.asan.3410444-    #12 0x7f163e593f16 in event_call lib/event.c:1995
    ./bgp_local_asn_dot.test_bgp_local_asn_dot_agg/r3.bgpd.asan.3410444-    #13 0x7f163e47c839 in frr_run lib/libfrr.c:1185
    ./bgp_local_asn_dot.test_bgp_local_asn_dot_agg/r3.bgpd.asan.3410444-    #14 0x562bf414e58d in main bgpd/bgp_main.c:505
    ./bgp_local_asn_dot.test_bgp_local_asn_dot_agg/r3.bgpd.asan.3410444-    #15 0x7f163de66d09 in __libc_start_main ../csu/libc-start.c:308
    ./bgp_local_asn_dot.test_bgp_local_asn_dot_agg/r3.bgpd.asan.3410444-
    ./bgp_local_asn_dot.test_bgp_local_asn_dot_agg/r3.bgpd.asan.3410444:Direct leak of 152 byte(s) in 1 object(s) allocated from:
    ./bgp_local_asn_dot.test_bgp_local_asn_dot_agg/r3.bgpd.asan.3410444-    #0 0x7f163e911037 in __interceptor_calloc ../../../../src/libsanitizer/asan/asan_malloc_linux.cpp:154
    ./bgp_local_asn_dot.test_bgp_local_asn_dot_agg/r3.bgpd.asan.3410444-    #1 0x7f163e4b9259 in qcalloc lib/memory.c:105
    ./bgp_local_asn_dot.test_bgp_local_asn_dot_agg/r3.bgpd.asan.3410444-    #2 0x562bf42ebbd5 in bgp_aggregate_new bgpd/bgp_route.c:7239
    ./bgp_local_asn_dot.test_bgp_local_asn_dot_agg/r3.bgpd.asan.3410444-    #3 0x562bf42f14e8 in bgp_aggregate_set bgpd/bgp_route.c:8421
    ./bgp_local_asn_dot.test_bgp_local_asn_dot_agg/r3.bgpd.asan.3410444-    #4 0x562bf42f1cde in aggregate_addressv4_magic bgpd/bgp_route.c:8543
    ./bgp_local_asn_dot.test_bgp_local_asn_dot_agg/r3.bgpd.asan.3410444-    #5 0x562bf42bd258 in aggregate_addressv4 bgpd/bgp_route_clippy.c:255
    ./bgp_local_asn_dot.test_bgp_local_asn_dot_agg/r3.bgpd.asan.3410444-    #6 0x7f163e3f1e1b in cmd_execute_command_real lib/command.c:988
    ./bgp_local_asn_dot.test_bgp_local_asn_dot_agg/r3.bgpd.asan.3410444-    #7 0x7f163e3f219c in cmd_execute_command lib/command.c:1048
    ./bgp_local_asn_dot.test_bgp_local_asn_dot_agg/r3.bgpd.asan.3410444-    #8 0x7f163e3f2df4 in cmd_execute lib/command.c:1215
    ./bgp_local_asn_dot.test_bgp_local_asn_dot_agg/r3.bgpd.asan.3410444-    #9 0x7f163e5a2d73 in vty_command lib/vty.c:544
    ./bgp_local_asn_dot.test_bgp_local_asn_dot_agg/r3.bgpd.asan.3410444-    #10 0x7f163e5a79c8 in vty_execute lib/vty.c:1307
    ./bgp_local_asn_dot.test_bgp_local_asn_dot_agg/r3.bgpd.asan.3410444-    #11 0x7f163e5ad299 in vtysh_read lib/vty.c:2216
    ./bgp_local_asn_dot.test_bgp_local_asn_dot_agg/r3.bgpd.asan.3410444-    #12 0x7f163e593f16 in event_call lib/event.c:1995
    ./bgp_local_asn_dot.test_bgp_local_asn_dot_agg/r3.bgpd.asan.3410444-    #13 0x7f163e47c839 in frr_run lib/libfrr.c:1185
    ./bgp_local_asn_dot.test_bgp_local_asn_dot_agg/r3.bgpd.asan.3410444-    #14 0x562bf414e58d in main bgpd/bgp_main.c:505
    ./bgp_local_asn_dot.test_bgp_local_asn_dot_agg/r3.bgpd.asan.3410444-    #15 0x7f163de66d09 in __libc_start_main ../csu/libc-start.c:308
    ./bgp_local_asn_dot.test_bgp_local_asn_dot_agg/r3.bgpd.asan.3410444-
    ./bgp_local_asn_dot.test_bgp_local_asn_dot_agg/r3.bgpd.asan.3410444-SUMMARY: AddressSanitizer: 304 byte(s) leaked in 2 allocation(s).

Signed-off-by: Samanvitha B Bhargav <bsamanvitha@vmware.com>
(cherry picked from commit 7a70d99038a67f693c40d4f1c03027a2b05bfaf8)
Signed-off-by: Donatas Abraitis <donatas@opensourcerouting.org>

diff --git a/bgpd/bgp_route.c b/bgpd/bgp_route.c
index 4ba9ef97162d70e3b7319ec04f6829a2ee3dacf8..37577cac3ad35536528c37d11bc87879e9cd82e3 100644 (file)
--- a/bgpd/bgp_route.c
+++ b/bgpd/bgp_route.c
@@ -8197,59 +8197,7 @@ static int bgp_aggregate_unset(struct vty *vty, const char *prefix_str,
        /* Unlock aggregate address configuration. */
        bgp_dest_set_bgp_aggregate_info(dest, NULL);
 
-       if (aggregate->community)
-               community_free(&aggregate->community);
-
-       if (aggregate->community_hash) {
-               /* Delete all communities in the hash.
-                */
-               hash_clean(aggregate->community_hash,
-                          bgp_aggr_community_remove);
-               /* Free up the community_hash.
-                */
-               hash_free(aggregate->community_hash);
-       }
-
-       if (aggregate->ecommunity)
-               ecommunity_free(&aggregate->ecommunity);
-
-       if (aggregate->ecommunity_hash) {
-               /* Delete all ecommunities in the hash.
-                */
-               hash_clean(aggregate->ecommunity_hash,
-                          bgp_aggr_ecommunity_remove);
-               /* Free up the ecommunity_hash.
-                */
-               hash_free(aggregate->ecommunity_hash);
-       }
-
-       if (aggregate->lcommunity)
-               lcommunity_free(&aggregate->lcommunity);
-
-       if (aggregate->lcommunity_hash) {
-               /* Delete all lcommunities in the hash.
-                */
-               hash_clean(aggregate->lcommunity_hash,
-                          bgp_aggr_lcommunity_remove);
-               /* Free up the lcommunity_hash.
-                */
-               hash_free(aggregate->lcommunity_hash);
-       }
-
-       if (aggregate->aspath)
-               aspath_free(aggregate->aspath);
-
-       if (aggregate->aspath_hash) {
-               /* Delete all as-paths in the hash.
-                */
-               hash_clean(aggregate->aspath_hash,
-                          bgp_aggr_aspath_remove);
-               /* Free up the aspath_hash.
-                */
-               hash_free(aggregate->aspath_hash);
-       }
-
-       bgp_aggregate_free(aggregate);
+       bgp_free_aggregate_info(aggregate);
        bgp_dest_unlock_node(dest);
        bgp_dest_unlock_node(dest);
 
@@ -8430,6 +8378,63 @@ DEFPY(aggregate_addressv4, aggregate_addressv4_cmd,
                                 match_med != NULL, suppress_map);
 }
 
+void bgp_free_aggregate_info(struct bgp_aggregate *aggregate)
+{
+       if (aggregate->community)
+               community_free(&aggregate->community);
+
+       if (aggregate->community_hash) {
+               /* Delete all communities in the hash.
+                */
+               hash_clean(aggregate->community_hash,
+                          bgp_aggr_community_remove);
+               /* Free up the community_hash.
+                */
+               hash_free(aggregate->community_hash);
+       }
+
+       if (aggregate->ecommunity)
+               ecommunity_free(&aggregate->ecommunity);
+
+       if (aggregate->ecommunity_hash) {
+               /* Delete all ecommunities in the hash.
+                */
+               hash_clean(aggregate->ecommunity_hash,
+                          bgp_aggr_ecommunity_remove);
+               /* Free up the ecommunity_hash.
+                */
+               hash_free(aggregate->ecommunity_hash);
+       }
+
+       if (aggregate->lcommunity)
+               lcommunity_free(&aggregate->lcommunity);
+
+       if (aggregate->lcommunity_hash) {
+               /* Delete all lcommunities in the hash.
+                */
+               hash_clean(aggregate->lcommunity_hash,
+                          bgp_aggr_lcommunity_remove);
+               /* Free up the lcommunity_hash.
+                */
+               hash_free(aggregate->lcommunity_hash);
+       }
+
+       if (aggregate->aspath)
+               aspath_free(aggregate->aspath);
+
+       if (aggregate->aspath_hash) {
+               /* Delete all as-paths in the hash.
+                */
+               hash_clean(aggregate->aspath_hash,
+                          bgp_aggr_aspath_remove);
+               /* Free up the aspath_hash.
+                */
+               hash_free(aggregate->aspath_hash);
+       }
+
+       bgp_aggregate_free(aggregate);
+}
+
 DEFPY(aggregate_addressv6, aggregate_addressv6_cmd,
       "[no] aggregate-address X:X::X:X/M$prefix [{"
       "as-set$as_set_s"

diff --git a/bgpd/bgp_route.h b/bgpd/bgp_route.h
index 1107e3e33e74062affa5c349ddc3915a10854fda..e776ae4b51c3fccd2066c0514ff13502678d58a4 100644 (file)
--- a/bgpd/bgp_route.h
+++ b/bgpd/bgp_route.h
@@ -667,6 +667,7 @@ extern void bgp_process_queue_init(struct bgp *bgp);
 extern void bgp_route_init(void);
 extern void bgp_route_finish(void);
 extern void bgp_cleanup_routes(struct bgp *);
+extern void bgp_free_aggregate_info(struct bgp_aggregate *aggregate);
 extern void bgp_announce_route(struct peer *peer, afi_t afi, safi_t safi,
                               bool force);
 extern void bgp_stop_announce_route_timer(struct peer_af *paf);

diff --git a/bgpd/bgpd.c b/bgpd/bgpd.c
index eaeed6c6000b863ad4305415a90fe72741c2c0c6..3dbd373f1225a2337e4bbfbc7ac6917b3e7ce821 100644 (file)
--- a/bgpd/bgpd.c
+++ b/bgpd/bgpd.c
@@ -3724,6 +3724,23 @@ int bgp_delete(struct bgp *bgp)
 #ifdef ENABLE_BGP_VNC
        rfapi_delete(bgp);
 #endif
+
+       /* Free memory allocated with aggregate address configuration. */
+       FOREACH_AFI_SAFI (afi, safi) {
+               struct bgp_aggregate *aggregate = NULL;
+
+               for (struct bgp_dest *dest =
+                            bgp_table_top(bgp->aggregate[afi][safi]);
+                    dest; dest = bgp_route_next(dest)) {
+                       aggregate = bgp_dest_get_bgp_aggregate_info(dest);
+                       if (aggregate == NULL)
+                               continue;
+
+                       bgp_dest_set_bgp_aggregate_info(dest, NULL);
+                       bgp_free_aggregate_info(aggregate);
+               }
+       }
+
        bgp_cleanup_routes(bgp);
 
        for (afi = 0; afi < AFI_MAX; ++afi) {