Bug #73126 Numerous Valgrind errors in OpenSSL

Description:
Server built with -DWITH_SSL=system shows many Valgrind errors. Some of these are one-time per server initialisation, but some appear to be per-connection thus their leaks might accumulate over time.

This might be a duplicate of Bug#15908967 VALGRIND ERRORS WHEN RUNNING WITH OPENSSL, in which case it is a request to backport its fixes (lp:mysql-server/5.6 revs 4591.1.60, 4591.1.81) to 5.5.

How to repeat:
cmake .. -DWITH_DEBUG=ON -DWITH_VALGRIND=ON -DCMAKE_C_FLAGS=-DHAVE_purify -DCMAKE_CXX_FLAGS=-DHAVE_purify -DWITH_SSL=system

$ ./mysql-test-run ssl --valgrind

Resulting error log has over 700 leak records. Some examples:

==16997== 24 bytes in 1 blocks are still reachable in loss record 1 of 695
==16997== at 0x4C2AB80: malloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==16997== by 0x5B6AD32: CRYPTO_malloc (in /lib/x86_64-linux-gnu/libcrypto.so.1.0.0)
==16997== by 0x5BF0C2A: lh_insert (in /lib/x86_64-linux-gnu/libcrypto.so.1.0.0)
==16997== by 0x5B6D829: OBJ_NAME_add (in /lib/x86_64-linux-gnu/libcrypto.so.1.0.0)
==16997== by 0x5BFAF74: EVP_add_cipher (in /lib/x86_64-linux-gnu/libcrypto.so.1.0.0)
==16997== by 0x58F0B80: SSL_library_init (in /lib/x86_64-linux-gnu/libssl.so.1.0.0)
==16997== by 0xC126F0: check_ssl_init (viosslfactories.c:155)
==16997== by 0xC127E8: new_VioSSLFd (viosslfactories.c:186)
==16997== by 0xC12C89: new_VioSSLAcceptorFd (viosslfactories.c:306)
==16997== by 0x555095: init_ssl() (mysqld.cc:3799)
==16997== by 0x556AB9: mysqld_main(int, char**) (mysqld.cc:4755)
==16997== by 0x54E27C: main (main.cc:25)

==16997== 24 bytes in 1 blocks are still reachable in loss record 65 of 695
==16997== at 0x4C2AB80: malloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==16997== by 0x5B6AD32: CRYPTO_malloc (in /lib/x86_64-linux-gnu/libcrypto.so.1.0.0)
==16997== by 0x5B6D806: OBJ_NAME_add (in /lib/x86_64-linux-gnu/libcrypto.so.1.0.0)
==16997== by 0x5BFAF74: EVP_add_cipher (in /lib/x86_64-linux-gnu/libcrypto.so.1.0.0)
==16997== by 0x5BFEF00: OpenSSL_add_all_ciphers (in /lib/x86_64-linux-gnu/libcrypto.so.1.0.0)
==16997== by 0x5BFEEDD: OPENSSL_add_all_algorithms_noconf (in /lib/x86_64-linux-gnu/libcrypto.so.1.0.0)
==16997== by 0xC126F5: check_ssl_init (viosslfactories.c:156)
==16997== by 0xC127E8: new_VioSSLFd (viosslfactories.c:186)
==16997== by 0xC12C89: new_VioSSLAcceptorFd (viosslfactories.c:306)
==16997== by 0x555095: init_ssl() (mysqld.cc:3799)
==16997== by 0x556AB9: mysqld_main(int, char**) (mysqld.cc:4755)
==16997== by 0x54E27C: main (main.cc:25)

==16997== 24 bytes in 1 blocks are still reachable in loss record 423 of 695
==16997== at 0x4C2AB80: malloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==16997== by 0x5B6AD32: CRYPTO_malloc (in /lib/x86_64-linux-gnu/libcrypto.so.1.0.0)
==16997== by 0x5B6D806: OBJ_NAME_add (in /lib/x86_64-linux-gnu/libcrypto.so.1.0.0)
==16997== by 0x5BFAFC7: EVP_add_digest (in /lib/x86_64-linux-gnu/libcrypto.so.1.0.0)
==16997== by 0x5BFF610: OpenSSL_add_all_digests (in /lib/x86_64-linux-gnu/libcrypto.so.1.0.0)
==16997== by 0xC126F5: check_ssl_init (viosslfactories.c:156)
==16997== by 0xC127E8: new_VioSSLFd (viosslfactories.c:186)
==16997== by 0xC12C89: new_VioSSLAcceptorFd (viosslfactories.c:306)
==16997== by 0x555095: init_ssl() (mysqld.cc:3799)
==16997== by 0x556AB9: mysqld_main(int, char**) (mysqld.cc:4755)
==16997== by 0x54E27C: main (main.cc:25)

==16997== 24 bytes in 1 blocks are still reachable in loss record 493 of 695
==16997== at 0x4C2AB80: malloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==16997== by 0x5B6AD32: CRYPTO_malloc (in /lib/x86_64-linux-gnu/libcrypto.so.1.0.0)
==16997== by 0x5B6C0A5: def_get_class (in /lib/x86_64-linux-gnu/libcrypto.so.1.0.0)
==16997== by 0x5B6C1D3: int_get_new_index (in /lib/x86_64-linux-gnu/libcrypto.so.1.0.0)
==16997== by 0x58E7264: SSL_get_ex_data_X509_STORE_CTX_idx (in /lib/x86_64-linux-gnu/libssl.so.1.0.0)
==16997== by 0x58E4336: SSL_CTX_new (in /lib/x86_64-linux-gnu/libssl.so.1.0.0)
==16997== by 0xC12837: new_VioSSLFd (viosslfactories.c:192)
==16997== by 0xC12C89: new_VioSSLAcceptorFd (viosslfactories.c:306)
==16997== by 0x555095: init_ssl() (mysqld.cc:3799)
==16997== by 0x556AB9: mysqld_main(int, char**) (mysqld.cc:4755)
==16997== by 0x54E27C: main (main.cc:25)

==16997== 24 bytes in 1 blocks are still reachable in loss record 499 of 695
==16997== at 0x4C2AB80: malloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==16997== by 0x5B6AD32: CRYPTO_malloc (in /lib/x86_64-linux-gnu/libcrypto.so.1.0.0)
==16997== by 0x5B6C0A5: def_get_class (in /lib/x86_64-linux-gnu/libcrypto.so.1.0.0)
==16997== by 0x5B6CA4A: int_new_ex_data (in /lib/x86_64-linux-gnu/libcrypto.so.1.0.0)
==16997== by 0x5BE74C3: BIO_set (in /lib/x86_64-linux-gnu/libcrypto.so.1.0.0)
==16997== by 0x5BE7541: BIO_new (in /lib/x86_64-linux-gnu/libcrypto.so.1.0.0)
==16997== by 0x5BE93B3: BIO_new_file (in /lib/x86_64-linux-gnu/libcrypto.so.1.0.0)
==16997== by 0x5C2DA43: X509_load_cert_crl_file (in /lib/x86_64-linux-gnu/libcrypto.so.1.0.0)
==16997== by 0x5C2DB8C: by_file_ctrl (in /lib/x86_64-linux-gnu/libcrypto.so.1.0.0)
==16997== by 0x5C24F0E: X509_STORE_load_locations (in /lib/x86_64-linux-gnu/libcrypto.so.1.0.0)
==16997== by 0xC12972: new_VioSSLFd (viosslfactories.c:220)
==16997== by 0xC12C89: new_VioSSLAcceptorFd (viosslfactories.c:306)
==16997== by 0x555095: init_ssl() (mysqld.cc:3799)
==16997== by 0x556AB9: mysqld_main(int, char**) (mysqld.cc:4755)
==16997== by 0x54E27C: main (main.cc:25)

Suggested fix:
Backport lp:mysql-server/5.6 revs 4591.1.60, 4591.1.81 to 5.5.

Actually all of 4591.1.81 looks wrong, it adds suppressions for stuff that can be deallocated instead. Will file a separate bug later if confirmed.

Hello Laurynas,

Thank you for the bug report.
Verified as described.

Thanks,
Umesh

The wrongness of commit 4591.1.81 has been fixed in 4591.1.161 except that it appears to put ERR_remove_state(0) calls all over the place (why does e.g. slave SQL thread need that?) and leaves one last suppression that can be replaced by a deallocation. Very minor, not sure if that's worth a separate bug report.

=== modified file 'mysql-test/valgrind.supp'
--- mysql-test/valgrind.supp 2013-10-01 14:35:36 +0000
+++ mysql-test/valgrind.supp 2014-07-10 10:26:41 +0000
@@ -1045,16 +1045,6 @@
 }

 {
- OpenSSL still reachable.
- Memcheck:Leak
- fun:malloc
- fun:CRYPTO_malloc
- fun:sk_new
- ...
- fun:SSL_COMP_get_compression_methods
- fun:SSL_library_init
-}
-{
    BUG#14801497 CONDITIONAL JUMP OR MOVE DEPENDS ON UNINITIALISED VALUE(S) IN CREATE_TMP_TABLE
    Memcheck:Cond
    fun:_Z16create_tmp_tableP3THDP15TMP_TABLE_PARAMR4ListI4ItemEP8st_orderbbyyPKc

=== modified file 'vio/vio.c'
--- vio/vio.c 2014-02-17 11:12:40 +0000
+++ vio/vio.c 2014-07-10 11:49:48 +0000
@@ -368,5 +368,6 @@
   ERR_free_strings();
   EVP_cleanup();
   CRYPTO_cleanup_all_ex_data();
+ sk_SSL_COMP_free(SSL_COMP_get_compression_methods());
 #endif
 }