From 280477fab59b789d924830e1a50dc9d2656915af Mon Sep 17 00:00:00 2001 From: "E. Scott Daniels" Date: Fri, 13 Nov 2020 15:13:46 -0500 Subject: [PATCH] Correct memory leak in the RTE cleanup When cleaning up a route table entry the round robin group block (rrg) was not being correctly freed. The leak was small, but would be noticed in a situation described by the indicated issue (many table updates). Issue-ID: RIC-674 Signed-off-by: E. Scott Daniels Change-Id: Ica7d0219574abd33392c7127f918ac71b2891702 Signed-off-by: E. Scott Daniels --- CHANGES_CORE.txt | 3 +++ CMakeLists.txt | 2 +- docs/rel-notes.rst | 7 ++++++ src/rmr/common/src/rt_generic_static.c | 42 ++++++++++++++++------------------ src/rmr/common/src/rtc_static.c | 5 ---- src/rmr/common/src/tools_static.c | 2 +- src/rmr/si/src/si95/siestablish.c | 28 +++++++++-------------- src/rmr/si/src/si95/sinewses.c | 5 ++-- 8 files changed, 46 insertions(+), 48 deletions(-) diff --git a/CHANGES_CORE.txt b/CHANGES_CORE.txt index 5d54c19..db2f2e6 100644 --- a/CHANGES_CORE.txt +++ b/CHANGES_CORE.txt @@ -5,6 +5,9 @@ # API and build change and fix summaries. Doc correctsions # and/or changes are not mentioned here; see the commit messages. +2020 November 4; Version 4.4.4 + Correct address memory leak in the RTE cleanup. (RIC-674) + 2020 November 4; Version 4.4.3 Correct bug introduced with race fix (4.4.0) (RIC-674) diff --git a/CMakeLists.txt b/CMakeLists.txt index 407a5a9..763c73f 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -41,7 +41,7 @@ cmake_minimum_required( VERSION 3.5 ) set( major_version "4" ) # should be automatically populated from git tag later, but until CI process sets a tag we use this set( minor_version "4" ) -set( patch_level "3" ) +set( patch_level "4" ) set( install_root "${CMAKE_INSTALL_PREFIX}" ) set( install_inc "include/rmr" ) diff --git a/docs/rel-notes.rst b/docs/rel-notes.rst index b7faccb..4384bdc 100644 --- a/docs/rel-notes.rst +++ b/docs/rel-notes.rst @@ -22,6 +22,13 @@ the need to leap frog versions ceased, and beginning with version 4.0.0, the RMR versions should no longer skip. +2020 November 4; Version 4.4.4 +------------------------------ + +Correct address memory leak in the RTE cleanup. (RIC-674) + + + 2020 November 4; Version 4.4.3 ------------------------------ diff --git a/src/rmr/common/src/rt_generic_static.c b/src/rmr/common/src/rt_generic_static.c index bf7bfdc..116b17f 100644 --- a/src/rmr/common/src/rt_generic_static.c +++ b/src/rmr/common/src/rt_generic_static.c @@ -211,6 +211,18 @@ static void rt_epcounts( route_table_t* rt, char* id ) { rmr_sym_foreach_class( rt->hash, 1, ep_counts, id ); // run endpoints in the active table } + +static void dump_tables( uta_ctx_t *ctx ) { + if( ctx->old_rtable != NULL ) { + rmr_vlog_force( RMR_VL_DEBUG, "old route table: (ref_count=%d)\n", ctx->old_rtable->ref_count ); + rt_stats( ctx->old_rtable ); + } else { + rmr_vlog_force( RMR_VL_DEBUG, "old route table was empty\n" ); + } + rmr_vlog_force( RMR_VL_DEBUG, "new route table:\n" ); + rt_stats( ctx->rtable ); +} + // ------------ route manager communication ------------------------------------------------- /* Send a request for a table update to the route manager. Updates come in @@ -829,18 +841,10 @@ static void parse_rt_rec( uta_ctx_t* ctx, uta_ctx_t* pctx, char* buf, int vleve } if( ctx->new_rtable ) { - if( DEBUG > 1 || (vlevel > 1) ) rmr_vlog( RMR_VL_DEBUG, "end of route table noticed\n" ); roll_tables( ctx ); // roll active to old, and new to active with proper locking - - if( vlevel > 0 ) { - if( ctx->old_rtable != NULL ) { - rmr_vlog_force( RMR_VL_DEBUG, "old route table: (ref_count=%d)\n", ctx->old_rtable->ref_count ); - rt_stats( ctx->old_rtable ); - } else { - rmr_vlog_force( RMR_VL_DEBUG, "old route table was empty\n" ); - } - rmr_vlog_force( RMR_VL_DEBUG, "new route table:\n" ); - rt_stats( ctx->rtable ); + if( DEBUG > 1 || (vlevel > 1) ) { + rmr_vlog( RMR_VL_DEBUG, "end of route table noticed\n" ); + dump_tables( ctx ); } send_rt_ack( pctx, mbuf, ctx->table_id, RMR_OK, NULL ); @@ -925,17 +929,9 @@ static void parse_rt_rec( uta_ctx_t* ctx, uta_ctx_t* pctx, char* buf, int vleve if( ctx->new_rtable ) { roll_tables( ctx ); // roll active to old, and new to active with proper locking - if( DEBUG > 1 || (vlevel > 1) ) rmr_vlog_force( RMR_VL_DEBUG, "end of rt update noticed\n" ); - - if( vlevel > 0 ) { - if( ctx->old_rtable != NULL ) { - rmr_vlog_force( RMR_VL_DEBUG, "old route table: (ref_count=%d)\n", ctx->old_rtable->ref_count ); - rt_stats( ctx->old_rtable ); - } else { - rmr_vlog_force( RMR_VL_DEBUG, "old route table was empty\n" ); - } - rmr_vlog_force( RMR_VL_DEBUG, "updated route table:\n" ); - rt_stats( ctx->rtable ); + if( DEBUG > 1 || (vlevel > 1) ) { + rmr_vlog_force( RMR_VL_DEBUG, "end of rt update noticed\n" ); + dump_tables( ctx ); } send_rt_ack( pctx, mbuf, ctx->table_id, RMR_OK, NULL ); @@ -1082,7 +1078,9 @@ static void del_rte( void* st, void* entry, char const* name, void* thing, void* for( i = 0; i < rte->nrrgroups; i++ ) { if( rte->rrgroups[i] ) { free( rte->rrgroups[i]->epts ); // ditch list of endpoint pointers (end points are reused; don't trash them) + free( rte->rrgroups[i] ); // but must free the rrg itself too } + } free( rte->rrgroups ); diff --git a/src/rmr/common/src/rtc_static.c b/src/rmr/common/src/rtc_static.c index 15fe567..ee84c91 100644 --- a/src/rmr/common/src/rtc_static.c +++ b/src/rmr/common/src/rtc_static.c @@ -274,11 +274,6 @@ static void* rtc( void* vctx ) { return NULL; } - if( (ctx->ephash = rmr_sym_alloc( RT_SIZE )) == NULL ) { // master hash table for endpoints (each rt will reference this) - rmr_vlog( RMR_VL_CRIT, "rmr_rtc: internal mishap: unable to allocate an endpoint hash table\n" ); - return NULL; - } - if( (eptr = getenv( ENV_VERBOSE_FILE )) != NULL ) { vfd = open( eptr, O_RDONLY ); vlevel = refresh_vlevel( vfd ); diff --git a/src/rmr/common/src/tools_static.c b/src/rmr/common/src/tools_static.c index cc95fc1..bbe49ee 100644 --- a/src/rmr/common/src/tools_static.c +++ b/src/rmr/common/src/tools_static.c @@ -187,7 +187,7 @@ static char* uta_h2ip( char const* hname ) { *(tok++) = 0; } - hent = gethostbyname( dname ); + hent = gethostbyname( dname ); // valgrind will complain that this leaks, but we cannot free it! if( hent == NULL || hent->h_addr_list == NULL ) { //rmr_vlog( RMR_VL_WARN, "h2ip: dns lookup failed for: %s\n", dname ); free( dname ); diff --git a/src/rmr/si/src/si95/siestablish.c b/src/rmr/si/src/si95/siestablish.c index a62490d..794ea07 100644 --- a/src/rmr/si/src/si95/siestablish.c +++ b/src/rmr/si/src/si95/siestablish.c @@ -65,30 +65,24 @@ Returns a transport struct which is the main context for the listener. */ extern struct tp_blk *SIlisten_prep( int type, char* abuf, int family ) { - struct tp_blk *tptr; // pointer at new tp block - int status = SI_OK; // processing status + struct tp_blk *tptr; // pointer at new tp block struct sockaddr *addr; // IP address we are requesting - int protocol; // protocol for socket call int optval = 0; int alen = 0; + int status = SI_OK; // processing status + int protocol; // protocol for socket call - tptr = (struct tp_blk *) SInew( TP_BLK ); // new transport info block + tptr = (struct tp_blk *) SInew( TP_BLK ); // transport info - if( tptr != NULL ) - { + if( tptr != NULL ) { addr = NULL; - switch( type ) // things specifc to tcp or udp - { - case UDP_DEVICE: - tptr->type = SOCK_DGRAM; - protocol = IPPROTO_UDP; - break; - - case TCP_DEVICE: - default: - tptr->type = SOCK_STREAM; - protocol = IPPROTO_TCP; + if( type == UDP_DEVICE ) { + tptr->type = SOCK_DGRAM; + protocol = IPPROTO_UDP; + } else { + tptr->type = SOCK_STREAM; + protocol = IPPROTO_TCP; } alen = SIgenaddr( abuf, protocol, family, tptr->type, &addr ); // family == 0 for type that suits the address passed in diff --git a/src/rmr/si/src/si95/sinewses.c b/src/rmr/si/src/si95/sinewses.c index 02a5e6d..56d9b10 100644 --- a/src/rmr/si/src/si95/sinewses.c +++ b/src/rmr/si/src/si95/sinewses.c @@ -88,8 +88,8 @@ extern int SInewsession( struct ginfo_blk *gptr, struct tp_blk *tpptr ) { } SETSOCKOPT( tpptr->fd, SOL_TCP, TCP_QUICKACK, (void *)&optval, sizeof( optval) ) ; - SIaddress( addr, (void **) &buf, AC_TODOT ); // get addr of remote side - if( (cbptr = gptr->cbtab[SI_CB_SECURITY].cbrtn) != NULL ) { // invoke the security callback function if there + SIaddress( addr, (void **) &buf, AC_TODOT ); // get addr of remote side; buf must be freed + if( (cbptr = gptr->cbtab[SI_CB_SECURITY].cbrtn) != NULL ) { // invoke the security callback function if there status = (*cbptr)( gptr->cbtab[SI_CB_SECURITY].cbdata, buf ); if( status == SI_RET_ERROR ) { // session to be rejected SIterm( gptr, newtp ); // terminate new tp block (do NOT call trash) @@ -110,6 +110,7 @@ extern int SInewsession( struct ginfo_blk *gptr, struct tp_blk *tpptr ) { SImap_fd( gptr, newtp->fd, newtp ); // add fd to the map + free( buf ); return SI_OK; } -- 2.16.6