Skip to content

Commit

Permalink
bgpd : aggregate-address memory leak fix
Browse files Browse the repository at this point in the history 
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-    FRRouting#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-    FRRouting#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-    FRRouting#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-    FRRouting#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-    FRRouting#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-    FRRouting#7 0x7f163e3f219c in cmd_execute_command lib/command.c:1048
    ./bgp_local_asn_dot.test_bgp_local_asn_dot_agg/r3.bgpd.asan.3410444-    FRRouting#8 0x7f163e3f2df4 in cmd_execute lib/command.c:1215
    ./bgp_local_asn_dot.test_bgp_local_asn_dot_agg/r3.bgpd.asan.3410444-    FRRouting#9 0x7f163e5a2d73 in vty_command lib/vty.c:544
    ./bgp_local_asn_dot.test_bgp_local_asn_dot_agg/r3.bgpd.asan.3410444-    FRRouting#10 0x7f163e5a79c8 in vty_execute lib/vty.c:1307
    ./bgp_local_asn_dot.test_bgp_local_asn_dot_agg/r3.bgpd.asan.3410444-    FRRouting#11 0x7f163e5ad299 in vtysh_read lib/vty.c:2216
    ./bgp_local_asn_dot.test_bgp_local_asn_dot_agg/r3.bgpd.asan.3410444-    FRRouting#12 0x7f163e593f16 in event_call lib/event.c:1995
    ./bgp_local_asn_dot.test_bgp_local_asn_dot_agg/r3.bgpd.asan.3410444-    FRRouting#13 0x7f163e47c839 in frr_run lib/libfrr.c:1185
    ./bgp_local_asn_dot.test_bgp_local_asn_dot_agg/r3.bgpd.asan.3410444-    FRRouting#14 0x562bf414e58d in main bgpd/bgp_main.c:505
    ./bgp_local_asn_dot.test_bgp_local_asn_dot_agg/r3.bgpd.asan.3410444-    FRRouting#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-    FRRouting#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-    FRRouting#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-    FRRouting#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-    FRRouting#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-    FRRouting#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-    FRRouting#7 0x7f163e3f219c in cmd_execute_command lib/command.c:1048
    ./bgp_local_asn_dot.test_bgp_local_asn_dot_agg/r3.bgpd.asan.3410444-    FRRouting#8 0x7f163e3f2df4 in cmd_execute lib/command.c:1215
    ./bgp_local_asn_dot.test_bgp_local_asn_dot_agg/r3.bgpd.asan.3410444-    FRRouting#9 0x7f163e5a2d73 in vty_command lib/vty.c:544
    ./bgp_local_asn_dot.test_bgp_local_asn_dot_agg/r3.bgpd.asan.3410444-    FRRouting#10 0x7f163e5a79c8 in vty_execute lib/vty.c:1307
    ./bgp_local_asn_dot.test_bgp_local_asn_dot_agg/r3.bgpd.asan.3410444-    FRRouting#11 0x7f163e5ad299 in vtysh_read lib/vty.c:2216
    ./bgp_local_asn_dot.test_bgp_local_asn_dot_agg/r3.bgpd.asan.3410444-    FRRouting#12 0x7f163e593f16 in event_call lib/event.c:1995
    ./bgp_local_asn_dot.test_bgp_local_asn_dot_agg/r3.bgpd.asan.3410444-    FRRouting#13 0x7f163e47c839 in frr_run lib/libfrr.c:1185
    ./bgp_local_asn_dot.test_bgp_local_asn_dot_agg/r3.bgpd.asan.3410444-    FRRouting#14 0x562bf414e58d in main bgpd/bgp_main.c:505
    ./bgp_local_asn_dot.test_bgp_local_asn_dot_agg/r3.bgpd.asan.3410444-    FRRouting#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 7a70d99)
Signed-off-by: Donatas Abraitis <donatas@opensourcerouting.org>
  • Loading branch information
@samanvithab @ton31337
samanvithab authored and ton31337 committed Mar 31, 2023
1 parent bc8baaf commit 8e64171
Show file tree
Hide file tree
Showing 3 changed files with 76 additions and 53 deletions.
111 changes: 58 additions & 53 deletions bgpd/bgp_route.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand Down Expand Up @@ -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"
Expand Down
1 change: 1 addition & 0 deletions bgpd/bgp_route.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
17 changes: 17 additions & 0 deletions bgpd/bgpd.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down

0 comments on commit 8e64171

Please sign in to comment.