Re: Two memory leak remaining, with fixes

[Nikos Mavrogiannopoulos]

Hello,
 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...

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
> (auth_cert.c:98)
> ==6235==    by 0x4C65519: _gnutls_proc_x509_server_certificate
> (auth_cert.c:1027)
> ==6235==    by 0x4C660AA: _gnutls_proc_cert_server_certificate
> (auth_cert.c:1262)
> ==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
> (gnutls_constate.c:721)
> ==3435==    by 0x4C4DCF0: _gnutls_send_handshake_final
> (gnutls_handshake.c:2462)
> ==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
> address@hidden
> http://lists.gnu.org/mailman/listinfo/gnutls-devel
>