From a7fe30e299773379ea5f9e6b5ebfff9303f62d8f Mon Sep 17 00:00:00 2001 From: Keelan10 Date: Thu, 7 Sep 2023 23:03:04 +0400 Subject: [PATCH] lib: Free Temporary Memory for AF_FLOWSPEC Prefix This commit addresses a leak where temporary memory allocated earlier by the `prefix_copy` function for AF_FLOWSPEC prefixes was not being freed. To ensure proper memory management, we now release this temporary memory by calling `prefix_flowspec_ptr_free`. The ASan leak log for reference: ``` *********************************************************************************** Address Sanitizer Error detected in bgp_flowspec.test_bgp_flowspec_topo/r1.asan.bgpd.11539 ================================================================= ==11539==ERROR: LeakSanitizer: detected memory leaks Direct leak of 56 byte(s) in 2 object(s) allocated from: #0 0x7feaa956ad28 in __interceptor_calloc (/usr/lib/x86_64-linux-gnu/libasan.so.4+0xded28) #1 0x7feaa8f670da in qcalloc lib/memory.c:105 #2 0x7feaa8fac1d4 in prefix_copy lib/prefix.c:346 #3 0x7feaa8ff43e8 in route_node_get lib/table.c:274 #4 0x56247cc798c0 in bgp_node_get bgpd/bgp_table.h:236 #5 0x56247cc798c0 in bgp_afi_node_get bgpd/bgp_route.c:145 #6 0x56247cc92622 in bgp_update bgpd/bgp_route.c:4188 #7 0x56247ce55b21 in bgp_nlri_parse_flowspec bgpd/bgp_flowspec.c:193 #8 0x56247cc4cdd8 in bgp_nlri_parse bgpd/bgp_packet.c:350 #9 0x56247cc4f37c in bgp_update_receive bgpd/bgp_packet.c:2153 #10 0x56247cc591e2 in bgp_process_packet bgpd/bgp_packet.c:3214 #11 0x7feaa9005b99 in event_call lib/event.c:1979 #12 0x7feaa8f4a379 in frr_run lib/libfrr.c:1213 #13 0x56247cb51b21 in main bgpd/bgp_main.c:510 #14 0x7feaa7f8dc86 in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x21c86) SUMMARY: AddressSanitizer: 56 byte(s) leaked in 2 allocation(s). *********************************************************************************** ``` Signed-off-by: Keelan Cannoo --- bgpd/bgp_table.c | 2 ++ lib/prefix.c | 16 ++++++++++++---- lib/prefix.h | 1 + lib/table.c | 7 +++++++ 4 files changed, 22 insertions(+), 4 deletions(-) diff --git a/bgpd/bgp_table.c b/bgpd/bgp_table.c index e01bf3911349..a64477047c3a 100644 --- a/bgpd/bgp_table.c +++ b/bgpd/bgp_table.c @@ -119,6 +119,8 @@ static void bgp_node_destroy(route_table_delegate_t *delegate, if (family2afi(node->p.family) == AFI_LINKSTATE) prefix_linkstate_ptr_free(&node->p); + else if (node->p.family == AF_FLOWSPEC) + prefix_flowspec_ptr_free(&node->p); XFREE(MTYPE_ROUTE_NODE, node); } diff --git a/lib/prefix.c b/lib/prefix.c index cde8677cf086..092581aeb836 100644 --- a/lib/prefix.c +++ b/lib/prefix.c @@ -1304,6 +1304,17 @@ void prefix_linkstate_ptr_free(struct prefix *p) p->u.prefix_linkstate.ptr = (uintptr_t)NULL; } +void prefix_flowspec_ptr_free(struct prefix *p) +{ + void *temp; + + if (!p || p->family != AF_FLOWSPEC || !p->u.prefix_flowspec.ptr) + return; + + temp = (void *)p->u.prefix_flowspec.ptr; + XFREE(MTYPE_PREFIX_FLOWSPEC, temp); + p->u.prefix_flowspec.ptr = (uintptr_t)NULL; +} struct prefix *prefix_new(void) { @@ -1455,7 +1466,6 @@ unsigned prefix_hash_key(const void *pp) { struct prefix copy; uint32_t len; - void *temp; /* make sure *all* unused bits are zero, particularly including * alignment / @@ -1467,9 +1477,7 @@ unsigned prefix_hash_key(const void *pp) len = jhash((void *)copy.u.prefix_flowspec.ptr, copy.u.prefix_flowspec.prefixlen, 0x55aa5a5a); - temp = (void *)copy.u.prefix_flowspec.ptr; - XFREE(MTYPE_PREFIX_FLOWSPEC, temp); - copy.u.prefix_flowspec.ptr = (uintptr_t)NULL; + prefix_flowspec_ptr_free(©); return len; } else if (((struct prefix *)pp)->family == AF_LINKSTATE) { len = jhash((void *)copy.u.prefix_linkstate.ptr, copy.prefixlen, diff --git a/lib/prefix.h b/lib/prefix.h index f1aff436899c..c8dc9d7d8bc9 100644 --- a/lib/prefix.h +++ b/lib/prefix.h @@ -438,6 +438,7 @@ static inline afi_t prefix_afi(union prefixconstptr pu) extern unsigned int prefix_bit(const uint8_t *prefix, const uint16_t bit_index); extern void prefix_linkstate_ptr_free(struct prefix *p); +extern void prefix_flowspec_ptr_free(struct prefix *p); extern struct prefix *prefix_new(void); extern void prefix_free(struct prefix **p); diff --git a/lib/table.c b/lib/table.c index dbfc3f8b915d..6af30ba211bb 100644 --- a/lib/table.c +++ b/lib/table.c @@ -284,6 +284,8 @@ struct route_node *route_node_get(struct route_table *table, if (node && node->info) { if (family2afi(p->family) == AFI_LINKSTATE) prefix_linkstate_ptr_free(p); + else if (p->family == AF_FLOWSPEC) + prefix_flowspec_ptr_free(p); return route_lock_node(node); } @@ -295,6 +297,9 @@ struct route_node *route_node_get(struct route_table *table, if (node->p.prefixlen == prefixlen) { if (family2afi(p->family) == AFI_LINKSTATE) prefix_linkstate_ptr_free(p); + else if (p->family == AF_FLOWSPEC) + prefix_flowspec_ptr_free(p); + return route_lock_node(node); } @@ -333,6 +338,8 @@ struct route_node *route_node_get(struct route_table *table, if (family2afi(p->family) == AFI_LINKSTATE) prefix_linkstate_ptr_free(p); + else if (p->family == AF_FLOWSPEC) + prefix_flowspec_ptr_free(p); return new; }