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-    #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-    #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 <[email protected]>
  • Loading branch information
@samanvithab
samanvithab committed Mar 30, 2023
1 parent 6baf6b1 commit 7a70d99
Show file tree
Hide file tree
Showing 3 changed files with 47 additions and 24 deletions.
53 changes: 29 additions & 24 deletions bgpd/bgp_route.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8338,30 +8338,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);

hash_clean_and_free(&aggregate->community_hash,
bgp_aggr_community_remove);

if (aggregate->ecommunity)
ecommunity_free(&aggregate->ecommunity);

hash_clean_and_free(&aggregate->ecommunity_hash,
bgp_aggr_ecommunity_remove);

if (aggregate->lcommunity)
lcommunity_free(&aggregate->lcommunity);

hash_clean_and_free(&aggregate->lcommunity_hash,
bgp_aggr_lcommunity_remove);

if (aggregate->aspath)
aspath_free(aggregate->aspath);

hash_clean_and_free(&aggregate->aspath_hash, bgp_aggr_aspath_remove);

bgp_aggregate_free(aggregate);
bgp_free_aggregate_info(aggregate);
bgp_dest_unlock_node(dest);
bgp_dest_unlock_node(dest);

Expand Down Expand Up @@ -8545,6 +8522,34 @@ 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);

hash_clean_and_free(&aggregate->community_hash,
bgp_aggr_community_remove);

if (aggregate->ecommunity)
ecommunity_free(&aggregate->ecommunity);

hash_clean_and_free(&aggregate->ecommunity_hash,
bgp_aggr_ecommunity_remove);

if (aggregate->lcommunity)
lcommunity_free(&aggregate->lcommunity);

hash_clean_and_free(&aggregate->lcommunity_hash,
bgp_aggr_lcommunity_remove);

if (aggregate->aspath)
aspath_free(aggregate->aspath);

hash_clean_and_free(&aggregate->aspath_hash, bgp_aggr_aspath_remove);

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 @@ -661,6 +661,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 @@ -3844,6 +3844,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 7a70d99

Please sign in to comment.