[openssl-dev] [OpenSSL][1.0.2h] Memory leaks - CAPI engine to blame?
> Works for me with mingw 32-bit: > > ALL OCSP TESTS SUCCESSFUL > Test X509v3_check_* > ../util/shlib_wrap.sh ./v3nametest > ../util/shlib_wrap.sh ./heartbeat_test > Test constant time utilites > ../util/shlib_wrap.sh ./constant_time_test > Testing constant time operations... > ok (ran 1908 tests) > test_verify_extra > ../util/shlib_wrap.sh ./verify_extra_test > PASS > test_clienthello > ../util/shlib_wrap.sh ./clienthellotest > test_sslv2conftest > ../util/shlib_wrap.sh ./sslv2conftest > SSLv2 CONF Test number 0 > SSLv2 CONF Test number 1 > SSLv2 CONF test: PASSED > make[1]: Leaving directory `/c/src/openssl-1.0.2h/test' > OPENSSL_CONF=apps/openssl.cnf util/opensslwrap.sh version -a > OpenSSL 1.0.2h-fips 3 May 2016 > built on: reproducible build, date unspecified > platform: mingw > options: bn(64,32) rc4(8x,mmx) des(ptr,risc1,16,long) idea(int) > blowfish(idx) > compiler: gcc -I. -I.. -I../include -DOPENSSL_THREADS -D_MT -DDSO_WIN32 > -DL_ENDIAN -DWIN32_LEAN_AND_MEAN -fomit-frame-pointer -O3 -march=i486 > -Wall -DOPENSSL_BN_ASM_PART_WORDS -DOPENSSL > _IA32_SSE2 -DOPENSSL_BN_ASM_MONT -DOPENSSL_BN_ASM_GF2m > -I/usr/local/ssl/fips-2.0/include -DSHA1_ASM -DSHA256_ASM -DSHA512_ASM > -DMD5_ASM -DRMD160_ASM -DAES_ASM -DVPAES_ASM -DWHIRLPOOL_ASM > -DGHASH_ASM > OPENSSLDIR: "/usr/local/ssl" > > Thanks for your email. Can I ask you to try one more time? Can you build OpenSSL from source including the CAPI engine and then run the tests again? Make sure the CAPI engine gets built (e.g. openssl engine command) I suspect the memory leaks come from there! I just built it again *without* this option 'enable-capieng' and the memory leaks are gone! Are there any patches available? Thanks again. ... ... ALL OCSP TESTS SUCCESSFUL Test X509v3_check_* ../util/shlib_wrap.sh ./v3nametest ../util/shlib_wrap.sh ./heartbeat_test Test constant time utilites ../util/shlib_wrap.sh ./constant_time_test Testing constant time operations... ok (ran 1908 tests) test_verify_extra ../util/shlib_wrap.sh ./verify_extra_test PASS test_clienthello ../util/shlib_wrap.sh ./clienthellotest test_sslv2conftest ../util/shlib_wrap.sh ./sslv2conftest SSLv2 CONF Test number 0 SSLv2 CONF Test number 1 SSLv2 CONF test: PASSED make[1]: Leaving directory `/src/openssl-1.0.2h/test' OPENSSL_CONF=apps/openssl.cnf util/opensslwrap.sh version -a OpenSSL 1.0.2h-fips 3 May 2016 built on: reproducible build, date unspecified platform: mingw options: bn(64,32) rc4(8x,mmx) des(ptr,risc1,16,long) idea(int) blowfish(idx) compiler: /mingw/bin/gcc.exe -I. -I.. -I../include -DZLIB -DOPENSSL_THREADS -D_ MT -DDSO_WIN32 -DL_ENDIAN -DWIN32_LEAN_AND_MEAN -DWINVER=0x0501 -D_WIN32_WINNT=0 x0501 -D_WIN32_IE=0x0501 -DPTW32_STATIC_LIB -O0 -pipe -mms-bitfields -fno-builti n -march=i686 -mtune=i686 -fomit-frame-pointer -Wall -DOPENSSL_BN_ASM_PART_WORDS -DOPENSSL_IA32_SSE2 -DOPENSSL_BN_ASM_MONT -DOPENSSL_BN_ASM_GF2m -I/mingw/includ e -DSHA1_ASM -DSHA256_ASM -DSHA512_ASM -DMD5_ASM -DRMD160_ASM -DAES_ASM -DVPAES_ ASM -DWHIRLPOOL_ASM -DGHASH_ASM OPENSSLDIR: "C:/OpenSSL" -- openssl-dev mailing list To unsubscribe: https://mta.openssl.org/mailman/listinfo/openssl-dev
[openssl-dev] [openssl.org #3198] [PATCH] Fix missing NULL pointer checks and memory leaks in crypto/asn1 files
The last patches from this have now been applied so closing this ticket. Thanks! Matt -- Ticket here: http://rt.openssl.org/Ticket/Display.html?id=3198 Please log in as guest with password guest if prompted -- openssl-dev mailing list To unsubscribe: https://mta.openssl.org/mailman/listinfo/openssl-dev
Re: [openssl-dev] [OpenSSL][1.0.2h] Memory leaks
Works for me with mingw 32-bit: ALL OCSP TESTS SUCCESSFUL Test X509v3_check_* ../util/shlib_wrap.sh ./v3nametest ../util/shlib_wrap.sh ./heartbeat_test Test constant time utilites ../util/shlib_wrap.sh ./constant_time_test Testing constant time operations... ok (ran 1908 tests) test_verify_extra ../util/shlib_wrap.sh ./verify_extra_test PASS test_clienthello ../util/shlib_wrap.sh ./clienthellotest test_sslv2conftest ../util/shlib_wrap.sh ./sslv2conftest SSLv2 CONF Test number 0 SSLv2 CONF Test number 1 SSLv2 CONF test: PASSED make[1]: Leaving directory `/c/src/openssl-1.0.2h/test' OPENSSL_CONF=apps/openssl.cnf util/opensslwrap.sh version -a OpenSSL 1.0.2h-fips 3 May 2016 built on: reproducible build, date unspecified platform: mingw options: bn(64,32) rc4(8x,mmx) des(ptr,risc1,16,long) idea(int) blowfish(idx) compiler: gcc -I. -I.. -I../include -DOPENSSL_THREADS -D_MT -DDSO_WIN32 -DL_ENDIAN -DWIN32_LEAN_AND_MEAN -fomit-frame-pointer -O3 -march=i486 -Wall -DOPENSSL_BN_ASM_PART_WORDS -DOPENSSL _IA32_SSE2 -DOPENSSL_BN_ASM_MONT -DOPENSSL_BN_ASM_GF2m -I/usr/local/ssl/fips-2.0/include -DSHA1_ASM -DSHA256_ASM -DSHA512_ASM -DMD5_ASM -DRMD160_ASM -DAES_ASM -DVPAES_ASM -DWHIRLPOOL_ASM -DGHASH_ASM OPENSSLDIR: "/usr/local/ssl" On 6/1/2016 3:18 PM, Sergio NNX wrote: How did you configure/build? What versions of mingw/msys did you use? ./configure mingw --prefix=/mingw make depend make all make test MSYS 1.0.18(0.48/3/2) 2012-11-21 22:34 i686 unknown; targ=MINGW32 GCC 6.1.0 32-bit Memory leaks didn't occur with previous OpenSSL versions using the same environment. I'm trying OpenSSL from master now but facing issues with Perl at the moment. Grrr. Can you try building it using MinGW/MSYS 32-bit? Thanks for looking at it. Cheers. Ser. -- openssl-dev mailing list To unsubscribe: https://mta.openssl.org/mailman/listinfo/openssl-dev
Re: [openssl-dev] [OpenSSL][1.0.2h] Memory leaks
> How did you configure/build? What versions of mingw/msys did you use? ./configure mingw --prefix=/mingw make depend make all make test MSYS 1.0.18(0.48/3/2) 2012-11-21 22:34 i686 unknown; targ=MINGW32 GCC 6.1.0 32-bit Memory leaks didn't occur with previous OpenSSL versions using the same environment. I'm trying OpenSSL from master now but facing issues with Perl at the moment. Grrr. Can you try building it using MinGW/MSYS 32-bit? Thanks for looking at it. Cheers. Ser. -- openssl-dev mailing list To unsubscribe: https://mta.openssl.org/mailman/listinfo/openssl-dev
Re: [openssl-dev] [OpenSSL][1.0.2h] Memory leaks
I tried to reproduce this last night, on the following platforms: linux x86_64 fips windows win64a fips windows mingw64 fips make test succeeded with no errors on the first two platforms. For windows/mingw64, though, I did get a different error: test SSL protocol test ssl3 is forbidden in FIPS mode 180668:error:2D07808C:FIPS routines:FIPS_module_mode_set:unsupported platform:fips.c:322: ssl2 disabled: skipping test test tls1 181056:error:2D07808C:FIPS routines:FIPS_module_mode_set:unsupported platform:fips.c:322: make[1]: *** [test_ssl] Error 1 make[1]: Leaving directory `/c/src/openssl-1.0.2h/test' make: *** [tests] Error 2 However, if I do make -k test, then I finish without the memory leaks you saw: ALL OCSP TESTS SUCCESSFUL Test X509v3_check_* ../util/shlib_wrap.sh ./v3nametest ../util/shlib_wrap.sh ./heartbeat_test Test constant time utilites ../util/shlib_wrap.sh ./constant_time_test Testing constant time operations... ok (ran 1908 tests) test_verify_extra ../util/shlib_wrap.sh ./verify_extra_test PASS test_clienthello ../util/shlib_wrap.sh ./clienthellotest test_sslv2conftest ../util/shlib_wrap.sh ./sslv2conftest SSLv2 CONF Test number 0 SSLv2 CONF Test number 1 SSLv2 CONF test: PASSED How did you configure/build? What versions of mingw/msys did you use? cheers, Joey On 05/31/2016 08:37 PM, Sergio NNX wrote: Ciao. Just built OpenSSL 1.0.2h from source and when running the tests I can see some memory leaks. The same did not happen when building previous versions on the same environment and same command line options. Thanks in advance. Find below the last bit of a long long long test output: ... ... ... ALL OCSP TESTS SUCCESSFUL Test X509v3_check_* ../util/shlib_wrap.sh ./v3nametest ../util/shlib_wrap.sh ./heartbeat_test Test constant time utilites ../util/shlib_wrap.sh ./constant_time_test Testing constant time operations... ok (ran 1908 tests) test_verify_extra ../util/shlib_wrap.sh ./verify_extra_test PASS test_clienthello ../util/shlib_wrap.sh ./clienthellotest [04:21:51] 221 file=buf_str.c, line=92, thread=1772, number=16, address=00867FC8 [04:21:51] 426 file=eng_lib.c, line=168, thread=1772, number=4, address=00869648 [04:21:51] 455 file=stack.c, line=162, thread=1772, number=20, address=0086ADE8 [04:21:51] 418 file=lhash.c, line=122, thread=1772, number=64, address=00869588 [04:21:51] 222 file=buf_str.c, line=92, thread=1772, number=7, address=00BBFDC8 [04:21:51] 419 file=eng_lib.c, line=168, thread=1772, number=4, address=008695D0 [04:21:51] 411 file=lhash.c, line=122, thread=1772, number=64, address=008694C8 [04:21:51] 428 file=stack.c, line=162, thread=1772, number=20, address=0086ACC8 [04:21:51] 412 file=eng_lib.c, line=168, thread=1772, number=4, address=00869510 [04:21:51] 266 file=stack.c, line=162, thread=1772, number=20, address=0086AC68 [04:21:51]72 file=eng_lib.c, line=69, thread=1772, number=112, address=00BBFD50 [04:21:51] 251 file=stack.c, line=162, thread=1772, number=20, address=0086AC08 [04:21:51] 236 file=stack.c, line=162, thread=1772, number=20, address=0086ABA8 [04:21:51] 233 file=lhash.c, line=122, thread=1772, number=64, address=0086AB48 [04:21:51] 234 file=eng_lib.c, line=168, thread=1772, number=4, address=00BB27F0 [04:21:51] 255 file=lhash.c, line=122, thread=1772, number=64, address=0086C0A8 [04:21:51] 256 file=eng_lib.c, line=168, thread=1772, number=4, address=0086C0F0 [04:21:51] 429 file=stack.c, line=164, thread=1772, number=32, address=00871E98 [04:21:51] 456 file=stack.c, line=164, thread=1772, number=16, address=0086E860 [04:21:51] 261 file=eng_table.c, line=150, thread=1772, number=16, address=00868520 [04:21:51] 417 file=lhash.c, line=120, thread=1772, number=96, address=00869520 [04:21:51] 421 file=stack.c, line=162, thread=1772, number=20, address=0086ACA8 [04:21:51] 260 file=lhash.c, line=191, thread=1772, number=12, address=008684F0 [04:21:51] 224 file=buf_str.c, line=92, thread=1772, number=18, address=0086A8C0 [04:21:51] 223 file=ameth_lib.c, line=289, thread=1772, number=108, address=0086A848 [04:21:51] 259 file=stack.c, line=164, thread=1772, number=16, address=008684C0 [04:21:51] 262 file=stack.c, line=162, thread=1772, number=20, address=0086AC48 [04:21:51] 431 file=lhash.c, line=120, thread=1772, number=96, address=00871D78 [04:21:51] 253 file=lhash.c, line=191, thread=1772, number=12, address=00868418 [04:21:51] 247 file=stack.c, line=162, thread=1772, number=20, address=0086ABE8 [04:21:51] 133 file=eng_lib.c, line=69, thread=1772, number=112, address=008693E8 [04:21:51] 252 file=stack.c, line=164, thread=1772, number=16, address=008683E8 [04:21:51] 410 file=lhash.c, line=120, thread=1772, number=96, address=00869460 [04:21:51] 220 file=ameth_lib.c, line=289, thread=1772, number=108, address=0086A7D0 [04:21:51] 219 file=eng_lib.c, line=69, thread=1772, number=112, address=0086A758 [04:21:51] 425 file=lhash.c, line=122, thread=1772, number=64
[openssl-dev] [OpenSSL][1.0.2h] Memory leaks
Ciao. Just built OpenSSL 1.0.2h from source and when running the tests I can see some memory leaks. The same did not happen when building previous versions on the same environment and same command line options. Thanks in advance. Find below the last bit of a long long long test output: ... ... ... ALL OCSP TESTS SUCCESSFUL Test X509v3_check_* ../util/shlib_wrap.sh ./v3nametest ../util/shlib_wrap.sh ./heartbeat_test Test constant time utilites ../util/shlib_wrap.sh ./constant_time_test Testing constant time operations... ok (ran 1908 tests) test_verify_extra ../util/shlib_wrap.sh ./verify_extra_test PASS test_clienthello ../util/shlib_wrap.sh ./clienthellotest [04:21:51] 221 file=buf_str.c, line=92, thread=1772, number=16, address=00867FC8 [04:21:51] 426 file=eng_lib.c, line=168, thread=1772, number=4, address=00869648 [04:21:51] 455 file=stack.c, line=162, thread=1772, number=20, address=0086ADE8 [04:21:51] 418 file=lhash.c, line=122, thread=1772, number=64, address=00869588 [04:21:51] 222 file=buf_str.c, line=92, thread=1772, number=7, address=00BBFDC8 [04:21:51] 419 file=eng_lib.c, line=168, thread=1772, number=4, address=008695D0 [04:21:51] 411 file=lhash.c, line=122, thread=1772, number=64, address=008694C8 [04:21:51] 428 file=stack.c, line=162, thread=1772, number=20, address=0086ACC8 [04:21:51] 412 file=eng_lib.c, line=168, thread=1772, number=4, address=00869510 [04:21:51] 266 file=stack.c, line=162, thread=1772, number=20, address=0086AC68 [04:21:51]72 file=eng_lib.c, line=69, thread=1772, number=112, address=00BBFD50 [04:21:51] 251 file=stack.c, line=162, thread=1772, number=20, address=0086AC08 [04:21:51] 236 file=stack.c, line=162, thread=1772, number=20, address=0086ABA8 [04:21:51] 233 file=lhash.c, line=122, thread=1772, number=64, address=0086AB48 [04:21:51] 234 file=eng_lib.c, line=168, thread=1772, number=4, address=00BB27F0 [04:21:51] 255 file=lhash.c, line=122, thread=1772, number=64, address=0086C0A8 [04:21:51] 256 file=eng_lib.c, line=168, thread=1772, number=4, address=0086C0F0 [04:21:51] 429 file=stack.c, line=164, thread=1772, number=32, address=00871E98 [04:21:51] 456 file=stack.c, line=164, thread=1772, number=16, address=0086E860 [04:21:51] 261 file=eng_table.c, line=150, thread=1772, number=16, address=00868520 [04:21:51] 417 file=lhash.c, line=120, thread=1772, number=96, address=00869520 [04:21:51] 421 file=stack.c, line=162, thread=1772, number=20, address=0086ACA8 [04:21:51] 260 file=lhash.c, line=191, thread=1772, number=12, address=008684F0 [04:21:51] 224 file=buf_str.c, line=92, thread=1772, number=18, address=0086A8C0 [04:21:51] 223 file=ameth_lib.c, line=289, thread=1772, number=108, address=0086A848 [04:21:51] 259 file=stack.c, line=164, thread=1772, number=16, address=008684C0 [04:21:51] 262 file=stack.c, line=162, thread=1772, number=20, address=0086AC48 [04:21:51] 431 file=lhash.c, line=120, thread=1772, number=96, address=00871D78 [04:21:51] 253 file=lhash.c, line=191, thread=1772, number=12, address=00868418 [04:21:51] 247 file=stack.c, line=162, thread=1772, number=20, address=0086ABE8 [04:21:51] 133 file=eng_lib.c, line=69, thread=1772, number=112, address=008693E8 [04:21:51] 252 file=stack.c, line=164, thread=1772, number=16, address=008683E8 [04:21:51] 410 file=lhash.c, line=120, thread=1772, number=96, address=00869460 [04:21:51] 220 file=ameth_lib.c, line=289, thread=1772, number=108, address=0086A7D0 [04:21:51] 219 file=eng_lib.c, line=69, thread=1772, number=112, address=0086A758 [04:21:51] 425 file=lhash.c, line=122, thread=1772, number=64, address=00871D30 [04:21:51] 437 file=lhash.c, line=191, thread=1772, number=12, address=0086E608 [04:21:51] 257 file=eng_table.c, line=150, thread=1772, number=16, address=008644A8 [04:21:51] 250 file=eng_table.c, line=150, thread=1772, number=16, address=008683A0 [04:21:51] 113 file=eng_lib.c, line=69, thread=1772, number=112, address=00868B70 [04:21:51] 246 file=eng_table.c, line=150, thread=1772, number=16, address=008682F8 [04:21:51] 249 file=lhash.c, line=191, thread=1772, number=12, address=00868370 [04:21:51] 248 file=stack.c, line=164, thread=1772, number=16, address=00868340 [04:21:51] 361 file=eng_lib.c, line=69, thread=1772, number=112, address=0086FD48 [04:21:51] 195 file=eng_lib.c, line=69, thread=1772, number=112, address=00869E68 [04:21:51] 218 file=eng_lib.c, line=69, thread=1772, number=112, address=0086A6E0 [04:21:51] 434 file=eng_table.c, line=150, thread=1772, number=16, address=0086E590 [04:21:51] 430 file=lhash.c, line=191, thread=1772, number=12, address=0086E518 [04:21:51] 231 file=pmeth_lib.c, line=200, thread=1772, number=108, address=0086AA68 [04:21:51] 232 file=lhash.c, line=120, thread=1772, number=96, address=0086AAE0 [04:21:51] 242 file=lhash.c, line=191, thread=1772, number=12, address=00868280 [04:21:51] 239 file=eng_table.c
[openssl-dev] [openssl.org #684] Memory Leaks in RSA_eay_private_decrypt
the code has changed a great deal in the past decade (!!!) -- Rich Salz, OpenSSL dev team; rs...@openssl.org -- Ticket here: http://rt.openssl.org/Ticket/Display.html?id=684 Please log in as guest with password guest if prompted -- openssl-dev mailing list To unsubscribe: https://mta.openssl.org/mailman/listinfo/openssl-dev
Re: [openssl-dev] memory leaks detected using libSSL 1.1
On 18/02/16 13:59, Michel wrote: > Yes ! > With your 2 patches applied, tls_decrypt_ticket.patch and > fix-win-thread-stop.patch, > (looks like I lost the first one yesterday), > none of my tests programs using libSSL v1.1 reports leaks. > > I feel better. :-) Great. I'll get those reviewed and committed. Matt -- openssl-dev mailing list To unsubscribe: https://mta.openssl.org/mailman/listinfo/openssl-dev
Re: [openssl-dev] memory leaks detected using libSSL 1.1
Yes ! With your 2 patches applied, tls_decrypt_ticket.patch and fix-win-thread-stop.patch, (looks like I lost the first one yesterday), none of my tests programs using libSSL v1.1 reports leaks. I feel better. :-) Thank you Matt. Regards, Michel. -Message d'origine- De : openssl-dev [mailto:openssl-dev-boun...@openssl.org] De la part de Matt Caswell Envoyé : jeudi 18 février 2016 13:27 À : openssl-dev@openssl.org Objet : Re: [openssl-dev] memory leaks detected using libSSL 1.1 On 18/02/16 11:37, Michel wrote: > Hi Matt, > > Here under is the new results after applying your patch. > Let me know anything I could do to investigate deeper. Thanks. That was very helpful. I think I may have identified the issue. Please can you try the attached patch and see if that fixes things? Thanks Matt -- openssl-dev mailing list To unsubscribe: https://mta.openssl.org/mailman/listinfo/openssl-dev
Re: [openssl-dev] memory leaks detected using libSSL 1.1
On 18/02/16 11:37, Michel wrote:
> Hi Matt,
>
> Here under is the new results after applying your patch.
> Let me know anything I could do to investigate deeper.
Thanks. That was very helpful. I think I may have identified the issue.
Please can you try the attached patch and see if that fixes things?
Thanks
Matt
>
> Regards,
>
> Michel.
>
> Thread serveur 5324 demarre
> Thread client 6348 demarre
> OPENSSL_INIT: ossl_init_base: Setting up stop handlers
> OPENSSL_INIT: ossl_init_add_all_ciphers: openssl_add_all_ciphers_internal()
> OPENSSL_INIT: ossl_init_add_all_digests: openssl_add_all_digests_internal()
> OPENSSL_INIT: ossl_init_ssl_base: Adding SSL ciphers and digests
> OPENSSL_INIT: ossl_init_ssl_base: SSL_COMP_get_compression_methods()
> OPENSSL_INIT: ossl_init_ssl_base: SSL_add_ssl_module()
> OPENSSL_INIT: ossl_init_load_ssl_strings: ERR_load_SSL_strings()
> OPENSSL_INIT: ossl_init_async: async_init()
> OPENSSL_INIT: ossl_init_load_crypto_strings:
> err_load_crypto_strings_intern()
> OPENSSL_INIT: ossl_init_get_thread_local: Starting: Local value is NULL,
> alloc is 1 (6348)
> OPENSSL_INIT: ossl_init_get_thread_local: Allocating a new local structure
> (6348)
> OPENSSL_INIT: ossl_init_get_thread_local: Ending: Local value is NOT NULL,
> alloc is 1 (6348)
> OPENSSL_INIT: ossl_init_thread_start: Starting (6348)
> OPENSSL_INIT: ossl_init_thread_start: marking thread for err_state (6348)
> OPENSSL_INIT: ossl_init_thread_start: End (6348)
> OPENSSL_INIT: ossl_init_get_thread_local: Starting: Local value is NULL,
> alloc is 1 (5324)
> OPENSSL_INIT: ossl_init_get_thread_local: Allocating a new local structure
> (5324)
> OPENSSL_INIT: ossl_init_get_thread_local: Ending: Local value is NOT NULL,
> alloc is 1 (5324)
> OPENSSL_INIT: ossl_init_thread_start: Starting (5324)
> OPENSSL_INIT: ossl_init_thread_start: marking thread for err_state (5324)
> OPENSSL_INIT: ossl_init_thread_start: End (5324)
> ...
> OPENSSL_INIT: OPENSSL_thread_stop: starting (6348)
> OPENSSL_INIT: ossl_init_get_thread_local: Starting: Local value is NOT NULL,
> alloc is 0 (6348)
> OPENSSL_INIT: ossl_init_get_thread_local: Clearing Thread Locals (6348)
> OPENSSL_INIT: ossl_init_get_thread_local: Ending: Local value is NOT NULL,
> alloc is 0 (6348)
> OPENSSL_INIT: ossl_init_thread_stop: starting (6348)
> OPENSSL_INIT: ossl_init_thread_stop: ERR_remove_thread_state(NULL) (6348)
> OPENSSL_INIT: OPENSSL_thread_stop: starting (5324)
> OPENSSL_INIT: ossl_init_get_thread_local: Starting: Local value is NULL,
> alloc is 0 (5324)
> OPENSSL_INIT: ossl_init_get_thread_local: Clearing Thread Locals (5324)
> OPENSSL_INIT: ossl_init_get_thread_local: Ending: Local value is NULL, alloc
> is 0 (5324)
> OPENSSL_INIT: ossl_init_thread_stop: starting (5324)
> OPENSSL_INIT: ossl_init_thread_stop: locals are NULL returning (5324)
> OPENSSL_INIT: OPENSSL_thread_stop: ending (5324)
> OPENSSL_INIT: ossl_init_thread_stop: end (6348)
> OPENSSL_INIT: OPENSSL_thread_stop: ending (6348)
>
> Thread serveur 5324 termine
> Thread client 6348 termine
>
> OPENSSL_INIT: ossl_init_get_thread_local: Starting: Local value is NULL,
> alloc is 0 (6240)
> OPENSSL_INIT: ossl_init_get_thread_local: Clearing Thread Locals (6240)
> OPENSSL_INIT: ossl_init_get_thread_local: Ending: Local value is NULL, alloc
> is 0 (6240)
> OPENSSL_INIT: ossl_init_thread_stop: starting (6240)
> OPENSSL_INIT: ossl_init_thread_stop: locals are NULL returning (6240)
> OPENSSL_INIT: ssl_library_stop: SSL_COMP_free_compression_methods()
> OPENSSL_INIT: ssl_library_stop: ERR_free_strings()
> OPENSSL_INIT: OPENSSL_cleanup: ERR_free_strings()
> OPENSSL_INIT: OPENSSL_INIT_library_stop: CRYPTO_cleanup_all_ex_data()
> OPENSSL_INIT: OPENSSL_INIT_library_stop: EVP_cleanup()
> OPENSSL_INIT: OPENSSL_INIT_library_stop: CONF_modules_free()
> OPENSSL_INIT: OPENSSL_INIT_library_stop: RAND_cleanup()
> Assertion failed: !bLeak, file p:\mes
> programmes\tests\_testsshared\teststls-11\testtls.cpp, line 165
>
> Detected memory leaks!
> Dumping objects ->
> {7025} normal block at 0x00A75628, 8 bytes long.
> Data: <> 00 00 00 00 01 00 00 00
> {5009} normal block at 0x00A3CB88, 12 bytes long.
> Data: < 4 > A8 0C A4 00 00 00 00 00 C0 34 01 00
> {5008} normal block at 0x00A40CA8, 400 bytes long.
> Data: <> 00 00 00 00 C0 17 00 00 00 00 00 00 00 00 00 00
> {5003} normal block at 0x00A3CCB8, 64 bytes long.
> Data: <> 88 CB A3 00 00 00 00 00 00 00 00 00 00 00 00 00
> {5001} normal block at 0x00A3C9B0, 96 bytes long.
> Data: <P9P p9P > B8 CC A3 00 50 39 50 00 70 39 50 00 08 00 00 00
> Object dump complete.
>
> WARNING: Visual Leak Dete
Re: [openssl-dev] memory leaks detected using libSSL 1.1
Hi Matt,
Here under is the new results after applying your patch.
Let me know anything I could do to investigate deeper.
Regards,
Michel.
Thread serveur 5324 demarre
Thread client 6348 demarre
OPENSSL_INIT: ossl_init_base: Setting up stop handlers
OPENSSL_INIT: ossl_init_add_all_ciphers: openssl_add_all_ciphers_internal()
OPENSSL_INIT: ossl_init_add_all_digests: openssl_add_all_digests_internal()
OPENSSL_INIT: ossl_init_ssl_base: Adding SSL ciphers and digests
OPENSSL_INIT: ossl_init_ssl_base: SSL_COMP_get_compression_methods()
OPENSSL_INIT: ossl_init_ssl_base: SSL_add_ssl_module()
OPENSSL_INIT: ossl_init_load_ssl_strings: ERR_load_SSL_strings()
OPENSSL_INIT: ossl_init_async: async_init()
OPENSSL_INIT: ossl_init_load_crypto_strings:
err_load_crypto_strings_intern()
OPENSSL_INIT: ossl_init_get_thread_local: Starting: Local value is NULL,
alloc is 1 (6348)
OPENSSL_INIT: ossl_init_get_thread_local: Allocating a new local structure
(6348)
OPENSSL_INIT: ossl_init_get_thread_local: Ending: Local value is NOT NULL,
alloc is 1 (6348)
OPENSSL_INIT: ossl_init_thread_start: Starting (6348)
OPENSSL_INIT: ossl_init_thread_start: marking thread for err_state (6348)
OPENSSL_INIT: ossl_init_thread_start: End (6348)
OPENSSL_INIT: ossl_init_get_thread_local: Starting: Local value is NULL,
alloc is 1 (5324)
OPENSSL_INIT: ossl_init_get_thread_local: Allocating a new local structure
(5324)
OPENSSL_INIT: ossl_init_get_thread_local: Ending: Local value is NOT NULL,
alloc is 1 (5324)
OPENSSL_INIT: ossl_init_thread_start: Starting (5324)
OPENSSL_INIT: ossl_init_thread_start: marking thread for err_state (5324)
OPENSSL_INIT: ossl_init_thread_start: End (5324)
...
OPENSSL_INIT: OPENSSL_thread_stop: starting (6348)
OPENSSL_INIT: ossl_init_get_thread_local: Starting: Local value is NOT NULL,
alloc is 0 (6348)
OPENSSL_INIT: ossl_init_get_thread_local: Clearing Thread Locals (6348)
OPENSSL_INIT: ossl_init_get_thread_local: Ending: Local value is NOT NULL,
alloc is 0 (6348)
OPENSSL_INIT: ossl_init_thread_stop: starting (6348)
OPENSSL_INIT: ossl_init_thread_stop: ERR_remove_thread_state(NULL) (6348)
OPENSSL_INIT: OPENSSL_thread_stop: starting (5324)
OPENSSL_INIT: ossl_init_get_thread_local: Starting: Local value is NULL,
alloc is 0 (5324)
OPENSSL_INIT: ossl_init_get_thread_local: Clearing Thread Locals (5324)
OPENSSL_INIT: ossl_init_get_thread_local: Ending: Local value is NULL, alloc
is 0 (5324)
OPENSSL_INIT: ossl_init_thread_stop: starting (5324)
OPENSSL_INIT: ossl_init_thread_stop: locals are NULL returning (5324)
OPENSSL_INIT: OPENSSL_thread_stop: ending (5324)
OPENSSL_INIT: ossl_init_thread_stop: end (6348)
OPENSSL_INIT: OPENSSL_thread_stop: ending (6348)
Thread serveur 5324 termine
Thread client 6348 termine
OPENSSL_INIT: ossl_init_get_thread_local: Starting: Local value is NULL,
alloc is 0 (6240)
OPENSSL_INIT: ossl_init_get_thread_local: Clearing Thread Locals (6240)
OPENSSL_INIT: ossl_init_get_thread_local: Ending: Local value is NULL, alloc
is 0 (6240)
OPENSSL_INIT: ossl_init_thread_stop: starting (6240)
OPENSSL_INIT: ossl_init_thread_stop: locals are NULL returning (6240)
OPENSSL_INIT: ssl_library_stop: SSL_COMP_free_compression_methods()
OPENSSL_INIT: ssl_library_stop: ERR_free_strings()
OPENSSL_INIT: OPENSSL_cleanup: ERR_free_strings()
OPENSSL_INIT: OPENSSL_INIT_library_stop: CRYPTO_cleanup_all_ex_data()
OPENSSL_INIT: OPENSSL_INIT_library_stop: EVP_cleanup()
OPENSSL_INIT: OPENSSL_INIT_library_stop: CONF_modules_free()
OPENSSL_INIT: OPENSSL_INIT_library_stop: RAND_cleanup()
Assertion failed: !bLeak, file p:\mes
programmes\tests\_testsshared\teststls-11\testtls.cpp, line 165
Detected memory leaks!
Dumping objects ->
{7025} normal block at 0x00A75628, 8 bytes long.
Data: <> 00 00 00 00 01 00 00 00
{5009} normal block at 0x00A3CB88, 12 bytes long.
Data: < 4 > A8 0C A4 00 00 00 00 00 C0 34 01 00
{5008} normal block at 0x00A40CA8, 400 bytes long.
Data: <> 00 00 00 00 C0 17 00 00 00 00 00 00 00 00 00 00
{5003} normal block at 0x00A3CCB8, 64 bytes long.
Data: <> 88 CB A3 00 00 00 00 00 00 00 00 00 00 00 00 00
{5001} normal block at 0x00A3C9B0, 96 bytes long.
Data: <P9P p9P > B8 CC A3 00 50 39 50 00 70 39 50 00 08 00 00 00
Object dump complete.
WARNING: Visual Leak Detector detected memory leaks!
-- Block 4993 at 0x00A3C9B0: 96 bytes --
Leak Hash: 0x7CDBED0B, Count: 1, Total 96 bytes
Call Stack (TID 4804):
ntdll.dll!RtlAllocateHeap()
f:\dd\vctools\crt\crtw32\misc\dbgmalloc.c (56): TestsTLS-11.exe!malloc()
+ 0x15 bytes
e:\openssl-1.1.git\crypto\mem.c (141): TestsTLS-11.exe!CRYPTO_malloc() +
0x9 bytes
e:\openssl-1.1.git\crypto\mem.c (161): TestsTLS-11.exe!CRYPTO_zalloc() +
0x11 bytes
e:\openssl-1.1.git\crypto\lhash\lhash.c (116): TestsTLS-11.exe!lh_new()
+ 0xE bytes
e:\openssl-1.1.git\crypto\err\err_lcl.h (2):
TestsTLS-11.exe!lh_ERR_STATE_new() + 0x10 bytes
e:\openssl-1.1.git\crypto
Re: [openssl-dev] memory leaks detected using libSSL 1.1
On 18/02/16 00:13, Michel wrote:
> Hi Matt,
>
> Thanks for the suggestion.
>
> This is what was printed to stderr :
> OPENSSL_INIT: ossl_init_base: Setting up stop handlers
> OPENSSL_INIT: ossl_init_add_all_ciphers: openssl_add_all_ciphers_internal()
> OPENSSL_INIT: ossl_init_add_all_digests: openssl_add_all_digests_internal()
> OPENSSL_INIT: ossl_init_ssl_base: Adding SSL ciphers and digests
> OPENSSL_INIT: ossl_init_ssl_base: SSL_COMP_get_compression_methods()
> OPENSSL_INIT: ossl_init_ssl_base: SSL_add_ssl_module()
> OPENSSL_INIT: ossl_init_load_ssl_strings: ERR_load_SSL_strings()
> OPENSSL_INIT: ossl_init_async: async_init()
> OPENSSL_INIT: ossl_init_load_crypto_strings:
> err_load_crypto_strings_intern()
> OPENSSL_INIT: ossl_init_thread_start: marking thread for err_state
> OPENSSL_INIT: ossl_init_thread_start: marking thread for err_state
> OPENSSL_INIT: ossl_init_thread_stop: ERR_remove_thread_state(NULL)
> OPENSSL_INIT: ssl_library_stop: SSL_COMP_free_compression_methods()
> OPENSSL_INIT: ssl_library_stop: ERR_free_strings()
> OPENSSL_INIT: OPENSSL_cleanup: ERR_free_strings()
> OPENSSL_INIT: OPENSSL_INIT_library_stop: CRYPTO_cleanup_all_ex_data()
> OPENSSL_INIT: OPENSSL_INIT_library_stop: EVP_cleanup()
> OPENSSL_INIT: OPENSSL_INIT_library_stop: CONF_modules_free()
> OPENSSL_INIT: OPENSSL_INIT_library_stop: RAND_cleanup()
>
> Shouldn't there be at least another line with ERR_remove_thread_state() (one
> for each thread) ?
Yes. I can see we have two of these:
OPENSSL_INIT: ossl_init_thread_start: marking thread for err_state
OPENSSL_INIT: ossl_init_thread_start: marking thread for err_state
Which means that the init code has spotted that there are two threads
running and has initialised the error system for both of them.
But we only get one of these:
OPENSSL_INIT: ossl_init_thread_stop: ERR_remove_thread_state(NULL)
Which means only one of the two threads has subsequently been de-inited.
That's very odd.
I have two possible theories:
1) OPENSSL_thread_stop() is not actually getting called as we think it is.
Or
2) The Thread Local Structure that keeps track of what things need
cleanup is not being obtained correctly for some reason during the
thread stop...so we have "forgotten" that we initialised the error system.
To try and help narrow down which of these possibilities it is I have
created a patch (attached) which bumps up the logging significantly.
Please can you apply it, rerun your code (with OPENSSL_INIT_DEBUG
defined still) and post the output here?
Thanks
Matt
> This test program launch 1 server thread and 1 client thread.
> Both of them have OPENSSL_thread_stop() in their [pre-]exit member function.
>
> Michel.
>
> -Message d'origine-
> De : openssl-dev [mailto:openssl-dev-boun...@openssl.org] De la part de Matt
> Caswell
> Envoyé : mercredi 17 février 2016 17:23
> À : openssl-dev@openssl.org
> Objet : Re: [openssl-dev] memory leaks detected using libSSL 1.1
>
>> Am I missing anything else ?
>
> That should be sufficient (although the OPENSSL_cleanup() should not be
> required).
>
> You could try compiling OpenSSL with OPENSSL_INIT_DEBUG defined, e.g.
>
> perl Configure your-platform-here -DOPENSSL_INIT_DEBUG
>
> This should print out some debugging information to stderr every time the
> init functions attempt to do something interesting. In particular when you
> call OPENSSL_thread_stop() you should see the following printed
> out:
>
> OPENSSL_INIT: ossl_init_thread_stop: ERR_remove_thread_state(NULL)
>
> Matt
>
From 6679ef43680a63aac9430bfbc6d49014d5675948 Mon Sep 17 00:00:00 2001
From: Matt Caswell <m...@openssl.org>
Date: Thu, 18 Feb 2016 09:32:42 +
Subject: [PATCH] Add some additional logging to thread stop code
---
crypto/init.c | 77 ++-
1 file changed, 71 insertions(+), 6 deletions(-)
diff --git a/crypto/init.c b/crypto/init.c
index 25e3dc7..c4d5c81 100644
--- a/crypto/init.c
+++ b/crypto/init.c
@@ -194,14 +194,36 @@ static struct thread_local_inits_st *ossl_init_get_thread_local(int alloc)
{
struct thread_local_inits_st *local = TlsGetValue(threadstopkey);
+#ifdef OPENSSL_INIT_DEBUG
+DWORD threadid;
+threadid = GetCurrentThreadId();
+fprintf(stderr, "OPENSSL_INIT: ossl_init_get_thread_local: "
+"Starting: Local value is %s, alloc is %d (%ld)\n",
+((local == NULL)?"NULL":"NOT NULL"), alloc, threadid);
+#endif
+
if (local == NULL && alloc) {
+#ifdef OPENSSL_INIT_DEBUG
+fprintf(stderr, "OPENSSL_INIT: ossl_init_get_thread_local: "
+"Allocating a new local structure (%ld)\n", threadid);
+#endif
local = OPENSSL_zalloc
Re: [openssl-dev] memory leaks detected using libSSL 1.1
Hi Matt, Thanks for the suggestion. This is what was printed to stderr : OPENSSL_INIT: ossl_init_base: Setting up stop handlers OPENSSL_INIT: ossl_init_add_all_ciphers: openssl_add_all_ciphers_internal() OPENSSL_INIT: ossl_init_add_all_digests: openssl_add_all_digests_internal() OPENSSL_INIT: ossl_init_ssl_base: Adding SSL ciphers and digests OPENSSL_INIT: ossl_init_ssl_base: SSL_COMP_get_compression_methods() OPENSSL_INIT: ossl_init_ssl_base: SSL_add_ssl_module() OPENSSL_INIT: ossl_init_load_ssl_strings: ERR_load_SSL_strings() OPENSSL_INIT: ossl_init_async: async_init() OPENSSL_INIT: ossl_init_load_crypto_strings: err_load_crypto_strings_intern() OPENSSL_INIT: ossl_init_thread_start: marking thread for err_state OPENSSL_INIT: ossl_init_thread_start: marking thread for err_state OPENSSL_INIT: ossl_init_thread_stop: ERR_remove_thread_state(NULL) OPENSSL_INIT: ssl_library_stop: SSL_COMP_free_compression_methods() OPENSSL_INIT: ssl_library_stop: ERR_free_strings() OPENSSL_INIT: OPENSSL_cleanup: ERR_free_strings() OPENSSL_INIT: OPENSSL_INIT_library_stop: CRYPTO_cleanup_all_ex_data() OPENSSL_INIT: OPENSSL_INIT_library_stop: EVP_cleanup() OPENSSL_INIT: OPENSSL_INIT_library_stop: CONF_modules_free() OPENSSL_INIT: OPENSSL_INIT_library_stop: RAND_cleanup() Shouldn't there be at least another line with ERR_remove_thread_state() (one for each thread) ? This test program launch 1 server thread and 1 client thread. Both of them have OPENSSL_thread_stop() in their [pre-]exit member function. Michel. -Message d'origine- De : openssl-dev [mailto:openssl-dev-boun...@openssl.org] De la part de Matt Caswell Envoyé : mercredi 17 février 2016 17:23 À : openssl-dev@openssl.org Objet : Re: [openssl-dev] memory leaks detected using libSSL 1.1 > Am I missing anything else ? That should be sufficient (although the OPENSSL_cleanup() should not be required). You could try compiling OpenSSL with OPENSSL_INIT_DEBUG defined, e.g. perl Configure your-platform-here -DOPENSSL_INIT_DEBUG This should print out some debugging information to stderr every time the init functions attempt to do something interesting. In particular when you call OPENSSL_thread_stop() you should see the following printed out: OPENSSL_INIT: ossl_init_thread_stop: ERR_remove_thread_state(NULL) Matt -- openssl-dev mailing list To unsubscribe: https://mta.openssl.org/mailman/listinfo/openssl-dev
Re: [openssl-dev] memory leaks detected using libSSL 1.1
On 16/02/16 23:25, Michel wrote:
> Hi Matt,
>
> Yes I am linking statically and I read the man about OPENSSL_init_crypto(),
> thanks.
> However I still have leaks reported.
> :-(
>
> What I have changed to adapt to v1.1 is calling OPENSSL_thread_stop() in
> each thread before it leaves,
> instead of ERR_remove_thread_state( NULL ),
> and I am calling OPENSSL_cleanup() before the main thread returns.
> Am I missing anything else ?
That should be sufficient (although the OPENSSL_cleanup() should not be
required).
You could try compiling OpenSSL with OPENSSL_INIT_DEBUG defined, e.g.
perl Configure your-platform-here -DOPENSSL_INIT_DEBUG
This should print out some debugging information to stderr every time
the init functions attempt to do something interesting. In particular
when you call OPENSSL_thread_stop() you should see the following printed
out:
OPENSSL_INIT: ossl_init_thread_stop: ERR_remove_thread_state(NULL)
Matt
>
> Now I have 2 tests programs, almost identical, one is linked against v1.0.2
> and the other against v1.1.
> The former (1.0.2) doesn't report any leak but the later (1.1) always report
> leaks.
>
> I tried lots of modifications to the 1.1 version in order to understand what
> is happening,
> but the only thing I noticed is leaks occur if at some point the program go
> to the OpenSSL error subsystem,
> like in the 2 reports below.
>
> Can you please help in this matter ?
>
> Detected memory leaks!
> Dumping objects ->
> {7453} normal block at 0x00895440, 8 bytes long.
> Data: <> 00 00 00 00 01 00 00 00
> {5460} normal block at 0x00871B98, 12 bytes long.
> Data: < e > C8 19 87 00 00 00 00 00 B4 65 01 00
> {5459} normal block at 0x008719C8, 400 bytes long.
> Data: <> 00 00 00 00 84 1B 00 00 00 00 00 00 00 00 00 00
> {5457} normal block at 0x00871900, 64 bytes long.
> Data: <> 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> {5455} normal block at 0x0086D7E8, 96 bytes long.
> Data: < ` > 00 19 87 00 60 CB 05 01 80 CB 05 01 08 00 00 00
> Object dump complete.
>
> WARNING: Visual Leak Detector detected memory leaks!
> -- Block 5450 at 0x0086D7E8: 96 bytes --
> Leak Hash: 0xE1A21867, Count: 1, Total 96 bytes
> Call Stack (TID 6956):
> ntdll.dll!RtlAllocateHeap()
> f:\dd\vctools\crt\crtw32\misc\dbgmalloc.c (56): TestsTLS-11.exe!malloc()
> + 0x15 bytes
> e:\openssl-1.1.git\crypto\mem.c (138): TestsTLS-11.exe!CRYPTO_malloc() +
> 0x9 bytes
> e:\openssl-1.1.git\crypto\mem.c (158): TestsTLS-11.exe!CRYPTO_zalloc() +
> 0x11 bytes
> e:\openssl-1.1.git\crypto\lhash\lhash.c (116): TestsTLS-11.exe!lh_new()
> + 0xB bytes
> e:\openssl-1.1.git\crypto\err\err_lcl.h (2):
> TestsTLS-11.exe!lh_ERR_STATE_new() + 0x10 bytes
> e:\openssl-1.1.git\crypto\err\err.c (321):
> TestsTLS-11.exe!int_thread_get() + 0xF bytes
> e:\openssl-1.1.git\crypto\err\err.c (369):
> TestsTLS-11.exe!int_thread_set_item() + 0x9 bytes
> e:\openssl-1.1.git\crypto\err\err.c (884):
> TestsTLS-11.exe!ERR_get_state() + 0xC bytes
> e:\openssl-1.1.git\crypto\err\err.c (581):
> TestsTLS-11.exe!ERR_put_error() + 0x5 bytes
> e:\openssl-1.1.git\crypto\pem\pem_lib.c (702):
> TestsTLS-11.exe!PEM_read_bio() + 0x15 bytes
> e:\openssl-1.1.git\crypto\pem\pem_info.c (117):
> TestsTLS-11.exe!PEM_X509_INFO_read_bio() + 0x19 bytes
> e:\openssl-1.1.git\crypto\x509\by_file.c (249):
> TestsTLS-11.exe!X509_load_cert_crl_file() + 0xF bytes
> e:\openssl-1.1.git\crypto\x509\by_file.c (113):
> TestsTLS-11.exe!by_file_ctrl() + 0xF bytes
> e:\openssl-1.1.git\crypto\x509\x509_lu.c (117):
> TestsTLS-11.exe!X509_LOOKUP_ctrl() + 0x1F bytes
> e:\openssl-1.1.git\crypto\x509\x509_d2.c (92):
> TestsTLS-11.exe!X509_STORE_load_locations() + 0x13 bytes
> e:\openssl-1.1.git\ssl\ssl_lib.c (3344):
> TestsTLS-11.exe!SSL_CTX_load_verify_locations() + 0x14 bytes
> p:\mes programmes\shared\ocrypto-11\tls.cpp (410):
> TestsTLS-11.exe!OTLS::TLSCtx::LoadTrustedCertsFile() + 0x12 bytes
> p:\mes programmes\tests\_testsshared\teststls-11\tlscltsetup.cpp (99):
> TestsTLS-11.exe!TLSCltConfig::Setup() + 0x26 bytes
> p:\mes programmes\tests\_testsshared\teststls-11\clttasks.cpp (90):
> TestsTLS-11.exe!CltThread::Main() + 0x17 bytes
> p:\mes programmes\shared\sthread.cpp (17):
> TestsTLS-11.exe!SThread::Run() + 0xE bytes
> f:\dd\vctools\crt\crtw32\startup\threadex.c (359):
> TestsTLS-11.exe!_threadstartex()
>
>
> -- Block 5452 at 0x00871900: 64 bytes --
> Leak Hash: 0x188D6136, Count: 1, Total 64 bytes
> Call Stack (TID 6956):
> ntdll.dll!RtlAllocateHeap()
> f:\dd\
Re: [openssl-dev] memory leaks detected using libSSL 1.1
Hi Matt,
Yes I am linking statically and I read the man about OPENSSL_init_crypto(),
thanks.
However I still have leaks reported.
:-(
What I have changed to adapt to v1.1 is calling OPENSSL_thread_stop() in
each thread before it leaves,
instead of ERR_remove_thread_state( NULL ),
and I am calling OPENSSL_cleanup() before the main thread returns.
Am I missing anything else ?
Now I have 2 tests programs, almost identical, one is linked against v1.0.2
and the other against v1.1.
The former (1.0.2) doesn't report any leak but the later (1.1) always report
leaks.
I tried lots of modifications to the 1.1 version in order to understand what
is happening,
but the only thing I noticed is leaks occur if at some point the program go
to the OpenSSL error subsystem,
like in the 2 reports below.
Can you please help in this matter ?
Detected memory leaks!
Dumping objects ->
{7453} normal block at 0x00895440, 8 bytes long.
Data: <> 00 00 00 00 01 00 00 00
{5460} normal block at 0x00871B98, 12 bytes long.
Data: < e > C8 19 87 00 00 00 00 00 B4 65 01 00
{5459} normal block at 0x008719C8, 400 bytes long.
Data: <> 00 00 00 00 84 1B 00 00 00 00 00 00 00 00 00 00
{5457} normal block at 0x00871900, 64 bytes long.
Data: <> 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
{5455} normal block at 0x0086D7E8, 96 bytes long.
Data: <` > 00 19 87 00 60 CB 05 01 80 CB 05 01 08 00 00 00
Object dump complete.
WARNING: Visual Leak Detector detected memory leaks!
-- Block 5450 at 0x0086D7E8: 96 bytes --
Leak Hash: 0xE1A21867, Count: 1, Total 96 bytes
Call Stack (TID 6956):
ntdll.dll!RtlAllocateHeap()
f:\dd\vctools\crt\crtw32\misc\dbgmalloc.c (56): TestsTLS-11.exe!malloc()
+ 0x15 bytes
e:\openssl-1.1.git\crypto\mem.c (138): TestsTLS-11.exe!CRYPTO_malloc() +
0x9 bytes
e:\openssl-1.1.git\crypto\mem.c (158): TestsTLS-11.exe!CRYPTO_zalloc() +
0x11 bytes
e:\openssl-1.1.git\crypto\lhash\lhash.c (116): TestsTLS-11.exe!lh_new()
+ 0xB bytes
e:\openssl-1.1.git\crypto\err\err_lcl.h (2):
TestsTLS-11.exe!lh_ERR_STATE_new() + 0x10 bytes
e:\openssl-1.1.git\crypto\err\err.c (321):
TestsTLS-11.exe!int_thread_get() + 0xF bytes
e:\openssl-1.1.git\crypto\err\err.c (369):
TestsTLS-11.exe!int_thread_set_item() + 0x9 bytes
e:\openssl-1.1.git\crypto\err\err.c (884):
TestsTLS-11.exe!ERR_get_state() + 0xC bytes
e:\openssl-1.1.git\crypto\err\err.c (581):
TestsTLS-11.exe!ERR_put_error() + 0x5 bytes
e:\openssl-1.1.git\crypto\pem\pem_lib.c (702):
TestsTLS-11.exe!PEM_read_bio() + 0x15 bytes
e:\openssl-1.1.git\crypto\pem\pem_info.c (117):
TestsTLS-11.exe!PEM_X509_INFO_read_bio() + 0x19 bytes
e:\openssl-1.1.git\crypto\x509\by_file.c (249):
TestsTLS-11.exe!X509_load_cert_crl_file() + 0xF bytes
e:\openssl-1.1.git\crypto\x509\by_file.c (113):
TestsTLS-11.exe!by_file_ctrl() + 0xF bytes
e:\openssl-1.1.git\crypto\x509\x509_lu.c (117):
TestsTLS-11.exe!X509_LOOKUP_ctrl() + 0x1F bytes
e:\openssl-1.1.git\crypto\x509\x509_d2.c (92):
TestsTLS-11.exe!X509_STORE_load_locations() + 0x13 bytes
e:\openssl-1.1.git\ssl\ssl_lib.c (3344):
TestsTLS-11.exe!SSL_CTX_load_verify_locations() + 0x14 bytes
p:\mes programmes\shared\ocrypto-11\tls.cpp (410):
TestsTLS-11.exe!OTLS::TLSCtx::LoadTrustedCertsFile() + 0x12 bytes
p:\mes programmes\tests\_testsshared\teststls-11\tlscltsetup.cpp (99):
TestsTLS-11.exe!TLSCltConfig::Setup() + 0x26 bytes
p:\mes programmes\tests\_testsshared\teststls-11\clttasks.cpp (90):
TestsTLS-11.exe!CltThread::Main() + 0x17 bytes
p:\mes programmes\shared\sthread.cpp (17):
TestsTLS-11.exe!SThread::Run() + 0xE bytes
f:\dd\vctools\crt\crtw32\startup\threadex.c (359):
TestsTLS-11.exe!_threadstartex()
-- Block 5452 at 0x00871900: 64 bytes --
Leak Hash: 0x188D6136, Count: 1, Total 64 bytes
Call Stack (TID 6956):
ntdll.dll!RtlAllocateHeap()
f:\dd\vctools\crt\crtw32\misc\dbgmalloc.c (56): TestsTLS-11.exe!malloc()
+ 0x15 bytes
e:\openssl-1.1.git\crypto\mem.c (138): TestsTLS-11.exe!CRYPTO_malloc() +
0x9 bytes
e:\openssl-1.1.git\crypto\mem.c (158): TestsTLS-11.exe!CRYPTO_zalloc() +
0x11 bytes
e:\openssl-1.1.git\crypto\lhash\lhash.c (118): TestsTLS-11.exe!lh_new()
+ 0xB bytes
e:\openssl-1.1.git\crypto\err\err_lcl.h (2):
TestsTLS-11.exe!lh_ERR_STATE_new() + 0x10 bytes
e:\openssl-1.1.git\crypto\err\err.c (321):
TestsTLS-11.exe!int_thread_get() + 0xF bytes
e:\openssl-1.1.git\crypto\err\err.c (369):
TestsTLS-11.exe!int_thread_set_item() + 0x9 bytes
e:\openssl-1.1.git\crypto\err\err.c (884):
TestsTLS-11.exe!ERR_get_state() + 0xC bytes
e:\openssl-1.1.git\crypto\err\err.c (581):
TestsTLS-11.exe!ERR_put_error() + 0x5 bytes
e:\openssl-1.1.git\crypto\pem\pem_lib.c (702):
TestsTLS-11.exe!PEM_read_bio() + 0x15 bytes
e:\openssl-1.1.git\crypto\pem\pem_info.c (117):
TestsTLS-11.exe!PEM_X509_INFO_read_bio() + 0x19 by
Re: [openssl-dev] memory leaks detected using libSSL 1.1
On 14/02/16 00:11, Michel wrote:
> Hi Matt,
>
> Thanks for your quick answer.
> I applied your patch and it fixes the leaks found in the simple test
> program.
>
> However, a more complex one, still report [other] leaks.
>
> Below is a new log if you can have a look at them.
> I will investigate deeper tomorrow concering 1.0.2.
Are you linking to OpenSSL statically?
Please see the "Notes" section on this page:
https://www.openssl.org/docs/manmaster/crypto/OPENSSL_atexit.html
Matt
>
> Thanks again,
>
> Michel.
>
> Detected memory leaks!
> Dumping objects ->
> {4383} normal block at 0x006472C8, 8 bytes long.
> Data: <> 00 00 00 00 01 00 00 00
> {4381} normal block at 0x00646B48, 12 bytes long.
> Data: < od } > D8 6F 64 00 00 00 00 00 20 7D 00 00
> {4379} normal block at 0x00647248, 64 bytes long.
> Data: 48 6B 64 00 00 00 00 00 00 00 00 00 00 00 00 00
> {4377} normal block at 0x006471A8, 96 bytes long.
> Data: 48 72 64 00 10 03 A5 00 30 03 A5 00 08 00 00 00
> {4375} normal block at 0x00646FD8, 400 bytes long.
> Data: <> 00 00 00 00 A0 09 00 00 00 00 00 00 00 00 00 00
> Object dump complete.
>
> WARNING: Visual Leak Detector detected memory leaks!
> -- Block 4363 at 0x00646B48: 12 bytes --
> Leak Hash: 0xF7E93E2A, Count: 1, Total 12 bytes
> Call Stack (TID 2464):
> ntdll.dll!RtlAllocateHeap()
> f:\dd\vctools\crt\crtw32\misc\dbgmalloc.c (56): TestsTLS-11.exe!malloc()
> + 0x15 bytes
> e:\openssl-1.1.git\crypto\mem.c (138): TestsTLS-11.exe!CRYPTO_malloc() +
> 0x9 bytes
> e:\openssl-1.1.git\crypto\lhash\lhash.c (168):
> TestsTLS-11.exe!lh_insert() + 0xB bytes
> e:\openssl-1.1.git\crypto\err\err_lcl.h (2):
> TestsTLS-11.exe!lh_ERR_STATE_insert() + 0x10 bytes
> e:\openssl-1.1.git\crypto\err\err.c (371):
> TestsTLS-11.exe!int_thread_set_item() + 0xD bytes
> e:\openssl-1.1.git\crypto\err\err.c (884):
> TestsTLS-11.exe!ERR_get_state() + 0xC bytes
> e:\openssl-1.1.git\crypto\err\err.c (598):
> TestsTLS-11.exe!ERR_clear_error() + 0x5 bytes
> e:\openssl-1.1.git\ssl\statem\statem.c (279):
> TestsTLS-11.exe!state_machine()
> e:\openssl-1.1.git\ssl\statem\statem.c (217):
> TestsTLS-11.exe!ossl_statem_connect() + 0xB bytes
> e:\openssl-1.1.git\ssl\ssl_lib.c (2908):
> TestsTLS-11.exe!SSL_do_handshake() + 0xC bytes
> p:\mes programmes\shared\ocrypto-11\tls.cpp (1017):
> TestsTLS-11.exe!OTLS::TLSSss::DoHandshake() + 0xC bytes
> p:\mes programmes\tests\_testsshared\teststls-11-leak\clttasks.cpp (63):
> TestsTLS-11.exe!CltThread::Main() + 0xB bytes
> p:\mes programmes\shared\sthread.cpp (17):
> TestsTLS-11.exe!SThread::Run() + 0xE bytes
> f:\dd\vctools\crt\crtw32\startup\threadex.c (359):
> TestsTLS-11.exe!_threadstartex()
>
> -- Block 4357 at 0x00646FD8: 400 bytes --
> Leak Hash: 0xC22F7275, Count: 1, Total 400 bytes
> Call Stack (TID 2464):
> ntdll.dll!RtlAllocateHeap()
> f:\dd\vctools\crt\crtw32\misc\dbgmalloc.c (56): TestsTLS-11.exe!malloc()
> + 0x15 bytes
> e:\openssl-1.1.git\crypto\mem.c (138): TestsTLS-11.exe!CRYPTO_malloc() +
> 0x9 bytes
> e:\openssl-1.1.git\crypto\err\err.c (874):
> TestsTLS-11.exe!ERR_get_state() + 0xE bytes
> e:\openssl-1.1.git\crypto\err\err.c (598):
> TestsTLS-11.exe!ERR_clear_error() + 0x5 bytes
> e:\openssl-1.1.git\ssl\statem\statem.c (279):
> TestsTLS-11.exe!state_machine()
> e:\openssl-1.1.git\ssl\statem\statem.c (217):
> TestsTLS-11.exe!ossl_statem_connect() + 0xB bytes
> e:\openssl-1.1.git\ssl\ssl_lib.c (2908):
> TestsTLS-11.exe!SSL_do_handshake() + 0xC bytes
> p:\mes programmes\shared\ocrypto-11\tls.cpp (1017):
> TestsTLS-11.exe!OTLS::TLSSss::DoHandshake() + 0xC bytes
> p:\mes programmes\tests\_testsshared\teststls-11-leak\clttasks.cpp (63):
> TestsTLS-11.exe!CltThread::Main() + 0xB bytes
> p:\mes programmes\shared\sthread.cpp (17):
> TestsTLS-11.exe!SThread::Run() + 0xE bytes
> f:\dd\vctools\crt\crtw32\startup\threadex.c (359):
> TestsTLS-11.exe!_threadstartex()
>
> -- Block 4359 at 0x006471A8: 96 bytes --
> Leak Hash: 0x1DBDD4B0, Count: 1, Total 96 bytes
> Call Stack (TID 2464):
> ntdll.dll!RtlAllocateHeap()
> f:\dd\vctools\crt\crtw32\misc\dbgmalloc.c (56): TestsTLS-11.exe!malloc()
> + 0x15 bytes
> e:\openssl-1.1.git\crypto\mem.c (138): TestsTLS-11.exe!CRYPTO_malloc() +
> 0x9 bytes
> e:\openssl-1.1.git\crypto\mem.c (158): TestsTLS-11.exe!CRYPTO_zalloc() +
> 0x11 bytes
> e:\openssl-1.1.git\crypto\lhash\lhash.c (116): TestsTLS-11.exe!lh_new()
> + 0xB bytes
> e:\openssl-1.1.git\crypto\err\err_lcl.h (2):
> Test
[openssl-dev] memory leaks detected using libSSL 1.1
Hi,
I have multithreaded test programs (client and server) that I use to test
some functionalities build with OpenSSL.
They started to warn about memory leaks when I linked them with version 1.1.
As I had to do some code changes to adapt the new version, I first thought I
forget some [new] init/free code.
I finally used OPENSSL_cleanup() and alikes instead of the previous litany
calls ;-), but still encounters leaks.
As it was hard to track them down, I write a simple server test program that
wait for a client and then return without even receiving data.
No certificate are loaded.
Leaks are detected only when a client handshake with the server.
I might be wrong, but I do not think this is a false positive.
Could you please have a look at the informations below and share your
feelings ?
Regards,
Michel.
Windows _CrtDumpMemoryLeaks() output :
Detected memory leaks!
Dumping objects ->
{4697} normal block at 0x00822660, 140 bytes long.
Data: <> 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
{4696} normal block at 0x00822608, 24 bytes long.
Data: <> 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
{4695} normal block at 0x008225B0, 24 bytes long.
Data: <> 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
{4694} normal block at 0x00822558, 24 bytes long.
Data: <> 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
{4693} normal block at 0x00822488, 148 bytes long.
Data: < & X% % > 00 00 00 00 08 26 82 00 58 25 82 00 B0 25 82 00
Object dump complete.
WARNING: Visual Leak Detector detected memory leaks!
-- Block 4677 at 0x00822488: 148 bytes --
Leak Hash: 0x76FECFA5, Count: 1, Total 148 bytes
Call Stack (TID 2904):
ntdll.dll!RtlAllocateHeap()
f:\dd\vctools\crt\crtw32\misc\dbgmalloc.c (56): TestsTLS-11.exe!malloc()
+ 0x15 bytes
e:\openssl-1.1.git\crypto\mem.c (138): TestsTLS-11.exe!CRYPTO_malloc() +
0x9 bytes
e:\openssl-1.1.git\crypto\mem.c (158): TestsTLS-11.exe!CRYPTO_zalloc() +
0x11 bytes
e:\openssl-1.1.git\crypto\hmac\hmac.c (174):
TestsTLS-11.exe!HMAC_CTX_new() + 0xE bytes
e:\openssl-1.1.git\ssl\t1_lib.c (3060):
TestsTLS-11.exe!tls_decrypt_ticket() + 0x5 bytes
e:\openssl-1.1.git\ssl\t1_lib.c (2994):
TestsTLS-11.exe!tls_check_serverhello_tlsext_early() + 0x2F bytes
e:\openssl-1.1.git\ssl\ssl_sess.c (536):
TestsTLS-11.exe!ssl_get_prev_session() + 0x18 bytes
e:\openssl-1.1.git\ssl\statem\statem_srvr.c (1181):
TestsTLS-11.exe!tls_process_client_hello() + 0x11 bytes
e:\openssl-1.1.git\ssl\statem\statem_srvr.c (804):
TestsTLS-11.exe!ossl_statem_server_process_message() + 0xD bytes
e:\openssl-1.1.git\ssl\statem\statem.c (609):
TestsTLS-11.exe!read_state_machine() + 0xB bytes
e:\openssl-1.1.git\ssl\statem\statem.c (429):
TestsTLS-11.exe!state_machine() + 0x9 bytes
e:\openssl-1.1.git\ssl\statem\statem.c (222):
TestsTLS-11.exe!ossl_statem_accept() + 0xB bytes
e:\openssl-1.1.git\ssl\ssl_lib.c (2908):
TestsTLS-11.exe!SSL_do_handshake() + 0xC bytes
p:\mes programmes\shared\ocrypto-11\tls.cpp (1017):
TestsTLS-11.exe!OTLS::TLSSss::DoHandshake() + 0xC bytes
p:\mes programmes\tests\_testsshared\teststls-11-leak -
copie\testtls.cpp (202): TestsTLS-11.exe!main() + 0xB bytes
f:\dd\vctools\crt\crtw32\startup\crt0.c (165):
TestsTLS-11.exe!mainCRTStartup()
-- Block 4678 at 0x00822558: 24 bytes --
Leak Hash: 0xEBA79111, Count: 1, Total 24 bytes
Call Stack (TID 2904):
ntdll.dll!RtlAllocateHeap()
f:\dd\vctools\crt\crtw32\misc\dbgmalloc.c (56): TestsTLS-11.exe!malloc()
+ 0x15 bytes
e:\openssl-1.1.git\crypto\mem.c (138): TestsTLS-11.exe!CRYPTO_malloc() +
0x9 bytes
e:\openssl-1.1.git\crypto\mem.c (158): TestsTLS-11.exe!CRYPTO_zalloc() +
0x11 bytes
e:\openssl-1.1.git\crypto\evp\digest.c (154):
TestsTLS-11.exe!EVP_MD_CTX_new() + 0xB bytes
e:\openssl-1.1.git\crypto\hmac\hmac.c (210):
TestsTLS-11.exe!HMAC_CTX_reset() + 0x5 bytes
e:\openssl-1.1.git\crypto\hmac\hmac.c (177):
TestsTLS-11.exe!HMAC_CTX_new() + 0x9 bytes
e:\openssl-1.1.git\ssl\t1_lib.c (3060):
TestsTLS-11.exe!tls_decrypt_ticket() + 0x5 bytes
e:\openssl-1.1.git\ssl\t1_lib.c (2994):
TestsTLS-11.exe!tls_check_serverhello_tlsext_early() + 0x2F bytes
e:\openssl-1.1.git\ssl\ssl_sess.c (536):
TestsTLS-11.exe!ssl_get_prev_session() + 0x18 bytes
e:\openssl-1.1.git\ssl\statem\statem_srvr.c (1181):
TestsTLS-11.exe!tls_process_client_hello() + 0x11 bytes
e:\openssl-1.1.git\ssl\statem\statem_srvr.c (804):
TestsTLS-11.exe!ossl_statem_server_process_message() + 0xD bytes
e:\openssl-1.1.git\ssl\statem\statem.c (609):
TestsTLS-11.exe!read_state_machine() + 0xB bytes
e:\openssl-1.1.git\ssl\statem\statem.c (429):
TestsTLS-11.exe!state_machine() + 0x9 bytes
e:\openssl-1.1.git\ssl\statem\statem.c (222):
TestsTLS-11.exe!ossl_statem_accept(
Re: [openssl-dev] memory leaks detected using libSSL 1.1
On 13/02/16 22:19, Michel wrote:
> Hi,
>
>
>
> I have multithreaded test programs (client and server) that I use to
> test some functionalities build with OpenSSL.
>
> They started to warn about memory leaks when I linked them with version 1.1.
>
> As I had to do some code changes to adapt the new version, I first
> thought I forget some [new] init/free code.
>
> I finally used OPENSSL_cleanup() and alikes instead of the previous
> litany calls ;-), but still encounters leaks.
>
> As it was hard to track them down, I write a simple server test program
> that wait for a client and then return without even receiving data.
>
> No certificate are loaded.
>
> Leaks are detected only when a client handshake with the server.
>
>
>
> I might be wrong, but I do not think this is a false positive.
>
> Could you please have a look at the informations below and share your
> feelings ?
Hmmm. It does look to me like there could be a memory leak here. What's
not clear to me is to why you are only seeing this in 1.1 and not
previous versions, as it looks like the same could happen in 1.0.2 as well!
Anyway, please try the attached patch to see if that helps.
Let me know how you get on.
Thanks
Matt
>From a47094a928f56cb62d57d4b53f2e4e20f9a0a031 Mon Sep 17 00:00:00 2001
From: Matt Caswell <m...@openssl.org>
Date: Sat, 13 Feb 2016 23:22:45 +
Subject: [PATCH] Fix memory leaks in tls_decrypt_ticket
Certain code paths in tls_decrypt_ticket could return early without first
freeing the HMAC_CTX or the EVP_CIPHER_CTX.
---
ssl/t1_lib.c | 22 +++---
1 file changed, 15 insertions(+), 7 deletions(-)
diff --git a/ssl/t1_lib.c b/ssl/t1_lib.c
index 522f0e6..0f6d51e 100644
--- a/ssl/t1_lib.c
+++ b/ssl/t1_lib.c
@@ -3048,7 +3048,7 @@ static int tls_decrypt_ticket(SSL *s, const unsigned char *etick,
SSL_SESSION *sess;
unsigned char *sdec;
const unsigned char *p;
-int slen, mlen, renew_ticket = 0;
+int slen, mlen, renew_ticket = 0, ret = -1;
unsigned char tick_hmac[EVP_MAX_MD_SIZE];
HMAC_CTX *hctx = NULL;
EVP_CIPHER_CTX *ctx;
@@ -3061,20 +3061,28 @@ static int tls_decrypt_ticket(SSL *s, const unsigned char *etick,
if (hctx == NULL)
return -2;
ctx = EVP_CIPHER_CTX_new();
+if (ctx == NULL) {
+ret = -2;
+goto err;
+}
if (tctx->tlsext_ticket_key_cb) {
unsigned char *nctick = (unsigned char *)etick;
int rv = tctx->tlsext_ticket_key_cb(s, nctick, nctick + 16,
ctx, hctx, 0);
if (rv < 0)
-return -1;
-if (rv == 0)
-return 2;
+goto err;
+if (rv == 0) {
+ret = 2;
+goto err;
+}
if (rv == 2)
renew_ticket = 1;
} else {
/* Check key name matches */
-if (memcmp(etick, tctx->tlsext_tick_key_name, 16))
-return 2;
+if (memcmp(etick, tctx->tlsext_tick_key_name, 16)) {
+ret = 2;
+goto err;
+}
if (HMAC_Init_ex(hctx, tctx->tlsext_tick_hmac_key, 16,
EVP_sha256(), NULL) <= 0
|| EVP_DecryptInit_ex(ctx, EVP_aes_128_cbc(), NULL,
@@ -3148,7 +3156,7 @@ static int tls_decrypt_ticket(SSL *s, const unsigned char *etick,
err:
EVP_CIPHER_CTX_free(ctx);
HMAC_CTX_free(hctx);
-return -1;
+return ret;
}
/* Tables to translate from NIDs to TLS v1.2 ids */
--
2.5.0
--
openssl-dev mailing list
To unsubscribe: https://mta.openssl.org/mailman/listinfo/openssl-dev
Re: [openssl-dev] memory leaks detected using libSSL 1.1
Hi Matt,
Thanks for your quick answer.
I applied your patch and it fixes the leaks found in the simple test
program.
However, a more complex one, still report [other] leaks.
Below is a new log if you can have a look at them.
I will investigate deeper tomorrow concering 1.0.2.
Thanks again,
Michel.
Detected memory leaks!
Dumping objects ->
{4383} normal block at 0x006472C8, 8 bytes long.
Data: <> 00 00 00 00 01 00 00 00
{4381} normal block at 0x00646B48, 12 bytes long.
Data: < od } > D8 6F 64 00 00 00 00 00 20 7D 00 00
{4379} normal block at 0x00647248, 64 bytes long.
Data: 48 6B 64 00 00 00 00 00 00 00 00 00 00 00 00 00
{4377} normal block at 0x006471A8, 96 bytes long.
Data: 48 72 64 00 10 03 A5 00 30 03 A5 00 08 00 00 00
{4375} normal block at 0x00646FD8, 400 bytes long.
Data: <> 00 00 00 00 A0 09 00 00 00 00 00 00 00 00 00 00
Object dump complete.
WARNING: Visual Leak Detector detected memory leaks!
-- Block 4363 at 0x00646B48: 12 bytes --
Leak Hash: 0xF7E93E2A, Count: 1, Total 12 bytes
Call Stack (TID 2464):
ntdll.dll!RtlAllocateHeap()
f:\dd\vctools\crt\crtw32\misc\dbgmalloc.c (56): TestsTLS-11.exe!malloc()
+ 0x15 bytes
e:\openssl-1.1.git\crypto\mem.c (138): TestsTLS-11.exe!CRYPTO_malloc() +
0x9 bytes
e:\openssl-1.1.git\crypto\lhash\lhash.c (168):
TestsTLS-11.exe!lh_insert() + 0xB bytes
e:\openssl-1.1.git\crypto\err\err_lcl.h (2):
TestsTLS-11.exe!lh_ERR_STATE_insert() + 0x10 bytes
e:\openssl-1.1.git\crypto\err\err.c (371):
TestsTLS-11.exe!int_thread_set_item() + 0xD bytes
e:\openssl-1.1.git\crypto\err\err.c (884):
TestsTLS-11.exe!ERR_get_state() + 0xC bytes
e:\openssl-1.1.git\crypto\err\err.c (598):
TestsTLS-11.exe!ERR_clear_error() + 0x5 bytes
e:\openssl-1.1.git\ssl\statem\statem.c (279):
TestsTLS-11.exe!state_machine()
e:\openssl-1.1.git\ssl\statem\statem.c (217):
TestsTLS-11.exe!ossl_statem_connect() + 0xB bytes
e:\openssl-1.1.git\ssl\ssl_lib.c (2908):
TestsTLS-11.exe!SSL_do_handshake() + 0xC bytes
p:\mes programmes\shared\ocrypto-11\tls.cpp (1017):
TestsTLS-11.exe!OTLS::TLSSss::DoHandshake() + 0xC bytes
p:\mes programmes\tests\_testsshared\teststls-11-leak\clttasks.cpp (63):
TestsTLS-11.exe!CltThread::Main() + 0xB bytes
p:\mes programmes\shared\sthread.cpp (17):
TestsTLS-11.exe!SThread::Run() + 0xE bytes
f:\dd\vctools\crt\crtw32\startup\threadex.c (359):
TestsTLS-11.exe!_threadstartex()
-- Block 4357 at 0x00646FD8: 400 bytes --
Leak Hash: 0xC22F7275, Count: 1, Total 400 bytes
Call Stack (TID 2464):
ntdll.dll!RtlAllocateHeap()
f:\dd\vctools\crt\crtw32\misc\dbgmalloc.c (56): TestsTLS-11.exe!malloc()
+ 0x15 bytes
e:\openssl-1.1.git\crypto\mem.c (138): TestsTLS-11.exe!CRYPTO_malloc() +
0x9 bytes
e:\openssl-1.1.git\crypto\err\err.c (874):
TestsTLS-11.exe!ERR_get_state() + 0xE bytes
e:\openssl-1.1.git\crypto\err\err.c (598):
TestsTLS-11.exe!ERR_clear_error() + 0x5 bytes
e:\openssl-1.1.git\ssl\statem\statem.c (279):
TestsTLS-11.exe!state_machine()
e:\openssl-1.1.git\ssl\statem\statem.c (217):
TestsTLS-11.exe!ossl_statem_connect() + 0xB bytes
e:\openssl-1.1.git\ssl\ssl_lib.c (2908):
TestsTLS-11.exe!SSL_do_handshake() + 0xC bytes
p:\mes programmes\shared\ocrypto-11\tls.cpp (1017):
TestsTLS-11.exe!OTLS::TLSSss::DoHandshake() + 0xC bytes
p:\mes programmes\tests\_testsshared\teststls-11-leak\clttasks.cpp (63):
TestsTLS-11.exe!CltThread::Main() + 0xB bytes
p:\mes programmes\shared\sthread.cpp (17):
TestsTLS-11.exe!SThread::Run() + 0xE bytes
f:\dd\vctools\crt\crtw32\startup\threadex.c (359):
TestsTLS-11.exe!_threadstartex()
-- Block 4359 at 0x006471A8: 96 bytes --
Leak Hash: 0x1DBDD4B0, Count: 1, Total 96 bytes
Call Stack (TID 2464):
ntdll.dll!RtlAllocateHeap()
f:\dd\vctools\crt\crtw32\misc\dbgmalloc.c (56): TestsTLS-11.exe!malloc()
+ 0x15 bytes
e:\openssl-1.1.git\crypto\mem.c (138): TestsTLS-11.exe!CRYPTO_malloc() +
0x9 bytes
e:\openssl-1.1.git\crypto\mem.c (158): TestsTLS-11.exe!CRYPTO_zalloc() +
0x11 bytes
e:\openssl-1.1.git\crypto\lhash\lhash.c (116): TestsTLS-11.exe!lh_new()
+ 0xB bytes
e:\openssl-1.1.git\crypto\err\err_lcl.h (2):
TestsTLS-11.exe!lh_ERR_STATE_new() + 0x10 bytes
e:\openssl-1.1.git\crypto\err\err.c (321):
TestsTLS-11.exe!int_thread_get() + 0xF bytes
e:\openssl-1.1.git\crypto\err\err.c (369):
TestsTLS-11.exe!int_thread_set_item() + 0x9 bytes
e:\openssl-1.1.git\crypto\err\err.c (884):
TestsTLS-11.exe!ERR_get_state() + 0xC bytes
e:\openssl-1.1.git\crypto\err\err.c (598):
TestsTLS-11.exe!ERR_clear_error() + 0x5 bytes
e:\openssl-1.1.git\ssl\statem\statem.c (279):
TestsTLS-11.exe!state_machine()
e:\openssl-1.1.git\ssl\statem\statem.c (217):
TestsTLS-11.exe!ossl_statem_connect() + 0xB bytes
e:\openssl-1.1.git\ssl\ssl_lib.c (2908):
TestsTLS-11.exe!SSL_do_handshake() + 0xC bytes
p:\mes programmes\shared\ocr
[openssl-dev] [openssl.org #1470] [PATCH] fix some memory leaks in asn1 crypto
This is reported against 0.9.8; please open a new ticket if still a problem with current releases. -- Rich Salz, OpenSSL dev team; rs...@openssl.org ___ openssl-dev mailing list To unsubscribe: https://mta.openssl.org/mailman/listinfo/openssl-dev
Re: [openssl-dev] [openssl.org #3198] [PATCH] Fix missing NULL pointer checks and memory leaks in crypto/asn1 files
On 10/06/14 21:48, Jonas Maebe via RT wrote: > On 13/12/13 11:54, The default queue via RT wrote: > >> In attachment you can find 7 patches against git master (generated via git >> format-patch) to fix a number of memory leaks (in case of failures) and >> missing NULL pointer checks (generally for malloc results) for source files >> under crypto/asn1. I've tried to follow the coding conventions of the >> surrounding code. >> >> I have about 50 more similar patches to various other parts of OpenSSL, but >> I'll wait with submitting them in case you'd like something different about >> the way I'm grouping/formatting/splitting them. > > I've rebased those 7 patches and all of my other similar patches against > current git HEAD and pushed them to a fork on github. I've created a > pull request for them at https://github.com/openssl/openssl/pull/131 All of my patches have been applied or superseded by other patches in the mean time. I've created a pull request for the four remaining, relevant patches at https://github.com/openssl/openssl/pull/511 Jonas ___ openssl-dev mailing list To unsubscribe: https://mta.openssl.org/mailman/listinfo/openssl-dev
Re: [openssl-dev] [libcrypto EVP] valgrind reports many memory leaks in wiki-example
>> Attached is the valgrind-output. It says: >>34 not-freed blocks The sample program doesn't clean up everything. Look at the function apps_shutdown in apps/openssl.c (in master) ___ openssl-dev mailing list To unsubscribe: https://mta.openssl.org/mailman/listinfo/openssl-dev
Re: [openssl-dev] [libcrypto EVP] valgrind reports many memory leaks in wiki-example
U.Mutlu wrote on 11/23/2015 06:43 AM: U.Mutlu wrote on 11/23/2015 06:32 AM: Hi, running the following example from the openssl wiki site under valgrind gives many memory-leaks in the underlying library functions: https://wiki.openssl.org/index.php/EVP_Symmetric_Encryption_and_Decryption $ valgrind --tool=memcheck --leak-check=full --show-leak-kinds=all --track-origins=yes -v ./a.out &>valgrind-output.txt Attached is the valgrind-output. It says: 34 not-freed blocks Are these maybe false-positives? The leak summary in the output says this: ==9895== LEAK SUMMARY: ==9895==definitely lost: 0 bytes in 0 blocks ==9895==indirectly lost: 0 bytes in 0 blocks ==9895== possibly lost: 0 bytes in 0 blocks ==9895==still reachable: 2,632 bytes in 34 blocks ==9895== suppressed: 0 bytes in 0 blocks Has nobody a clue what the above output means? ___ openssl-dev mailing list To unsubscribe: https://mta.openssl.org/mailman/listinfo/openssl-dev
[openssl-dev] [libcrypto EVP] valgrind reports many memory leaks in wiki-example
Hi, running the following example from the openssl wiki site under valgrind gives many memory-leaks in the underlying library functions: https://wiki.openssl.org/index.php/EVP_Symmetric_Encryption_and_Decryption $ gcc -Wall -O2 test_encdec.c -lcrypto $ valgrind --tool=memcheck --leak-check=full --show-leak-kinds=all --track-origins=yes -v ./a.out &>valgrind-output.txt Attached is the valgrind-output. It says: 34 not-freed blocks Are these maybe false-positives? The leak summary in the output says this: ==9895== LEAK SUMMARY: ==9895==definitely lost: 0 bytes in 0 blocks ==9895==indirectly lost: 0 bytes in 0 blocks ==9895== possibly lost: 0 bytes in 0 blocks ==9895==still reachable: 2,632 bytes in 34 blocks ==9895== suppressed: 0 bytes in 0 blocks OS: Debian 8 amd64, using libs from the Debian repo: libcrypto: 5.6.1-6+deb8u1 openssl: 1.0.1k-3+deb8u1 -- U.Mutlu valgrind-output.txt.gz Description: application/gzip ___ openssl-dev mailing list To unsubscribe: https://mta.openssl.org/mailman/listinfo/openssl-dev
Re: [openssl-dev] [libcrypto EVP] valgrind reports many memory leaks in wiki-example
U.Mutlu wrote on 11/23/2015 06:32 AM: Hi, running the following example from the openssl wiki site under valgrind gives many memory-leaks in the underlying library functions: https://wiki.openssl.org/index.php/EVP_Symmetric_Encryption_and_Decryption $ gcc -Wall -O2 test_encdec.c -lcrypto Typo above: I rather had used the -ggdb switch: $ gcc -Wall -ggdb test_encdec.c -lcrypto $ valgrind --tool=memcheck --leak-check=full --show-leak-kinds=all --track-origins=yes -v ./a.out &>valgrind-output.txt Attached is the valgrind-output. It says: 34 not-freed blocks Are these maybe false-positives? The leak summary in the output says this: ==9895== LEAK SUMMARY: ==9895==definitely lost: 0 bytes in 0 blocks ==9895==indirectly lost: 0 bytes in 0 blocks ==9895== possibly lost: 0 bytes in 0 blocks ==9895==still reachable: 2,632 bytes in 34 blocks ==9895== suppressed: 0 bytes in 0 blocks OS: Debian 8 amd64, using libs from the Debian repo: libcrypto: 5.6.1-6+deb8u1 openssl: 1.0.1k-3+deb8u1 -- U.Mutlu ___ openssl-dev mailing list To unsubscribe: https://mta.openssl.org/mailman/listinfo/openssl-dev
[openssl-dev] [openssl.org #4030] others quick patches for memory leaks in pk7_smime.c and pk7_mime.c
created by mistake. -- Rich Salz, OpenSSL dev team; rs...@openssl.org ___ openssl-dev mailing list To unsubscribe: https://mta.openssl.org/mailman/listinfo/openssl-dev
[openssl-dev] [openssl.org #1542] others quick patches for memory leaks in pk7_smime.c and pk7_mime.c
Yes, it looks like these have been fixed; closing. -- Rich Salz, OpenSSL dev team; rs...@openssl.org ___ openssl-dev mailing list To unsubscribe: https://mta.openssl.org/mailman/listinfo/openssl-dev
Re: [openssl-dev] [openssl.org #1542] others quick patches for memory leaks in pk7_smime.c and pk7_mime.c
The proposed patch is mangled and very hard to read, but I think all proposed changes have already been committed, or the code has been removed. So I think this can be closed now. Cheers ___ openssl-dev mailing list To unsubscribe: https://mta.openssl.org/mailman/listinfo/openssl-dev
Re: [openssl-dev] [openssl.org #4030] Re: [openssl-dev #1542] others quick patches for memory leaks in pk7_smime.c and pk7_mime.c
On Sat, Sep 05, 2015 at 01:49:23pm +, Alessandro Ghedini via RT wrote: > The proposed patch is mangled and very hard to read, but I think all proposed > changes have already been committed, or the code has been removed. > > So I think this can be closed now. Ugh, wrong subject... this was supposed to be a reply to RT#1542 and can be closed. Sorry for the noise. Cheers ___ openssl-dev mailing list To unsubscribe: https://mta.openssl.org/mailman/listinfo/openssl-dev
[openssl-dev] [openssl.org #4030] Re: [openssl-dev #1542] others quick patches for memory leaks in pk7_smime.c and pk7_mime.c
The proposed patch is mangled and very hard to read, but I think all proposed changes have already been committed, or the code has been removed. So I think this can be closed now. Cheers ___ openssl-bugs-mod mailing list openssl-bugs-...@openssl.org https://mta.openssl.org/mailman/listinfo/openssl-bugs-mod ___ openssl-dev mailing list To unsubscribe: https://mta.openssl.org/mailman/listinfo/openssl-dev
Re: [openssl-dev] [openssl.org #3985] [PATCH] Fix potential memory leaks
The corresponding GitHub pull request was merged, so this can be closed now. Cheers ___ openssl-dev mailing list To unsubscribe: https://mta.openssl.org/mailman/listinfo/openssl-dev
[openssl-dev] [openssl.org #3985] [PATCH] Fix potential memory leaks
Hello, see GitHub pull request at https://github.com/openssl/openssl/pull/354 which fixes memory leaks on error conditions in X509_add1_reject_object() and PKCS7_verify(). Cheers ___ openssl-bugs-mod mailing list openssl-bugs-...@openssl.org https://mta.openssl.org/mailman/listinfo/openssl-bugs-mod ___ openssl-dev mailing list To unsubscribe: https://mta.openssl.org/mailman/listinfo/openssl-dev
Re: [openssl-dev] [openssl.org #3985] [PATCH] Fix potential memory leaks
On Wed, Aug 05, 2015 at 11:01:13am +, Alessandro Ghedini via RT wrote: Hello, see GitHub pull request at https://github.com/openssl/openssl/pull/354 which fixes memory leaks on error conditions in X509_add1_reject_object() and PKCS7_verify(). I also added a couple more patches fixing memory leaks in the test suite. Now it's possible to run make test with GCC's address sanitizer, without any errors. ___ openssl-dev mailing list To unsubscribe: https://mta.openssl.org/mailman/listinfo/openssl-dev
[openssl-dev] [openssl.org #3856] [PATCH] Fix memory leaks in test code
Fixed in master and 1.0.2; thanks! OpenSSL_1_0_2-stable 295c629 RT3856: Fix memory leaks in test code master 2d54040 RT3856: Fix memory leaks in test code Author: Russell Webb russell.w...@intel.com Date: Sat Jun 13 10:35:55 2015 -0400 RT3856: Fix memory leaks in test code Reviewed-by: Matt Caswell m...@openssl.org -- Rich Salz, OpenSSL dev team; rs...@openssl.org ___ openssl-dev mailing list To unsubscribe: https://mta.openssl.org/mailman/listinfo/openssl-dev
[openssl-dev] [openssl.org #3856] [PATCH] Fix memory leaks in test code
From: Russell Webb russell.w...@intel.com
Several tests were not properly cleaning up everything on
process exit, which prevented 'make test' from succeeding
when running with address sanitizer enabled. Fix the tests
to properly free all resources.
Note that this was found with gcc version 6.0 and was not
observed using gcc 4.9.2.
Signed-off-by: Russell Webb russell.w...@intel.com
---
test/bntest.c | 1 +
test/hmactest.c | 3 +++
test/srptest.c | 1 +
3 files changed, 5 insertions(+)
diff --git a/test/bntest.c b/test/bntest.c
index 1ce6db1..891db62 100644
--- a/test/bntest.c
+++ b/test/bntest.c
@@ -1100,6 +1100,7 @@ int test_mod_exp_mont5(BIO *bp, BN_CTX *ctx)
fprintf(stderr, Modular exponentiation test failed!\n);
return 0;
}
+BN_MONT_CTX_free(mont);
BN_free(a);
BN_free(p);
BN_free(m);
diff --git a/test/hmactest.c b/test/hmactest.c
index 13344d6..3b01598 100644
--- a/test/hmactest.c
+++ b/test/hmactest.c
@@ -188,6 +188,7 @@ int main(int argc, char *argv[])
}
printf(test 4 ok\n);
test5:
+HMAC_CTX_cleanup(ctx);
HMAC_CTX_init(ctx);
if (HMAC_Init_ex(ctx, test[4].key, test[4].key_len, NULL, NULL)) {
printf(Should fail to initialise HMAC with empty MD (test 5)\n);
@@ -272,6 +273,7 @@ test5:
printf(test 5 ok\n);
}
test6:
+HMAC_CTX_cleanup(ctx);
HMAC_CTX_init(ctx);
if (!HMAC_Init_ex(ctx, test[7].key, test[7].key_len, EVP_sha1(), NULL)) {
printf(Failed to initialise HMAC (test 6)\n);
@@ -302,6 +304,7 @@ test6:
printf(test 6 ok\n);
}
end:
+HMAC_CTX_cleanup(ctx);
EXIT(err);
}
diff --git a/test/srptest.c b/test/srptest.c
index 1d463cd..8075218 100644
--- a/test/srptest.c
+++ b/test/srptest.c
@@ -148,6 +148,7 @@ int main(int argc, char **argv)
ERR_remove_thread_state(NULL);
ERR_free_strings();
CRYPTO_mem_leaks(bio_err);
+BIO_free(bio_err);
return 0;
}
--
2.1.0
___
openssl-dev mailing list
To unsubscribe: https://mta.openssl.org/mailman/listinfo/openssl-dev
[openssl-dev] SRP memory leaks and more leaks
Hi, Trying to use the 'SRP' code, I found two kinds of memory leaks in srp_vfy.c . 1) The first kind was due to bad use of OpenSSL 'stack' code and cumbersome / confusing names of structure / functions. In this case, leaks occurs when loading a verifier file containing 'index' lines. Two structures are used : SRP_gN and SRP_gN_cache. And unfortunatly, the SRP_gN_free function free a SRP_gN_cache structure. The SRP_VBASE_free() function, line 276, should call : sk_SRP_gN_cache_pop_free(vb-gN_cache, SRP_gN_cache_free); instead of : sk_SRP_gN_cache_free(vb-gN_cache); And consequently the SRP_gN_cache_free() function, lines 305-312, has to move before SRP_VBASE_free() The attached patch solve this kind of leaks. 2) When simulating a 'fake user' as per RFC 5054, § '2.5.1.3. Unknown SRP User Name', the SRP_VBASE_get_by_user() function returns a newly allocated SRP_user_pwd structure whose adress is not kept anywhere else. So, the caller should free it later, but from outside the function, there is no way to distinguish between 'fake users' and real ones which are managed and freed throught the use of the SRP_VBASE structure / functions. I am afraid there is no other solution than changing the SRP_VBASE_get_by_user() function prototype. It is better I do not share here my opinion about the comments below : /* need to check whether there are memory leaks */, s_server.c line 433 or : /* there may be still some leaks to fix, */ srp_vfy.c line 449 :( Hope this will save time to other users, Michel. srp_vfy.patch Description: Binary data ___ openssl-dev mailing list To unsubscribe: https://mta.openssl.org/mailman/listinfo/openssl-dev
[openssl.org #3507] [PATCH] Fix memory leaks.
Hello,
The attached patch fixes some memory leaks that were found via Coverity.
---
Kurt Cancemi
https://www.x64architecture.com
From 3d2c713113545255b61efe433e130078d4cf2e22 Mon Sep 17 00:00:00 2001
From: Kurt Cancemi k...@x64architecture.com
Date: Wed, 27 Aug 2014 20:21:33 -0400
Subject: [PATCH] Fix memory leaks.
---
crypto/asn1/x_x509a.c| 20 +++-
crypto/ec/ec_ameth.c | 1 +
crypto/ec/ec_mult.c | 1 +
crypto/ec/ecp_mont.c | 7 +--
crypto/pkcs7/pk7_smime.c | 1 +
crypto/x509/x509_trs.c | 2 ++
crypto/x509/x509_vfy.c | 1 +
crypto/x509v3/pcy_data.c | 4
crypto/x509v3/pcy_tree.c | 3 +++
9 files changed, 33 insertions(+), 7 deletions(-)
diff --git a/crypto/asn1/x_x509a.c b/crypto/asn1/x_x509a.c
index 03a9c45..1603e4b 100644
--- a/crypto/asn1/x_x509a.c
+++ b/crypto/asn1/x_x509a.c
@@ -159,12 +159,22 @@ int X509_add1_trust_object(X509 *x, ASN1_OBJECT *obj)
int X509_add1_reject_object(X509 *x, ASN1_OBJECT *obj)
{
X509_CERT_AUX *aux;
- ASN1_OBJECT *objtmp;
- if(!(objtmp = OBJ_dup(obj))) return 0;
- if(!(aux = aux_get(x))) return 0;
- if(!aux-reject
- !(aux-reject = sk_ASN1_OBJECT_new_null())) return 0;
+ ASN1_OBJECT *objtmp = NULL;
+ if (obj) {
+ objtmp = OBJ_dup(obj);
+ if (!objtmp)
+ return 0;
+ }
+ if(!(aux = aux_get(x)))
+ goto err;
+ if(!aux-reject !(aux-reject = sk_ASN1_OBJECT_new_null()))
+ goto err;
return sk_ASN1_OBJECT_push(aux-reject, objtmp);
+
+ err:
+ if (objtmp)
+ ASN1_OBJECT_free(objtmp);
+ return 0;
}
void X509_trust_clear(X509 *x)
diff --git a/crypto/ec/ec_ameth.c b/crypto/ec/ec_ameth.c
index a149bf6..15e86c4 100644
--- a/crypto/ec/ec_ameth.c
+++ b/crypto/ec/ec_ameth.c
@@ -387,6 +387,7 @@ static int ec_bits(const EVP_PKEY *pkey)
group = EC_KEY_get0_group(pkey-pkey.ec);
if (!EC_GROUP_get_order(group, order, NULL))
{
+ BN_free(order);
ERR_clear_error();
return 0;
}
diff --git a/crypto/ec/ec_mult.c b/crypto/ec/ec_mult.c
index fb693c3..3b23c5d 100644
--- a/crypto/ec/ec_mult.c
+++ b/crypto/ec/ec_mult.c
@@ -535,6 +535,7 @@ int ec_wNAF_mul(const EC_GROUP *group, EC_POINT *r, const BIGNUM *scalar,
if (numblocks pre_comp-numblocks)
{
ECerr(EC_F_EC_WNAF_MUL, ERR_R_INTERNAL_ERROR);
+ OPENSSL_free(tmp_wNAF);
goto err;
}
totalnum = num + numblocks;
diff --git a/crypto/ec/ecp_mont.c b/crypto/ec/ecp_mont.c
index 232ae34..2735957 100644
--- a/crypto/ec/ecp_mont.c
+++ b/crypto/ec/ecp_mont.c
@@ -229,8 +229,11 @@ int ec_GFp_mont_group_set_curve(EC_GROUP *group, const BIGNUM *p, const BIGNUM *
}
one = BN_new();
if (one == NULL) goto err;
- if (!BN_to_montgomery(one, BN_value_one(), mont, ctx)) goto err;
-
+ if (!BN_to_montgomery(one, BN_value_one(), mont, ctx))
+ {
+ BN_free(one);
+ goto err;
+ }
group-field_data1 = mont;
mont = NULL;
group-field_data2 = one;
diff --git a/crypto/pkcs7/pk7_smime.c b/crypto/pkcs7/pk7_smime.c
index a5104f8..9024ce8 100644
--- a/crypto/pkcs7/pk7_smime.c
+++ b/crypto/pkcs7/pk7_smime.c
@@ -364,6 +364,7 @@ int PKCS7_verify(PKCS7 *p7, STACK_OF(X509) *certs, X509_STORE *store,
if (tmpin == NULL)
{
PKCS7err(PKCS7_F_PKCS7_VERIFY,ERR_R_MALLOC_FAILURE);
+ sk_X509_free(signers);
return 0;
}
}
diff --git a/crypto/x509/x509_trs.c b/crypto/x509/x509_trs.c
index 3d7e068..5781573 100644
--- a/crypto/x509/x509_trs.c
+++ b/crypto/x509/x509_trs.c
@@ -206,10 +206,12 @@ int X509_TRUST_add(int id, int flags, int (*ck)(X509_TRUST *, X509 *, int),
if(idx == -1) {
if(!trtable !(trtable = sk_X509_TRUST_new(tr_cmp))) {
X509err(X509_F_X509_TRUST_ADD,ERR_R_MALLOC_FAILURE);
+ OPENSSL_free(trtmp);
return 0;
}
if (!sk_X509_TRUST_push(trtable, trtmp)) {
X509err(X509_F_X509_TRUST_ADD,ERR_R_MALLOC_FAILURE);
+ OPENSSL_free(trtmp);
return 0;
}
}
diff --git a/crypto/x509/x509_vfy.c b/crypto/x509/x509_vfy.c
index 85aa113..ae4fff9 100644
--- a/crypto/x509/x509_vfy.c
+++ b/crypto/x509/x509_vfy.c
@@ -353,6 +353,7 @@ int X509_verify_cert(X509_STORE_CTX *ctx)
x = xtmp;
if (!sk_X509_push(ctx-chain,x))
{
+ sk_X509_free(sktmp);
X509_free(xtmp);
X509err(X509_F_X509_VERIFY_CERT,ERR_R_MALLOC_FAILURE);
return 0;
diff --git a/crypto/x509v3/pcy_data.c b/crypto/x509v3/pcy_data.c
index 3444b03..2bb5868 100644
--- a/crypto/x509v3/pcy_data.c
+++ b/crypto/x509v3/pcy_data.c
@@ -99,7 +99,11 @@ X509_POLICY_DATA *policy_data_new(POLICYINFO *policy,
id = NULL;
ret = OPENSSL_malloc(sizeof(X509_POLICY_DATA));
if (!ret)
+ {
+ if (id)
+ ASN1_OBJECT_free(id);
return NULL;
+ }
ret-expected_policy_set = sk_ASN1_OBJECT_new_null();
if (!ret-expected_policy_set)
{
diff --git a/crypto/x509v3/pcy_tree.c b/crypto/x509v3/pcy_tree.c
index 47b1bf8..f8658ba 100644
--- a/crypto/x509v3/pcy_tree.c
+++ b/crypto/x509v3/pcy_tree.c
@@ -684,7 +684,10 @@ static int tree_calculate_user_set(X509_POLICY_TREE *tree,
{
tree-user_policies = sk_X509_POLICY_NODE_new_null();
if (!tree-user_policies
Re: [openssl.org #3507] [PATCH] Fix memory leaks.
The attached updated patch fixes a style error.
---
Kurt Cancemi
https://www.x64architecture.com
From d112c3f7b36a60f8af109b90fe5299f7ac049cc6 Mon Sep 17 00:00:00 2001
From: Kurt Cancemi k...@x64architecture.com
Date: Wed, 27 Aug 2014 20:37:45 -0400
Subject: [PATCH] Fix memory leaks.
---
crypto/asn1/x_x509a.c | 21 -
crypto/ec/ec_ameth.c | 1 +
crypto/ec/ec_mult.c | 1 +
crypto/ec/ecp_mont.c | 7 +--
crypto/objects/obj_xref.h | 2 +-
crypto/pkcs7/pk7_smime.c | 1 +
crypto/x509/x509_trs.c| 2 ++
crypto/x509/x509_vfy.c| 1 +
crypto/x509v3/pcy_data.c | 4
crypto/x509v3/pcy_tree.c | 3 +++
10 files changed, 35 insertions(+), 8 deletions(-)
diff --git a/crypto/asn1/x_x509a.c b/crypto/asn1/x_x509a.c
index 03a9c45..ec3da38 100644
--- a/crypto/asn1/x_x509a.c
+++ b/crypto/asn1/x_x509a.c
@@ -159,12 +159,23 @@ int X509_add1_trust_object(X509 *x, ASN1_OBJECT *obj)
int X509_add1_reject_object(X509 *x, ASN1_OBJECT *obj)
{
X509_CERT_AUX *aux;
- ASN1_OBJECT *objtmp;
- if(!(objtmp = OBJ_dup(obj))) return 0;
- if(!(aux = aux_get(x))) return 0;
- if(!aux-reject
- !(aux-reject = sk_ASN1_OBJECT_new_null())) return 0;
+ ASN1_OBJECT *objtmp = NULL;
+ if (obj)
+ {
+ objtmp = OBJ_dup(obj);
+ if (!objtmp)
+ return 0;
+ }
+ if(!(aux = aux_get(x)))
+ goto err;
+ if(!aux-reject !(aux-reject = sk_ASN1_OBJECT_new_null()))
+ goto err;
return sk_ASN1_OBJECT_push(aux-reject, objtmp);
+
+ err:
+ if (objtmp)
+ ASN1_OBJECT_free(objtmp);
+ return 0;
}
void X509_trust_clear(X509 *x)
diff --git a/crypto/ec/ec_ameth.c b/crypto/ec/ec_ameth.c
index a149bf6..15e86c4 100644
--- a/crypto/ec/ec_ameth.c
+++ b/crypto/ec/ec_ameth.c
@@ -387,6 +387,7 @@ static int ec_bits(const EVP_PKEY *pkey)
group = EC_KEY_get0_group(pkey-pkey.ec);
if (!EC_GROUP_get_order(group, order, NULL))
{
+ BN_free(order);
ERR_clear_error();
return 0;
}
diff --git a/crypto/ec/ec_mult.c b/crypto/ec/ec_mult.c
index fb693c3..3b23c5d 100644
--- a/crypto/ec/ec_mult.c
+++ b/crypto/ec/ec_mult.c
@@ -535,6 +535,7 @@ int ec_wNAF_mul(const EC_GROUP *group, EC_POINT *r, const BIGNUM *scalar,
if (numblocks pre_comp-numblocks)
{
ECerr(EC_F_EC_WNAF_MUL, ERR_R_INTERNAL_ERROR);
+ OPENSSL_free(tmp_wNAF);
goto err;
}
totalnum = num + numblocks;
diff --git a/crypto/ec/ecp_mont.c b/crypto/ec/ecp_mont.c
index 232ae34..2735957 100644
--- a/crypto/ec/ecp_mont.c
+++ b/crypto/ec/ecp_mont.c
@@ -229,8 +229,11 @@ int ec_GFp_mont_group_set_curve(EC_GROUP *group, const BIGNUM *p, const BIGNUM *
}
one = BN_new();
if (one == NULL) goto err;
- if (!BN_to_montgomery(one, BN_value_one(), mont, ctx)) goto err;
-
+ if (!BN_to_montgomery(one, BN_value_one(), mont, ctx))
+ {
+ BN_free(one);
+ goto err;
+ }
group-field_data1 = mont;
mont = NULL;
group-field_data2 = one;
diff --git a/crypto/objects/obj_xref.h b/crypto/objects/obj_xref.h
index cfd628a..2b3dc6d 100644
--- a/crypto/objects/obj_xref.h
+++ b/crypto/objects/obj_xref.h
@@ -54,8 +54,8 @@ static const nid_triple sigoid_srt[] =
static const nid_triple * const sigoid_srt_xref[] =
{
sigoid_srt[29],
- sigoid_srt[17],
sigoid_srt[18],
+ sigoid_srt[17],
sigoid_srt[0],
sigoid_srt[1],
sigoid_srt[7],
diff --git a/crypto/pkcs7/pk7_smime.c b/crypto/pkcs7/pk7_smime.c
index a5104f8..9024ce8 100644
--- a/crypto/pkcs7/pk7_smime.c
+++ b/crypto/pkcs7/pk7_smime.c
@@ -364,6 +364,7 @@ int PKCS7_verify(PKCS7 *p7, STACK_OF(X509) *certs, X509_STORE *store,
if (tmpin == NULL)
{
PKCS7err(PKCS7_F_PKCS7_VERIFY,ERR_R_MALLOC_FAILURE);
+ sk_X509_free(signers);
return 0;
}
}
diff --git a/crypto/x509/x509_trs.c b/crypto/x509/x509_trs.c
index 3d7e068..5781573 100644
--- a/crypto/x509/x509_trs.c
+++ b/crypto/x509/x509_trs.c
@@ -206,10 +206,12 @@ int X509_TRUST_add(int id, int flags, int (*ck)(X509_TRUST *, X509 *, int),
if(idx == -1) {
if(!trtable !(trtable = sk_X509_TRUST_new(tr_cmp))) {
X509err(X509_F_X509_TRUST_ADD,ERR_R_MALLOC_FAILURE);
+ OPENSSL_free(trtmp);
return 0;
}
if (!sk_X509_TRUST_push(trtable, trtmp)) {
X509err(X509_F_X509_TRUST_ADD,ERR_R_MALLOC_FAILURE);
+ OPENSSL_free(trtmp);
return 0;
}
}
diff --git a/crypto/x509/x509_vfy.c b/crypto/x509/x509_vfy.c
index 85aa113..ae4fff9 100644
--- a/crypto/x509/x509_vfy.c
+++ b/crypto/x509/x509_vfy.c
@@ -353,6 +353,7 @@ int X509_verify_cert(X509_STORE_CTX *ctx)
x = xtmp;
if (!sk_X509_push(ctx-chain,x))
{
+ sk_X509_free(sktmp);
X509_free(xtmp);
X509err(X509_F_X509_VERIFY_CERT,ERR_R_MALLOC_FAILURE);
return 0;
diff --git a/crypto/x509v3/pcy_data.c b/crypto/x509v3/pcy_data.c
index 3444b03..2bb5868 100644
--- a/crypto/x509v3/pcy_data.c
+++ b/crypto/x509v3/pcy_data.c
@@ -99,7 +99,11 @@ X509_POLICY_DATA *policy_data_new(POLICYINFO *policy,
id = NULL;
ret = OPENSSL_malloc(sizeof(X509_POLICY_DATA));
if (!ret)
+ {
+ if (id)
+ ASN1_OBJECT_free(id);
return NULL;
+ }
ret
Re: [openssl.org #3507] [PATCH] Fix memory leaks.
On Thu, Aug 28, 2014 at 03:11:14PM +0200, Kurt Cancemi via RT wrote:
The attached updated patch fixes a style error.
I still have a bunch of other patches like this to go thru, but
did a quick look at this, and at least this looks weird:
--- a/crypto/objects/obj_xref.h
+++ b/crypto/objects/obj_xref.h
@@ -54,8 +54,8 @@ static const nid_triple sigoid_srt[] =
static const nid_triple * const sigoid_srt_xref[] =
{
sigoid_srt[29],
- sigoid_srt[17],
sigoid_srt[18],
+ sigoid_srt[17],
sigoid_srt[0],
sigoid_srt[1],
sigoid_srt[7],
Can you explain that?
Kurt
__
OpenSSL Project http://www.openssl.org
Development Mailing List openssl-dev@openssl.org
Automated List Manager majord...@openssl.org
Re: [openssl.org #3507] [PATCH] Fix memory leaks.
I ran make which regenerated the objects, thanks for pointing that
out, I attached an updated patch without the change.
---
Kurt Cancemi
https://www.x64architecture.com
On Thu, Aug 28, 2014 at 12:41 PM, Kurt Roeckx k...@roeckx.be wrote:
On Thu, Aug 28, 2014 at 03:11:14PM +0200, Kurt Cancemi via RT wrote:
The attached updated patch fixes a style error.
I still have a bunch of other patches like this to go thru, but
did a quick look at this, and at least this looks weird:
--- a/crypto/objects/obj_xref.h
+++ b/crypto/objects/obj_xref.h
@@ -54,8 +54,8 @@ static const nid_triple sigoid_srt[] =
static const nid_triple * const sigoid_srt_xref[] =
{
sigoid_srt[29],
- sigoid_srt[17],
sigoid_srt[18],
+ sigoid_srt[17],
sigoid_srt[0],
sigoid_srt[1],
sigoid_srt[7],
Can you explain that?
Kurt
__
OpenSSL Project http://www.openssl.org
Development Mailing List openssl-dev@openssl.org
Automated List Manager majord...@openssl.org
From e5ec6311b407e52e096ad0197814e77176b4c9f9 Mon Sep 17 00:00:00 2001
From: Kurt Cancemi k...@x64architecture.com
Date: Thu, 28 Aug 2014 13:48:39 -0400
Subject: [PATCH] Fix memory leaks.
---
crypto/asn1/x_x509a.c| 21 -
crypto/ec/ec_ameth.c | 1 +
crypto/ec/ec_mult.c | 1 +
crypto/ec/ecp_mont.c | 7 +--
crypto/pkcs7/pk7_smime.c | 1 +
crypto/x509/x509_trs.c | 2 ++
crypto/x509/x509_vfy.c | 1 +
crypto/x509v3/pcy_data.c | 4
crypto/x509v3/pcy_tree.c | 3 +++
9 files changed, 34 insertions(+), 7 deletions(-)
diff --git a/crypto/asn1/x_x509a.c b/crypto/asn1/x_x509a.c
index 03a9c45..ec3da38 100644
--- a/crypto/asn1/x_x509a.c
+++ b/crypto/asn1/x_x509a.c
@@ -159,12 +159,23 @@ int X509_add1_trust_object(X509 *x, ASN1_OBJECT *obj)
int X509_add1_reject_object(X509 *x, ASN1_OBJECT *obj)
{
X509_CERT_AUX *aux;
- ASN1_OBJECT *objtmp;
- if(!(objtmp = OBJ_dup(obj))) return 0;
- if(!(aux = aux_get(x))) return 0;
- if(!aux-reject
- !(aux-reject = sk_ASN1_OBJECT_new_null())) return 0;
+ ASN1_OBJECT *objtmp = NULL;
+ if (obj)
+ {
+ objtmp = OBJ_dup(obj);
+ if (!objtmp)
+ return 0;
+ }
+ if(!(aux = aux_get(x)))
+ goto err;
+ if(!aux-reject !(aux-reject = sk_ASN1_OBJECT_new_null()))
+ goto err;
return sk_ASN1_OBJECT_push(aux-reject, objtmp);
+
+ err:
+ if (objtmp)
+ ASN1_OBJECT_free(objtmp);
+ return 0;
}
void X509_trust_clear(X509 *x)
diff --git a/crypto/ec/ec_ameth.c b/crypto/ec/ec_ameth.c
index a149bf6..15e86c4 100644
--- a/crypto/ec/ec_ameth.c
+++ b/crypto/ec/ec_ameth.c
@@ -387,6 +387,7 @@ static int ec_bits(const EVP_PKEY *pkey)
group = EC_KEY_get0_group(pkey-pkey.ec);
if (!EC_GROUP_get_order(group, order, NULL))
{
+ BN_free(order);
ERR_clear_error();
return 0;
}
diff --git a/crypto/ec/ec_mult.c b/crypto/ec/ec_mult.c
index fb693c3..3b23c5d 100644
--- a/crypto/ec/ec_mult.c
+++ b/crypto/ec/ec_mult.c
@@ -535,6 +535,7 @@ int ec_wNAF_mul(const EC_GROUP *group, EC_POINT *r, const BIGNUM *scalar,
if (numblocks pre_comp-numblocks)
{
ECerr(EC_F_EC_WNAF_MUL, ERR_R_INTERNAL_ERROR);
+ OPENSSL_free(tmp_wNAF);
goto err;
}
totalnum = num + numblocks;
diff --git a/crypto/ec/ecp_mont.c b/crypto/ec/ecp_mont.c
index 232ae34..2735957 100644
--- a/crypto/ec/ecp_mont.c
+++ b/crypto/ec/ecp_mont.c
@@ -229,8 +229,11 @@ int ec_GFp_mont_group_set_curve(EC_GROUP *group, const BIGNUM *p, const BIGNUM *
}
one = BN_new();
if (one == NULL) goto err;
- if (!BN_to_montgomery(one, BN_value_one(), mont, ctx)) goto err;
-
+ if (!BN_to_montgomery(one, BN_value_one(), mont, ctx))
+ {
+ BN_free(one);
+ goto err;
+ }
group-field_data1 = mont;
mont = NULL;
group-field_data2 = one;
diff --git a/crypto/pkcs7/pk7_smime.c b/crypto/pkcs7/pk7_smime.c
index a5104f8..9024ce8 100644
--- a/crypto/pkcs7/pk7_smime.c
+++ b/crypto/pkcs7/pk7_smime.c
@@ -364,6 +364,7 @@ int PKCS7_verify(PKCS7 *p7, STACK_OF(X509) *certs, X509_STORE *store,
if (tmpin == NULL)
{
PKCS7err(PKCS7_F_PKCS7_VERIFY,ERR_R_MALLOC_FAILURE);
+ sk_X509_free(signers);
return 0;
}
}
diff --git a/crypto/x509/x509_trs.c b/crypto/x509/x509_trs.c
index 3d7e068..5781573 100644
--- a/crypto/x509/x509_trs.c
+++ b/crypto/x509/x509_trs.c
@@ -206,10 +206,12 @@ int X509_TRUST_add(int id, int flags, int (*ck)(X509_TRUST *, X509 *, int),
if(idx == -1) {
if(!trtable !(trtable = sk_X509_TRUST_new(tr_cmp))) {
X509err(X509_F_X509_TRUST_ADD,ERR_R_MALLOC_FAILURE);
+ OPENSSL_free(trtmp);
return 0;
}
if (!sk_X509_TRUST_push(trtable, trtmp)) {
X509err(X509_F_X509_TRUST_ADD,ERR_R_MALLOC_FAILURE);
+ OPENSSL_free(trtmp);
return 0;
}
}
diff --git a/crypto/x509/x509_vfy.c b/crypto/x509/x509_vfy.c
index 85aa113..ae4fff9 100644
--- a/crypto/x509/x509_vfy.c
+++ b/crypto/x509/x509_vfy.c
[openssl.org #2846] [PATCH 1/4] Remove unfinished/unused code with memory leaks (to silence static analyzer)
It was already fixed in the next release after 1.0.2; see rsalz-monolith branch in akamai/openssl fork on github. thanks. -- Rich Salz, OpenSSL dev team; rs...@openssl.org __ OpenSSL Project http://www.openssl.org Development Mailing List openssl-dev@openssl.org Automated List Manager majord...@openssl.org
Re: [openssl.org #3198] [PATCH] Fix missing NULL pointer checks and memory leaks in crypto/asn1 files
On 13/12/13 11:54, The default queue via RT wrote: In attachment you can find 7 patches against git master (generated via git format-patch) to fix a number of memory leaks (in case of failures) and missing NULL pointer checks (generally for malloc results) for source files under crypto/asn1. I've tried to follow the coding conventions of the surrounding code. I have about 50 more similar patches to various other parts of OpenSSL, but I'll wait with submitting them in case you'd like something different about the way I'm grouping/formatting/splitting them. I've rebased those 7 patches and all of my other similar patches against current git HEAD and pushed them to a fork on github. I've created a pull request for them at https://github.com/openssl/openssl/pull/131 Jonas __ OpenSSL Project http://www.openssl.org Development Mailing List openssl-dev@openssl.org Automated List Manager majord...@openssl.org
Re: [openssl.org #3198] [PATCH] Fix missing NULL pointer checks and memory leaks in crypto/asn1 files
On Tue, Jun 10, 2014 at 09:48:19PM +0200, Jonas Maebe via RT wrote: On 13/12/13 11:54, The default queue via RT wrote: In attachment you can find 7 patches against git master (generated via git format-patch) to fix a number of memory leaks (in case of failures) and missing NULL pointer checks (generally for malloc results) for source files under crypto/asn1. I've tried to follow the coding conventions of the surrounding code. I have about 50 more similar patches to various other parts of OpenSSL, but I'll wait with submitting them in case you'd like something different about the way I'm grouping/formatting/splitting them. I've rebased those 7 patches and all of my other similar patches against current git HEAD and pushed them to a fork on github. I've created a pull request for them at https://github.com/openssl/openssl/pull/131 So that actually contains 57 commits. It's going to take some time for someone to review it. Kurt __ OpenSSL Project http://www.openssl.org Development Mailing List openssl-dev@openssl.org Automated List Manager majord...@openssl.org
Re: [openssl.org #3198] [PATCH] Fix missing NULL pointer checks and memory leaks in crypto/asn1 files
On 10/06/14 21:59, Kurt Roeckx via RT wrote: On Tue, Jun 10, 2014 at 09:48:19PM +0200, Jonas Maebe via RT wrote: On 13/12/13 11:54, The default queue via RT wrote: In attachment you can find 7 patches against git master (generated via git format-patch) to fix a number of memory leaks (in case of failures) and missing NULL pointer checks (generally for malloc results) for source files under crypto/asn1. I've tried to follow the coding conventions of the surrounding code. I have about 50 more similar patches to various other parts of OpenSSL, but I'll wait with submitting them in case you'd like something different about the way I'm grouping/formatting/splitting them. I've rebased those 7 patches and all of my other similar patches against current git HEAD and pushed them to a fork on github. I've created a pull request for them at https://github.com/openssl/openssl/pull/131 So that actually contains 57 commits. It's going to take some time for someone to review it. Yes, sorry. I was originally planning on submitting them piecemeal, but as nothing happened with the initial batch I kind of lost motivation... Jonas __ OpenSSL Project http://www.openssl.org Development Mailing List openssl-dev@openssl.org Automated List Manager majord...@openssl.org
[openssl.org #3198] [PATCH] Fix missing NULL pointer checks and memory leaks in crypto/asn1 files
Hi, In attachment you can find 7 patches against git master (generated via git format-patch) to fix a number of memory leaks (in case of failures) and missing NULL pointer checks (generally for malloc results) for source files under crypto/asn1. I've tried to follow the coding conventions of the surrounding code. I have about 50 more similar patches to various other parts of OpenSSL, but I'll wait with submitting them in case you'd like something different about the way I'm grouping/formatting/splitting them. Jonas 0001-SetBlob-free-rgSetBlob-on-error-path.patch Description: Binary data 0002-ASN1_verify-ASN1_item_verify-cleanse-and-free-buf_in.patch Description: Binary data 0003-mime_hdr_new-free-mhdr-tmpname-tmpval-on-error-path.patch Description: Binary data 0004-mime_hdr_addparam-free-tmpname-tmpval-and-mparam-on-.patch Description: Binary data 0005-multi_split-check-for-NULL-when-allocating-parts-and.patch Description: Binary data 0006-asn1_bio_new-free-ctx-on-error-path.patch Description: Binary data 0007-asn1_set_seq_out-free-derlist-and-tmpdat-on-error-pa.patch Description: Binary data
Re: [openssl.org #3198] [PATCH] Fix missing NULL pointer checks and memory leaks in crypto/asn1 files
On 13 Dec 2013, at 11:54, The default queue via RT wrote: In attachment you can find 7 patches against git master (generated via git format-patch) to fix a number of memory leaks (in case of failures) and missing NULL pointer checks (generally for malloc results) for source files under crypto/asn1. I've tried to follow the coding conventions of the surrounding code. Of course, right before I sent those patches I discovered some more issues, changed the code, rebased/fixed up the previous commits and then forgot to test the result. Please find corrected replacements for patches 0003 and 0004 in the previous series in attachments. Sorry. Jonas 0003-mime_hdr_new-free-mhdr-tmpname-tmpval-on-error-path.patch Description: Binary data 0004-mime_hdr_addparam-free-tmpname-tmpval-and-mparam-on-.patch Description: Binary data
[openssl.org #2941] Memory leaks in ca.c
Greetings! In case of error updating ca database a memory leak occur: $ openssl ca -config z/caCA/ca.conf -in z/user1/req.pem -batch -notext ... skipped ... failed to update database TXT_DB error number 2 [19:00:30] 4957 file=ca.c, line=2199, thread=3074324104, number=28, address=086466F0 [19:00:30] 4956 file=buffer.c, line=121, thread=3074324104, number=268, address=08646A18 [19:00:30] 4953 file=ca.c, line=2179, thread=3074324104, number=14, address=0864D790 [19:00:30] 4668 file=bn_print.c, line=74, thread=3074324104, number=10, address=08648780 [19:00:30] 4954 file=ca.c, line=2186, thread=3074324104, number=8, address=08649618 [19:00:30] 4952 file=ca.c, line=2176, thread=3074324104, number=2, address=086466E0 330 bytes leaked in 6 chunks Thank you! -- SY, Dmitry Belyavsky Greetings!In case of error updating ca database a memory leak occur:$ openssl ca -config z/caCA/ca.conf -in z/user1/req.pem -batch -notext... skipped ... failed to update databaseTXT_DB error number 2[19:00:30] 4957 file=ca.c, line=2199, thread=3074324104, number=28, address=086466F0[19:00:30] 4956 file=buffer.c, line=121, thread=3074324104, number=268, address=08646A18 [19:00:30] 4953 file=ca.c, line=2179, thread=3074324104, number=14, address=0864D790[19:00:30] 4668 file=bn_print.c, line=74, thread=3074324104, number=10, address=08648780[19:00:30] 4954 file=ca.c, line=2186, thread=3074324104, number=8, address=08649618 [19:00:30] 4952 file=ca.c, line=2176, thread=3074324104, number=2, address=086466E0330 bytes leaked in 6 chunksThank you!-- SY, Dmitry Belyavsky
Re: [openssl.org #2295] Resolved: [PATCH] OOM checks and memory leaks fixes
On Mon, 22 Nov 2010 16:52:27 +0100, Alexei Khlebnikov via RT r...@openssl.org wrote: I see that the patch was applied fully to branches OpenSSL_0_9_8-stable, OpenSSL_1_0_0-stable and OpenSSL_1_0_1-stable. But it was applied incompletely to HEAD. I suggest to apply the attached tiny patch error-handling.2010-11-22.patch for completion. The mentioned tiny patch has been committed: http://cvs.openssl.org/filediff?f=openssl/crypto/pkcs12/p12_key.cv1=1.24v2=1.25 But the ticket has not been closed. I suggest to close it, because the patch has been merged properly. Direct link to a page with status changing control, for your convenience: http://rt.openssl.org/Ticket/Modify.html?id=2295 Thanks! __ OpenSSL Project http://www.openssl.org Development Mailing List openssl-dev@openssl.org Automated List Manager majord...@openssl.org
Re: [openssl.org #2295] AutoReply: [PATCH] OOM checks and memory leaks fixes
Hi! I just want to remind about my little patch. More than 3 months passed without any activity on it. I'll appreciate if someone looks at it before OpenSSL code changes so much that the patch becomes non-mergeable. The patch is quite trivial, shouldn't create any regressions. The patch file size is relatively big (7 KB), because I included 10 lines of context (diff -U10) instead of default 3, in hope of simplifying review and speeding up merging. Best regards, Alexei. __ OpenSSL Project http://www.openssl.org Development Mailing List openssl-dev@openssl.org Automated List Manager majord...@openssl.org
[openssl.org #2295] [PATCH] OOM checks and memory leaks fixes
Mandatory information: Request type: PATCH OS and arch: GNU/Linux on IA-32 OpenSSL version: 1.0.0a Hi OpenSSL project, I'd like to submit a patch which includes a handful of small fixes: 1) Out-of-memory error checking. 2) Memory leak fix for OOM situation. 3) Fall-through comment for a case without break. 4) Duplicate code elimination. Hoping the patch will be useful. -- Best regards, Alexei Khlebnikov, Opera Software ASA, Norway. error-handling.2010-06-28.patch Description: Binary data
memory leaks in speed.c?
hi, we've developed a tool for automatically detecting memory errors. We ran our tool on openssl and it has detected the following (possible) memory leaks in apps/speed.c function static int do_multi(int multi) At line 2759 the variable fds is allocated. But at line 2776 and 2925 the procedure returns without freeing fds. Moreover, at line 2766 and 2788, fds is dereferenced without checking that mallloc has actually allocated memory. This may cause a memory fault. Regards, The SpaceInvader team. __ OpenSSL Project http://www.openssl.org Development Mailing List openssl-dev@openssl.org Automated List Manager [EMAIL PROTECTED]
[openssl.org #1542] others quick patches for memory leaks in pk7_smime.c and pk7_mime.c
Hi,
I suggest some quick patches (memory leaks):
All OS.
All openssl versions.
1) in pk7_smime.c:
255a256
sk_X509_free(signers);
New code:
tmpin = BIO_new_mem_buf(ptr, len);
if (tmpin == NULL)
{
PKCS7err(PKCS7_F_PKCS7_VERIFY,ERR_R_MALLOC_FAILURE);
sk_X509_free(signers);
return 0;
}
2) in pk7_mime.c:
653c653,657
if(!mhdr) return NULL;
---
if(!mhdr) {
if(tmpname) OPENSSL_free(tmpname);
if(tmpval) OPENSSL_free(tmpval);
return NULL;
}
656c660,663
if(!(mhdr-params = sk_MIME_PARAM_new(mime_param_cmp))) return
NULL;
---
if(!(mhdr-params = sk_MIME_PARAM_new(mime_param_cmp))) {
mime_hdr_free(mhdr);
return NULL;
}
Line 678:
if(!tmpval) return 0;
-
if(!tmpval) {
if(tmpname) OPENSSL_free(tmpname);
return 0;
}
Line 682:
if(!mparam) return 0;
-
if(!mparam) {
if(tmpname) OPENSSL_free(tmpname);
if(tmpval) OPENSSL_free(tmpval);
return 0;
}
Thanks
Eric
__
OpenSSL Project http://www.openssl.org
Development Mailing List openssl-dev@openssl.org
Automated List Manager [EMAIL PROTECTED]
Re: NEW FINDINGS-More Leaks-Memory Leaks in SSL_Library_init()
Hi!
There is a new finding to this. There is one more place for porbable memory
leak. The earlier solution which I proposed(using a funtion
SSL_free_comp_methods) is usfficient to arrest the memory leaks if zlib is
not being dynamically loaded.
if I enable -DZLIB -DZLIB_SHARED in openssl build, and run the simple
application mentioned in earlier post through valgrind, This is the output
==23867== 86 bytes in 5 blocks are still reachable in loss record 5 of 6
==23867==at 0x1B903CC8: malloc (vg_replace_malloc.c:131)
==23867==by 0x1B983840: default_malloc_ex (mem.c:79)
==23867==by 0x1B983EF6: CRYPTO_malloc (mem.c:304)
==23867==by 0x1B9E24A9: DSO_new_method (dso_lib.c:103)
|
|
|
==23867== LEAK SUMMARY:
==23867==definitely lost: 0 bytes in 0 blocks.
==23867==possibly lost: 0 bytes in 0 blocks.
==23867==still reachable: 804 bytes in 10 blocks.
==23867== suppressed: 0 bytes in 0 blocks.
On further investigation, I found that the variable zlib_dso which get
allocated during call to COMP_zlib(), is not being free through any control
path.
I wrote a funtion like this in the file c_zlib.c and called it before
calling SSL_free_comp_methods() as part of the cleanup sequence from
application.
/* defined in c_zlib.c *.
387 void UNLOAD_zlib(void)
388 {
389 if(zlib_dso != NULL)
390 DSO_free(zlib_dso);
391 }
With this change, I do not get the still reachable error message from the
valgrind run.
regards
-Nitin
From: Darryl Miles [EMAIL PROTECTED]
Reply-To: openssl-dev@openssl.org
To: openssl-dev@openssl.org
Subject: Re: Memory Leaks in SSL_Library_init()
Date: Mon, 02 Apr 2007 18:56:40 +0100
Nitin M wrote:
Darryl, What is your opinion on this finding? As you were also keen on
knowing the result of this experimentation. :)
What is you opinion?
Here is the valgrind output for the above program for your reference.
==10877== ERROR SUMMARY: 0 errors from 0 contexts (suppressed: 17 from 1)
--10877--
--10877-- supp: 17 Ugly strchr error in /lib/ld-2.3.2.so
==10877== malloc/free: in use at exit: 36 bytes in 2 blocks.
==10877== malloc/free: 89 allocs, 87 frees, 1632 bytes allocated.
==10877==
==10877== searching for pointers to 2 not-freed blocks.
==10877== checked 2852348 bytes.
Here is my output from 0.9.7f:
--5448-- supp:4 dl_relocate_object
==5448== malloc/free: in use at exit: 0 bytes in 0 blocks.
==5448== malloc/free: 79 allocs, 79 frees, 2,640 bytes allocated.
No leaks, but you are testing with 0.9.8, which version?
I don't have time at the minute to investigate with 0.9.8.
I'd also like to see a SSL_library_cleanup(), which in turn calls
SSL_free_comp_methods() just to start the ball rolling on reducing the
OpenSSL voodoo.
This is still true. The cleanup is an area of the library which could do
with the rough edges taking off it and the officially agreed methods to be
documented in the same place as SSL_library_init().
With a call SSL_library_cleanup(), which internally calls
EVP_cleanup() and SSL_free_comp_methods() do you see any harm or any
scenario in which it might break. I feel that if used properly for
every corresponding SSL_library_init(), it should not causse any
problem.
Agreed on this. A function SSL_library_cleanup() should be created,
which will reverse the action of calling SSL_library_init() and using the
OpenSSL library in any way possible.
It is fine by me to document that the SSL_library_cleanup() is not
thread-safe to clearly warn users to ensure they revert back to single
threaded access to the OpenSSL library for calling cleanup functions. It
should also be documented that the application code should have already
free'ed any OpenSSL objects it created and retained before calling
SSL_library_cleanup(). If should be documented that the state of the
OpenSSL library reverts to the same state as when an process first loads
the library, so the same rules apply again (i.e. its maybe necessary to
call SSL_library_init() before making use of some OpenSSL provided
functions).
It is fine by me to implement the necessary locking within the
sub-functions it calls to make those respective function thread-safe (since
there maybe legitimate reasons to call them independently of
SSL_library_cleanup() function) I think this is already done anyway.
It is fine by me for the proposed SSL_free_comp_methods() to be implemented
and also fine to remove the previously discussed first to lines from the
posted patch which attempt to reduce lock contention and locking overhead.
You will have to petition one of the project comitters to actually get any
action on these points. Maybe posting a fresh patch which adds the above
and the necessary new documentation will help get things moving.
rant_modeI already have an outstanding contribution for the project that
still
Re: Memory Leaks in SSL_Library_init()
Nitin M wrote: Darryl, What is your opinion on this finding? As you were also keen on knowing the result of this experimentation. :) What is you opinion? Here is the valgrind output for the above program for your reference. ==10877== ERROR SUMMARY: 0 errors from 0 contexts (suppressed: 17 from 1) --10877-- --10877-- supp: 17 Ugly strchr error in /lib/ld-2.3.2.so ==10877== malloc/free: in use at exit: 36 bytes in 2 blocks. ==10877== malloc/free: 89 allocs, 87 frees, 1632 bytes allocated. ==10877== ==10877== searching for pointers to 2 not-freed blocks. ==10877== checked 2852348 bytes. Here is my output from 0.9.7f: --5448-- supp:4 dl_relocate_object ==5448== malloc/free: in use at exit: 0 bytes in 0 blocks. ==5448== malloc/free: 79 allocs, 79 frees, 2,640 bytes allocated. No leaks, but you are testing with 0.9.8, which version? I don't have time at the minute to investigate with 0.9.8. I'd also like to see a SSL_library_cleanup(), which in turn calls SSL_free_comp_methods() just to start the ball rolling on reducing the OpenSSL voodoo. This is still true. The cleanup is an area of the library which could do with the rough edges taking off it and the officially agreed methods to be documented in the same place as SSL_library_init(). With a call SSL_library_cleanup(), which internally calls EVP_cleanup() and SSL_free_comp_methods() do you see any harm or any scenario in which it might break. I feel that if used properly for every corresponding SSL_library_init(), it should not causse any problem. Agreed on this. A function SSL_library_cleanup() should be created, which will reverse the action of calling SSL_library_init() and using the OpenSSL library in any way possible. It is fine by me to document that the SSL_library_cleanup() is not thread-safe to clearly warn users to ensure they revert back to single threaded access to the OpenSSL library for calling cleanup functions. It should also be documented that the application code should have already free'ed any OpenSSL objects it created and retained before calling SSL_library_cleanup(). If should be documented that the state of the OpenSSL library reverts to the same state as when an process first loads the library, so the same rules apply again (i.e. its maybe necessary to call SSL_library_init() before making use of some OpenSSL provided functions). It is fine by me to implement the necessary locking within the sub-functions it calls to make those respective function thread-safe (since there maybe legitimate reasons to call them independently of SSL_library_cleanup() function) I think this is already done anyway. It is fine by me for the proposed SSL_free_comp_methods() to be implemented and also fine to remove the previously discussed first to lines from the posted patch which attempt to reduce lock contention and locking overhead. You will have to petition one of the project comitters to actually get any action on these points. Maybe posting a fresh patch which adds the above and the necessary new documentation will help get things moving. rant_modeI already have an outstanding contribution for the project that still remains unaddressed (see postings on SSL_shutdown()) so although this issue is something I'd normally pickup on and try to formalize (as I think I understand the issue well enough to do that) I can only contribute as fast as the consortium allows. My extra energy which would otherwise be spent assisting the greater good is eaten up by maintaining a local fork./rant_mode Darryl __ OpenSSL Project http://www.openssl.org Development Mailing List openssl-dev@openssl.org Automated List Manager [EMAIL PROTECTED]
Re: Memory Leaks in SSL_Library_init()
From: Darryl Miles [EMAIL PROTECTED] Reply-To: openssl-dev@openssl.org To: openssl-dev@openssl.org Subject: Re: Memory Leaks in SSL_Library_init() Date: Mon, 02 Apr 2007 18:56:40 +0100 Nitin M wrote: Darryl, What is your opinion on this finding? As you were also keen on knowing the result of this experimentation. :) What is you opinion? Here is the valgrind output for the above program for your reference. ==10877== ERROR SUMMARY: 0 errors from 0 contexts (suppressed: 17 from 1) --10877-- --10877-- supp: 17 Ugly strchr error in /lib/ld-2.3.2.so ==10877== malloc/free: in use at exit: 36 bytes in 2 blocks. ==10877== malloc/free: 89 allocs, 87 frees, 1632 bytes allocated. ==10877== ==10877== searching for pointers to 2 not-freed blocks. ==10877== checked 2852348 bytes. Here is my output from 0.9.7f: --5448-- supp:4 dl_relocate_object ==5448== malloc/free: in use at exit: 0 bytes in 0 blocks. ==5448== malloc/free: 79 allocs, 79 frees, 2,640 bytes allocated. No leaks, but you are testing with 0.9.8, which version? I am testing with 0.9.8d. I don't have time at the minute to investigate with 0.9.8. I'd also like to see a SSL_library_cleanup(), which in turn calls SSL_free_comp_methods() just to start the ball rolling on reducing the OpenSSL voodoo. This is still true. The cleanup is an area of the library which could do with the rough edges taking off it and the officially agreed methods to be documented in the same place as SSL_library_init(). With a call SSL_library_cleanup(), which internally calls EVP_cleanup() and SSL_free_comp_methods() do you see any harm or any scenario in which it might break. I feel that if used properly for every corresponding SSL_library_init(), it should not causse any problem. Agreed on this. A function SSL_library_cleanup() should be created, which will reverse the action of calling SSL_library_init() and using the OpenSSL library in any way possible. Just out of curiosity I wanted to know in case any one has any Idea why was the earlier patch submitted by Jonatahan Green was not included in mainstream? Any specific technical reason? It is fine by me to document that the SSL_library_cleanup() is not thread-safe to clearly warn users to ensure they revert back to single threaded access to the OpenSSL library for calling cleanup functions. It should also be documented that the application code should have already free'ed any OpenSSL objects it created and retained before calling SSL_library_cleanup(). If should be documented that the state of the OpenSSL library reverts to the same state as when an process first loads the library, so the same rules apply again (i.e. its maybe necessary to call SSL_library_init() before making use of some OpenSSL provided functions). It is fine by me to implement the necessary locking within the sub-functions it calls to make those respective function thread-safe (since there maybe legitimate reasons to call them independently of SSL_library_cleanup() function) I think this is already done anyway. It is fine by me for the proposed SSL_free_comp_methods() to be implemented and also fine to remove the previously discussed first to lines from the posted patch which attempt to reduce lock contention and locking overhead. U mean that these lines as per the previous arguments and discusssions is not required ? 1289 if (ssl_comp_methods == NULL) 1290 return; You will have to petition one of the project comitters to actually get any action on these points. Maybe posting a fresh patch which adds the above and the necessary new documentation will help get things moving. rant_modeI already have an outstanding contribution for the project that still remains unaddressed (see postings on SSL_shutdown()) so although this issue is something I'd normally pickup on and try to formalize (as I think I understand the issue well enough to do that) I can only contribute as fast as the consortium allows. My extra energy which would otherwise be spent assisting the greater good is eaten up by maintaining a local fork./rant_mode Darryl __ OpenSSL Project http://www.openssl.org Development Mailing List openssl-dev@openssl.org Automated List Manager [EMAIL PROTECTED] _ Sign in and get updated on all the action from Formula One http://content.msn.co.in/Sports/FormulaOne/Default __ OpenSSL Project http://www.openssl.org Development Mailing List openssl-dev@openssl.org Automated List Manager [EMAIL PROTECTED]
Re: Memory Leaks in SSL_Library_init()
Hi All!
Any inputs on this finding?
I would like to check with the core development team also, if their opinion
differs on this, whether there is a need of SSL_free_comp_methods() funtion
to cleanup the ssl_comp_methods?
Darryl, What is your opinion on this finding? As you were also keen on
knowing the result of this experimentation. :)
Thanks in advance
regards
-Nitin
From: Nitin M [EMAIL PROTECTED]
Reply-To: openssl-dev@openssl.org
To: openssl-dev@openssl.org
CC: [EMAIL PROTECTED]
Subject: Re: Memory Leaks in SSL_Library_init()
Date: Wed, 28 Mar 2007 04:14:17 +
1 #includeopenssl/ssl.h
2 int main()
3 {
4 SSL_library_init();
5
6 /* thread-local cleanup */
7 ERR_remove_state(0);
8
9 /* Globals we finished with */
10 //
my_application_function_to_destroy_OpenSSL_objects_attached_to_local_variables();
11 // In the particular application I am keeping an RSA
12 // // object around.
13 // if(tmp_rsa != NULL) {
14 // RSA_free(tmp_rsa);
15 // tmp_rsa = NULL;
16 // }
17
18
19 // /* thread-safe cleanup */
20 ENGINE_cleanup();
21 CONF_modules_unload(1);
22 //
23 // /* global application exit cleanup (after all SSL activity
is shutdown) */
24 ERR_free_strings();
25 EVP_cleanup();
26 // SSL_free_comp_methods();
27 CRYPTO_cleanup_all_ex_data();
28 }
I also tried with the above code and found that 36 bytes are still
reachable. Then after retining only EVP_cleanup() from the clanup sequence
of calls, I felt that EVP_clenup is doing the job of cleaning up most of
what SSL_library_init() created.
In one of your earlier posts you have said that while you use the mentioned
sequence of cleanup calls you did not get any memory leak at all. You dont
even get the Still reachable msg with that sequence? if this is true,is
it possible for you to post a small example code here?
Also I think the function
my_application_function_to_destroy_OpenSSL_objects_attached_to_local_variables();
is your application specific function and since I did not have any RSA
object I did not call RSA_free(tmp_rsa);
With a call SSL_library_cleanup(), which internally calls EVP_cleanup()
and SSL_free_comp_methods() do you see any harm or any scenario in which it
might break. I feel that if used properly for every corresponding
SSL_library_init(), it should not causse any problem.
What is you opinion?
Here is the valgrind output for the above program for your reference.
==10877== ERROR SUMMARY: 0 errors from 0 contexts (suppressed: 17 from 1)
--10877--
--10877-- supp: 17 Ugly strchr error in /lib/ld-2.3.2.so
==10877== malloc/free: in use at exit: 36 bytes in 2 blocks.
==10877== malloc/free: 89 allocs, 87 frees, 1632 bytes allocated.
==10877==
==10877== searching for pointers to 2 not-freed blocks.
==10877== checked 2852348 bytes.
==10877==
==10877== 36 bytes in 2 blocks are still reachable in loss record 1 of 1
==10877==at 0x1B903CC8: malloc (vg_replace_malloc.c:131)
==10877==by 0x1B978CAA: default_malloc_ex (in
/home/nitin/software/lib/libcrypto.so.0.9.8)
==10877==
==10877== LEAK SUMMARY:
==10877==definitely lost: 0 bytes in 0 blocks.
==10877==possibly lost: 0 bytes in 0 blocks.
==10877==still reachable: 36 bytes in 2 blocks.
==10877== suppressed: 0 bytes in 0 blocks.
From: Darryl Miles [EMAIL PROTECTED]
Reply-To: openssl-dev@openssl.org
To: openssl-dev@openssl.org
Subject: Re: Memory Leaks in SSL_Library_init()
Date: Wed, 28 Mar 2007 04:53:02 +0100
Nitin M wrote:
I wrote a simple program like this to find out the possibility of a
memory leak. The program goes like this.
#includeopenssl/ssl.h
int main()
{
SSL_library_init();
EVP_cleanup();
}
when I run this programm through the valgrind I get the following output.
Ok, it is not clear if tried out the cleanup sequence I posted and checked
for results after using it ?
If you are still getting leaks after; then the function
SSL_free_comp_methods(); might be addressing a genuine leak which can't be
reclaimed using any of the existing cleanup functions.
So I'd vote for it to get more attention and be included.
I'd also like to see a SSL_library_cleanup(), which in turn calls
SSL_free_comp_methods() just to start the ball rolling on reducing the
OpenSSL voodoo.
Darryl
__
OpenSSL Project http://www.openssl.org
Development Mailing List openssl-dev@openssl.org
Automated List Manager [EMAIL PROTECTED]
_
Spice up your IM conversations. New, colorful and animated emoticons. Get
chatting! http://server1.msn.co.in/SP05/emoticons
RE: Memory Leaks in SSL_Library_init()
Richard Salz: Suppose another thread does this: *p=99; *p=98; Out of scope -- the C standard does not define ANY semantics for multiple threads of execution. Exactly. The original example was: extern volatile char* p; int i, j; i = *p; j = *p; The standard says that the compiler may not treat this as j = i = *p; *and that's all it says.* The only case in scope that I can think of is if the process catches a signal. I believe the standard does require that no matter when the process catches a signal, it must not be the case that j's value has changed but not i's. Because the standard has an 'as-if' rule, saying the compiler may not treat A as B only makes sense when you can show a detectable difference between A and B. In any event, you definitely won't get two fetches from memory on most modern computers. So if that's the requirement, everyone's violating it. DS __ OpenSSL Project http://www.openssl.org Development Mailing List openssl-dev@openssl.org Automated List Manager [EMAIL PROTECTED]
Re: Memory Leaks in SSL_Library_init()
On 3/28/07, Darryl Miles [EMAIL PROTECTED] wrote:
Actually 'volatile' doesn't provide a useful fix.
[...]
The problem occurs after the beginning of the function. If the compare is
done on a cached copy in a register. Look at this example:
if (variable!=NULL)
{
free(variable);
variable=NULL;
}
This could easily be optimized to (cached_copy is a register or other
fast
place to store data):
int cached_copy=variable;
if(cached_copy!=NULL)
{
acquire_lock();
cached_copy=variable;
free(cached_copy);
cached_copy=NULL;
variable=cached_copy;
release_lock();
}
else variable=cached_copy;
If you think this cannot happen, I defy you to explain to me why. What
standard or guarantee is being violated? How can POSIX-compliant or
C-compliant code break with this optimization? How can you say it can
never
be faster?
This is the precise optimization that 'volatile' inhibits. 'volatile'
requires that the value not be cached in cheap-to-access locations
like registers, instead being re-loaded from expensive-to-access
locations like the original memory -- because it may be changed from
outside the control flow that the compiler knows about.
-Kyle H
__
OpenSSL Project http://www.openssl.org
Development Mailing List openssl-dev@openssl.org
Automated List Manager [EMAIL PROTECTED]
RE: Memory Leaks in SSL_Library_init()
This is the precise optimization that 'volatile' inhibits. For single-threaded code, you are right. But we are talking about multi-threaded code. 'volatile' requires that the value not be cached in cheap-to-access locations like registers, instead being re-loaded from expensive-to-access locations like the original memory -- because it may be changed from outside the control flow that the compiler knows about. I had this exact same argument a long time ago about code like this: int foo; volatile int have_pointer=0; volatile int *pointer=NULL; In one thread: pointer=foo; have_pointer=1; In another: if(have_pointer!=0) *pointer++; All I could say was that this code breaks the standard. One thread accesses 'have_pointer' while another thread might be modifying it. Did I know how it could fail? No. In fact, nobody could think of a way it could fail because we didn't really realize that the ordering could break and not just the concurrent access/modification. Did it fail? Yes. I believe we first had faults on new UltraSparc CPUs (might have been new Alphas, I can't remember for sure). It was a classic case of worked on old CPUs, failed on new ones. The new ones had some write reordering capability. If you look at Itanium compilers, you will see that they don't put memory barriers between 'volatile' accesses. So 'volatile' does not provide ordering. It is known not to provide atomicity. There are platforms where a 'uint64_t' *can't* be written atomically, so what's a read of a volatile 'uint64_t' even supposed to do? So you have to be saying, you don't get atomicity or ordering, but you do get ... What exactly? And what standard says so? My copy of the pthreads standard says that accessing a variable in one thread while another thread is or might be modifying it is undefined behavior. IMO, it may break on a new CPU is totally unacceptable for code that is going to be distributed and especially for libraries with expected wide distribution. DS PS: There are many, many more stories of I couldn't think of any way it could break code breaking when the real world thought of ways. You have to code based on guarantees in standards, not your own lack of imagination. __ OpenSSL Project http://www.openssl.org Development Mailing List openssl-dev@openssl.org Automated List Manager [EMAIL PROTECTED]
Re: Memory Leaks in SSL_Library_init()
If you have multiple threads accessing it, you manage access using a mutex. If locking is important to the application that it's in. (Clearing the compression is as important as clearing the library state. If there's a lock around the library state clearing, a lock needs to exist around the compression. If not, it's the same level of priority.) A read of a 'volatile uint64_t', btw, is supposed to make sure that it reads from the original memory locations, not cached copies of it in register or spread across multiple registers. Even if it can't be accessed atomically, the guarantee is that it will fetch from memory, so that it can be changed outside the current program flow (signal handlers, shm, mmap, and the like). Yes, it needs to be locked (all possible concurrent access needs to be locked, ideally, even though that's a performance hog). I'd not doubt that there are platforms where uint32_t can't be accessed atomically. Or even uint16_t. The question becomes, at what point does it become not-cost-effective to support platforms that cannot guarantee things that can be guaranteed by the platforms used by a majority of the user base? So, here's a novel idea: why don't you write a patch to clear the compression structs that adheres to your view of appropriate design, and submit it? -Kyle H On 3/29/07, David Schwartz [EMAIL PROTECTED] wrote: This is the precise optimization that 'volatile' inhibits. For single-threaded code, you are right. But we are talking about multi-threaded code. 'volatile' requires that the value not be cached in cheap-to-access locations like registers, instead being re-loaded from expensive-to-access locations like the original memory -- because it may be changed from outside the control flow that the compiler knows about. I had this exact same argument a long time ago about code like this: int foo; volatile int have_pointer=0; volatile int *pointer=NULL; In one thread: pointer=foo; have_pointer=1; In another: if(have_pointer!=0) *pointer++; All I could say was that this code breaks the standard. One thread accesses 'have_pointer' while another thread might be modifying it. Did I know how it could fail? No. In fact, nobody could think of a way it could fail because we didn't really realize that the ordering could break and not just the concurrent access/modification. Did it fail? Yes. I believe we first had faults on new UltraSparc CPUs (might have been new Alphas, I can't remember for sure). It was a classic case of worked on old CPUs, failed on new ones. The new ones had some write reordering capability. If you look at Itanium compilers, you will see that they don't put memory barriers between 'volatile' accesses. So 'volatile' does not provide ordering. It is known not to provide atomicity. There are platforms where a 'uint64_t' *can't* be written atomically, so what's a read of a volatile 'uint64_t' even supposed to do? So you have to be saying, you don't get atomicity or ordering, but you do get ... What exactly? And what standard says so? My copy of the pthreads standard says that accessing a variable in one thread while another thread is or might be modifying it is undefined behavior. IMO, it may break on a new CPU is totally unacceptable for code that is going to be distributed and especially for libraries with expected wide distribution. DS PS: There are many, many more stories of I couldn't think of any way it could break code breaking when the real world thought of ways. You have to code based on guarantees in standards, not your own lack of imagination. __ OpenSSL Project http://www.openssl.org Development Mailing List openssl-dev@openssl.org Automated List Manager [EMAIL PROTECTED] -- -Kyle H __ OpenSSL Project http://www.openssl.org Development Mailing List openssl-dev@openssl.org Automated List Manager [EMAIL PROTECTED]
RE: Memory Leaks in SSL_Library_init()
A read of a 'volatile uint64_t', btw, is supposed to make sure that it reads from the original memory locations, not cached copies of it in register or spread across multiple registers. Which it doesn't do on any platform I know of. On every platform, 'volatile' reads through the caches and 'volatile' writes are writes to a cache to be flushed to main memory when it's convenient. The rationale is this simple -- people use 'volatile' in very important cases, such as longjmp and signals. Anything that is very expensive to do that isn't needed in those cases simply will not be put into 'volatile'. If that means you don't get the multi-thread semantics you erroneously thought you guaranteed, that's your problem. Even if it can't be accessed atomically, the guarantee is that it will fetch from memory, so that it can be changed outside the current program flow (signal handlers, shm, mmap, and the like). The problem is that on modern machines fetch from memory is ill-defined because memory is not all in one place. What is a fetch from memory on a ccNUMA system? The 'volatile' keyword comes from the C standard. The C standard doesn't say anything about sharing memory between processes or threads. There are standards for doing that, and 'volatile' only means anything in those contexts if the standards that let you share memory or create threads say so. A multi-threaded standard could say that 'volatile' has some particular set of semantics. However, as far as I know, *none* *does*. At one time, people thought ordering was part of those semantics. Now there are significant platforms where it isn't. At one point, people thought some types of atomicity were part of those semantics. Now there are significant platforms where it isn't. The 'volatile' keyword is only required to work for signals and longjmp. It may or may not work in other cases. If you try to state precisely what semantics you think 'volatile' assures you, I think you'll find that it's impossible. And even if you did, next year it could go on the list of semantics people thought they had. In this case, the semantic you need is basically that a concurrent read/write will not get a value other than one in the variable before the write or the one after. We know for a fact 'volatile' doesn't provide this guarantee, since it doesn't provide it for 'uint64_t' on platforms that don't have atomic 64-bit writes. And it's not because it's impossible to provide it. Atomic 64-bit writes can be faked with spinlocks around all 64-bit atomic accesses. You know why those platforms don't provide those spinlocks? Because 'volatile' does not provide the read/write guarantee you need, so there is no reason to put them there. Yes, it needs to be locked (all possible concurrent access needs to be locked, ideally, even though that's a performance hog). Since a lock is both necessary and sufficient, what help is 'volatile'? I'd not doubt that there are platforms where uint32_t can't be accessed atomically. Or even uint16_t. The question becomes, at what point does it become not-cost-effective to support platforms that cannot guarantee things that can be guaranteed by the platforms used by a majority of the user base? No, that's not the question. The question is, are we going to follow the standards and use the guarantees we have or senselessly write code that relies on behavior that is explicitly undefined by the standards and risk having our code break on new CPUs? So, here's a novel idea: why don't you write a patch to clear the compression structs that adheres to your view of appropriate design, and submit it? I suggested either removing the locking entirely or simply removing the unlocked check for NULL. I think cutting/pasting a patch would be more work than simply making the change. DS __ OpenSSL Project http://www.openssl.org Development Mailing List openssl-dev@openssl.org Automated List Manager [EMAIL PROTECTED]
Re: Memory Leaks in SSL_Library_init()
A read of a 'volatile uint64_t', btw, is supposed to make sure that it reads from the original memory locations, not cached copies of it in register or spread across multiple registers. No. The computing model in ANSI/ISO C doesn't really go below the level of source code. Volatile only guarantees that the compiler will not remove reads/writes that, based on its analysis of the program source, it thinks are redundant. For example: volatile unsigned char* p = get_address(); *p = *p = 5 The compiler is not allowed to remove either assignment. int i = *p + *p; The compiler must dereference the pointer twice. There is no guarantee about caching, atomicity, etc. In fact, there is no mention of it. Even if it can't be accessed atomically, the guarantee is that it will fetch from memory, so that it can be changed outside the current program flow (signal handlers, shm, mmap, and the like). Yes, it needs to be locked (all possible concurrent access needs to be locked, ideally, even though that's a performance hog). This is mostly right, but the one wrong bit (the guarantee) is the most important part. -- STSM Senior Security Architect DataPower SOA Appliances __ OpenSSL Project http://www.openssl.org Development Mailing List openssl-dev@openssl.org Automated List Manager [EMAIL PROTECTED]
Re: Memory Leaks in SSL_Library_init()
Kyle Hamilton wrote:
On 3/28/07, Darryl Miles [EMAIL PROTECTED] wrote:
The problem occurs after the beginning of the function. If the
compare is
done on a cached copy in a register. Look at this example:
if (variable!=NULL)
{
free(variable);
variable=NULL;
}
This could easily be optimized to (cached_copy is a register or other
fast
place to store data):
int cached_copy=variable;
if(cached_copy!=NULL)
{
acquire_lock();
cached_copy=variable;
free(cached_copy);
cached_copy=NULL;
variable=cached_copy;
release_lock();
}
else variable=cached_copy;
If you think this cannot happen, I defy you to explain to me why. What
standard or guarantee is being violated? How can POSIX-compliant or
C-compliant code break with this optimization? How can you say it can
never
be faster?
This is the precise optimization that 'volatile' inhibits. 'volatile'
requires that the value not be cached in cheap-to-access locations
like registers, instead being re-loaded from expensive-to-access
locations like the original memory -- because it may be changed from
outside the control flow that the compiler knows about.
Agreed with the sorts of things volatile inhibits.
But this whole tangent is irrelevant to the original posters code. The
above pseudo code is not a valid interpretation. The last line David
wrote variable=cached_copy; can't happen for the code:
if(variable) {
free(variable);
variable=NULL;
}
Like David has claimed.
A compiler will not generate a store instruction to put back a
cached_copy into the variable location. Principally because there was
no assignment operation in the original code and because even a
non-optimizing compiler knows it can just dump the cached_copy
temporary register on the floor.
The conversation stemmed from the code:
1289 if (ssl_comp_methods == NULL)
1290 return;
1291 CRYPTO_w_lock(CRYPTO_LOCK_SSL);
1292 if (ssl_comp_methods != NULL)
1993 {
...SNIP...
1296 }
1297 CRYPTO_w_unlock(CRYPTO_LOCK_SSL);
If ssl_comp_methods is NULL, then no store instruction to the memory
location of ssl_comp_methods will ever happen. So nothing is subject to
the so called lost write concurrency problem.
So marking it 'volatile' will not gain the above code anything. But it
will inhibit valid optimizations the compiler might make to all code
that uses the variable 'ssl_comp_methods' that are inside the locked
regions.
Darryl
__
OpenSSL Project http://www.openssl.org
Development Mailing List openssl-dev@openssl.org
Automated List Manager [EMAIL PROTECTED]
Re: Memory Leaks in SSL_Library_init()
This is the precise optimization that 'volatile' inhibits. 'volatile' requires that the value not be cached in cheap-to-access locations like registers, instead being re-loaded from expensive-to-access locations like the original memory -- because it may be changed from outside the control flow that the compiler knows about. Agreed with the sorts of things volatile inhibits. You are both wrong. The C standard is *silent* on the concept of underlying hardware. It only talks about the flow of control as defined in the module being translated. It created a new term, sequence point, to talk about control flow in an abstract way. The ISO C web site seems to be: http://www.open-std.org/jtc1/sc22/wg14/ and if you go there you can find copies of the standard and the committee mailings, e.g. /r$ -- STSM Senior Security Architect DataPower SOA Appliances __ OpenSSL Project http://www.openssl.org Development Mailing List openssl-dev@openssl.org Automated List Manager [EMAIL PROTECTED]
Re: Memory Leaks in SSL_Library_init()
Richard Salz wrote: This is the precise optimization that 'volatile' inhibits. 'volatile' requires that the value not be cached in cheap-to-access locations like registers, instead being re-loaded from expensive-to-access locations like the original memory -- because it may be changed from outside the control flow that the compiler knows about. Agreed with the sorts of things volatile inhibits. You are both wrong. Just to clarify what exactly your comment is aimed at. Is it still on the tangent subject of volatile usage ? or is this in relation to the original poster subject matter ? If its the meaning of volatile, I used the term sorts of to be deliberately vague to mean that Kyle is in the right ball park (which he is even under your double *p++ *p++ example I can relate that to what Kyle wrote above) and as it is a tangent subject the main point of my comment was to close the tangent down; so thats that on volatile usage from me. However if your comment is addressed at the more useful part of my replies (the bit you didn't quote) then I am definitely interested if you would elaborate on that. The C standard is *silent* on the concept of underlying hardware. It only talks about the flow of control as defined in the module being translated. It created a new term, sequence point, to talk about control flow in an abstract way. I can't actually relate that comment to either discussion. No one is disputing the definition of flow control. No one is claiming that the C standard defines anything at the hardware layer. When the hardware layer has been talked about it was either cited as an example of usage (not cited as the definition); or it was talked about in relation to what a compiler does and does not do when it generates assembly language instructions. Darryl __ OpenSSL Project http://www.openssl.org Development Mailing List openssl-dev@openssl.org Automated List Manager [EMAIL PROTECTED]
RE: Memory Leaks in SSL_Library_init()
Darryl Mile wrote:
A compiler will not generate a store instruction to put back a
cached_copy into the variable location. Principally because there was
no assignment operation in the original code and because even a
non-optimizing compiler knows it can just dump the cached_copy
temporary register on the floor.
That may be true, but it doesn't help. The compiler generating a store
instruction is only one way a cached copy can be written back to memory.
Anything a compiler can do could also be done by the CPU or memory hardware.
Only operations with specific guaranteed semantics solve that problem.
The conversation stemmed from the code:
1289 if (ssl_comp_methods == NULL)
1290 return;
1291 CRYPTO_w_lock(CRYPTO_LOCK_SSL);
1292 if (ssl_comp_methods != NULL)
1993 {
...SNIP...
1296 }
1297 CRYPTO_w_unlock(CRYPTO_LOCK_SSL);
If ssl_comp_methods is NULL, then no store instruction to the memory
location of ssl_comp_methods will ever happen. So nothing is subject to
the so called lost write concurrency problem.
You are correct that no store *instruction* will happen, but that doesn't mean
the CPU won't generate a store anyway. While it's true that current caches keep
track of whether data is modified and don't write back data unless it is, that
doesn't mean future CPUs might not find it more efficient to avoid having to
keep the flag and write back in all cases. What if the flag gets to be
expensive but write operations are cheap?
There is precedent. On the x86, for example, at least some compare-exchange
operations will write back the original data if the compare fails.
In other words, if you ask for:
if(a==7) a=3;
You might get (in *microcode* even though the instructions say what you want):
a = (a==7) ? 3 : a;
This can be an optimization because the the code as specified requires a
micro-branch internal to the compare-exchange instruction, which can be
expensive if mispredicted. The unconditional store avoids the branch and can be
implemented mathematically.
If the compiler was designed before anyone knew a CPU might do this, it could
easily generate a compare-exchange instruction for a volatile variable that had
an access of this form. The compiler author knows the CPU follows the
requested code precisely. The next CPU, however, makes the store unconditional
as an optimization because a branch is cheaper than a store. So sorry, your new
CPU breaks your old executables.
The code we are talking about is of this precise form. It compares the value of
a pointer and changes the value of that pointer only if the pointer was not
NULL to begin with. Now the code we are talking about contains locks inside the
conditional, so it will not fail this exact way. (I believe the x86 will only
do this for bus-locked instructions anyway, but there is no reason why the next
x86 couldn't do the same thing for non-bus-locked transactions too.)
You argument is of the form I can't think of any way it could fail. This says
more about your imagination than the validity of the code. ;)
I agree none of the way I can think of that it can fail seem particularly
likely. The problem is the ways it can fail that neither of us can think of --
until it fails. Experience has shown me (and I cited a few examples in this
thread) that will be ways it can fail you can't think of.
That is why standards provide guarantees.
So marking it 'volatile' will not gain the above code anything. But it
will inhibit valid optimizations the compiler might make to all code
that uses the variable 'ssl_comp_methods' that are inside the locked
regions.
I agree with that.
DS
__
OpenSSL Project http://www.openssl.org
Development Mailing List openssl-dev@openssl.org
Automated List Manager [EMAIL PROTECTED]
Re: Memory Leaks in SSL_Library_init()
I was not commenting on any part of the message that I didn't quote. :) Kyle's claim about things like cache's and registers is wrong, not even sort-of right. The standard talks about only in terms of sequence points, and volatile limits what can be done in terms of sequence points. So extern volatile char* p; int i, j; i = *p; j = *p; The standard says that the compiler may not treat this as j = i = *p; *and that's all it says.* -- STSM Senior Security Architect DataPower SOA Appliances __ OpenSSL Project http://www.openssl.org Development Mailing List openssl-dev@openssl.org Automated List Manager [EMAIL PROTECTED]
RE: Memory Leaks in SSL_Library_init()
Richard Salz wrote: Kyle's claim about things like cache's and registers is wrong, not even sort-of right. The standard talks about only in terms of sequence points, and volatile limits what can be done in terms of sequence points. So extern volatile char* p; int i, j; i = *p; j = *p; The standard says that the compiler may not treat this as j = i = *p; *and that's all it says.* Suppose another thread does this: *p=99; *p=98; Would a compiler that let another code running your code above get 'i' equal to 98 and 'p' equal to 99 be broken in your opinion? I've seen code generated on weakly-ordered memory platforms, and memory barriers are not placed between stores to volatiles. The problem is that it is not well-defined to talk about the order in which operations take place. The order can be different depending upon where you look (one can be first in the instruction stream but last out of the write posting buffer). The standard is discussed in terms of an abstract machine and the machine the code is running on may bear only as much resemblance to the abstract machine as is needed to make strictly-compliant code work. DS __ OpenSSL Project http://www.openssl.org Development Mailing List openssl-dev@openssl.org Automated List Manager [EMAIL PROTECTED]
RE: Memory Leaks in SSL_Library_init()
Suppose another thread does this: *p=99; *p=98; Out of scope -- the C standard does not define ANY semantics for multiple threads of execution. The standard is discussed in terms of an abstract machine and the machine the code is running on may bear only as much resemblance to the abstract machine as is needed to make strictly-compliant code work. Yes. Google for mashey volatile will turn up some interesting stories from an expert, including this one: http://yarchive.net/comp/volatile.html /r$ -- STSM Senior Security Architect DataPower SOA Appliances __ OpenSSL Project http://www.openssl.org Development Mailing List openssl-dev@openssl.org Automated List Manager [EMAIL PROTECTED]
Re: Memory Leaks in SSL_Library_init()
Kyle Hamilton wrote:
Oh. I'm sorry. Someone needs to use a keyword 'volatile'.
Bingo. Problem solved on the improper optimization issue.
Actually 'volatile' doesn't provide a useful fix.
Ignoring the fact there is no problem with any CPU that currently
exists. The volatile is most useful for memory mapped I/O where
touching a memory location (for load or store) can have side effects on
the I/O device. So 'volatile' can be used to enforce a strict access
policy based on the code as written (as opposed to how the compiler
might re-order load/stores if it was regular memory).
My points here are that even under the best optimizing compiler
following 100% safe optimizations will never have a problem with the
original posters code. There is no need to use volatile, it provide no
benefit and will impact correct optimizations within code making use of
the variable.
David seems to be thinking ahead into the realms of CPUs that have not
been invented yet. I'm don't give two hoots about those CPUs and I
certainly wouldn't waste brain cells trying to make my code portable to
a piece of hardware that does not exist.
On 3/27/07, David Schwartz [EMAIL PROTECTED] wrote:
For POSIX threads, the result of reading a variable in one
thread while it
might be modified in another thread is undefined. Line 1289 and
1290 should
be removed.
Not this old chestnut again.
Like it or not, it's a fact.
I would disagree. Its merely your opinion; and in my opinion your
understanding of C, CPU architecture, CPU cache cohearancy and memory
access isn't as good as you think it is.
You did point out a scenario that was not at all relevant to the
situation in the code of the original post.
Of course, that's because those CPUs don't exist yet. But I expect
code that
I write and compile today to work on future CPUs because I don't rely on
guarantees I don't actually ahve.
LOL. Hey that trans-wrap-cpu which converts charged ions into bits uses
an equally spartan language better able describe all those new features
it brings to the party.
Umm, what CPU? *Every* CPU OpenSSL does or might ever run on? And that
would
have to include every compiler OpenSSL does or might support, because the
compiler can change things around too.
I already pointed out the hows and whys the compiler CAN NOT change
anything around in the situation embodied in the original post.
The function entrance, the return and the CRYPTO_w_lock() calls all
act as the correct memory barriers with regards to compiler
optimizations and load/store rescheduling concerns. Even if the
CRYPTO_w_lock() is a macro or inline, since the writer would have been
sure to insert the correct barriers if he chose to implement
CRYPTO_w_lock() as a macro/inline.
Where is that documented or guaranteed? You mean that they *happen* to be
barriers on the platforms you have used. You have no idea what the next
version of GCC might do. We are talking about saving *NOTHING* here.
Its documented in the multi-CPU erratum for each CPU that can be wired
up to access the same memory.
There are different levels to the entire problem area, there is the
electrical and timing of the memory bus access to the same memory
location, there is the area of CPU cache and shared CPU cache
cohearancy. There are all documented for each architecture.
Then there is the area of the C language optimizing compiler. Which is
less documented as its the language constructs which are documented
rather than the optimization.
The barriers being discussed in this point are those for the compiler
that affect code generation and how load/store operations are managed.
But in principle the C language is a sequential language, that means the
observable order of events that occurs is STRICTLY in the order laid out
in the source code. Any optimization that defies that basic principal
is a bug.
The problem occurs after the beginning of the function. If the compare is
done on a cached copy in a register. Look at this example:
if (variable!=NULL)
{
free(variable);
variable=NULL;
}
This could easily be optimized to (cached_copy is a register or other
fast
place to store data):
int cached_copy=variable;
if(cached_copy!=NULL)
{
acquire_lock();
cached_copy=variable;
free(cached_copy);
cached_copy=NULL;
variable=cached_copy;
release_lock();
}
else variable=cached_copy;
If you think this cannot happen, I defy you to explain to me why. What
standard or guarantee is being violated? How can POSIX-compliant or
C-compliant code break with this optimization? How can you say it can
never
be faster?
I presume in this example the variable 'cached_copy' is not a real
variable (as in it does not exist in the original C source) but a
compiler generated variable during the optimization process.
It can not happen because when the compiler makes a function call to
acquire_lock() (in the o/p this is CRYPTO_w_lock(); by definition of C
RE: Memory Leaks in SSL_Library_init()
So the point you are trying to make is, while the function would solve the purpose of freeing the compression methods, However the lock are not really required in the usage secnario of this function? If the usage scenario is solely final shutdown of the library, then the lock is not required. If the usage scenario is larger than that, then the double-checked locking should be removed and all tests should be done under the lock. The current code is an insensible compromise. The double-checked locking makes no sense in either usage scenario. If it's purely a shutdown function, then the optimization is a pure complication -- there's no need to acquire the lock in any case. If it's not purely a shutdown function, then the optimization is prohibited by the pthreads standard and its behavior is undefined and it could break on future CPUs, compilers, or platforms. I admit it's unlikely to break on future x86 platforms -- too much existing x86-only code would break. But I have seen similar types of 'optimizations' break on new CPUs when those CPUs used optimizations that were not imaginable at the time the code was written. OpenSSL has a way for platforms to provide safe multi-threading primitives. If additional primitives are needed or beneficial for optimization, they should be provided the same way. OpenSSL itself should not make assumptions that are specifically prohibited by the standards it is designed to work with. DS __ OpenSSL Project http://www.openssl.org Development Mailing List openssl-dev@openssl.org Automated List Manager [EMAIL PROTECTED]
RE: Memory Leaks in SSL_Library_init()
David seems to be thinking ahead into the realms of CPUs that have not been invented yet. Exactly. That's why there are standards and guarantees. If you follow the standards and rely on the guarantees you have, your code will work on all future platforms that provide those same guarantees you have. At one time, the idea that a compiler would enforce the C standard's aliasing rules was inconceivable. Did a lot of code break on newer compilers that relied on them in optimization? Yes, it did. At one time, the idea that a CPU would reorder writes that had been placed in order in assembly code was inconceivable. Did some code break on newer CPUs that had posted write buffers? Yes, it did. I'm don't give two hoots about those CPUs and I certainly wouldn't waste brain cells trying to make my code portable to a piece of hardware that does not exist. Forgive me for being blunt, but that's a totally idiotic attitude. I for one don't expect to have to recompile or even debug my software when I upgrade my CPU. It is unreasonable to design code so that it breaks on new CPUs, and when you rely on behavior that is not documented or guaranteed, that's exactly what happens. I understand that you think that this one assumption is safe and won't break. But I can tell you from a long history of such assumptions that they are *not* safe and they *do* break. Can I tell you how and when this particular one will break? No, I can't. Have I seen many similar assumptions break in the past? Yes, I have. I have worked as a professional programmer for more than 20 years and have worked directly for or consulted for about 8 companies that develop software as a primary or significant aspect of their business. This type of code violates the published coding standards of the six of those companies that have them. DS __ OpenSSL Project http://www.openssl.org Development Mailing List openssl-dev@openssl.org Automated List Manager [EMAIL PROTECTED]
Re: Memory Leaks in SSL_Library_init()
From: Darryl Miles [EMAIL PROTECTED]
Reply-To: openssl-dev@openssl.org
To: openssl-dev@openssl.org
Subject: Re: Memory Leaks in SSL_Library_init()
Date: Wed, 21 Mar 2007 11:12:38 +
Nitin M wrote:
Does this mean that in such scenario the application need not call
SSL_library_init since it does not need those extra initialisations and
can achieve only the required initialisations with specific calls?
If this is true I have two more questions here,
1. In what scenario then an application should use SSL_library_init()? I
think not for the simplest use of SSL/Crypto functionality in the
application.
2. If SSL_library_init() should be called only once in the lifetime of a
process and it is is creating some global data structures only once within
that process, why cant we have a function to clear all these global
variables which can be called at the the process termination time?
If you are using SSL then you are touching many parts of the OpenSSL
library as all the digests and crypto algorithms are pulled in to allow
them to be available for selection and negotiation. Most users want to
simple few function calls to auto-register all those methods.
The simplest use of OpenSSL is more inline with how I explained, an app
just wants to use a single digest algorithm (like MD5 or SHA1, but not
SSL).
As I still maintain you need to provide information about the leak(s) in
order to progress.
Hi!
I wrote a simple program like this to find out the possibility of a memory
leak. The program goes like this.
#includeopenssl/ssl.h
int main()
{
SSL_library_init();
EVP_cleanup();
}
when I run this programm through the valgrind I get the following output.
==10036== ERROR SUMMARY: 0 errors from 0 contexts (suppressed: 17 from 1)
--10036--
--10036-- supp: 17 Ugly strchr error in /lib/ld-2.3.2.so
==10036== malloc/free: in use at exit: 36 bytes in 2 blocks.
==10036== malloc/free: 87 allocs, 85 frees, 1472 bytes allocated.
==10036==
==10036== searching for pointers to 2 not-freed blocks.
==10036== checked 2852348 bytes.
==10036==
==10036== 36 bytes in 2 blocks are still reachable in loss record 1 of 1
==10036==at 0x1B903CC8: malloc (vg_replace_malloc.c:131)
==10036==by 0x1B978CAA: default_malloc_ex (in
/home/nitin/software/lib/libcrypto.so.0.9.8)
==10036==
==10036== LEAK SUMMARY:
==10036==definitely lost: 0 bytes in 0 blocks.
==10036==possibly lost: 0 bytes in 0 blocks.
==10036==still reachable: 36 bytes in 2 blocks.
==10036== suppressed: 0 bytes in 0 blocks.
This shows that 36 bytes in 2 block are still reachable.
In Valgrinds language A pointer to the start of the block is found. This
usually indicates programming sloppiness. Since the block is still pointed
at, the programmer could, at least in principle, free it before program
exit. But I think these would get accumulated over a period of time if the
application uses pluggin DLLs which inturn use OpenSSL and it is not known
how often the plugin DLLs are loaded and unloaded.
After further looking into source I found that there is no way/function for
the The SSL compression methods global stack (ssl_comp_methods) to be
freed.
I added a small function SSL_free_comp_methods to cleanup this stack based
on the patch submitted by Jonathan Green.
http://marc.info/?l=openssl-devm=112200683926727w=2
1287 void SSL_free_comp_methods(void)
1288 {
1289 if (ssl_comp_methods == NULL)
1290 return;
1291 CRYPTO_w_lock(CRYPTO_LOCK_SSL);
1292 if (ssl_comp_methods != NULL)
1293 {
1294 sk_SSL_COMP_pop_free(ssl_comp_methods,CRYPTO_free);
1295 ssl_comp_methods = NULL;
1296 }
1297 CRYPTO_w_unlock(CRYPTO_LOCK_SSL);
1298 }
So when I call this function also in the application program in addition to
EVP_cleanup(),
#includeopenssl/ssl.h
int main()
{
SSL_library_init();
EVP_cleanup();
SSL_free_comp_methods();
}
the valgrind output is like this
==10055== ERROR SUMMARY: 0 errors from 0 contexts (suppressed: 17 from 1)
--10055--
--10055-- supp: 17 Ugly strchr error in /lib/ld-2.3.2.so
==10055== malloc/free: in use at exit: 0 bytes in 0 blocks.
==10055== malloc/free: 87 allocs, 87 frees, 1472 bytes allocated.
==10055==
==10055== No malloc'd blocks -- no leaks are possible.
Which looks like a more cleaner output.
I was just wondering if this issue has been higlighted in some earlier posts
and the patch submitted looks like solving the problem, technically was
there any problem in including it into mainstream code?
Application usages goes like this:
* Application initializes the OpenSSL libary.
* Application creates and uses data object with the OpenSSL library.
* Application wants to shutdown.
* Application destroys all OpenSSL data objects it is holding.
* Application calls cleanup routines in the OpenSSL library.
This is pretty standard stuff, what David maybe highlighting is that many
RE: Memory Leaks in SSL_Library_init()
1287 void SSL_free_comp_methods(void)
1288 {
1289 if (ssl_comp_methods == NULL)
1290 return;
1291 CRYPTO_w_lock(CRYPTO_LOCK_SSL);
1292 if (ssl_comp_methods != NULL)
1293 {
1294 sk_SSL_COMP_pop_free(ssl_comp_methods,CRYPTO_free);
1295 ssl_comp_methods = NULL;
1296 }
1297 CRYPTO_w_unlock(CRYPTO_LOCK_SSL);
1298 }
For POSIX threads, the result of reading a variable in one thread while it
might be modified in another thread is undefined. Line 1289 and 1290 should
be removed.
Alternatively, lines 1288, 1289, 1290, and 1297 should be removed. It's hard
to understand any reasonable scenario in which one thread might be calling
SSL_free_comp_methods while another thread is or might be accessing the
ssl_comp_methods variable or any of the registered compression methods.
But the code as posted is an unreasonable compromise between two reasonable
positions. It is sort of safe if called while another thread is or might
be accesing ssl_comp_methods.
If you think there is no way this code could fail, imagine a hypothetical
compiler/platform in which if(ssl_comp_methods == NULL) translates into an
operation through a fast caching shadow register like this:
shadow=ssl_comp_methods;
if(ssl_comp_methods==NULL)
{
ssl_comp_methods=shadow; /* flush cache, writing back entries */
/* whatever */
}
If another thread modified 'ssl_comp_methods' in between the read and the
cache flush, that modification would be lost. I, for one, don't like
needlessly making code potentially disastrous on future CPUs and compilers.
(To save what?!)
... the evils of premature optimization.
DS
__
OpenSSL Project http://www.openssl.org
Development Mailing List openssl-dev@openssl.org
Automated List Manager [EMAIL PROTECTED]
Re: Memory Leaks in SSL_Library_init()
David Schwartz wrote:
1287 void SSL_free_comp_methods(void)
1288 {
1289 if (ssl_comp_methods == NULL)
1290 return;
1291 CRYPTO_w_lock(CRYPTO_LOCK_SSL);
1292 if (ssl_comp_methods != NULL)
1293 {
1294 sk_SSL_COMP_pop_free(ssl_comp_methods,CRYPTO_free);
1295 ssl_comp_methods = NULL;
1296 }
1297 CRYPTO_w_unlock(CRYPTO_LOCK_SSL);
1298 }
For POSIX threads, the result of reading a variable in one thread while it
might be modified in another thread is undefined. Line 1289 and 1290 should
be removed.
Not this old chestnut again.
I can't name a CPU in which an aligned load/store of a natural/primitive
data size between overlapping threads actually has undefined behavior.
The underlying assembly instructions generated by the C compiler have
a defined behavior on multi-CPU systems.
Due to this defined behavior of the underlying CPU at a lower level than
the scope of the POSIX specification is designed to cover you should
treat this behavior as defined. This would not contradict POSIX which
is effectively saying out of the scope of the POSIX spec by using the
term undefined.
The function entrance, the return and the CRYPTO_w_lock() calls all
act as the correct memory barriers with regards to compiler
optimizations and load/store rescheduling concerns. Even if the
CRYPTO_w_lock() is a macro or inline, since the writer would have been
sure to insert the correct barriers if he chose to implement
CRYPTO_w_lock() as a macro/inline.
Alternatively, lines 1288, 1289, 1290, and 1297 should be removed. It's hard
to understand any reasonable scenario in which one thread might be calling
SSL_free_comp_methods while another thread is or might be accessing the
ssl_comp_methods variable or any of the registered compression methods.
I would agree with your overall view, but I would on the basis that this
function is not performance critical to deserve any special lock
contention avoidance treatment.
But there is a case for locking to not be necessary at all. If the
SSL_free_comp_methods() is only designed to be used during the cleanup
phase. During a clean shutdown sequence of the OpenSSL library
I'm pretty sure you are meant to have reverted the OpenSSL call
execution path to single threaded mode.
Meaning while you are executing any cleanup functions it is not safe to
be calling any other OpenSSL functions at the same time. By virtue of
this requirement locking should not be necessary (but should do more
good than harm if implemented). If your application needs to repeatedly
init and cleanup, init and cleanup, then your application needs to also
manage the state and the serialization of the init/cleanup calls (since
they may not be safe to be called out-of-order, called twice,
interleved, etc...).
But the code as posted is an unreasonable compromise between two reasonable
positions. It is sort of safe if called while another thread is or might
be accesing ssl_comp_methods.
This I'm not so sure I agree with; the It is 'sort of' safe if called
while another thread ... part (see the last 2 paragraphs of mine above).
If you think there is no way this code could fail, imagine a hypothetical
compiler/platform in which if(ssl_comp_methods == NULL) translates into an
operation through a fast caching shadow register like this:
shadow=ssl_comp_methods;
if(ssl_comp_methods==NULL)
{
ssl_comp_methods=shadow; /* flush cache, writing back entries */
/* whatever */
}
If another thread modified 'ssl_comp_methods' in between the read and the
cache flush, that modification would be lost. I, for one, don't like
needlessly making code potentially disastrous on future CPUs and compilers.
(To save what?!)
... the evils of premature optimization.
No as I said by virtue of it being a function it already forces the
compiler to perform the correct barriers. Maybe you can name a CPU
target which does not automatically flush pending writes for return
from function and call a function instructions, I can't.
The example you cite is not valid for line 1289 and 1290. The only
write operation to 'ssl_comp_methods' occurs inside the locked region
and after the text condition has been re-evaluated.
So your pseudo-code serves a useful purpose to illustrate to others on
the list about the concern you raise, but the concern itself is not
relevant to the example given.
Darryl
__
OpenSSL Project http://www.openssl.org
Development Mailing List openssl-dev@openssl.org
Automated List Manager [EMAIL PROTECTED]
Re: Memory Leaks in SSL_Library_init()
Nitin M wrote:
I wrote a simple program like this to find out the possibility of a
memory leak. The program goes like this.
#includeopenssl/ssl.h
int main()
{
SSL_library_init();
EVP_cleanup();
}
when I run this programm through the valgrind I get the following output.
Ok, it is not clear if tried out the cleanup sequence I posted and
checked for results after using it ?
If you are still getting leaks after; then the function
SSL_free_comp_methods(); might be addressing a genuine leak which can't
be reclaimed using any of the existing cleanup functions.
So I'd vote for it to get more attention and be included.
I'd also like to see a SSL_library_cleanup(), which in turn calls
SSL_free_comp_methods() just to start the ball rolling on reducing the
OpenSSL voodoo.
Darryl
__
OpenSSL Project http://www.openssl.org
Development Mailing List openssl-dev@openssl.org
Automated List Manager [EMAIL PROTECTED]
Re: Memory Leaks in SSL_Library_init()
1 #includeopenssl/ssl.h
2 int main()
3 {
4 SSL_library_init();
5
6 /* thread-local cleanup */
7 ERR_remove_state(0);
8
9 /* Globals we finished with */
10 //
my_application_function_to_destroy_OpenSSL_objects_attached_to_local_variables();
11 // In the particular application I am keeping an RSA
12 // // object around.
13 // if(tmp_rsa != NULL) {
14 // RSA_free(tmp_rsa);
15 // tmp_rsa = NULL;
16 // }
17
18
19 // /* thread-safe cleanup */
20 ENGINE_cleanup();
21 CONF_modules_unload(1);
22 //
23 // /* global application exit cleanup (after all SSL activity is
shutdown) */
24 ERR_free_strings();
25 EVP_cleanup();
26 // SSL_free_comp_methods();
27 CRYPTO_cleanup_all_ex_data();
28 }
I also tried with the above code and found that 36 bytes are still
reachable. Then after retining only EVP_cleanup() from the clanup sequence
of calls, I felt that EVP_clenup is doing the job of cleaning up most of
what SSL_library_init() created.
In one of your earlier posts you have said that while you use the mentioned
sequence of cleanup calls you did not get any memory leak at all. You dont
even get the Still reachable msg with that sequence? if this is true,is it
possible for you to post a small example code here?
Also I think the function
my_application_function_to_destroy_OpenSSL_objects_attached_to_local_variables();
is your application specific function and since I did not have any RSA
object I did not call RSA_free(tmp_rsa);
With a call SSL_library_cleanup(), which internally calls EVP_cleanup()
and SSL_free_comp_methods() do you see any harm or any scenario in which it
might break. I feel that if used properly for every corresponding
SSL_library_init(), it should not causse any problem.
What is you opinion?
Here is the valgrind output for the above program for your reference.
==10877== ERROR SUMMARY: 0 errors from 0 contexts (suppressed: 17 from 1)
--10877--
--10877-- supp: 17 Ugly strchr error in /lib/ld-2.3.2.so
==10877== malloc/free: in use at exit: 36 bytes in 2 blocks.
==10877== malloc/free: 89 allocs, 87 frees, 1632 bytes allocated.
==10877==
==10877== searching for pointers to 2 not-freed blocks.
==10877== checked 2852348 bytes.
==10877==
==10877== 36 bytes in 2 blocks are still reachable in loss record 1 of 1
==10877==at 0x1B903CC8: malloc (vg_replace_malloc.c:131)
==10877==by 0x1B978CAA: default_malloc_ex (in
/home/nitin/software/lib/libcrypto.so.0.9.8)
==10877==
==10877== LEAK SUMMARY:
==10877==definitely lost: 0 bytes in 0 blocks.
==10877==possibly lost: 0 bytes in 0 blocks.
==10877==still reachable: 36 bytes in 2 blocks.
==10877== suppressed: 0 bytes in 0 blocks.
From: Darryl Miles [EMAIL PROTECTED]
Reply-To: openssl-dev@openssl.org
To: openssl-dev@openssl.org
Subject: Re: Memory Leaks in SSL_Library_init()
Date: Wed, 28 Mar 2007 04:53:02 +0100
Nitin M wrote:
I wrote a simple program like this to find out the possibility of a memory
leak. The program goes like this.
#includeopenssl/ssl.h
int main()
{
SSL_library_init();
EVP_cleanup();
}
when I run this programm through the valgrind I get the following output.
Ok, it is not clear if tried out the cleanup sequence I posted and checked
for results after using it ?
If you are still getting leaks after; then the function
SSL_free_comp_methods(); might be addressing a genuine leak which can't be
reclaimed using any of the existing cleanup functions.
So I'd vote for it to get more attention and be included.
I'd also like to see a SSL_library_cleanup(), which in turn calls
SSL_free_comp_methods() just to start the ball rolling on reducing the
OpenSSL voodoo.
Darryl
__
OpenSSL Project http://www.openssl.org
Development Mailing List openssl-dev@openssl.org
Automated List Manager [EMAIL PROTECTED]
_
Spice up your IM conversations. New, colorful and animated emoticons. Get
chatting! http://server1.msn.co.in/SP05/emoticons/
__
OpenSSL Project http://www.openssl.org
Development Mailing List openssl-dev@openssl.org
Automated List Manager [EMAIL PROTECTED]
RE: Memory Leaks in SSL_Library_init()
For POSIX threads, the result of reading a variable in one
thread while it
might be modified in another thread is undefined. Line 1289 and
1290 should
be removed.
Not this old chestnut again.
Like it or not, it's a fact.
I can't name a CPU in which an aligned load/store of a natural/primitive
data size between overlapping threads actually has undefined behavior.
The underlying assembly instructions generated by the C compiler have
a defined behavior on multi-CPU systems.
Of course, that's because those CPUs don't exist yet. But I expect code that
I write and compile today to work on future CPUs because I don't rely on
guarantees I don't actually ahve.
Due to this defined behavior of the underlying CPU at a lower level than
the scope of the POSIX specification is designed to cover you should
treat this behavior as defined. This would not contradict POSIX which
is effectively saying out of the scope of the POSIX spec by using the
term undefined.
Umm, what CPU? *Every* CPU OpenSSL does or might ever run on? And that would
have to include every compiler OpenSSL does or might support, because the
compiler can change things around too.
The function entrance, the return and the CRYPTO_w_lock() calls all
act as the correct memory barriers with regards to compiler
optimizations and load/store rescheduling concerns. Even if the
CRYPTO_w_lock() is a macro or inline, since the writer would have been
sure to insert the correct barriers if he chose to implement
CRYPTO_w_lock() as a macro/inline.
Where is that documented or guaranteed? You mean that they *happen* to be
barriers on the platforms you have used. You have no idea what the next
version of GCC might do. We are talking about saving *NOTHING* here.
Alternatively, lines 1288, 1289, 1290, and 1297 should be
removed. It's hard
to understand any reasonable scenario in which one thread might
be calling
SSL_free_comp_methods while another thread is or might be accessing the
ssl_comp_methods variable or any of the registered compression methods.
I would agree with your overall view, but I would on the basis that this
function is not performance critical to deserve any special lock
contention avoidance treatment.
Exactly. This is the worst kind of premature optimization.
But there is a case for locking to not be necessary at all. If the
SSL_free_comp_methods() is only designed to be used during the cleanup
phase. During a clean shutdown sequence of the OpenSSL library
I'm pretty sure you are meant to have reverted the OpenSSL call
execution path to single threaded mode.
Right. You can't let one thread unconditionally shut down a library while
another thread is or might be using it. However, it's possible this function
was also intended to 'unregister' the compression functions without harming
the library. In that case, the lock is needed and the optimization is
broken.
Meaning while you are executing any cleanup functions it is not safe to
be calling any other OpenSSL functions at the same time. By virtue of
this requirement locking should not be necessary (but should do more
good than harm if implemented). If your application needs to repeatedly
init and cleanup, init and cleanup, then your application needs to also
manage the state and the serialization of the init/cleanup calls (since
they may not be safe to be called out-of-order, called twice,
interleved, etc...).
It will do more good than harm if the locks haven't been cleaned up!
If another thread modified 'ssl_comp_methods' in between the
read and the
cache flush, that modification would be lost. I, for one, don't like
needlessly making code potentially disastrous on future CPUs
and compilers.
(To save what?!)
... the evils of premature optimization.
No as I said by virtue of it being a function it already forces the
compiler to perform the correct barriers. Maybe you can name a CPU
target which does not automatically flush pending writes for return
from function and call a function instructions, I can't.
The problem occurs after the beginning of the function. If the compare is
done on a cached copy in a register. Look at this example:
if (variable!=NULL)
{
free(variable);
variable=NULL;
}
This could easily be optimized to (cached_copy is a register or other fast
place to store data):
int cached_copy=variable;
if(cached_copy!=NULL)
{
acquire_lock();
cached_copy=variable;
free(cached_copy);
cached_copy=NULL;
variable=cached_copy;
release_lock();
}
else variable=cached_copy;
If you think this cannot happen, I defy you to explain to me why. What
standard or guarantee is being violated? How can POSIX-compliant or
C-compliant code break with this optimization? How can you say it can never
be faster?
The example you cite is not valid for line 1289 and 1290. The only
write operation to 'ssl_comp_methods' occurs inside the locked region
and after the text condition has been re-evaluated.
So
Re: Memory Leaks in SSL_Library_init()
Oh. I'm sorry. Someone needs to use a keyword 'volatile'.
Bingo. Problem solved on the improper optimization issue.
Can we commit the patch so that we don't have to keep getting hit by 2
or 3 threads about valgrind complaining about reachable pointers at
the end of program execution every month? This /is/ an OpenSSL
library bug, and it /does/ need to be addressed -- regardless of it's
only 36 bytes, relying on an operating system cleaning up after you
is sloppy programming at best and bug-laden at worst.
Remember, not all architectures have memory management units, either.
-Kyle H
On 3/27/07, David Schwartz [EMAIL PROTECTED] wrote:
For POSIX threads, the result of reading a variable in one
thread while it
might be modified in another thread is undefined. Line 1289 and
1290 should
be removed.
Not this old chestnut again.
Like it or not, it's a fact.
I can't name a CPU in which an aligned load/store of a natural/primitive
data size between overlapping threads actually has undefined behavior.
The underlying assembly instructions generated by the C compiler have
a defined behavior on multi-CPU systems.
Of course, that's because those CPUs don't exist yet. But I expect code that
I write and compile today to work on future CPUs because I don't rely on
guarantees I don't actually ahve.
Due to this defined behavior of the underlying CPU at a lower level than
the scope of the POSIX specification is designed to cover you should
treat this behavior as defined. This would not contradict POSIX which
is effectively saying out of the scope of the POSIX spec by using the
term undefined.
Umm, what CPU? *Every* CPU OpenSSL does or might ever run on? And that would
have to include every compiler OpenSSL does or might support, because the
compiler can change things around too.
The function entrance, the return and the CRYPTO_w_lock() calls all
act as the correct memory barriers with regards to compiler
optimizations and load/store rescheduling concerns. Even if the
CRYPTO_w_lock() is a macro or inline, since the writer would have been
sure to insert the correct barriers if he chose to implement
CRYPTO_w_lock() as a macro/inline.
Where is that documented or guaranteed? You mean that they *happen* to be
barriers on the platforms you have used. You have no idea what the next
version of GCC might do. We are talking about saving *NOTHING* here.
Alternatively, lines 1288, 1289, 1290, and 1297 should be
removed. It's hard
to understand any reasonable scenario in which one thread might
be calling
SSL_free_comp_methods while another thread is or might be accessing the
ssl_comp_methods variable or any of the registered compression methods.
I would agree with your overall view, but I would on the basis that this
function is not performance critical to deserve any special lock
contention avoidance treatment.
Exactly. This is the worst kind of premature optimization.
But there is a case for locking to not be necessary at all. If the
SSL_free_comp_methods() is only designed to be used during the cleanup
phase. During a clean shutdown sequence of the OpenSSL library
I'm pretty sure you are meant to have reverted the OpenSSL call
execution path to single threaded mode.
Right. You can't let one thread unconditionally shut down a library while
another thread is or might be using it. However, it's possible this function
was also intended to 'unregister' the compression functions without harming
the library. In that case, the lock is needed and the optimization is
broken.
Meaning while you are executing any cleanup functions it is not safe to
be calling any other OpenSSL functions at the same time. By virtue of
this requirement locking should not be necessary (but should do more
good than harm if implemented). If your application needs to repeatedly
init and cleanup, init and cleanup, then your application needs to also
manage the state and the serialization of the init/cleanup calls (since
they may not be safe to be called out-of-order, called twice,
interleved, etc...).
It will do more good than harm if the locks haven't been cleaned up!
If another thread modified 'ssl_comp_methods' in between the
read and the
cache flush, that modification would be lost. I, for one, don't like
needlessly making code potentially disastrous on future CPUs
and compilers.
(To save what?!)
... the evils of premature optimization.
No as I said by virtue of it being a function it already forces the
compiler to perform the correct barriers. Maybe you can name a CPU
target which does not automatically flush pending writes for return
from function and call a function instructions, I can't.
The problem occurs after the beginning of the function. If the compare is
done on a cached copy in a register. Look at this example:
if (variable!=NULL)
{
free(variable);
variable=NULL;
}
This could easily be optimized to (cached_copy is a register or other fast
RE: Memory Leaks in SSL_Library_init()
Oh. I'm sorry. Someone needs to use a keyword 'volatile'. Sorry, doesn't help. Bingo. Problem solved on the improper optimization issue. What specification says that 'volatile' causes any particular semantics across threads? I must not have read that one. The 'volatile' keyword is only defined in the C and C++ standards, and neither of those address multi-threading. You might think that multi-threading standards would extend 'volatile' to have multi-thread semantics. The fact is, they don't. If you can cite one threading standard that specifies semantics for 'volatile', please do so. The reason they don't is quite logical. First, it penalizes code that uses 'volatile' for its documented purposes (such as 'longjmp' and signals). Second, they instead provide much more flexible primitives (such as mutexes) which solve a larger range of problems than a variable qualifier ever should. In short, *nobody* has any idea what effect 'volatile' might have on muli-threaded code on future compilers and future CPUs because *no* standard says what it should do. For example, Itanium compilers do not put memory barriers before or after accesses to 'volatile' variables precisely because it heavily penalizes sensible code and better ways to access shared variables are provided. Can we commit the patch so that we don't have to keep getting hit by 2 or 3 threads about valgrind complaining about reachable pointers at the end of program execution every month? This /is/ an OpenSSL library bug, and it /does/ need to be addressed -- regardless of it's only 36 bytes, relying on an operating system cleaning up after you is sloppy programming at best and bug-laden at worst. Where these kinds of things can easily be fixed, they *definitely* should be. Anyone who doesn't want to waste time cleaning up doesn't have to call the cleanup functions at all. I do not in any way oppose making clean up functions do a better job of cleaning up where that is essentially free. Remember, not all architectures have memory management units, either. Even architectures without memory management units keep track of what memory belongs to a process and free it when that process terminates. It's hard to imagine an architecture that couldn't do that where you wouldn't just keep your libraries spun up all the time. I can't think of any. In any event, on such a platform, you probably couldn't use much commodity software. The trade-offs in design would skew very differently. There are SSL libraries aimed at that market, and I don't think OpenSSL is really trying to do that. DS __ OpenSSL Project http://www.openssl.org Development Mailing List openssl-dev@openssl.org Automated List Manager [EMAIL PROTECTED]
RE: Memory Leaks in SSL_Library_init()
So the point you are trying to make is, while the function would solve the
purpose of freeing the compression methods, However the lock are not really
required in the usage secnario of this function?
From: David Schwartz [EMAIL PROTECTED]
Reply-To: openssl-dev@openssl.org
To: openssl-dev@openssl.org
Subject: RE: Memory Leaks in SSL_Library_init()
Date: Tue, 27 Mar 2007 21:23:45 -0700
For POSIX threads, the result of reading a variable in one
thread while it
might be modified in another thread is undefined. Line 1289 and
1290 should
be removed.
Not this old chestnut again.
Like it or not, it's a fact.
I can't name a CPU in which an aligned load/store of a natural/primitive
data size between overlapping threads actually has undefined behavior.
The underlying assembly instructions generated by the C compiler have
a defined behavior on multi-CPU systems.
Of course, that's because those CPUs don't exist yet. But I expect code
that
I write and compile today to work on future CPUs because I don't rely on
guarantees I don't actually ahve.
Due to this defined behavior of the underlying CPU at a lower level than
the scope of the POSIX specification is designed to cover you should
treat this behavior as defined. This would not contradict POSIX which
is effectively saying out of the scope of the POSIX spec by using the
term undefined.
Umm, what CPU? *Every* CPU OpenSSL does or might ever run on? And that
would
have to include every compiler OpenSSL does or might support, because the
compiler can change things around too.
The function entrance, the return and the CRYPTO_w_lock() calls all
act as the correct memory barriers with regards to compiler
optimizations and load/store rescheduling concerns. Even if the
CRYPTO_w_lock() is a macro or inline, since the writer would have been
sure to insert the correct barriers if he chose to implement
CRYPTO_w_lock() as a macro/inline.
Where is that documented or guaranteed? You mean that they *happen* to be
barriers on the platforms you have used. You have no idea what the next
version of GCC might do. We are talking about saving *NOTHING* here.
Alternatively, lines 1288, 1289, 1290, and 1297 should be
removed. It's hard
to understand any reasonable scenario in which one thread might
be calling
SSL_free_comp_methods while another thread is or might be accessing
the
ssl_comp_methods variable or any of the registered compression
methods.
I would agree with your overall view, but I would on the basis that this
function is not performance critical to deserve any special lock
contention avoidance treatment.
Exactly. This is the worst kind of premature optimization.
But there is a case for locking to not be necessary at all. If the
SSL_free_comp_methods() is only designed to be used during the cleanup
phase. During a clean shutdown sequence of the OpenSSL library
I'm pretty sure you are meant to have reverted the OpenSSL call
execution path to single threaded mode.
Right. You can't let one thread unconditionally shut down a library while
another thread is or might be using it. However, it's possible this
function
was also intended to 'unregister' the compression functions without harming
the library. In that case, the lock is needed and the optimization is
broken.
Meaning while you are executing any cleanup functions it is not safe to
be calling any other OpenSSL functions at the same time. By virtue of
this requirement locking should not be necessary (but should do more
good than harm if implemented). If your application needs to repeatedly
init and cleanup, init and cleanup, then your application needs to also
manage the state and the serialization of the init/cleanup calls (since
they may not be safe to be called out-of-order, called twice,
interleved, etc...).
It will do more good than harm if the locks haven't been cleaned up!
If another thread modified 'ssl_comp_methods' in between the
read and the
cache flush, that modification would be lost. I, for one, don't like
needlessly making code potentially disastrous on future CPUs
and compilers.
(To save what?!)
... the evils of premature optimization.
No as I said by virtue of it being a function it already forces the
compiler to perform the correct barriers. Maybe you can name a CPU
target which does not automatically flush pending writes for return
from function and call a function instructions, I can't.
The problem occurs after the beginning of the function. If the compare is
done on a cached copy in a register. Look at this example:
if (variable!=NULL)
{
free(variable);
variable=NULL;
}
This could easily be optimized to (cached_copy is a register or other fast
place to store data):
int cached_copy=variable;
if(cached_copy!=NULL)
{
acquire_lock();
cached_copy=variable;
free(cached_copy);
cached_copy=NULL;
variable=cached_copy;
release_lock();
}
else variable=cached_copy;
If you think
RE: Memory Leaks in SSL_Library_init()
Hi! I have an example case where by the unused memoy allocated by SSL_library_init when not freed, would accumulate. There is an application which takes services from some of the libraries say A, B and C. These libraries are dynamically loaded and unloaded into the application as and when required depending on certain conditions and using g_module_ calls. All of these plugin libraries call SSL_library_init() and since we do not know how many times in the lifespan of the application these libraries would be loaded/unloaded, we dont know how many times SSL_library_init() would get called. This issue is also encountered in pretty much every library of sufficient complexity. All modules in a process that access a complex library must cooperate to meet that library's process-scope rules. Failure to meet this requirement can lead to unexpected and spectacular failures. As a simple example, consider algorithms that are added. These are process-scope. A plugin may find algorithms added that it didn't want added. So you have to design your plugin to tolerate algorithms added by other plugins. In other words, being a plugin that works with other plugins puts design constraints and imposes cooperation requirements. Imagine two plugins that each want to call CRYPTO_set_mem_functions. Whose locking functions do we use? What if one module uses a kernel threads implementation and the user uses emulated threads inside a single kernel thread? Sorry, it's not pretty, but if plugins are going to use OpenSSL, then the program they are plugged into has to coordinate (or there has to be an OpenSSL plugin they both use or some other sensible arrangement). A process has to agree on how it's going to use OpenSSL because the process loads and initializes the library. Making sure initialization is only done once is the least of your problems. DS __ OpenSSL Project http://www.openssl.org Development Mailing List openssl-dev@openssl.org Automated List Manager [EMAIL PROTECTED]
RE: Memory Leaks in SSL_Library_init()
HI! Thanks again for highlighting those issues. What would be the best way for the application using those pluggins to avoid this issue of SSL_library_init()? regards -Nitin From: David Schwartz [EMAIL PROTECTED] Reply-To: openssl-dev@openssl.org To: openssl-dev@openssl.org Subject: RE: Memory Leaks in SSL_Library_init() Date: Wed, 21 Mar 2007 00:14:53 -0700 Hi! I have an example case where by the unused memoy allocated by SSL_library_init when not freed, would accumulate. There is an application which takes services from some of the libraries say A, B and C. These libraries are dynamically loaded and unloaded into the application as and when required depending on certain conditions and using g_module_ calls. All of these plugin libraries call SSL_library_init() and since we do not know how many times in the lifespan of the application these libraries would be loaded/unloaded, we dont know how many times SSL_library_init() would get called. This issue is also encountered in pretty much every library of sufficient complexity. All modules in a process that access a complex library must cooperate to meet that library's process-scope rules. Failure to meet this requirement can lead to unexpected and spectacular failures. As a simple example, consider algorithms that are added. These are process-scope. A plugin may find algorithms added that it didn't want added. So you have to design your plugin to tolerate algorithms added by other plugins. In other words, being a plugin that works with other plugins puts design constraints and imposes cooperation requirements. Imagine two plugins that each want to call CRYPTO_set_mem_functions. Whose locking functions do we use? What if one module uses a kernel threads implementation and the user uses emulated threads inside a single kernel thread? Sorry, it's not pretty, but if plugins are going to use OpenSSL, then the program they are plugged into has to coordinate (or there has to be an OpenSSL plugin they both use or some other sensible arrangement). A process has to agree on how it's going to use OpenSSL because the process loads and initializes the library. Making sure initialization is only done once is the least of your problems. DS __ OpenSSL Project http://www.openssl.org Development Mailing List openssl-dev@openssl.org Automated List Manager [EMAIL PROTECTED] _ Catch the complete World Cup coverage with MSN http://content.msn.co.in/Sports/Cricket/Default.aspx __ OpenSSL Project http://www.openssl.org Development Mailing List openssl-dev@openssl.org Automated List Manager [EMAIL PROTECTED]
RE: Memory Leaks in SSL_Library_init()
hi! If we say that the call SSL_library_init() would initialze some data structures which have process scope and are initialized only once. In such case what is the problem in having a *single* function which exacly cleans up those data structures at the time of process termination? regards -Nitin From: David Schwartz [EMAIL PROTECTED] Reply-To: openssl-dev@openssl.org To: openssl-dev@openssl.org Subject: RE: Memory Leaks in SSL_Library_init() Date: Wed, 21 Mar 2007 00:14:53 -0700 Hi! I have an example case where by the unused memoy allocated by SSL_library_init when not freed, would accumulate. There is an application which takes services from some of the libraries say A, B and C. These libraries are dynamically loaded and unloaded into the application as and when required depending on certain conditions and using g_module_ calls. All of these plugin libraries call SSL_library_init() and since we do not know how many times in the lifespan of the application these libraries would be loaded/unloaded, we dont know how many times SSL_library_init() would get called. This issue is also encountered in pretty much every library of sufficient complexity. All modules in a process that access a complex library must cooperate to meet that library's process-scope rules. Failure to meet this requirement can lead to unexpected and spectacular failures. As a simple example, consider algorithms that are added. These are process-scope. A plugin may find algorithms added that it didn't want added. So you have to design your plugin to tolerate algorithms added by other plugins. In other words, being a plugin that works with other plugins puts design constraints and imposes cooperation requirements. Imagine two plugins that each want to call CRYPTO_set_mem_functions. Whose locking functions do we use? What if one module uses a kernel threads implementation and the user uses emulated threads inside a single kernel thread? Sorry, it's not pretty, but if plugins are going to use OpenSSL, then the program they are plugged into has to coordinate (or there has to be an OpenSSL plugin they both use or some other sensible arrangement). A process has to agree on how it's going to use OpenSSL because the process loads and initializes the library. Making sure initialization is only done once is the least of your problems. DS __ OpenSSL Project http://www.openssl.org Development Mailing List openssl-dev@openssl.org Automated List Manager [EMAIL PROTECTED] _ Post ads for free. On new MSN Classifieds. http://www.yello.in/home.php?utm_source=hotmailtagutm_medium=textlinkutm_content=inutm_campaign=intro __ OpenSSL Project http://www.openssl.org Development Mailing List openssl-dev@openssl.org Automated List Manager [EMAIL PROTECTED]
RE: Memory Leaks in SSL_Library_init()
HI! Thanks again for highlighting those issues. What would be the best way for the application using those pluggins to avoid this issue of SSL_library_init()? There are really two good ways that ensure that all problems are resolved. Other ways just deal with problems as they crop up and can never ensure there are no more problems (because you never really know what a future plug-in might do when you write a plug-in so you can never be sure it won't break something you rely on). One way is to design into the application a specification for accessing SSL and cryptographic routines. This specification would generally put one master module (or the application itself) in charge of initializing the library and managing all process-level library parameters. Thus the program 'wraps' OpenSSL. This may mean just replacing the startup and shutdown routines and forbidding changes to process-level state. It could mean wrapping a significant part of the library's API. Another way is to have each plugin use its own copy of OpenSSL with its own everything. This guarantees you start up and shut down your own build of OpenSSL and get exactly the version you want. (For some libraries, you have to change the symbols so the names don't conflict.) The first way has the disadvantage that plugins cannot use differing versions of OpenSSL and must settle on process-level settings that might not make any plugin perfectly happy. It also requires someone to actually create the rules and implement them and all plugins using the common library must follow the specification. The second way has the disadvantage that it's ugly and wastes resource, but it does allow each plugin to own its own process-level OpenSSL. Each plugin can get the version of OpenSSL it wants. Where it's too late for the first way, what is generally resorted to is fixing problems that crop up. Where possible, libraries can be redesigned to minimize process-level state. What happens if two plugins need different versions of OpenSSL? Generally, a plug-in library is strictly limited in what it is permitted to do and going outside that scope can cause weird instabilities. Consider a plugin that always worked fine because it only added 64-bit ciphers, then another plugin adds 256-bit ciphers, and the first plugin starts getting larger ciphers because they are preferred but then copies a key into a 64-bit buffer. Oopsie. Which plug-in is at fault? Plug-ins share all kinds of things, and this type of sharing requires agreement and cooperation. You cannot have all the plug-ins use the same library, where that library has process-level state, and just hope it will all work out somehow. Designing programs with plugin modules is not easy. It takes a *lot* of thinking from the beginning to keep it sensible and be relatively sure new modules won't break old ones. For some strange reason, this thread is making me feel old. DS __ OpenSSL Project http://www.openssl.org Development Mailing List openssl-dev@openssl.org Automated List Manager [EMAIL PROTECTED]
RE: Memory Leaks in SSL_Library_init()
If we say that the call SSL_library_init() would initialze some data structures which have process scope and are initialized only once. In such case what is the problem in having a *single* function which exacly cleans up those data structures at the time of process termination? See my other posts, particularly this section: Again, you can't put everything last. And anything that goes after something else cannot touch what has already been cleaned up unless you add code to make that work at the point where you touch the thing that might be cleaned up. And that point is usually *not* the cleanup code. The perfect solution involves a lot of very complex dependency work to make absolutely sure that nothing can be used after it's cleaned up. What if the cleanup code for A might need B, and the cleanup code for B might need C, and the cleanup code for C might need A? Do you un-cleanup A? Then when do you clean it back up again? Might you never terminate? There are many examples. Engineering a library for perfect cleanup is a *major* design requirement and if you insist on it, things will have to give in other areas. The problem, in a nutshell, is dependencies. DS __ OpenSSL Project http://www.openssl.org Development Mailing List openssl-dev@openssl.org Automated List Manager [EMAIL PROTECTED]
Re: Memory Leaks in SSL_Library_init()
Nitin M wrote: Does this mean that in such scenario the application need not call SSL_library_init since it does not need those extra initialisations and can achieve only the required initialisations with specific calls? If this is true I have two more questions here, 1. In what scenario then an application should use SSL_library_init()? I think not for the simplest use of SSL/Crypto functionality in the application. 2. If SSL_library_init() should be called only once in the lifetime of a process and it is is creating some global data structures only once within that process, why cant we have a function to clear all these global variables which can be called at the the process termination time? If you are using SSL then you are touching many parts of the OpenSSL library as all the digests and crypto algorithms are pulled in to allow them to be available for selection and negotiation. Most users want to simple few function calls to auto-register all those methods. The simplest use of OpenSSL is more inline with how I explained, an app just wants to use a single digest algorithm (like MD5 or SHA1, but not SSL). As I still maintain you need to provide information about the leak(s) in order to progress. Application usages goes like this: * Application initializes the OpenSSL libary. * Application creates and uses data object with the OpenSSL library. * Application wants to shutdown. * Application destroys all OpenSSL data objects it is holding. * Application calls cleanup routines in the OpenSSL library. This is pretty standard stuff, what David maybe highlighting is that many programmers or a short-lived applications running on a process segmented operating system might skip the last 2 points to save time/money. Darryl __ OpenSSL Project http://www.openssl.org Development Mailing List openssl-dev@openssl.org Automated List Manager [EMAIL PROTECTED]
Re: Memory Leaks in SSL_Library_init()
Is it required to call SSL_library_init() if I only want to use some crypto functionalities? -Nitin From: Darryl Miles [EMAIL PROTECTED] Reply-To: openssl-dev@openssl.org To: openssl-dev@openssl.org Subject: Re: Memory Leaks in SSL_Library_init() Date: Wed, 21 Mar 2007 11:12:38 + Nitin M wrote: Does this mean that in such scenario the application need not call SSL_library_init since it does not need those extra initialisations and can achieve only the required initialisations with specific calls? If this is true I have two more questions here, 1. In what scenario then an application should use SSL_library_init()? I think not for the simplest use of SSL/Crypto functionality in the application. 2. If SSL_library_init() should be called only once in the lifetime of a process and it is is creating some global data structures only once within that process, why cant we have a function to clear all these global variables which can be called at the the process termination time? If you are using SSL then you are touching many parts of the OpenSSL library as all the digests and crypto algorithms are pulled in to allow them to be available for selection and negotiation. Most users want to simple few function calls to auto-register all those methods. The simplest use of OpenSSL is more inline with how I explained, an app just wants to use a single digest algorithm (like MD5 or SHA1, but not SSL). As I still maintain you need to provide information about the leak(s) in order to progress. Application usages goes like this: * Application initializes the OpenSSL libary. * Application creates and uses data object with the OpenSSL library. * Application wants to shutdown. * Application destroys all OpenSSL data objects it is holding. * Application calls cleanup routines in the OpenSSL library. This is pretty standard stuff, what David maybe highlighting is that many programmers or a short-lived applications running on a process segmented operating system might skip the last 2 points to save time/money. Darryl __ OpenSSL Project http://www.openssl.org Development Mailing List openssl-dev@openssl.org Automated List Manager [EMAIL PROTECTED] _ Sign in and get updated on all the action from Formula One http://content.msn.co.in/Sports/FormulaOne/Default __ OpenSSL Project http://www.openssl.org Development Mailing List openssl-dev@openssl.org Automated List Manager [EMAIL PROTECTED]
RE: Memory Leaks in SSL_Library_init()
Is it required to call SSL_library_init() if I only want to use some crypto functionalities? All SSL_library_init does is add ciphers and digests to the EVP table. If you don't need any ciphers and digests accessible through the EVP interface or you add those ciphers and digests yourself, you do not need to call SSL_library_init. Look at the ssl/ssl_algs.c file. DS __ OpenSSL Project http://www.openssl.org Development Mailing List openssl-dev@openssl.org Automated List Manager [EMAIL PROTECTED]
Re: Memory Leaks in SSL_Library_init()
Hi! for using valgrind this way of configuring is OK ./config --prefix=/home/user -DPURIFY or this is required ./config --prefix=/home/user -DPURIFY -ggdb regards -Nitin From: Darryl Miles [EMAIL PROTECTED] Reply-To: openssl-dev@openssl.org To: openssl-dev@openssl.org Subject: Re: Memory Leaks in SSL_Library_init() Date: Wed, 21 Mar 2007 11:12:38 + Nitin M wrote: Does this mean that in such scenario the application need not call SSL_library_init since it does not need those extra initialisations and can achieve only the required initialisations with specific calls? If this is true I have two more questions here, 1. In what scenario then an application should use SSL_library_init()? I think not for the simplest use of SSL/Crypto functionality in the application. 2. If SSL_library_init() should be called only once in the lifetime of a process and it is is creating some global data structures only once within that process, why cant we have a function to clear all these global variables which can be called at the the process termination time? If you are using SSL then you are touching many parts of the OpenSSL library as all the digests and crypto algorithms are pulled in to allow them to be available for selection and negotiation. Most users want to simple few function calls to auto-register all those methods. The simplest use of OpenSSL is more inline with how I explained, an app just wants to use a single digest algorithm (like MD5 or SHA1, but not SSL). As I still maintain you need to provide information about the leak(s) in order to progress. Application usages goes like this: * Application initializes the OpenSSL libary. * Application creates and uses data object with the OpenSSL library. * Application wants to shutdown. * Application destroys all OpenSSL data objects it is holding. * Application calls cleanup routines in the OpenSSL library. This is pretty standard stuff, what David maybe highlighting is that many programmers or a short-lived applications running on a process segmented operating system might skip the last 2 points to save time/money. Darryl __ OpenSSL Project http://www.openssl.org Development Mailing List openssl-dev@openssl.org Automated List Manager [EMAIL PROTECTED] _ Post ads for free. On new MSN Classifieds. http://www.yello.in/home.php?utm_source=hotmailtagutm_medium=textlinkutm_content=inutm_campaign=intro __ OpenSSL Project http://www.openssl.org Development Mailing List openssl-dev@openssl.org Automated List Manager [EMAIL PROTECTED]
Re: Memory Leaks in SSL_Library_init()
David Schwartz wrote: The function SSL_library_init() is observed to be introudcing memory leak in the application code. There is still some amount of memory leak left even after the series of cleanup calls suggested in the openssl FAQ. Your first sentence is pretty funny though. How can a function you can only call once in the lifetime of a process introduce a memory leak? DS Once you figure that out, it won't be so funny. ;) You're making assumptions about the OS that aren't always true. b.c. __ OpenSSL Project http://www.openssl.org Development Mailing List openssl-dev@openssl.org Automated List Manager [EMAIL PROTECTED]
Re: Memory Leaks in SSL_Library_init()
David Schwartz wrote:
The function SSL_library_init() is observed to be introudcing
memory leak in
the application code. There is still some amount of memory leak left even
after the series of cleanup calls suggested in the openssl FAQ.
Can someone help understand that technically what is the problem
in having a
single cleanup call like SSL_library_cleanup, instead of a using
a series of
cleanup calls?
Because there are multiple layers in use at the same time. Think of
OpenSSL more as a collection of cryptographic functions bundled together
rather than a single entity.
OpenSSL is able to load dynamically at runtime additional functionality
and allow the application to provide functionality to hook in. For
example I could expose a bespoke homebrew digest or encryption algorithm
in such a way that it could be negotiated via SSL.
At the other end of the scale there are users who simply want to use
OpenSSL to provide just a single digest algorithm, and consequentially
don't need all the extra initialization for SSL support.
Out of interest where are those memory leaks that you find ?
As for the series of cleanup calls, there are several reasons. The two main
ones are that you might need to cleanup different things and some cleanup
functions need to be called from a particular context.
I'm not heard the argument that doing correct cleanup introduces extra
code complexity in relation to OpenSSL. David seems to be providing
generic reasons why this leak might exist as opposed to the reasons for
this specific leak you have found.
Sure I can think up many reasons why this leak exists, at the top of my
list would be, maybe there is a genuine bug here that the developers
dont know about it yet.
So I'd like to continue on the path to proving or disproving that by
asking the O/P'er to specify what exactly he has found that is being
leaked. valgrind can be a useful in this regard, but dont forget to
compile OpenSSL with -DPURIFY or disable uninitialized data checking.
Despite what the man page / FAQ suggests you should do, would it be
possible to try the following cleanup sequence and confirm if this
changes the situation with your leaks:
/* thread-local cleanup */
ERR_remove_state(0);
/* Globals we finished with */
my_application_function_to_destroy_OpenSSL_objects_attached_to_local_variables();
// In the particular application I am keeping an RSA
// object around.
// if(tmp_rsa != NULL) {
// RSA_free(tmp_rsa);
// tmp_rsa = NULL;
// }
/* thread-safe cleanup */
ENGINE_cleanup();
CONF_modules_unload(1);
/* global application exit cleanup (after all SSL activity is shutdown) */
ERR_free_strings();
EVP_cleanup();
CRYPTO_cleanup_all_ex_data();
Last time I run my application through valgrind I did not have any leaks
with my OpenSSL usage.
Darryl
__
OpenSSL Project http://www.openssl.org
Development Mailing List openssl-dev@openssl.org
Automated List Manager [EMAIL PROTECTED]
RE: Memory Leaks in SSL_Library_init()
From: David Schwartz [EMAIL PROTECTED] Reply-To: openssl-dev@openssl.org To: openssl-dev@openssl.org Subject: RE: Memory Leaks in SSL_Library_init() Date: Tue, 20 Mar 2007 05:02:01 -0700 The function SSL_library_init() is observed to be introudcing memory leak in the application code. There is still some amount of memory leak left even after the series of cleanup calls suggested in the openssl FAQ. Can someone help understand that technically what is the problem in having a single cleanup call like SSL_library_cleanup, instead of a using a series of cleanup calls? It's pretty much the same issue as every other library in the world. Perfect cleanup is extremely difficult and often adds performance overhead during use. Keepin it apart from the memory leak, i would like to know by example how a perfect cleanup can casue performance problems? Since nobody really cares whether you can free every single byte before you terminate a process, nobody bothers to code it. Not just for a single byte, but in embedded systems and in deviced with resource contraints woudn't we care about even a small amount of memory not being freed after it is no more required? For example, singletons can be very difficult to remove. How and when do you remove a memory manager? As for the series of cleanup calls, there are several reasons. The two main ones are that you might need to cleanup different things and some cleanup functions need to be called from a particular context. Your first sentence is pretty funny though. How can a function you can only call once in the lifetime of a process introduce a memory leak? DS __ OpenSSL Project http://www.openssl.org Development Mailing List openssl-dev@openssl.org Automated List Manager [EMAIL PROTECTED] _ Voice your questions and our experts will answer them http://content.msn.co.in/Lifestyle/AskExpert/Default01.htm __ OpenSSL Project http://www.openssl.org Development Mailing List openssl-dev@openssl.org Automated List Manager [EMAIL PROTECTED]
RE: Memory Leaks in SSL_Library_init()
Keepin it apart from the memory leak, i would like to know by example how a perfect cleanup can casue performance problems? One common case goes like this: 1) You have an object you create very early in the library initialization. 2) The object is accessed a lot, and having to check if it's available or initialized would be tricky to do every place it might be invoked. 3) So long as you never cleanup the object, you know it must exist in any code that library could call, since the first thing the library does is create it. 4) If you destroy it in cleanup, any other cleanup code that runs after that has to deal with the fact that the object doesn't exist. This could in some cases be most of the library. 5) You could destroy it last, but you can't destroy everything last. Consider a linked list of algorithms with sentinels to save pointer compares to NULL. For perfect cleanup, you have to get rid of those dummy objects. But that means any code that runs after that cleanup will blow up if it accesses the list (or all code that accesses the list will have to make sure it hasn't been cleaned up, defeating the whole point of the sentinels). So you have a new ordering rule that affects any code called from cleanup code that might go after those objects are cleaned up -- it most not ever touch that list, and no code it calls can ever touch the list. If you get too many of these rules, it's easier to change to a less-efficient list algorithm that doesn't require sentinels. Again, you can't put everything last. And anything that goes after something else cannot touch what has already been cleaned up unless you add code to make that work at the point where you touch the thing that might be cleaned up. And that point is usually *not* the cleanup code. The perfect solution involves a lot of very complex dependency work to make absolutely sure that nothing can be used after it's cleaned up. What if the cleanup code for A might need B, and the cleanup code for B might need C, and the cleanup code for C might need A? Do you un-cleanup A? Then when do you clean it back up again? Might you never terminate? There are many examples. Engineering a library for perfect cleanup is a *major* design requirement and if you insist on it, things will have to give in other areas. Since nobody really cares whether you can free every single byte before you terminate a process, nobody bothers to code it. Not just for a single byte, but in embedded systems and in deviced with resource contraints woudn't we care about even a small amount of memory not being freed after it is no more required? Sure and in those special cases, you have totally different design requirements and thus totally different designs. Unless you have embedded systems that have enough resources that they can run commodity code, in which case you don't care -- that's why you put the resources there. If you want to do the most with the very least, you have a very special case that bears no resemblance to typical desktop/server code (and there exist SSL/crypto libraries for these unusual cases). If you want to use mainstream code, you have to make the mainstream tradeoffs. Otherwise, you were wrong to want to use mainstream code. One of the reasons people put lots of CPU and lots of memory in modern embedded systems is because it's cheap and allows you to use more mainstream engineering tradeoffs. This is one of those tradeoffs. Welcome to the real world. DS __ OpenSSL Project http://www.openssl.org Development Mailing List openssl-dev@openssl.org Automated List Manager [EMAIL PROTECTED]
Re: Memory Leaks in SSL_Library_init()
Hi! All,
Thanks very much for your inouts on this.
From: Darryl Miles [EMAIL PROTECTED]
Reply-To: openssl-dev@openssl.org
To: openssl-dev@openssl.org
Subject: Re: Memory Leaks in SSL_Library_init()
Date: Tue, 20 Mar 2007 17:18:40 +
David Schwartz wrote:
The function SSL_library_init() is observed to be introudcing
memory leak in
the application code. There is still some amount of memory leak left even
after the series of cleanup calls suggested in the openssl FAQ.
Can someone help understand that technically what is the problem
in having a
single cleanup call like SSL_library_cleanup, instead of a using
a series of
cleanup calls?
Because there are multiple layers in use at the same time. Think of
OpenSSL more as a collection of cryptographic functions bundled together
rather than a single entity.
OpenSSL is able to load dynamically at runtime additional functionality and
allow the application to provide functionality to hook in. For example I
could expose a bespoke homebrew digest or encryption algorithm in such a
way that it could be negotiated via SSL.
At the other end of the scale there are users who simply want to use
OpenSSL to provide just a single digest algorithm, and consequentially
don't need all the extra initialization for SSL support.
Does this mean that in such scenario the application need not call
SSL_library_init since it does not need those extra initialisations and can
achieve only the required initialisations with specific calls?
If this is true I have two more questions here,
1. In what scenario then an application should use SSL_library_init()? I
think not for the simplest use of SSL/Crypto functionality in the
application.
2. If SSL_library_init() should be called only once in the lifetime of a
process and it is is creating some global data structures only once within
that process, why cant we have a function to clear all these global
variables which can be called at the the process termination time?
Out of interest where are those memory leaks that you find ?
As for the series of cleanup calls, there are several reasons. The two
main
ones are that you might need to cleanup different things and some cleanup
functions need to be called from a particular context.
I'm not heard the argument that doing correct cleanup introduces extra code
complexity in relation to OpenSSL. David seems to be providing generic
reasons why this leak might exist as opposed to the reasons for this
specific leak you have found.
Sure I can think up many reasons why this leak exists, at the top of my
list would be, maybe there is a genuine bug here that the developers dont
know about it yet.
So I'd like to continue on the path to proving or disproving that by asking
the O/P'er to specify what exactly he has found that is being leaked.
valgrind can be a useful in this regard, but dont forget to compile OpenSSL
with -DPURIFY or disable uninitialized data checking.
Despite what the man page / FAQ suggests you should do, would it be
possible to try the following cleanup sequence and confirm if this changes
the situation with your leaks:
/* thread-local cleanup */
ERR_remove_state(0);
/* Globals we finished with */
my_application_function_to_destroy_OpenSSL_objects_attached_to_local_variables();
// In the particular application I am keeping an RSA
// object around.
// if(tmp_rsa != NULL) {
// RSA_free(tmp_rsa);
// tmp_rsa = NULL;
// }
/* thread-safe cleanup */
ENGINE_cleanup();
CONF_modules_unload(1);
/* global application exit cleanup (after all SSL activity is shutdown) */
ERR_free_strings();
EVP_cleanup();
CRYPTO_cleanup_all_ex_data();
Last time I run my application through valgrind I did not have any leaks
with my OpenSSL usage.
Darryl
__
OpenSSL Project http://www.openssl.org
Development Mailing List openssl-dev@openssl.org
Automated List Manager [EMAIL PROTECTED]
regards
-Nitin
_
Watch free concerts with Pink, Rod Stewart, Oasis and more. Visit MSN
Presents today.
http://music.msn.com/presents?icid=ncmsnpresentstaglineocid=T002MSN03A07001
__
OpenSSL Project http://www.openssl.org
Development Mailing List openssl-dev@openssl.org
Automated List Manager [EMAIL PROTECTED]
RE: Memory Leaks in SSL_Library_init()
Hi! I have an example case where by the unused memoy allocated by SSL_library_init when not freed, would accumulate. There is an application which takes services from some of the libraries say A, B and C. These libraries are dynamically loaded and unloaded into the application as and when required depending on certain conditions and using g_module_ calls. All of these plugin libraries call SSL_library_init() and since we do not know how many times in the lifespan of the application these libraries would be loaded/unloaded, we dont know how many times SSL_library_init() would get called. regards -Nitin From: David Schwartz [EMAIL PROTECTED] Reply-To: openssl-dev@openssl.org To: openssl-dev@openssl.org Subject: RE: Memory Leaks in SSL_Library_init() Date: Tue, 20 Mar 2007 05:02:01 -0700 The function SSL_library_init() is observed to be introudcing memory leak in the application code. There is still some amount of memory leak left even after the series of cleanup calls suggested in the openssl FAQ. Can someone help understand that technically what is the problem in having a single cleanup call like SSL_library_cleanup, instead of a using a series of cleanup calls? It's pretty much the same issue as every other library in the world. Perfect cleanup is extremely difficult and often adds performance overhead during use. Since nobody really cares whether you can free every single byte before you terminate a process, nobody bothers to code it. For example, singletons can be very difficult to remove. How and when do you remove a memory manager? As for the series of cleanup calls, there are several reasons. The two main ones are that you might need to cleanup different things and some cleanup functions need to be called from a particular context. Your first sentence is pretty funny though. How can a function you can only call once in the lifetime of a process introduce a memory leak? DS __ OpenSSL Project http://www.openssl.org Development Mailing List openssl-dev@openssl.org Automated List Manager [EMAIL PROTECTED] _ Spice up your IM conversations. New, colorful and animated emoticons. Get chatting! http://server1.msn.co.in/SP05/emoticons/ __ OpenSSL Project http://www.openssl.org Development Mailing List openssl-dev@openssl.org Automated List Manager [EMAIL PROTECTED]
Memory Leaks in SSL_Library_init()
Hi All! The function SSL_library_init() is observed to be introudcing memory leak in the application code. There is still some amount of memory leak left even after the series of cleanup calls suggested in the openssl FAQ. Can someone help understand that technically what is the problem in having a single cleanup call like SSL_library_cleanup, instead of a using a series of cleanup calls? regards -Nitin _ Spice up your IM conversations. New, colorful and animated emoticons. Get chatting! http://server1.msn.co.in/SP05/emoticons/ __ OpenSSL Project http://www.openssl.org Development Mailing List openssl-dev@openssl.org Automated List Manager [EMAIL PROTECTED]
[openssl.org #1462] [PATCH] fix some memory leaks in the pkcs7 crypto
committed (with minor modifications). Please test a recent snapshot. Thanks, Nils __ OpenSSL Project http://www.openssl.org Development Mailing List openssl-dev@openssl.org Automated List Manager [EMAIL PROTECTED]
[openssl.org #1462] [PATCH] fix some memory leaks in the pkcs7 crypto
Below is a patch against openssl-0.9.8d.tar.gz to fix some memory leaks in the
pkcs7 functions when errors occur.
Since I am in the US, I have already submitted the patch to the US Bureau of
Industry
and Security and the Encryption Request Coordinator.
Thanks in advance,
Charles
diff -Naur openssl-0.9.8d.orig/crypto/pkcs7/pk7_doit.c
openssl-0.9.8d/crypto/pkcs7/pk7_doit.c
--- openssl-0.9.8d.orig/crypto/pkcs7/pk7_doit.c 2005-08-04 15:10:05.0
-0700
+++ openssl-0.9.8d/crypto/pkcs7/pk7_doit.c 2007-01-24 18:44:16.0
-0800
@@ -217,7 +217,9 @@
keylen=EVP_CIPHER_key_length(evp_cipher);
ivlen=EVP_CIPHER_iv_length(evp_cipher);
xalg-algorithm = OBJ_nid2obj(EVP_CIPHER_type(evp_cipher));
- if (ivlen 0) RAND_pseudo_bytes(iv,ivlen);
+ if (ivlen 0)
+ if (RAND_pseudo_bytes(iv,ivlen) = 0)
+ goto err;
if (EVP_CipherInit_ex(ctx, evp_cipher, NULL, NULL, NULL, 1)=0)
goto err;
if (EVP_CIPHER_CTX_rand_key(ctx, key) = 0)
@@ -226,10 +228,13 @@
goto err;
if (ivlen 0) {
- if (xalg-parameter == NULL)
- xalg-parameter=ASN1_TYPE_new();
+ if (xalg-parameter == NULL) {
+ xalg-parameter=ASN1_TYPE_new();
+ if (xalg-parameter == NULL)
+ goto err;
+ }
if(EVP_CIPHER_param_to_asn1(ctx, xalg-parameter) 0)
- goto err;
+ goto err;
}
/* Lets do the pub key stuff :-) */
@@ -242,7 +247,8 @@
PKCS7err(PKCS7_F_PKCS7_DATAINIT,PKCS7_R_MISSING_CERIPEND_INFO);
goto err;
}
- pkey=X509_get_pubkey(ri-cert);
+ if ((pkey=X509_get_pubkey(ri-cert)) == NULL)
+ goto err;
jj=EVP_PKEY_size(pkey);
EVP_PKEY_free(pkey);
if (max jj) max=jj;
@@ -255,7 +261,8 @@
for (i=0; isk_PKCS7_RECIP_INFO_num(rsk); i++)
{
ri=sk_PKCS7_RECIP_INFO_value(rsk,i);
- pkey=X509_get_pubkey(ri-cert);
+ if ((pkey=X509_get_pubkey(ri-cert)) == NULL)
+ goto err;
jj=EVP_PKEY_encrypt(tmp,key,keylen,pkey);
EVP_PKEY_free(pkey);
if (jj = 0)
@@ -291,6 +298,8 @@
if(bio == NULL)
{
bio=BIO_new(BIO_s_mem());
+ if (bio == NULL)
+ goto err;
BIO_set_mem_eof_return(bio,0);
}
}
@@ -541,6 +550,8 @@
bio=BIO_new(BIO_s_mem());
BIO_set_mem_eof_return(bio,0);
}
+ if (bio == NULL)
+ goto err;
#endif
}
BIO_push(out,bio);
@@ -695,9 +706,13 @@
ERR_R_MALLOC_FAILURE);
goto err;
}
- PKCS7_add_signed_attribute(si,
+ if (!PKCS7_add_signed_attribute(si,
NID_pkcs9_signingTime,
- V_ASN1_UTCTIME,sign_time);
+ V_ASN1_UTCTIME,sign_time))
+ {
+ M_ASN1_UTCTIME_free(sign_time);
+ goto err;
+ }
}
/* Add digest */
@@ -714,11 +729,16 @@
{
PKCS7err(PKCS7_F_PKCS7_DATAFINAL,
ERR_R_MALLOC_FAILURE);
+ M_ASN1_OCTET_STRING_free(digest);
goto err;
}
- PKCS7_add_signed_attribute(si,
+ if (!PKCS7_add_signed_attribute(si,
NID_pkcs9_messageDigest,
- V_ASN1_OCTET_STRING,digest
Re: [openssl.org #1070] PATCH: fix for memory leaks in smime verification
On Mon, May 16, 2005 at 06:30:26PM +0200, Riaz Rahaman via RT wrote: I don't see any patch attached? The message came to the openssl-dev mailing list via the OpenSSL RT2 request tracker. Attachments are not forwarded on this path, but can be viewed on the web -- in this case: https://www.aet.tu-cottbus.de/rt2/Ticket/Display.html?id=1070 (Log in with name guest and password guest.) __ OpenSSL Project http://www.openssl.org Development Mailing List openssl-dev@openssl.org Automated List Manager [EMAIL PROTECTED]
