Fix an init-deadlock in OPENSSL_INIT_LOAD_CRYPTO_STRINGS#21683
Fix an init-deadlock in OPENSSL_INIT_LOAD_CRYPTO_STRINGS#21683bernd-edlinger wants to merge 1 commit intoopenssl:masterfrom
Conversation
|
It might be good to include the offending OPENSSL_init_crypto() call in the commit message? |
crypto/err/err.c
Outdated
| if (!OPENSSL_init_crypto(OPENSSL_INIT_LOAD_CRYPTO_STRINGS, NULL)) | ||
| return 0; | ||
|
|
There was a problem hiding this comment.
Should it be moved to ossl_err_string_int() instead?
|
Also the pyca external test failure seems relevant. Unfortunately it won't be trivial to fix I am afraid. Except it might be actually fixed by the suggestion I've made above. It is worth trying. |
|
@t8m I tried to follow your suggestion, but since |
|
No, this does not work at all, and caused: |
crypto/err/err.c
Outdated
| unsigned long l; | ||
|
|
||
| if (!RUN_ONCE(&err_string_init, do_err_strings_init)) { | ||
| if (!OPENSSL_init_crypto(OPENSSL_INIT_LOAD_CRYPTO_STRINGS, NULL) |
There was a problem hiding this comment.
Perhaps you need to also add it to ERR_reason_error_string() ?
|
The ci failure is quite strange now. |
4617789 to
bd2a20e
Compare
t8m
left a comment
There was a problem hiding this comment.
Could you please amend the commit message so it tells about the final fix and not the previous attempts? Also I do not think it needs all the backtraces but it is up to you how detailed you want it to be.
bd2a20e to
9323bd4
Compare
…STRINGS)
A recursive OPENSSL_init_crypto(OPENSSL_INIT_LOAD_CRYPTO_STRINGS) call
may happen if an out-of-memory error happens at the first callstack,
and the dead-lock happens at the second callstack, because ossl_err_get_state_int
calls OPENSSL_init_crypto(OPENSSL_INIT_LOAD_CRYPTO_STRINGS) although that
call is currently already executing.
At least on posix system this causes the process to freeze at this
point, and must be avoided whatever it takes.
The fix is using err_shelve_state around the critical region, which
makes ossl_err_get_state_int return early and not call the recursive
OPENSSL_init_crypto(OPENSSL_INIT_LOAD_CRYPTO_STRINGS).
This can be reproduced with my error injection patch.
The test vector has been validated on the master branch:
$ ERROR_INJECT=1692279870 ../util/shlib_wrap.sh ./asn1parse-test ./corpora/asn1parse/027f6e82ba01d9db9a9167b83e56cc9f2c602550
ERROR_INJECT=1692279870
#0 0x7f280b42fef8 in __sanitizer_print_stack_trace ../../../../src/libsanitizer/asan/asan_stack.cpp:86
openssl#1 0x5610a3f396b4 in my_malloc fuzz/test-corpus.c:114
openssl#2 0x7f280a2eb94c in CRYPTO_malloc crypto/mem.c:177
openssl#3 0x7f280a2dafdb in OPENSSL_LH_insert crypto/lhash/lhash.c:114
openssl#4 0x7f280a1c87fe in err_load_strings crypto/err/err.c:264
openssl#5 0x7f280a1c87fe in err_load_strings crypto/err/err.c:259
openssl#6 0x7f280a1c87fe in ERR_load_strings_const crypto/err/err.c:301
openssl#7 0x7f280a6f513b in ossl_err_load_PROV_strings providers/common/provider_err.c:233
openssl#8 0x7f280a1cf015 in ossl_err_load_crypto_strings crypto/err/err_all.c:109
openssl#9 0x7f280a2e9b8c in ossl_init_load_crypto_strings crypto/init.c:190
openssl#10 0x7f280a2e9b8c in ossl_init_load_crypto_strings_ossl_ crypto/init.c:181
openssl#11 0x7f2808cfbf67 (/lib/x86_64-linux-gnu/libc.so.6+0x99f67)
openssl#12 0x7f280a32301e in CRYPTO_THREAD_run_once crypto/threads_pthread.c:154
openssl#13 0x7f280a2ea1da in OPENSSL_init_crypto crypto/init.c:553
openssl#14 0x5610a3f38e2f in FuzzerInitialize fuzz/asn1parse.c:29
openssl#15 0x5610a3f38783 in main fuzz/test-corpus.c:194
openssl#16 0x7f2808c8bd8f (/lib/x86_64-linux-gnu/libc.so.6+0x29d8f)
openssl#17 0x7f2808c8be3f in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x29e3f)
openssl#18 0x5610a3f38d34 in _start (/home/runner/work/openssl/openssl/fuzz/asn1parse-test+0x3d34)
AddressSanitizer:DEADLYSIGNAL
=================================================================
==27629==ERROR: AddressSanitizer: ABRT on unknown address 0x03e900006e23 (pc 0x7f2808cfbef8 bp 0x7f280b36afe0 sp 0x7ffd545b2460 T0)
#0 0x7f2808cfbef8 (/lib/x86_64-linux-gnu/libc.so.6+0x99ef8)
openssl#1 0x7f280a32301e in CRYPTO_THREAD_run_once crypto/threads_pthread.c:154
openssl#2 0x7f280a2ea1da in OPENSSL_init_crypto crypto/init.c:553
openssl#3 0x7f280a1c935e in ossl_err_get_state_int crypto/err/err.c:705
openssl#4 0x7f280a1cf1f9 in ERR_new crypto/err/err_blocks.c:20
openssl#5 0x7f280a2eb9ac in CRYPTO_malloc crypto/mem.c:205
openssl#6 0x7f280a2dafdb in OPENSSL_LH_insert crypto/lhash/lhash.c:114
openssl#7 0x7f280a1c87fe in err_load_strings crypto/err/err.c:264
openssl#8 0x7f280a1c87fe in err_load_strings crypto/err/err.c:259
openssl#9 0x7f280a1c87fe in ERR_load_strings_const crypto/err/err.c:301
openssl#10 0x7f280a6f513b in ossl_err_load_PROV_strings providers/common/provider_err.c:233
openssl#11 0x7f280a1cf015 in ossl_err_load_crypto_strings crypto/err/err_all.c:109
openssl#12 0x7f280a2e9b8c in ossl_init_load_crypto_strings crypto/init.c:190
openssl#13 0x7f280a2e9b8c in ossl_init_load_crypto_strings_ossl_ crypto/init.c:181
openssl#14 0x7f2808cfbf67 (/lib/x86_64-linux-gnu/libc.so.6+0x99f67)
openssl#15 0x7f280a32301e in CRYPTO_THREAD_run_once crypto/threads_pthread.c:154
openssl#16 0x7f280a2ea1da in OPENSSL_init_crypto crypto/init.c:553
openssl#17 0x5610a3f38e2f in FuzzerInitialize fuzz/asn1parse.c:29
openssl#18 0x5610a3f38783 in main fuzz/test-corpus.c:194
openssl#19 0x7f2808c8bd8f (/lib/x86_64-linux-gnu/libc.so.6+0x29d8f)
openssl#20 0x7f2808c8be3f in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x29e3f)
openssl#21 0x5610a3f38d34 in _start (/home/runner/work/openssl/openssl/fuzz/asn1parse-test+0x3d34)
AddressSanitizer can not provide additional info.
SUMMARY: AddressSanitizer: ABRT (/lib/x86_64-linux-gnu/libc.so.6+0x99ef8)
==27629==ABORTING
9323bd4 to
7602ee3
Compare
|
I updated the commit message once more: I added this statement describing how the fix works (and removed the accidental stuff at the end): But I have still no Idea why |
|
This pull request is ready to merge |
|
Merged to master as 6b1a127. Thanks! |
…STRINGS)
A recursive OPENSSL_init_crypto(OPENSSL_INIT_LOAD_CRYPTO_STRINGS) call
may happen if an out-of-memory error happens at the first callstack,
and the dead-lock happens at the second callstack, because ossl_err_get_state_int
calls OPENSSL_init_crypto(OPENSSL_INIT_LOAD_CRYPTO_STRINGS) although that
call is currently already executing.
At least on posix system this causes the process to freeze at this
point, and must be avoided whatever it takes.
The fix is using err_shelve_state around the critical region, which
makes ossl_err_get_state_int return early and not call the recursive
OPENSSL_init_crypto(OPENSSL_INIT_LOAD_CRYPTO_STRINGS).
This can be reproduced with my error injection patch.
The test vector has been validated on the master branch:
$ ERROR_INJECT=1692279870 ../util/shlib_wrap.sh ./asn1parse-test ./corpora/asn1parse/027f6e82ba01d9db9a9167b83e56cc9f2c602550
ERROR_INJECT=1692279870
#0 0x7f280b42fef8 in __sanitizer_print_stack_trace ../../../../src/libsanitizer/asan/asan_stack.cpp:86
#1 0x5610a3f396b4 in my_malloc fuzz/test-corpus.c:114
#2 0x7f280a2eb94c in CRYPTO_malloc crypto/mem.c:177
#3 0x7f280a2dafdb in OPENSSL_LH_insert crypto/lhash/lhash.c:114
#4 0x7f280a1c87fe in err_load_strings crypto/err/err.c:264
#5 0x7f280a1c87fe in err_load_strings crypto/err/err.c:259
#6 0x7f280a1c87fe in ERR_load_strings_const crypto/err/err.c:301
#7 0x7f280a6f513b in ossl_err_load_PROV_strings providers/common/provider_err.c:233
#8 0x7f280a1cf015 in ossl_err_load_crypto_strings crypto/err/err_all.c:109
#9 0x7f280a2e9b8c in ossl_init_load_crypto_strings crypto/init.c:190
#10 0x7f280a2e9b8c in ossl_init_load_crypto_strings_ossl_ crypto/init.c:181
#11 0x7f2808cfbf67 (/lib/x86_64-linux-gnu/libc.so.6+0x99f67)
#12 0x7f280a32301e in CRYPTO_THREAD_run_once crypto/threads_pthread.c:154
#13 0x7f280a2ea1da in OPENSSL_init_crypto crypto/init.c:553
#14 0x5610a3f38e2f in FuzzerInitialize fuzz/asn1parse.c:29
#15 0x5610a3f38783 in main fuzz/test-corpus.c:194
#16 0x7f2808c8bd8f (/lib/x86_64-linux-gnu/libc.so.6+0x29d8f)
#17 0x7f2808c8be3f in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x29e3f)
#18 0x5610a3f38d34 in _start (/home/runner/work/openssl/openssl/fuzz/asn1parse-test+0x3d34)
AddressSanitizer:DEADLYSIGNAL
=================================================================
==27629==ERROR: AddressSanitizer: ABRT on unknown address 0x03e900006e23 (pc 0x7f2808cfbef8 bp 0x7f280b36afe0 sp 0x7ffd545b2460 T0)
#0 0x7f2808cfbef8 (/lib/x86_64-linux-gnu/libc.so.6+0x99ef8)
#1 0x7f280a32301e in CRYPTO_THREAD_run_once crypto/threads_pthread.c:154
#2 0x7f280a2ea1da in OPENSSL_init_crypto crypto/init.c:553
#3 0x7f280a1c935e in ossl_err_get_state_int crypto/err/err.c:705
#4 0x7f280a1cf1f9 in ERR_new crypto/err/err_blocks.c:20
#5 0x7f280a2eb9ac in CRYPTO_malloc crypto/mem.c:205
#6 0x7f280a2dafdb in OPENSSL_LH_insert crypto/lhash/lhash.c:114
#7 0x7f280a1c87fe in err_load_strings crypto/err/err.c:264
#8 0x7f280a1c87fe in err_load_strings crypto/err/err.c:259
#9 0x7f280a1c87fe in ERR_load_strings_const crypto/err/err.c:301
#10 0x7f280a6f513b in ossl_err_load_PROV_strings providers/common/provider_err.c:233
#11 0x7f280a1cf015 in ossl_err_load_crypto_strings crypto/err/err_all.c:109
#12 0x7f280a2e9b8c in ossl_init_load_crypto_strings crypto/init.c:190
#13 0x7f280a2e9b8c in ossl_init_load_crypto_strings_ossl_ crypto/init.c:181
#14 0x7f2808cfbf67 (/lib/x86_64-linux-gnu/libc.so.6+0x99f67)
#15 0x7f280a32301e in CRYPTO_THREAD_run_once crypto/threads_pthread.c:154
#16 0x7f280a2ea1da in OPENSSL_init_crypto crypto/init.c:553
#17 0x5610a3f38e2f in FuzzerInitialize fuzz/asn1parse.c:29
#18 0x5610a3f38783 in main fuzz/test-corpus.c:194
#19 0x7f2808c8bd8f (/lib/x86_64-linux-gnu/libc.so.6+0x29d8f)
#20 0x7f2808c8be3f in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x29e3f)
#21 0x5610a3f38d34 in _start (/home/runner/work/openssl/openssl/fuzz/asn1parse-test+0x3d34)
AddressSanitizer can not provide additional info.
SUMMARY: AddressSanitizer: ABRT (/lib/x86_64-linux-gnu/libc.so.6+0x99ef8)
==27629==ABORTING
Reviewed-by: Tomas Mraz <[email protected]>
Reviewed-by: Tom Cosgrove <[email protected]>
(Merged from #21683)
This can be reproduced with my error injection patch.
The test vector has been validated on the master branch:
$ ERROR_INJECT=1692279870 ../util/shlib_wrap.sh ./asn1parse-test ./corpora/asn1parse/027f6e82ba01d9db9a9167b83e56cc9f2c602550 ERROR_INJECT=1692279870
#0 0x7f280b42fef8 in _sanitizer_print_stack_trace ../../../../src/libsanitizer/asan/asan_stack.cpp:86
#1 0x5610a3f396b4 in my_malloc fuzz/test-corpus.c:114
#2 0x7f280a2eb94c in CRYPTO_malloc crypto/mem.c:177
#3 0x7f280a2dafdb in OPENSSL_LH_insert crypto/lhash/lhash.c:114
#4 0x7f280a1c87fe in err_load_strings crypto/err/err.c:264
#5 0x7f280a1c87fe in err_load_strings crypto/err/err.c:259
#6 0x7f280a1c87fe in ERR_load_strings_const crypto/err/err.c:301
#7 0x7f280a6f513b in ossl_err_load_PROV_strings providers/common/provider_err.c:233
#8 0x7f280a1cf015 in ossl_err_load_crypto_strings crypto/err/err_all.c:109
#9 0x7f280a2e9b8c in ossl_init_load_crypto_strings crypto/init.c:190
#10 0x7f280a2e9b8c in ossl_init_load_crypto_strings_ossl crypto/init.c:181
#11 0x7f2808cfbf67 (/lib/x86_64-linux-gnu/libc.so.6+0x99f67)
#12 0x7f280a32301e in CRYPTO_THREAD_run_once crypto/threads_pthread.c:154
#13 0x7f280a2ea1da in OPENSSL_init_crypto crypto/init.c:553
#14 0x5610a3f38e2f in FuzzerInitialize fuzz/asn1parse.c:29
#15 0x5610a3f38783 in main fuzz/test-corpus.c:194
#16 0x7f2808c8bd8f (/lib/x86_64-linux-gnu/libc.so.6+0x29d8f)
#17 0x7f2808c8be3f in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x29e3f)
#18 0x5610a3f38d34 in _start (/home/runner/work/openssl/openssl/fuzz/asn1parse-test+0x3d34)
AddressSanitizer:DEADLYSIGNAL
================================================================= ==27629==ERROR: AddressSanitizer: ABRT on unknown address 0x03e900006e23 (pc 0x7f2808cfbef8 bp 0x7f280b36afe0 sp 0x7ffd545b2460 T0)
#0 0x7f2808cfbef8 (/lib/x86_64-linux-gnu/libc.so.6+0x99ef8)
#1 0x7f280a32301e in CRYPTO_THREAD_run_once crypto/threads_pthread.c:154
#2 0x7f280a2ea1da in OPENSSL_init_crypto crypto/init.c:553
#3 0x7f280a1c935e in ossl_err_get_state_int crypto/err/err.c:705
#4 0x7f280a1cf1f9 in ERR_new crypto/err/err_blocks.c:20
#5 0x7f280a2eb9ac in CRYPTO_malloc crypto/mem.c:205
#6 0x7f280a2dafdb in OPENSSL_LH_insert crypto/lhash/lhash.c:114
#7 0x7f280a1c87fe in err_load_strings crypto/err/err.c:264
#8 0x7f280a1c87fe in err_load_strings crypto/err/err.c:259
#9 0x7f280a1c87fe in ERR_load_strings_const crypto/err/err.c:301
#10 0x7f280a6f513b in ossl_err_load_PROV_strings providers/common/provider_err.c:233
#11 0x7f280a1cf015 in ossl_err_load_crypto_strings crypto/err/err_all.c:109
#12 0x7f280a2e9b8c in ossl_init_load_crypto_strings crypto/init.c:190
#13 0x7f280a2e9b8c in ossl_init_load_crypto_strings_ossl_ crypto/init.c:181
#14 0x7f2808cfbf67 (/lib/x86_64-linux-gnu/libc.so.6+0x99f67)
#15 0x7f280a32301e in CRYPTO_THREAD_run_once crypto/threads_pthread.c:154
#16 0x7f280a2ea1da in OPENSSL_init_crypto crypto/init.c:553
#17 0x5610a3f38e2f in FuzzerInitialize fuzz/asn1parse.c:29
#18 0x5610a3f38783 in main fuzz/test-corpus.c:194
#19 0x7f2808c8bd8f (/lib/x86_64-linux-gnu/libc.so.6+0x29d8f)
#20 0x7f2808c8be3f in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x29e3f)
#21 0x5610a3f38d34 in _start (/home/runner/work/openssl/openssl/fuzz/asn1parse-test+0x3d34)
AddressSanitizer can not provide additional info.
SUMMARY: AddressSanitizer: ABRT (/lib/x86_64-linux-gnu/libc.so.6+0x99ef8) ==27629==ABORTING
Checklist