From eb2b9892065cf5b69400b98ca82e4e99a525eb29 Mon Sep 17 00:00:00 2001 From: Bernd Edlinger Date: Fri, 20 Apr 2018 15:45:06 +0200 Subject: [PATCH] Ensure the thread keys are always allocated in the same order Fixes: #5899 Reviewed-by: Rich Salz Reviewed-by: Richard Levitte (Merged from https://github.com/openssl/openssl/pull/5911) --- crypto/bio/b_addr.c | 3 +- crypto/engine/eng_lib.c | 3 +- crypto/err/err.c | 49 +++++++++++++++++++--- crypto/ex_data.c | 3 +- crypto/include/internal/cryptlib_int.h | 1 + crypto/include/internal/err_int.h | 2 + crypto/init.c | 57 +++++++++++++++++++------- doc/man3/OPENSSL_init_crypto.pod | 6 +-- include/openssl/crypto.h | 3 +- 9 files changed, 100 insertions(+), 27 deletions(-) diff --git a/crypto/bio/b_addr.c b/crypto/bio/b_addr.c index 9832c0c89c..abec7bb8db 100644 --- a/crypto/bio/b_addr.c +++ b/crypto/bio/b_addr.c @@ -603,7 +603,8 @@ static int addrinfo_wrap(int family, int socktype, DEFINE_RUN_ONCE_STATIC(do_bio_lookup_init) { - OPENSSL_init_crypto(0, NULL); + if (!OPENSSL_init_crypto(0, NULL)) + return 0; bio_lookup_lock = CRYPTO_THREAD_lock_new(); return bio_lookup_lock != NULL; } diff --git a/crypto/engine/eng_lib.c b/crypto/engine/eng_lib.c index f5031d3eb7..95736eda6a 100644 --- a/crypto/engine/eng_lib.c +++ b/crypto/engine/eng_lib.c @@ -20,7 +20,8 @@ CRYPTO_ONCE engine_lock_init = CRYPTO_ONCE_STATIC_INIT; DEFINE_RUN_ONCE(do_engine_lock_init) { - OPENSSL_init_crypto(0, NULL); + if (!OPENSSL_init_crypto(0, NULL)) + return 0; global_engine_lock = CRYPTO_THREAD_lock_new(); return global_engine_lock != NULL; } diff --git a/crypto/err/err.c b/crypto/err/err.c index f55655c6b6..03cbd738e1 100644 --- a/crypto/err/err.c +++ b/crypto/err/err.c @@ -265,11 +265,19 @@ static void ERR_STATE_free(ERR_STATE *s) DEFINE_RUN_ONCE_STATIC(do_err_strings_init) { - OPENSSL_init_crypto(0, NULL); + if (!OPENSSL_init_crypto(0, NULL)) + return 0; err_string_lock = CRYPTO_THREAD_lock_new(); + if (err_string_lock == NULL) + return 0; int_error_hash = lh_ERR_STRING_DATA_new(err_string_data_hash, err_string_data_cmp); - return err_string_lock != NULL && int_error_hash != NULL; + if (int_error_hash == NULL) { + CRYPTO_THREAD_lock_free(err_string_lock); + err_string_lock = NULL; + return 0; + } + return 1; } void err_cleanup(void) @@ -662,7 +670,10 @@ DEFINE_RUN_ONCE_STATIC(err_do_init) ERR_STATE *ERR_get_state(void) { - ERR_STATE *state = NULL; + ERR_STATE *state; + + if (!OPENSSL_init_crypto(OPENSSL_INIT_BASE_ONLY, NULL)) + return NULL; if (!RUN_ONCE(&err_init, err_do_init)) return NULL; @@ -694,13 +705,41 @@ ERR_STATE *ERR_get_state(void) return state; } +/* + * err_shelve_state returns the current thread local error state + * and freezes the error module until err_unshelve_state is called. + */ +int err_shelve_state(void **state) +{ + if (!OPENSSL_init_crypto(OPENSSL_INIT_BASE_ONLY, NULL)) + return 0; + + if (!RUN_ONCE(&err_init, err_do_init)) + return 0; + + *state = CRYPTO_THREAD_get_local(&err_thread_local); + if (!CRYPTO_THREAD_set_local(&err_thread_local, (ERR_STATE*)-1)) + return 0; + + return 1; +} + +/* + * err_unshelve_state restores the error state that was returned + * by err_shelve_state previously. + */ +void err_unshelve_state(void* state) +{ + if (state != (void*)-1) + CRYPTO_THREAD_set_local(&err_thread_local, (ERR_STATE*)state); +} + int ERR_get_next_error_library(void) { int ret; - if (!RUN_ONCE(&err_string_init, do_err_strings_init)) { + if (!RUN_ONCE(&err_string_init, do_err_strings_init)) return 0; - } CRYPTO_THREAD_write_lock(err_string_lock); ret = int_err_library_number++; diff --git a/crypto/ex_data.c b/crypto/ex_data.c index ed9820cfad..08dc7c4073 100644 --- a/crypto/ex_data.c +++ b/crypto/ex_data.c @@ -37,7 +37,8 @@ static CRYPTO_ONCE ex_data_init = CRYPTO_ONCE_STATIC_INIT; DEFINE_RUN_ONCE_STATIC(do_ex_data_init) { - OPENSSL_init_crypto(0, NULL); + if (!OPENSSL_init_crypto(0, NULL)) + return 0; ex_data_lock = CRYPTO_THREAD_lock_new(); return ex_data_lock != NULL; } diff --git a/crypto/include/internal/cryptlib_int.h b/crypto/include/internal/cryptlib_int.h index 8f5650fe55..38b5dac9a3 100644 --- a/crypto/include/internal/cryptlib_int.h +++ b/crypto/include/internal/cryptlib_int.h @@ -25,6 +25,7 @@ int ossl_init_thread_start(uint64_t opts); * use". */ # define OPENSSL_INIT_ZLIB 0x00010000L +# define OPENSSL_INIT_BASE_ONLY 0x00040000L /* OPENSSL_INIT_THREAD flags */ # define OPENSSL_INIT_THREAD_ASYNC 0x01 diff --git a/crypto/include/internal/err_int.h b/crypto/include/internal/err_int.h index 7fec3ed767..4a7e43abcd 100644 --- a/crypto/include/internal/err_int.h +++ b/crypto/include/internal/err_int.h @@ -13,5 +13,7 @@ int err_load_crypto_strings_int(void); void err_cleanup(void); void err_delete_thread_state(void); +int err_shelve_state(void **); +void err_unshelve_state(void *); #endif diff --git a/crypto/init.c b/crypto/init.c index c79c32c17a..fb7e3ace87 100644 --- a/crypto/init.c +++ b/crypto/init.c @@ -81,22 +81,36 @@ DEFINE_RUN_ONCE_STATIC(ossl_init_base) * We use a dummy thread local key here. We use the destructor to detect * when the thread is going to stop (where that feature is available) */ - CRYPTO_THREAD_init_local(&threadstopkey, ossl_init_thread_stop_wrap); + if (!CRYPTO_THREAD_init_local(&threadstopkey, ossl_init_thread_stop_wrap)) + return 0; + if ((init_lock = CRYPTO_THREAD_lock_new()) == NULL) + goto err; #ifndef OPENSSL_SYS_UEFI - atexit(OPENSSL_cleanup); + if (atexit(OPENSSL_cleanup) != 0) + goto err; #endif - if ((init_lock = CRYPTO_THREAD_lock_new()) == NULL) - return 0; OPENSSL_cpuid_setup(); - /* - * BIG FAT WARNING! - * Everything needed to be initialized in this function before threads - * come along MUST happen before base_inited is set to 1, or we will - * see race conditions. - */ base_inited = 1; + return 1; +err: +#ifdef OPENSSL_INIT_DEBUG + fprintf(stderr, "OPENSSL_INIT: ossl_init_base not ok!\n"); +#endif + CRYPTO_THREAD_lock_free(init_lock); + init_lock = NULL; + + CRYPTO_THREAD_cleanup_local(&threadstopkey); + return 0; +} + +static CRYPTO_ONCE load_crypto_nodelete = CRYPTO_ONCE_STATIC_INIT; +DEFINE_RUN_ONCE_STATIC(ossl_init_load_crypto_nodelete) +{ +#ifdef OPENSSL_INIT_DEBUG + fprintf(stderr, "OPENSSL_INIT: ossl_init_load_crypto_nodelete()\n"); +#endif #if !defined(OPENSSL_NO_DSO) && !defined(OPENSSL_USE_NODELETE) # ifdef DSO_WIN32 { @@ -108,6 +122,10 @@ DEFINE_RUN_ONCE_STATIC(ossl_init_base) | GET_MODULE_HANDLE_EX_FLAG_PIN, (void *)&base_inited, &handle); +# ifdef OPENSSL_INIT_DEBUG + fprintf(stderr, "OPENSSL_INIT: obtained DSO reference? %s\n", + (ret == TRUE ? "No!" : "Yes.")); +# endif return (ret == TRUE) ? 1 : 0; } # else @@ -116,9 +134,12 @@ DEFINE_RUN_ONCE_STATIC(ossl_init_base) * to remain loaded until the atexit() handler is run at process exit. */ { - DSO *dso = NULL; + DSO *dso; + void *err; + + if (!err_shelve_state(&err)) + return 0; - ERR_set_mark(); dso = DSO_dsobyaddr(&base_inited, DSO_FLAG_NO_UNLOAD_ON_FREE); # ifdef OPENSSL_INIT_DEBUG fprintf(stderr, "OPENSSL_INIT: obtained DSO reference? %s\n", @@ -130,7 +151,7 @@ DEFINE_RUN_ONCE_STATIC(ossl_init_base) */ # endif DSO_free(dso); - ERR_pop_to_mark(); + err_unshelve_state(err); } # endif #endif @@ -541,11 +562,17 @@ void OPENSSL_cleanup(void) int OPENSSL_init_crypto(uint64_t opts, const OPENSSL_INIT_SETTINGS *settings) { if (stopped) { - CRYPTOerr(CRYPTO_F_OPENSSL_INIT_CRYPTO, ERR_R_INIT_FAIL); + if (!(opts & OPENSSL_INIT_BASE_ONLY)) + CRYPTOerr(CRYPTO_F_OPENSSL_INIT_CRYPTO, ERR_R_INIT_FAIL); return 0; } - if (!base_inited && !RUN_ONCE(&base, ossl_init_base)) + if (!RUN_ONCE(&base, ossl_init_base)) + return 0; + + if (!(opts & OPENSSL_INIT_BASE_ONLY) + && !RUN_ONCE(&load_crypto_nodelete, + ossl_init_load_crypto_nodelete)) return 0; if ((opts & OPENSSL_INIT_NO_LOAD_CRYPTO_STRINGS) diff --git a/doc/man3/OPENSSL_init_crypto.pod b/doc/man3/OPENSSL_init_crypto.pod index 606885b9e4..a259539f05 100644 --- a/doc/man3/OPENSSL_init_crypto.pod +++ b/doc/man3/OPENSSL_init_crypto.pod @@ -141,15 +141,15 @@ CAPI engine (if available). This not a default option. With this option the library will automatically load and initialise the padlock engine (if available). This not a default option. -=item OPENSSL_INIT_ENGINE_DASYNC +=item OPENSSL_INIT_ENGINE_AFALG With this option the library will automatically load and initialise the -DASYNC engine. This not a default option. +AFALG engine. This not a default option. =item OPENSSL_INIT_ENGINE_ALL_BUILTIN With this option the library will automatically load and initialise all the -built in engines listed above with the exception of the openssl and dasync +built in engines listed above with the exception of the openssl and afalg engines. This not a default option. =item OPENSSL_INIT_ATFORK diff --git a/include/openssl/crypto.h b/include/openssl/crypto.h index 27119ffa7b..c2ad65e044 100644 --- a/include/openssl/crypto.h +++ b/include/openssl/crypto.h @@ -376,8 +376,9 @@ int CRYPTO_memcmp(const void * in_a, const void * in_b, size_t len); # define OPENSSL_INIT_ENGINE_CAPI 0x00002000L # define OPENSSL_INIT_ENGINE_PADLOCK 0x00004000L # define OPENSSL_INIT_ENGINE_AFALG 0x00008000L -# define OPENSSL_INIT_reserved_internal 0x00010000L +/* OPENSSL_INIT_ZLIB 0x00010000L */ # define OPENSSL_INIT_ATFORK 0x00020000L +/* OPENSSL_INIT_BASE_ONLY 0x00040000L */ /* OPENSSL_INIT flag range 0xfff00000 reserved for OPENSSL_init_ssl() */ /* Max OPENSSL_INIT flag value is 0x80000000 */ -- 2.34.1