Re: [libssh2] #182: Various memory leaks

#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 will try to provide some test cases/code.

 The problem as I see it is that there are a number of functions that are
 called many times, with state engines to control the process flow. Memory
 is allocated at one state and released at another. This works fine when
 everything runs smoothly and connect correctly. However when something
 goes wrong the function may never be called again, so the memory is never
 released.
 The best way I have found of ensuring the memory is released is as
 follows.

 Ensure the socket is closed. It would be useful to have a function to
 clear the socket from the '''session''' object.
 Call the last negotiation function again, eg if a call to
 '''libssh2_session_startup''' returned '''LIBSSH2_ERROR_EAGAIN''' then
 call again, this should ensure that any allocated memory is released.

 Whilst this technique seem to work I am not happy relying on a side effect
 for releasing memory, if '''libssh2_session_free''' is called I would
 expect any memory allocated to a session to be released.

 The following function is an example that allocate and release memory only
 within the function

 diffie_hellman_sha1
 allocates memory to '''exchange_state->e_packet''' when
 '''exchange_state->state == libssh2_NB_state_idle''', releases memory
 '''ONLY''' when the negotiation compleates/fails.

 I believe the following changes (diff -u session.c) release memory that is
 never released.

 --- session.c Thu Apr 29 22:55:49 2010
 +++ /usr2/other/libssh2/libssh2-1.2.6/src/session.c Tue Jun 22
 13:10:49 2010
 @@ -747,6 +747,7 @@
      LIBSSH2_CHANNEL *ch;
      LIBSSH2_LISTENER *l;
      struct transportpacket *p = &session->packet;
 +printf("%s: 0x%p, free_state: %d, kexinit_data
 0x%p\n",__func__,session,session->free_state,session->kexinit_data);

      if (session->free_state == libssh2_NB_state_idle) {
          _libssh2_debug(session, LIBSSH2_TRACE_TRANS, "Freeing session
 resource",
 @@ -836,6 +837,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 +853,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 +872,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);
      }

 You will also need to change kex.c (diff -c kex.c) to ensure there is not
 a double free

 --- 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);

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