Skip to content

Commit 3ed9af7

Browse files
samanvithabton31337
samanvithab
authored and
ton31337
committed
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- sonic-net#1 0x7f163e4b9259 in qcalloc lib/memory.c:105 ./bgp_local_asn_dot.test_bgp_local_asn_dot_agg/r3.bgpd.asan.3410444- sonic-net#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- sonic-net#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- sonic-net#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- sonic-net#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- sonic-net#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- sonic-net#7 0x7f163e3f219c in cmd_execute_command lib/command.c:1048 ./bgp_local_asn_dot.test_bgp_local_asn_dot_agg/r3.bgpd.asan.3410444- sonic-net#8 0x7f163e3f2df4 in cmd_execute lib/command.c:1215 ./bgp_local_asn_dot.test_bgp_local_asn_dot_agg/r3.bgpd.asan.3410444- sonic-net#9 0x7f163e5a2d73 in vty_command lib/vty.c:544 ./bgp_local_asn_dot.test_bgp_local_asn_dot_agg/r3.bgpd.asan.3410444- sonic-net#10 0x7f163e5a79c8 in vty_execute lib/vty.c:1307 ./bgp_local_asn_dot.test_bgp_local_asn_dot_agg/r3.bgpd.asan.3410444- sonic-net#11 0x7f163e5ad299 in vtysh_read lib/vty.c:2216 ./bgp_local_asn_dot.test_bgp_local_asn_dot_agg/r3.bgpd.asan.3410444- sonic-net#12 0x7f163e593f16 in event_call lib/event.c:1995 ./bgp_local_asn_dot.test_bgp_local_asn_dot_agg/r3.bgpd.asan.3410444- sonic-net#13 0x7f163e47c839 in frr_run lib/libfrr.c:1185 ./bgp_local_asn_dot.test_bgp_local_asn_dot_agg/r3.bgpd.asan.3410444- sonic-net#14 0x562bf414e58d in main bgpd/bgp_main.c:505 ./bgp_local_asn_dot.test_bgp_local_asn_dot_agg/r3.bgpd.asan.3410444- sonic-net#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- sonic-net#1 0x7f163e4b9259 in qcalloc lib/memory.c:105 ./bgp_local_asn_dot.test_bgp_local_asn_dot_agg/r3.bgpd.asan.3410444- sonic-net#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- sonic-net#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- sonic-net#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- sonic-net#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- sonic-net#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- sonic-net#7 0x7f163e3f219c in cmd_execute_command lib/command.c:1048 ./bgp_local_asn_dot.test_bgp_local_asn_dot_agg/r3.bgpd.asan.3410444- sonic-net#8 0x7f163e3f2df4 in cmd_execute lib/command.c:1215 ./bgp_local_asn_dot.test_bgp_local_asn_dot_agg/r3.bgpd.asan.3410444- sonic-net#9 0x7f163e5a2d73 in vty_command lib/vty.c:544 ./bgp_local_asn_dot.test_bgp_local_asn_dot_agg/r3.bgpd.asan.3410444- sonic-net#10 0x7f163e5a79c8 in vty_execute lib/vty.c:1307 ./bgp_local_asn_dot.test_bgp_local_asn_dot_agg/r3.bgpd.asan.3410444- sonic-net#11 0x7f163e5ad299 in vtysh_read lib/vty.c:2216 ./bgp_local_asn_dot.test_bgp_local_asn_dot_agg/r3.bgpd.asan.3410444- sonic-net#12 0x7f163e593f16 in event_call lib/event.c:1995 ./bgp_local_asn_dot.test_bgp_local_asn_dot_agg/r3.bgpd.asan.3410444- sonic-net#13 0x7f163e47c839 in frr_run lib/libfrr.c:1185 ./bgp_local_asn_dot.test_bgp_local_asn_dot_agg/r3.bgpd.asan.3410444- sonic-net#14 0x562bf414e58d in main bgpd/bgp_main.c:505 ./bgp_local_asn_dot.test_bgp_local_asn_dot_agg/r3.bgpd.asan.3410444- sonic-net#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]> (cherry picked from commit 7a70d99) Signed-off-by: Donatas Abraitis <[email protected]>
1 parent da0eee2 commit 3ed9af7

File tree

3 files changed

+76
-53
lines changed

3 files changed

+76
-53
lines changed

‎bgpd/bgp_route.c

+58-53
Original file line numberDiff line numberDiff line change
@@ -8322,59 +8322,7 @@ static int bgp_aggregate_unset(struct vty *vty, const char *prefix_str,
83228322
/* Unlock aggregate address configuration. */
83238323
bgp_dest_set_bgp_aggregate_info(dest, NULL);
83248324

8325-
if (aggregate->community)
8326-
community_free(&aggregate->community);
8327-
8328-
if (aggregate->community_hash) {
8329-
/* Delete all communities in the hash.
8330-
*/
8331-
hash_clean(aggregate->community_hash,
8332-
bgp_aggr_community_remove);
8333-
/* Free up the community_hash.
8334-
*/
8335-
hash_free(aggregate->community_hash);
8336-
}
8337-
8338-
if (aggregate->ecommunity)
8339-
ecommunity_free(&aggregate->ecommunity);
8340-
8341-
if (aggregate->ecommunity_hash) {
8342-
/* Delete all ecommunities in the hash.
8343-
*/
8344-
hash_clean(aggregate->ecommunity_hash,
8345-
bgp_aggr_ecommunity_remove);
8346-
/* Free up the ecommunity_hash.
8347-
*/
8348-
hash_free(aggregate->ecommunity_hash);
8349-
}
8350-
8351-
if (aggregate->lcommunity)
8352-
lcommunity_free(&aggregate->lcommunity);
8353-
8354-
if (aggregate->lcommunity_hash) {
8355-
/* Delete all lcommunities in the hash.
8356-
*/
8357-
hash_clean(aggregate->lcommunity_hash,
8358-
bgp_aggr_lcommunity_remove);
8359-
/* Free up the lcommunity_hash.
8360-
*/
8361-
hash_free(aggregate->lcommunity_hash);
8362-
}
8363-
8364-
if (aggregate->aspath)
8365-
aspath_free(aggregate->aspath);
8366-
8367-
if (aggregate->aspath_hash) {
8368-
/* Delete all as-paths in the hash.
8369-
*/
8370-
hash_clean(aggregate->aspath_hash,
8371-
bgp_aggr_aspath_remove);
8372-
/* Free up the aspath_hash.
8373-
*/
8374-
hash_free(aggregate->aspath_hash);
8375-
}
8376-
8377-
bgp_aggregate_free(aggregate);
8325+
bgp_free_aggregate_info(aggregate);
83788326
bgp_dest_unlock_node(dest);
83798327
bgp_dest_unlock_node(dest);
83808328

@@ -8555,6 +8503,63 @@ DEFPY(aggregate_addressv4, aggregate_addressv4_cmd,
85558503
match_med != NULL, suppress_map);
85568504
}
85578505

8506+
void bgp_free_aggregate_info(struct bgp_aggregate *aggregate)
8507+
{
8508+
if (aggregate->community)
8509+
community_free(&aggregate->community);
8510+
8511+
if (aggregate->community_hash) {
8512+
/* Delete all communities in the hash.
8513+
*/
8514+
hash_clean(aggregate->community_hash,
8515+
bgp_aggr_community_remove);
8516+
/* Free up the community_hash.
8517+
*/
8518+
hash_free(aggregate->community_hash);
8519+
}
8520+
8521+
if (aggregate->ecommunity)
8522+
ecommunity_free(&aggregate->ecommunity);
8523+
8524+
if (aggregate->ecommunity_hash) {
8525+
/* Delete all ecommunities in the hash.
8526+
*/
8527+
hash_clean(aggregate->ecommunity_hash,
8528+
bgp_aggr_ecommunity_remove);
8529+
/* Free up the ecommunity_hash.
8530+
*/
8531+
hash_free(aggregate->ecommunity_hash);
8532+
}
8533+
8534+
if (aggregate->lcommunity)
8535+
lcommunity_free(&aggregate->lcommunity);
8536+
8537+
if (aggregate->lcommunity_hash) {
8538+
/* Delete all lcommunities in the hash.
8539+
*/
8540+
hash_clean(aggregate->lcommunity_hash,
8541+
bgp_aggr_lcommunity_remove);
8542+
/* Free up the lcommunity_hash.
8543+
*/
8544+
hash_free(aggregate->lcommunity_hash);
8545+
}
8546+
8547+
if (aggregate->aspath)
8548+
aspath_free(aggregate->aspath);
8549+
8550+
if (aggregate->aspath_hash) {
8551+
/* Delete all as-paths in the hash.
8552+
*/
8553+
hash_clean(aggregate->aspath_hash,
8554+
bgp_aggr_aspath_remove);
8555+
/* Free up the aspath_hash.
8556+
*/
8557+
hash_free(aggregate->aspath_hash);
8558+
}
8559+
8560+
bgp_aggregate_free(aggregate);
8561+
}
8562+
85588563
DEFPY(aggregate_addressv6, aggregate_addressv6_cmd,
85598564
"[no] aggregate-address X:X::X:X/M$prefix [{"
85608565
"as-set$as_set_s"

‎bgpd/bgp_route.h

+1
Original file line numberDiff line numberDiff line change
@@ -674,6 +674,7 @@ extern void bgp_process_queue_init(struct bgp *bgp);
674674
extern void bgp_route_init(void);
675675
extern void bgp_route_finish(void);
676676
extern void bgp_cleanup_routes(struct bgp *);
677+
extern void bgp_free_aggregate_info(struct bgp_aggregate *aggregate);
677678
extern void bgp_announce_route(struct peer *peer, afi_t afi, safi_t safi,
678679
bool force);
679680
extern void bgp_stop_announce_route_timer(struct peer_af *paf);

‎bgpd/bgpd.c

+17
Original file line numberDiff line numberDiff line change
@@ -3801,6 +3801,23 @@ int bgp_delete(struct bgp *bgp)
38013801
#ifdef ENABLE_BGP_VNC
38023802
rfapi_delete(bgp);
38033803
#endif
3804+
3805+
/* Free memory allocated with aggregate address configuration. */
3806+
FOREACH_AFI_SAFI (afi, safi) {
3807+
struct bgp_aggregate *aggregate = NULL;
3808+
3809+
for (struct bgp_dest *dest =
3810+
bgp_table_top(bgp->aggregate[afi][safi]);
3811+
dest; dest = bgp_route_next(dest)) {
3812+
aggregate = bgp_dest_get_bgp_aggregate_info(dest);
3813+
if (aggregate == NULL)
3814+
continue;
3815+
3816+
bgp_dest_set_bgp_aggregate_info(dest, NULL);
3817+
bgp_free_aggregate_info(aggregate);
3818+
}
3819+
}
3820+
38043821
bgp_cleanup_routes(bgp);
38053822

38063823
for (afi = 0; afi < AFI_MAX; ++afi) {

0 commit comments

Comments
 (0)