Re: [PATCH] BUG/MINOR: ssl: fix potential memory leak in ssl_sock_load_dh_params()

[Roberto Guimaraes]

It’s a double-free…. we need to NULL local_dh_1024 after the free because your 
code has the following destructor (1.5 doesn’t):

__attribute__((destructor))
static void __ssl_sock_deinit(void)
{
#ifdef SSL_CTRL_SET_TLSEXT_HOSTNAME
        lru64_destroy(ssl_ctx_lru_tree);
#endif

#ifndef OPENSSL_NO_DH
        if (local_dh_1024) {
                DH_free(local_dh_1024);
                local_dh_1024 = NULL;
        }

So I was able to reproduce it by adding the destructor and NULLing the var 
after freeing fixes it.


Also, it’s not just nginx that does the freeing - haproxy also does that in 
different code paths (loading it from file):

/* Loads Diffie-Hellman parameter from a file. Returns 1 if loaded, else -1
   if an error occured, and 0 if parameter not found. */
int ssl_sock_load_dh_params(SSL_CTX *ctx, const char *file)
{
….
….
end:
        if (dh)
                DH_free(dh);

        return ret;
}

so I think the DH_free is indeed correct since it will leak otherwise. however, 
sometime along the way the destructor was added which will double-free the 
global variable.

once again, apologize for the bug.

thanks,
roberto

> On Jul 12, 2016, at 2:55 PM, Roberto Guimaraes <rguimar...@fastly.com> wrote:
> 
> o