[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: Two memory leak remaining, with fixes

From: Sam Varshavchik
Subject: Re: Two memory leak remaining, with fixes
Date: Sat, 27 Sep 2008 23:16:13 -0400

Nikos Mavrogiannopoulos writes:

 Thank you, I applied it (the last one with some additional removals).
I'm curious though how there could be a leak. Maybe it's too late for
me anyway...

Both patches address leaks caused by the same underlying problem. The functions in question allocate some session-specific objects, during a handshake, and save the pointer in the session structure.

When a /rehandshake/ occurs, gnutls_handshake() gets called again, a second time. The existing code just allocates new objects and puts them into the gnutls_session_int structure again. Pointers to objects allocated by the first call to gnutls_handshake(), during the first handshake, never get freed.

All of these leaks occur when gnutls_handshake() gets called a second time, not the first time.

There is no test case for this, but I can confirm that GnuTLS seems to properly implement a rehandshake of an existing GnuTLS session. At least, when the GnuTLS client connects to a GnuTLS server. If the server requests a handshake, the client accepts, gnutls_handshake() -- on both the client and the server -- seems to do the right thing. Except for leaking memory, of course.

On Sun, Sep 28, 2008 at 1:40 AM, Sam Varshavchik <address@hidden> wrote:
Rerunning my valgrind checks against the latest CVS, two memory leaks remain
that are triggered by a session rehandshake.

First leak:

==6235== 719 bytes in 13 blocks are definitely lost in loss record 3 of 5
==6235==    at 0x4A0739E: malloc (vg_replace_malloc.c:207)
==6235==    by 0x4C59EA0: _gnutls_set_datum_m (gnutls_datum.c:80)
==6235==    by 0x4C63899: _gnutls_copy_certificate_auth_info
==6235==    by 0x4C65519: _gnutls_proc_x509_server_certificate
==6235==    by 0x4C660AA: _gnutls_proc_cert_server_certificate
==6235==    by 0x4C52DE4: _gnutls_recv_server_certificate (gnutls_kx.c:719)
==6235==    by 0x4C4D6AD: _gnutls_handshake_client (gnutls_handshake.c:2359)
==6235==    by 0x4C4D268: gnutls_handshake (gnutls_handshake.c:2262)

Second leak:

==3435==    at 0x4A0739E: malloc (vg_replace_malloc.c:207)
==3435==    by 0x4C59EA0: _gnutls_set_datum_m (gnutls_datum.c:80)
==3435==    by 0x4C61352: _gnutls_set_keys (gnutls_constate.c:130)
==3435==    by 0x4C61FA0: _gnutls_set_write_keys (gnutls_constate.c:424)
==3435==    by 0x4C62AD4: _gnutls_write_connection_state_init
==3435==    by 0x4C4DCF0: _gnutls_send_handshake_final
==3435==    by 0x4C4E89C: _gnutls_handshake_common (gnutls_handshake.c:2675)
==3435==    by 0x4C4D2B1: gnutls_handshake (gnutls_handshake.c:2279)

Investigating the second leak also found a number of other potential leaks
in gnutls_session_int.cipher_specs. It's probably a good idea to audit all
other parts of gnutls_session_int that might need explicit cleanup before
they are allocated, given that gnutls_handshake() may be called again as a
result of a rehandshake.

--- lib/auth_cert.c~    2008-09-23 20:30:32.000000000 -0400
+++ lib/auth_cert.c     2008-09-27 17:55:28.000000000 -0400
@@ -73,6 +73,9 @@

 if (info->raw_certificate_list != NULL)
+      for (j = 0; j < info->ncerts; j++)
+        _gnutls_free_datum (&info->raw_certificate_list[j]);
     gnutls_free( info->raw_certificate_list);

--- lib/gnutls_constate.c~      2008-09-25 20:30:41.000000000 -0400
+++ lib/gnutls_constate.c       2008-09-27 18:18:15.000000000 -0400
@@ -124,6 +124,13 @@
                   _gnutls_bin2hex (key_block, block_size, buf,
                                    sizeof (buf)));

+  _gnutls_free_datum (&session->cipher_specs.server_write_mac_secret);
+  _gnutls_free_datum (&session->cipher_specs.client_write_mac_secret);
+  _gnutls_free_datum (&session->cipher_specs.server_write_IV);
+  _gnutls_free_datum (&session->cipher_specs.client_write_IV);
+  _gnutls_free_datum (&session->cipher_specs.server_write_key);
+  _gnutls_free_datum (&session->cipher_specs.client_write_key);
 pos = 0;
 if (hash_size > 0)

Gnutls-devel mailing list

Attachment: pgpu5F3kFC9Ds.pgp
Description: PGP signature

reply via email to

[Prev in Thread] Current Thread [Next in Thread]