Skip to content

Commit

Permalink
Fix memleak in OpenSSL uninitialization
Browse files Browse the repository at this point in the history
On my version of OpenSSL (openssl-1.0.2j on Gentoo Linux), the original
version of this code resulted in a small memleak reoprted by ASAN:

  Direct leak of 24 byte(s) in 1 object(s) allocated from:
    #0 0x4d5460 in malloc /var/tmp/portage/sys-devel/llvm-3.9.0/work/llvm-3.9.0.src/projects/compiler-rt/lib/asan/asan_malloc_linux.cc:64
    CESNET#1 0x7f8fff9cb7fc in CRYPTO_malloc /var/tmp/portage/dev-libs/openssl-1.0.2j/work/openssl-1.0.2j-abi_x86_64.amd64/crypto/mem.c:346
    CESNET#2 0x7f8fffcd372b in load_builtin_compressions /var/tmp/portage/dev-libs/openssl-1.0.2j/work/openssl-1.0.2j-abi_x86_64.amd64/ssl/ssl_ciph.c:487
    CESNET#3 0x7f8fffcd5b5a in SSL_COMP_get_compression_methods /var/tmp/portage/dev-libs/openssl-1.0.2j/work/openssl-1.0.2j-abi_x86_64.amd64/ssl/ssl_ciph.c:1962
    CESNET#4 0x7f8fffcdc6b1 in SSL_library_init /var/tmp/portage/dev-libs/openssl-1.0.2j/work/openssl-1.0.2j-abi_x86_64.amd64/ssl/ssl_algs.c:150
    CESNET#5 0x7f90005ea466 in nc_ssh_tls_init /home/jkt/work/prog/libnetconf2/build/../src/session.c:1165:5
    CESNET#6 0x7f90005ea448 in nc_init /home/jkt/work/prog/libnetconf2/build/../src/session.c:1209:5
    CESNET#7 0x7f90005fe7c5 in nc_server_init /home/jkt/work/prog/libnetconf2/build/../src/session_server.c:430:5
    CESNET#8 0x50ffd8 in main /home/jkt/work/prog/libnetconf2/build/../tests/test_fd_comm.c:335:5
    CESNET#9 0x7f8ffe93e733 in __libc_start_main /var/tmp/portage/sys-libs/glibc-2.22-r4/work/glibc-2.22/csu/libc-start.c:289
    CESNET#10 0x419e18 in _start (/home/jkt/work/prog/libnetconf2/build/tests/test_fd_comm+0x419e18)

Upstream OpenSSL's issue tracker suggests [1] to use
SSL_COMP_add_compression_method [2]. That particular overload was only
added in 1.0.2, and since 1.1.0, it's deprecated and documented [3] to
be a no-op. RHEL6 comes with 1.0.1.

TL;DR: it's an API-compatibility mess, but it fixes a leak. It shouldn't
break the build.

[1] https://rt.openssl.org/Ticket/Display.html?id=2561&user=guest&pass=guest
[2] https://www.openssl.org/docs/man1.0.2/ssl/SSL_COMP_add_compression_method.html
[3] https://www.openssl.org/docs/man1.1.0/ssl/SSL_COMP_add_compression_method.html
  • Loading branch information
jktjkt committed Nov 21, 2016
1 parent d714746 commit 7361b98
Showing 1 changed file with 12 additions and 0 deletions.
12 changes: 12 additions & 0 deletions src/session.c
Original file line number Diff line number Diff line change
Expand Up @@ -1139,7 +1139,13 @@ nc_tls_destroy(void)
nc_thread_destroy();
EVP_cleanup();
ERR_free_strings();
#if OPENSSL_VERSION_NUMBER >= 0x10100000L // >= 1.1.0
// no de-init needed
#elif OPENSSL_VERSION_NUMBER >= 0x10002000L // >= 1.0.2
SSL_COMP_free_compression_methods();
#else
sk_SSL_COMP_free(SSL_COMP_get_compression_methods());
#endif

CRYPTO_THREADID_set_callback(NULL);
CRYPTO_set_locking_callback(NULL);
Expand Down Expand Up @@ -1175,7 +1181,13 @@ static void
nc_ssh_tls_destroy(void)
{
ERR_free_strings();
#if OPENSSL_VERSION_NUMBER >= 0x10100000L // >= 1.1.0
// no de-init needed
#elif OPENSSL_VERSION_NUMBER >= 0x10002000L // >= 1.0.2
SSL_COMP_free_compression_methods();
#else
sk_SSL_COMP_free(SSL_COMP_get_compression_methods());
#endif

nc_ssh_destroy();

Expand Down

0 comments on commit 7361b98

Please sign in to comment.