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 <daniels@research.att.com>
Change-Id: Ica7d0219574abd33392c7127f918ac71b2891702
Signed-off-by: E. Scott Daniels <daniels@research.att.com>

diff --git a/CHANGES_CORE.txt b/CHANGES_CORE.txt
index 5d54c19..db2f2e6 100644 (file)
--- 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 (file)
--- 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 (file)
--- 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 (file)
--- 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 (file)
--- 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 (file)
--- 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 (file)
--- 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 (file)
--- 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;
 }