From 188f352c1f88d2995a4eff86176648e544f88832 Mon Sep 17 00:00:00 2001 From: Philippe Guibert Date: Tue, 31 Dec 2024 14:42:17 +0100 Subject: [PATCH] bgpd: fix bgp evpn memory leaks when adj-rib-in is disabled Some bgp evpn memory contexts are not freed at the end of the bgp process. > ================================================================= > ==1208677==ERROR: LeakSanitizer: detected memory leaks > > Direct leak of 96 byte(s) in 2 object(s) allocated from: > #0 0x7f93ad4b4a57 in __interceptor_calloc ../../../../src/libsanitizer/asan/asan_malloc_linux.cpp:154 > #1 0x7f93ace77233 in qcalloc lib/memory.c:106 > #2 0x563bb68f4df1 in process_type5_route bgpd/bgp_evpn.c:5084 > #3 0x563bb68fb663 in bgp_nlri_parse_evpn bgpd/bgp_evpn.c:6302 > #4 0x563bb69ea2a9 in bgp_nlri_parse bgpd/bgp_packet.c:347 > #5 0x563bb69f7716 in bgp_update_receive bgpd/bgp_packet.c:2482 > #6 0x563bb6a04d3b in bgp_process_packet bgpd/bgp_packet.c:4091 > #7 0x7f93acf8082d in event_call lib/event.c:1996 > #8 0x7f93ace48931 in frr_run lib/libfrr.c:1232 > #9 0x563bb6880ae1 in main bgpd/bgp_main.c:557 > #10 0x7f93ac829d8f in __libc_start_call_main ../sysdeps/nptl/libc_start_call_main.h:58 Actually, the bgp evpn context may noy be used if adj rib in is unused. This may lead to memory leaks. Fix this by freeing the context in those corner cases. Signed-off-by: Philippe Guibert --- bgpd/bgp_route.c | 28 +++++++++++++++++----------- 1 file changed, 17 insertions(+), 11 deletions(-) diff --git a/bgpd/bgp_route.c b/bgpd/bgp_route.c index f27f0b97c089..1bc6ad6d4e1f 100644 --- a/bgpd/bgp_route.c +++ b/bgpd/bgp_route.c @@ -4910,6 +4910,7 @@ void bgp_update(struct peer *peer, const struct prefix *p, uint32_t addpath_id, bool force_evpn_import = false; safi_t orig_safi = safi; struct bgp_labels bgp_labels = {}; + struct bgp_route_evpn *p_evpn = evpn; uint8_t i; if (frrtrace_enabled(frr_bgp, process_update)) { @@ -4951,11 +4952,9 @@ void bgp_update(struct peer *peer, const struct prefix *p, uint32_t addpath_id, * will not be interned. In which case, it is ok to update the * attr->evpn_overlay, so that, this can be stored in adj_in. */ - if (evpn) { - if (afi == AFI_L2VPN) - bgp_attr_set_evpn_overlay(attr, evpn); - else - evpn_overlay_free(evpn); + if (evpn && afi == AFI_L2VPN) { + bgp_attr_set_evpn_overlay(attr, evpn); + p_evpn = NULL; } bgp_adj_in_set(dest, peer, attr, addpath_id, &bgp_labels); } @@ -5128,11 +5127,9 @@ void bgp_update(struct peer *peer, const struct prefix *p, uint32_t addpath_id, * attr->evpn_overlay with evpn directly. Instead memcpy * evpn to new_atr.evpn_overlay before it is interned. */ - if (soft_reconfig && evpn) { - if (afi == AFI_L2VPN) - bgp_attr_set_evpn_overlay(&new_attr, evpn); - else - evpn_overlay_free(evpn); + if (soft_reconfig && evpn && afi == AFI_L2VPN) { + bgp_attr_set_evpn_overlay(&new_attr, evpn); + p_evpn = NULL; } /* Apply incoming route-map. @@ -5301,7 +5298,8 @@ void bgp_update(struct peer *peer, const struct prefix *p, uint32_t addpath_id, bgp_dest_unlock_node(dest); bgp_attr_unintern(&attr_new); - + if (p_evpn) + evpn_overlay_free(p_evpn); return; } @@ -5466,6 +5464,8 @@ void bgp_update(struct peer *peer, const struct prefix *p, uint32_t addpath_id, ret = bgp_damp_update(pi, dest, afi, safi); if (ret == BGP_DAMP_SUPPRESSED) { bgp_dest_unlock_node(dest); + if (p_evpn) + evpn_overlay_free(p_evpn); return; } } @@ -5552,6 +5552,8 @@ void bgp_update(struct peer *peer, const struct prefix *p, uint32_t addpath_id, type, sub_type, NULL); } #endif + if (p_evpn) + evpn_overlay_free(p_evpn); return; } // End of implicit withdraw @@ -5646,6 +5648,8 @@ void bgp_update(struct peer *peer, const struct prefix *p, uint32_t addpath_id, } #endif + if (p_evpn) + evpn_overlay_free(p_evpn); return; /* This BGP update is filtered. Log the reason then update BGP @@ -5709,6 +5713,8 @@ void bgp_update(struct peer *peer, const struct prefix *p, uint32_t addpath_id, } #endif + if (p_evpn) + evpn_overlay_free(p_evpn); return; }