Subject: Re: [libssh2] #182: Various memory leaks

Re: [libssh2] #182: Various memory leaks

From: libssh2 Trac <trac_at_libssh2.stuge.se>
Date: Fri, 25 Jun 2010 08:32:13 -0000

#182: Various memory leaks
-------------------------------+--------------------------------------------
  Reporter: john@… | Owner: bagder
      Type: defect | Status: assigned
  Priority: normal | Milestone: 1.2.6
 Component: API | Version: 1.2.6
Resolution: | Keywords:
    Blocks: | Blocked By:
-------------------------------+--------------------------------------------

Comment (by john@…):

 I have found and fixed a number of leaks, this may be a repeat of
 information above but I feel it will be useful to summarize in one place.

 The software needs to be able to detect and remember that the socket is
 dead, if the socket state is not set to LIBSSH2_SOCKET_DISCONNECTED the
 '''_libssh2_channel_free''' will not release any memory. To do this I
 added an extra function to '''misc.c''' as shown below

 --- misc.c Thu Jun 3 12:52:02 2010
 +++ /usr2/other/libssh2/libssh2-1.2.6/src/misc.c Thu Jun 24
 12:30:57 2010
 @@ -130,6 +130,20 @@
  }
  #endif /* _libssh2_recv */

 +/* libssh2_socket_failed_error
 + */
 +int
 +_libssh2_socket_failed_error(int err)
 +{
 + switch(err) {
 + /* Add errors that dont show the socket is bad here */
 + case EINTR : /* an interupt doesn't indicate that the socket has
 failed */
 + return 0;
 + }
 + /* the socket is failed if we get here */
 + return 1;
 +}
 +
  /* libssh2_ntohu32
   */
  unsigned int

 This function allows people to add errors that don't indicate socket
 failures, I know that an interrupt is not a failure, and as I am only
 testing on Solaris 10 x86 there may be machine specific ones.

 I used this function where ever data is sent or received on the socket
 (_libssh2_recv and _libssh2_send) to detect a dead socket error and set
 the session->socket_state = LIBSSH2_SOCKET_DISCONNECTED; as show below,
 only '''session.c''' and '''transport.c''' send or receive


 --- session.c Thu Apr 29 22:55:49 2010
 +++ /usr2/other/libssh2/libssh2-1.2.6/src/session.c Thu Jun 24
 12:10:05 2010
 @@ -129,6 +129,9 @@
                  return LIBSSH2_ERROR_EAGAIN;
              }

 + if(_libssh2_socket_failed_error(errno)) {
 + session->socket_state = LIBSSH2_SOCKET_DISCONNECTED; /*
 show socket disconnected */
 + }
              /* Some kinda error */
              session->banner_TxRx_state = libssh2_NB_state_idle;
              session->banner_TxRx_total_send = 0;
 @@ -243,6 +246,9 @@
              session->banner_TxRx_total_send += ret;
              return LIBSSH2_ERROR_EAGAIN;
          }
 + if((ret < 0) && _libssh2_socket_failed_error(errno)) {
 + session->socket_state = LIBSSH2_SOCKET_DISCONNECTED; /* show
 socket disconnected */
 + }
          session->banner_TxRx_state = libssh2_NB_state_idle;
          session->banner_TxRx_total_send = 0;
          return LIBSSH2_ERROR_SOCKET_NONE;

 --- transport.c Sun Apr 25 18:35:43 2010
 +++ /usr2/other/libssh2/libssh2-1.2.6/src/transport.c Thu Jun 24
 12:13:03 2010
 @@ -393,6 +393,10 @@
                          LIBSSH2_SESSION_BLOCK_INBOUND;
                      return LIBSSH2_ERROR_EAGAIN;
                  }
 + /* a 0 byte read indicates that the socket has failed */
 + if((nread == 0) || _libssh2_socket_failed_error(errno)) {
 + session->socket_state = LIBSSH2_SOCKET_DISCONNECTED; /*
 show socket disconnected */
 + }
                  return LIBSSH2_ERROR_SOCKET_NONE;
              }

 @@ -666,6 +670,9 @@
      else if (rc < 0) {
          /* nothing was sent */
          if (errno != EAGAIN) {
 + if(_libssh2_socket_failed_error(errno)) {
 + session->socket_state = LIBSSH2_SOCKET_DISCONNECTED; /*
 show socket disconnected */
 + }
              /* send failure! */
              return LIBSSH2_ERROR_SOCKET_NONE;
          }
 @@ -846,6 +853,9 @@
              p->ototal_num = total_length;
              return LIBSSH2_ERROR_EAGAIN;
          }
 + if((ret < 0) && _libssh2_socket_failed_error(errno)) {
 + session->socket_state = LIBSSH2_SOCKET_DISCONNECTED; /* show
 socket disconnected */
 + }
          return LIBSSH2_ERROR_SOCKET_NONE;
      }


 These changes ensure that memory is released when as socket is closed.

 I found that some memory was never release from the session object so
 added the following to '''session.c''' as shown below

 --- session.c Thu Apr 29 22:55:49 2010
 +++ /usr2/other/libssh2/libssh2-1.2.6/src/session.c Thu Jun 24
 12:10:05 2010

 @@ -836,6 +842,9 @@
          LIBSSH2_FREE(session, session->hostkey_prefs);
      }

 + if (session->local.kexinit) {
 + LIBSSH2_FREE(session, session->local.kexinit);
 + }
      if (session->local.crypt_prefs) {
          LIBSSH2_FREE(session, session->local.crypt_prefs);
      }
 @@ -849,6 +858,9 @@
          LIBSSH2_FREE(session, session->local.lang_prefs);
      }

 + if (session->remote.kexinit) {
 + LIBSSH2_FREE(session, session->remote.kexinit);
 + }
      if (session->remote.crypt_prefs) {
          LIBSSH2_FREE(session, session->remote.crypt_prefs);
      }
 @@ -865,6 +877,9 @@
      /*
       * Make sure all memory used in the state variables are free
       */
 + if (session->kexinit_data) {
 + LIBSSH2_FREE(session, session->kexinit_data);
 + }
      if (session->startup_data) {
          LIBSSH2_FREE(session, session->startup_data);
      }

 And because '''session->kexinit_data''' may be released in 2 places
 '''kex.c''' mus be modified to clear the pointer.

 --- kex.c Thu Apr 29 22:56:50 2010
 +++ /usr2/other/libssh2/libssh2-1.2.6/src/kex.c Tue Jun 22 12:43:54 2010
 @@ -1099,6 +1099,8 @@
      } else {
          data = session->kexinit_data;
          data_len = session->kexinit_data_len;
 + session->kexinit_data = NULL;
 + session->kexinit_data_len = 0;
      }

      rc = _libssh2_transport_write(session, data, data_len);

 When shutting down a channel/session I ensured all memory is released from
 the protocol negotiation functions by modifying my software to loop
 calling a libssh function whilst it returned LIBSSH2_ERROR_EAGAIN as shown
 below. '''Note''' at this point the socket has been closed so the
 functions will return an error, I am looping to ensure all memory is
 released.

     do {
     rc = libssh2_session_free(session[idx]);
     if(rc) Error(">>> FAILED libssh2_session_free for idx: %d, session
 0x%p, error %d\n",idx,session[idx],rc);
     } while(rc == LIBSSH2_ERROR_EAGAIN);


     do {
     rc = libssh2_session_startup(session[idx],sshSock[idx]);
     } while(rc == LIBSSH2_ERROR_EAGAIN);

 This ensures that any memory allocated within the
 '''libssh2_session_startup''' function is released

-- 
Ticket URL: <http://trac.libssh2.org/ticket/182#comment:5>
libssh2 <http://trac.libssh2.org/>
C library for writing portable SSH2 clients
_______________________________________________
libssh2-devel http://cool.haxx.se/cgi-bin/mailman/listinfo/libssh2-devel
Received on 2010-06-25