The branch master has been updated via cd3f8c1b11b0b9f4163bc8c62cbae38aec1b4030 (commit) from f62846b703d163265176fe960ec7d087b4c3fa96 (commit)
- Log ----------------------------------------------------------------- commit cd3f8c1b11b0b9f4163bc8c62cbae38aec1b4030 Author: Rich Salz <rs...@akamai.com> Date: Thu Feb 18 15:31:56 2021 -0500 Always check CRYPTO_LOCK_{read,write}_lock Some functions that lock things are void, so we just return early. Also make ossl_namemap_empty return 0 on error. Updated the docs, and added some code to ossl_namemap_stored() to handle the failure, and updated the tests to allow for failure. Fixes: #14230 Reviewed-by: Shane Lontis <shane.lon...@oracle.com> Reviewed-by: Paul Dale <pa...@openssl.org> (Merged from https://github.com/openssl/openssl/pull/14238) ----------------------------------------------------------------------- Summary of changes: crypto/bio/b_addr.c | 5 ++++- crypto/bn/bn_mont.c | 9 ++++++-- crypto/context.c | 37 ++++++++++++++++++++++++--------- crypto/core_namemap.c | 33 ++++++++++++++++++++++------- crypto/engine/eng_ctrl.c | 3 ++- crypto/engine/eng_dyn.c | 15 ++++++++------ crypto/engine/eng_init.c | 9 +++++--- crypto/engine/eng_list.c | 25 ++++++++++++++-------- crypto/engine/eng_pkey.c | 25 ++++++++++++---------- crypto/engine/eng_table.c | 14 +++++++++---- crypto/engine/tb_asnmth.c | 3 ++- crypto/err/err.c | 12 +++++++---- crypto/evp/keymgmt_lib.c | 8 +++++-- crypto/ex_data.c | 6 ++++-- crypto/init.c | 3 ++- crypto/initthread.c | 14 ++++++++----- crypto/mem_sec.c | 15 +++++++++----- crypto/objects/o_names.c | 20 +++++++++++------- crypto/provider_core.c | 38 +++++++++++++++++++++++----------- crypto/rand/rand_lib.c | 12 ++++++++--- crypto/rsa/rsa_ossl.c | 3 ++- crypto/store/store_register.c | 9 +++++--- crypto/threads_none.c | 4 ++-- crypto/threads_pthread.c | 4 ++-- crypto/threads_win.c | 4 ++-- crypto/trace.c | 3 ++- crypto/ui/ui_openssl.c | 3 ++- crypto/x509/by_dir.c | 6 ++++-- crypto/x509/pcy_cache.c | 3 ++- crypto/x509/v3_purp.c | 3 ++- crypto/x509/x_crl.c | 3 ++- doc/internal/man3/ossl_namemap_new.pod | 3 ++- include/openssl/crypto.h.in | 4 ++-- providers/fips/self_test.c | 19 +++++++++++------ ssl/ssl_lib.c | 9 +++++--- ssl/ssl_sess.c | 33 ++++++++++++++++++++--------- test/namemap_internal_test.c | 6 +++--- 37 files changed, 286 insertions(+), 139 deletions(-) diff --git a/crypto/bio/b_addr.c b/crypto/bio/b_addr.c index 635f84e183..cefa449af0 100644 --- a/crypto/bio/b_addr.c +++ b/crypto/bio/b_addr.c @@ -782,7 +782,10 @@ int BIO_lookup_ex(const char *host, const char *service, int lookup_type, goto err; } - CRYPTO_THREAD_write_lock(bio_lookup_lock); + if (!CRYPTO_THREAD_write_lock(bio_lookup_lock)) { + ret = 0; + goto err; + } he_fallback_address = INADDR_ANY; if (host == NULL) { he = &he_fallback; diff --git a/crypto/bn/bn_mont.c b/crypto/bn/bn_mont.c index 934c55850a..b6c4c06bc5 100644 --- a/crypto/bn/bn_mont.c +++ b/crypto/bn/bn_mont.c @@ -430,7 +430,8 @@ BN_MONT_CTX *BN_MONT_CTX_set_locked(BN_MONT_CTX **pmont, CRYPTO_RWLOCK *lock, { BN_MONT_CTX *ret; - CRYPTO_THREAD_read_lock(lock); + if (!CRYPTO_THREAD_read_lock(lock)) + return NULL; ret = *pmont; CRYPTO_THREAD_unlock(lock); if (ret) @@ -453,7 +454,11 @@ BN_MONT_CTX *BN_MONT_CTX_set_locked(BN_MONT_CTX **pmont, CRYPTO_RWLOCK *lock, } /* The locked compare-and-set, after the local work is done. */ - CRYPTO_THREAD_write_lock(lock); + if (!CRYPTO_THREAD_write_lock(lock)) { + BN_MONT_CTX_free(ret); + return NULL; + } + if (*pmont) { BN_MONT_CTX_free(ret); ret = *pmont; diff --git a/crypto/context.c b/crypto/context.c index 5a46921778..803111459a 100644 --- a/crypto/context.c +++ b/crypto/context.c @@ -232,7 +232,12 @@ static void ossl_lib_ctx_generic_new(void *parent_ign, void *ptr_ign, void *ptr = meth->new_func(ctx); if (ptr != NULL) { - CRYPTO_THREAD_write_lock(ctx->lock); + if (!CRYPTO_THREAD_write_lock(ctx->lock)) + /* + * Can't return something, so best to hope that something will + * fail later. :( + */ + return; CRYPTO_set_ex_data(ad, index, ptr); CRYPTO_THREAD_unlock(ctx->lock); } @@ -277,21 +282,30 @@ void *ossl_lib_ctx_get_data(OSSL_LIB_CTX *ctx, int index, if (ctx == NULL) return NULL; - CRYPTO_THREAD_read_lock(ctx->lock); + if (!CRYPTO_THREAD_read_lock(ctx->lock)) + return NULL; dynidx = ctx->dyn_indexes[index]; CRYPTO_THREAD_unlock(ctx->lock); if (dynidx != -1) { - CRYPTO_THREAD_read_lock(ctx->index_locks[index]); - CRYPTO_THREAD_read_lock(ctx->lock); + if (!CRYPTO_THREAD_read_lock(ctx->index_locks[index])) + return NULL; + if (!CRYPTO_THREAD_read_lock(ctx->lock)) { + CRYPTO_THREAD_unlock(ctx->index_locks[index]); + return NULL; + } data = CRYPTO_get_ex_data(&ctx->data, dynidx); CRYPTO_THREAD_unlock(ctx->lock); CRYPTO_THREAD_unlock(ctx->index_locks[index]); return data; } - CRYPTO_THREAD_write_lock(ctx->index_locks[index]); - CRYPTO_THREAD_write_lock(ctx->lock); + if (!CRYPTO_THREAD_write_lock(ctx->index_locks[index])) + return NULL; + if (!CRYPTO_THREAD_write_lock(ctx->lock)) { + CRYPTO_THREAD_unlock(ctx->index_locks[index]); + return NULL; + } dynidx = ctx->dyn_indexes[index]; if (dynidx != -1) { @@ -321,13 +335,14 @@ void *ossl_lib_ctx_get_data(OSSL_LIB_CTX *ctx, int index, */ if (ossl_crypto_alloc_ex_data_intern(CRYPTO_EX_INDEX_OSSL_LIB_CTX, NULL, &ctx->data, ctx->dyn_indexes[index])) { - CRYPTO_THREAD_read_lock(ctx->lock); + if (!CRYPTO_THREAD_read_lock(ctx->lock)) + goto end; data = CRYPTO_get_ex_data(&ctx->data, ctx->dyn_indexes[index]); CRYPTO_THREAD_unlock(ctx->lock); } +end: CRYPTO_THREAD_unlock(ctx->index_locks[index]); - return data; } @@ -348,7 +363,8 @@ int ossl_lib_ctx_run_once(OSSL_LIB_CTX *ctx, unsigned int idx, if (ctx == NULL) return 0; - CRYPTO_THREAD_read_lock(ctx->oncelock); + if (!CRYPTO_THREAD_read_lock(ctx->oncelock)) + return 0; done = ctx->run_once_done[idx]; if (done) ret = ctx->run_once_ret[idx]; @@ -357,7 +373,8 @@ int ossl_lib_ctx_run_once(OSSL_LIB_CTX *ctx, unsigned int idx, if (done) return ret; - CRYPTO_THREAD_write_lock(ctx->oncelock); + if (!CRYPTO_THREAD_write_lock(ctx->oncelock)) + return 0; if (ctx->run_once_done[idx]) { ret = ctx->run_once_ret[idx]; CRYPTO_THREAD_unlock(ctx->oncelock); diff --git a/crypto/core_namemap.c b/crypto/core_namemap.c index b19ca50786..1cc76bf030 100644 --- a/crypto/core_namemap.c +++ b/crypto/core_namemap.c @@ -107,7 +107,8 @@ int ossl_namemap_empty(OSSL_NAMEMAP *namemap) if (namemap == NULL) return 1; - CRYPTO_THREAD_read_lock(namemap->lock); + if (!CRYPTO_THREAD_read_lock(namemap->lock)) + return -1; rv = namemap->max_number == 0; CRYPTO_THREAD_unlock(namemap->lock); return rv; @@ -149,9 +150,10 @@ int ossl_namemap_doall_names(const OSSL_NAMEMAP *namemap, int number, * the user function, so that we're not holding the read lock when in user * code. This could lead to deadlocks. */ - CRYPTO_THREAD_read_lock(namemap->lock); - num_names = lh_NAMENUM_ENTRY_num_items(namemap->namenum); + if (!CRYPTO_THREAD_read_lock(namemap->lock)) + return 0; + num_names = lh_NAMENUM_ENTRY_num_items(namemap->namenum); if (num_names == 0) { CRYPTO_THREAD_unlock(namemap->lock); return 0; @@ -199,7 +201,8 @@ int ossl_namemap_name2num_n(const OSSL_NAMEMAP *namemap, if (namemap == NULL) return 0; - CRYPTO_THREAD_read_lock(namemap->lock); + if (!CRYPTO_THREAD_read_lock(namemap->lock)) + return 0; number = namemap_name2num_n(namemap, name, name_len); CRYPTO_THREAD_unlock(namemap->lock); @@ -281,7 +284,8 @@ int ossl_namemap_add_name_n(OSSL_NAMEMAP *namemap, int number, if (name == NULL || name_len == 0 || namemap == NULL) return 0; - CRYPTO_THREAD_write_lock(namemap->lock); + if (!CRYPTO_THREAD_write_lock(namemap->lock)) + return 0; tmp_number = namemap_add_name_n(namemap, number, name, name_len); CRYPTO_THREAD_unlock(namemap->lock); return tmp_number; @@ -307,7 +311,8 @@ int ossl_namemap_add_names(OSSL_NAMEMAP *namemap, int number, return 0; } - CRYPTO_THREAD_write_lock(namemap->lock); + if (!CRYPTO_THREAD_write_lock(namemap->lock)) + return 0; /* * Check that no name is an empty string, and that all names have at * most one numeric identity together. @@ -422,12 +427,26 @@ static void get_legacy_md_names(const OBJ_NAME *on, void *arg) OSSL_NAMEMAP *ossl_namemap_stored(OSSL_LIB_CTX *libctx) { +#ifndef FIPS_MODULE + int nms; +#endif OSSL_NAMEMAP *namemap = ossl_lib_ctx_get_data(libctx, OSSL_LIB_CTX_NAMEMAP_INDEX, &stored_namemap_method); + if (namemap == NULL) + return NULL; + #ifndef FIPS_MODULE - if (namemap != NULL && ossl_namemap_empty(namemap)) { + nms = ossl_namemap_empty(namemap); + if (nms < 0) { + /* + * Could not get lock to make the count, so maybe internal objects + * weren't added. This seems safest. + */ + return NULL; + } + if (nms == 1) { /* Before pilfering, we make sure the legacy database is populated */ OPENSSL_init_crypto(OPENSSL_INIT_ADD_ALL_CIPHERS | OPENSSL_INIT_ADD_ALL_DIGESTS, NULL); diff --git a/crypto/engine/eng_ctrl.c b/crypto/engine/eng_ctrl.c index ac770bdeb3..2dc798c1a3 100644 --- a/crypto/engine/eng_ctrl.c +++ b/crypto/engine/eng_ctrl.c @@ -132,7 +132,8 @@ int ENGINE_ctrl(ENGINE *e, int cmd, long i, void *p, void (*f) (void)) ERR_raise(ERR_LIB_ENGINE, ERR_R_PASSED_NULL_PARAMETER); return 0; } - CRYPTO_THREAD_write_lock(global_engine_lock); + if (!CRYPTO_THREAD_write_lock(global_engine_lock)) + return 0; ref_exists = ((e->struct_ref > 0) ? 1 : 0); CRYPTO_THREAD_unlock(global_engine_lock); ctrl_exists = ((e->ctrl == NULL) ? 0 : 1); diff --git a/crypto/engine/eng_dyn.c b/crypto/engine/eng_dyn.c index 0ce8146f16..2463029c98 100644 --- a/crypto/engine/eng_dyn.c +++ b/crypto/engine/eng_dyn.c @@ -157,7 +157,7 @@ static void dynamic_data_ctx_free_func(void *parent, void *ptr, static int dynamic_set_data_ctx(ENGINE *e, dynamic_data_ctx **ctx) { dynamic_data_ctx *c = OPENSSL_zalloc(sizeof(*c)); - int ret = 1; + int ret = 0; if (c == NULL) { ERR_raise(ERR_LIB_ENGINE, ERR_R_MALLOC_FAILURE); @@ -166,13 +166,13 @@ static int dynamic_set_data_ctx(ENGINE *e, dynamic_data_ctx **ctx) c->dirs = sk_OPENSSL_STRING_new_null(); if (c->dirs == NULL) { ERR_raise(ERR_LIB_ENGINE, ERR_R_MALLOC_FAILURE); - OPENSSL_free(c); - return 0; + goto end; } c->DYNAMIC_F1 = "v_check"; c->DYNAMIC_F2 = "bind_engine"; c->dir_load = 1; - CRYPTO_THREAD_write_lock(global_engine_lock); + if (!CRYPTO_THREAD_write_lock(global_engine_lock)) + goto end; if ((*ctx = (dynamic_data_ctx *)ENGINE_get_ex_data(e, dynamic_ex_data_idx)) == NULL) { @@ -184,11 +184,13 @@ static int dynamic_set_data_ctx(ENGINE *e, dynamic_data_ctx **ctx) } } CRYPTO_THREAD_unlock(global_engine_lock); + ret = 1; /* * If we lost the race to set the context, c is non-NULL and *ctx is the * context of the thread that won. */ - if (c) +end: + if (c != NULL) sk_OPENSSL_STRING_free(c->dirs); OPENSSL_free(c); return ret; @@ -213,7 +215,8 @@ static dynamic_data_ctx *dynamic_get_data_ctx(ENGINE *e) ERR_raise(ERR_LIB_ENGINE, ENGINE_R_NO_INDEX); return NULL; } - CRYPTO_THREAD_write_lock(global_engine_lock); + if (!CRYPTO_THREAD_write_lock(global_engine_lock)) + return NULL; /* Avoid a race by checking again inside this lock */ if (dynamic_ex_data_idx < 0) { /* Good, someone didn't beat us to it */ diff --git a/crypto/engine/eng_init.c b/crypto/engine/eng_init.c index 6c9a58fb7a..12a33c5d94 100644 --- a/crypto/engine/eng_init.c +++ b/crypto/engine/eng_init.c @@ -63,7 +63,8 @@ int engine_unlocked_finish(ENGINE *e, int unlock_for_handlers) CRYPTO_THREAD_unlock(global_engine_lock); to_return = e->finish(e); if (unlock_for_handlers) - CRYPTO_THREAD_write_lock(global_engine_lock); + if (!CRYPTO_THREAD_write_lock(global_engine_lock)) + return 0; if (!to_return) return 0; } @@ -88,7 +89,8 @@ int ENGINE_init(ENGINE *e) ERR_raise(ERR_LIB_ENGINE, ERR_R_MALLOC_FAILURE); return 0; } - CRYPTO_THREAD_write_lock(global_engine_lock); + if (!CRYPTO_THREAD_write_lock(global_engine_lock)) + return 0; ret = engine_unlocked_init(e); CRYPTO_THREAD_unlock(global_engine_lock); return ret; @@ -101,7 +103,8 @@ int ENGINE_finish(ENGINE *e) if (e == NULL) return 1; - CRYPTO_THREAD_write_lock(global_engine_lock); + if (!CRYPTO_THREAD_write_lock(global_engine_lock)) + return 0; to_return = engine_unlocked_finish(e, 1); CRYPTO_THREAD_unlock(global_engine_lock); if (!to_return) { diff --git a/crypto/engine/eng_list.c b/crypto/engine/eng_list.c index be08804665..3fa32fa81e 100644 --- a/crypto/engine/eng_list.c +++ b/crypto/engine/eng_list.c @@ -138,7 +138,8 @@ ENGINE *ENGINE_get_first(void) return NULL; } - CRYPTO_THREAD_write_lock(global_engine_lock); + if (!CRYPTO_THREAD_write_lock(global_engine_lock)) + return NULL; ret = engine_list_head; if (ret) { ret->struct_ref++; @@ -157,7 +158,8 @@ ENGINE *ENGINE_get_last(void) return NULL; } - CRYPTO_THREAD_write_lock(global_engine_lock); + if (!CRYPTO_THREAD_write_lock(global_engine_lock)) + return NULL; ret = engine_list_tail; if (ret) { ret->struct_ref++; @@ -173,9 +175,10 @@ ENGINE *ENGINE_get_next(ENGINE *e) ENGINE *ret = NULL; if (e == NULL) { ERR_raise(ERR_LIB_ENGINE, ERR_R_PASSED_NULL_PARAMETER); - return 0; + return NULL; } - CRYPTO_THREAD_write_lock(global_engine_lock); + if (!CRYPTO_THREAD_write_lock(global_engine_lock)) + return NULL; ret = e->next; if (ret) { /* Return a valid structural reference to the next ENGINE */ @@ -193,9 +196,10 @@ ENGINE *ENGINE_get_prev(ENGINE *e) ENGINE *ret = NULL; if (e == NULL) { ERR_raise(ERR_LIB_ENGINE, ERR_R_PASSED_NULL_PARAMETER); - return 0; + return NULL; } - CRYPTO_THREAD_write_lock(global_engine_lock); + if (!CRYPTO_THREAD_write_lock(global_engine_lock)) + return NULL; ret = e->prev; if (ret) { /* Return a valid structural reference to the next ENGINE */ @@ -220,7 +224,8 @@ int ENGINE_add(ENGINE *e) ERR_raise(ERR_LIB_ENGINE, ENGINE_R_ID_OR_NAME_MISSING); return 0; } - CRYPTO_THREAD_write_lock(global_engine_lock); + if (!CRYPTO_THREAD_write_lock(global_engine_lock)) + return 0; if (!engine_list_add(e)) { ERR_raise(ERR_LIB_ENGINE, ENGINE_R_INTERNAL_LIST_ERROR); to_return = 0; @@ -237,7 +242,8 @@ int ENGINE_remove(ENGINE *e) ERR_raise(ERR_LIB_ENGINE, ERR_R_PASSED_NULL_PARAMETER); return 0; } - CRYPTO_THREAD_write_lock(global_engine_lock); + if (!CRYPTO_THREAD_write_lock(global_engine_lock)) + return 0; if (!engine_list_remove(e)) { ERR_raise(ERR_LIB_ENGINE, ENGINE_R_INTERNAL_LIST_ERROR); to_return = 0; @@ -289,7 +295,8 @@ ENGINE *ENGINE_by_id(const char *id) return NULL; } - CRYPTO_THREAD_write_lock(global_engine_lock); + if (!CRYPTO_THREAD_write_lock(global_engine_lock)) + return NULL; iterator = engine_list_head; while (iterator && (strcmp(id, iterator->id) != 0)) iterator = iterator->next; diff --git a/crypto/engine/eng_pkey.c b/crypto/engine/eng_pkey.c index 9feb52d83b..8ba39a46b7 100644 --- a/crypto/engine/eng_pkey.c +++ b/crypto/engine/eng_pkey.c @@ -60,23 +60,24 @@ EVP_PKEY *ENGINE_load_private_key(ENGINE *e, const char *key_id, if (e == NULL) { ERR_raise(ERR_LIB_ENGINE, ERR_R_PASSED_NULL_PARAMETER); - return 0; + return NULL; } - CRYPTO_THREAD_write_lock(global_engine_lock); + if (!CRYPTO_THREAD_write_lock(global_engine_lock)) + return NULL; if (e->funct_ref == 0) { CRYPTO_THREAD_unlock(global_engine_lock); ERR_raise(ERR_LIB_ENGINE, ENGINE_R_NOT_INITIALISED); - return 0; + return NULL; } CRYPTO_THREAD_unlock(global_engine_lock); if (!e->load_privkey) { ERR_raise(ERR_LIB_ENGINE, ENGINE_R_NO_LOAD_FUNCTION); - return 0; + return NULL; } pkey = e->load_privkey(e, key_id, ui_method, callback_data); if (pkey == NULL) { ERR_raise(ERR_LIB_ENGINE, ENGINE_R_FAILED_LOADING_PRIVATE_KEY); - return 0; + return NULL; } return pkey; } @@ -88,23 +89,24 @@ EVP_PKEY *ENGINE_load_public_key(ENGINE *e, const char *key_id, if (e == NULL) { ERR_raise(ERR_LIB_ENGINE, ERR_R_PASSED_NULL_PARAMETER); - return 0; + return NULL; } - CRYPTO_THREAD_write_lock(global_engine_lock); + if (!CRYPTO_THREAD_write_lock(global_engine_lock)) + return NULL; if (e->funct_ref == 0) { CRYPTO_THREAD_unlock(global_engine_lock); ERR_raise(ERR_LIB_ENGINE, ENGINE_R_NOT_INITIALISED); - return 0; + return NULL; } CRYPTO_THREAD_unlock(global_engine_lock); if (!e->load_pubkey) { ERR_raise(ERR_LIB_ENGINE, ENGINE_R_NO_LOAD_FUNCTION); - return 0; + return NULL; } pkey = e->load_pubkey(e, key_id, ui_method, callback_data); if (pkey == NULL) { ERR_raise(ERR_LIB_ENGINE, ENGINE_R_FAILED_LOADING_PUBLIC_KEY); - return 0; + return NULL; } return pkey; } @@ -119,7 +121,8 @@ int ENGINE_load_ssl_client_cert(ENGINE *e, SSL *s, ERR_raise(ERR_LIB_ENGINE, ERR_R_PASSED_NULL_PARAMETER); return 0; } - CRYPTO_THREAD_write_lock(global_engine_lock); + if (!CRYPTO_THREAD_write_lock(global_engine_lock)) + return 0; if (e->funct_ref == 0) { CRYPTO_THREAD_unlock(global_engine_lock); ERR_raise(ERR_LIB_ENGINE, ENGINE_R_NOT_INITIALISED); diff --git a/crypto/engine/eng_table.c b/crypto/engine/eng_table.c index be5704e93d..aa9b31aeff 100644 --- a/crypto/engine/eng_table.c +++ b/crypto/engine/eng_table.c @@ -86,7 +86,9 @@ int engine_table_register(ENGINE_TABLE **table, ENGINE_CLEANUP_CB *cleanup, { int ret = 0, added = 0; ENGINE_PILE tmplate, *fnd; - CRYPTO_THREAD_write_lock(global_engine_lock); + + if (!CRYPTO_THREAD_write_lock(global_engine_lock)) + return 0; if (!(*table)) added = 1; if (!int_table_check(table, 1)) @@ -161,7 +163,9 @@ IMPLEMENT_LHASH_DOALL_ARG(ENGINE_PILE, ENGINE); void engine_table_unregister(ENGINE_TABLE **table, ENGINE *e) { - CRYPTO_THREAD_write_lock(global_engine_lock); + if (!CRYPTO_THREAD_write_lock(global_engine_lock)) + /* Can't return a value. :( */ + return; if (int_table_check(table, 0)) lh_ENGINE_PILE_doall_ENGINE(&(*table)->piles, int_unregister_cb, e); CRYPTO_THREAD_unlock(global_engine_lock); @@ -179,7 +183,8 @@ static void int_cleanup_cb_doall(ENGINE_PILE *p) void engine_table_cleanup(ENGINE_TABLE **table) { - CRYPTO_THREAD_write_lock(global_engine_lock); + if (!CRYPTO_THREAD_write_lock(global_engine_lock)) + return; if (*table) { lh_ENGINE_PILE_doall(&(*table)->piles, int_cleanup_cb_doall); lh_ENGINE_PILE_free(&(*table)->piles); @@ -206,7 +211,8 @@ ENGINE *engine_table_select_int(ENGINE_TABLE **table, int nid, const char *f, return NULL; } ERR_set_mark(); - CRYPTO_THREAD_write_lock(global_engine_lock); + if (!CRYPTO_THREAD_write_lock(global_engine_lock)) + goto end; /* * Check again inside the lock otherwise we could race against cleanup * operations. But don't worry about a debug printout diff --git a/crypto/engine/tb_asnmth.c b/crypto/engine/tb_asnmth.c index d1465227b9..fbc0d6f02a 100644 --- a/crypto/engine/tb_asnmth.c +++ b/crypto/engine/tb_asnmth.c @@ -199,7 +199,8 @@ const EVP_PKEY_ASN1_METHOD *ENGINE_pkey_asn1_find_str(ENGINE **pe, return NULL; } - CRYPTO_THREAD_write_lock(global_engine_lock); + if (!CRYPTO_THREAD_write_lock(global_engine_lock)) + return NULL; engine_table_doall(pkey_asn1_meth_table, look_str_cb, &fstr); /* If found obtain a structural reference to engine */ if (fstr.e) { diff --git a/crypto/err/err.c b/crypto/err/err.c index a8bde92674..23836b844b 100644 --- a/crypto/err/err.c +++ b/crypto/err/err.c @@ -176,7 +176,8 @@ static ERR_STRING_DATA *int_err_get_item(const ERR_STRING_DATA *d) { ERR_STRING_DATA *p = NULL; - CRYPTO_THREAD_read_lock(err_string_lock); + if (!CRYPTO_THREAD_read_lock(err_string_lock)) + return NULL; p = lh_ERR_STRING_DATA_retrieve(int_error_hash, d); CRYPTO_THREAD_unlock(err_string_lock); @@ -238,7 +239,8 @@ static void err_patch(int lib, ERR_STRING_DATA *str) */ static int err_load_strings(const ERR_STRING_DATA *str) { - CRYPTO_THREAD_write_lock(err_string_lock); + if (!CRYPTO_THREAD_write_lock(err_string_lock)) + return 0; for (; str->error; str++) (void)lh_ERR_STRING_DATA_insert(int_error_hash, (ERR_STRING_DATA *)str); @@ -281,7 +283,8 @@ int ERR_unload_strings(int lib, ERR_STRING_DATA *str) if (!RUN_ONCE(&err_string_init, do_err_strings_init)) return 0; - CRYPTO_THREAD_write_lock(err_string_lock); + if (!CRYPTO_THREAD_write_lock(err_string_lock)) + return 0; /* * We don't need to ERR_PACK the lib, since that was done (to * the table) when it was loaded. @@ -728,7 +731,8 @@ int ERR_get_next_error_library(void) if (!RUN_ONCE(&err_string_init, do_err_strings_init)) return 0; - CRYPTO_THREAD_write_lock(err_string_lock); + if (!CRYPTO_THREAD_write_lock(err_string_lock)) + return 0; ret = int_err_library_number++; CRYPTO_THREAD_unlock(err_string_lock); return ret; diff --git a/crypto/evp/keymgmt_lib.c b/crypto/evp/keymgmt_lib.c index 27b9f6b907..4f0dc2d4a7 100644 --- a/crypto/evp/keymgmt_lib.c +++ b/crypto/evp/keymgmt_lib.c @@ -101,7 +101,8 @@ void *evp_keymgmt_util_export_to_provider(EVP_PKEY *pk, EVP_KEYMGMT *keymgmt) if (pk->keymgmt == keymgmt) return pk->keydata; - CRYPTO_THREAD_read_lock(pk->lock); + if (!CRYPTO_THREAD_read_lock(pk->lock)) + return NULL; /* * If the provider native "origin" hasn't changed since last time, we * try to find our keymgmt in the operation cache. If it has changed @@ -156,7 +157,10 @@ void *evp_keymgmt_util_export_to_provider(EVP_PKEY *pk, EVP_KEYMGMT *keymgmt) return NULL; } - CRYPTO_THREAD_write_lock(pk->lock); + if (!CRYPTO_THREAD_write_lock(pk->lock)) { + evp_keymgmt_freedata(keymgmt, import_data.keydata); + return NULL; + } /* Check to make sure some other thread didn't get there first */ op = evp_keymgmt_util_find_operation_cache(pk, keymgmt); if (op != NULL && op->keydata != NULL) { diff --git a/crypto/ex_data.c b/crypto/ex_data.c index 291e5a6498..da5b5b69ba 100644 --- a/crypto/ex_data.c +++ b/crypto/ex_data.c @@ -43,7 +43,8 @@ static EX_CALLBACKS *get_and_lock(OSSL_EX_DATA_GLOBAL *global, int class_index) return NULL; } - CRYPTO_THREAD_write_lock(global->ex_data_lock); + if (!CRYPTO_THREAD_write_lock(global->ex_data_lock)) + return NULL; ip = &global->ex_data[class_index]; return ip; } @@ -367,7 +368,8 @@ void CRYPTO_free_ex_data(int class_index, void *obj, CRYPTO_EX_DATA *ad) if (storage != NULL) f = storage[i]; else { - CRYPTO_THREAD_write_lock(global->ex_data_lock); + if (!CRYPTO_THREAD_write_lock(global->ex_data_lock)) + continue; f = sk_EX_CALLBACK_value(ip->meth, i); CRYPTO_THREAD_unlock(global->ex_data_lock); } diff --git a/crypto/init.c b/crypto/init.c index 50aec32c3d..3b6a16a76d 100644 --- a/crypto/init.c +++ b/crypto/init.c @@ -586,7 +586,8 @@ int OPENSSL_init_crypto(uint64_t opts, const OPENSSL_INIT_SETTINGS *settings) if (settings == NULL) { ret = RUN_ONCE(&config, ossl_init_config); } else { - CRYPTO_THREAD_write_lock(init_lock); + if (!CRYPTO_THREAD_write_lock(init_lock)) + return 0; conf_settings = settings; ret = RUN_ONCE_ALT(&config, ossl_init_config_settings, ossl_init_config); diff --git a/crypto/initthread.c b/crypto/initthread.c index 0697013514..993bf74473 100644 --- a/crypto/initthread.c +++ b/crypto/initthread.c @@ -157,7 +157,8 @@ static int init_thread_push_handlers(THREAD_EVENT_HANDLER **hands) if (gtr == NULL) return 0; - CRYPTO_THREAD_write_lock(gtr->lock); + if (!CRYPTO_THREAD_write_lock(gtr->lock)) + return 0; ret = (sk_THREAD_EVENT_HANDLER_PTR_push(gtr->skhands, hands) != 0); CRYPTO_THREAD_unlock(gtr->lock); @@ -172,7 +173,8 @@ static void init_thread_remove_handlers(THREAD_EVENT_HANDLER **handsin) gtr = get_global_tevent_register(); if (gtr == NULL) return; - CRYPTO_THREAD_write_lock(gtr->lock); + if (!CRYPTO_THREAD_write_lock(gtr->lock)) + return; for (i = 0; i < sk_THREAD_EVENT_HANDLER_PTR_num(gtr->skhands); i++) { THREAD_EVENT_HANDLER **hands = sk_THREAD_EVENT_HANDLER_PTR_value(gtr->skhands, i); @@ -389,10 +391,12 @@ static int init_thread_deregister(void *index, int all) gtr = get_global_tevent_register(); if (gtr == NULL) return 0; - if (!all) - CRYPTO_THREAD_write_lock(gtr->lock); - else + if (!all) { + if (!CRYPTO_THREAD_write_lock(gtr->lock)) + return 0; + } else { glob_tevent_reg = NULL; + } for (i = 0; i < sk_THREAD_EVENT_HANDLER_PTR_num(gtr->skhands); i++) { THREAD_EVENT_HANDLER **hands = sk_THREAD_EVENT_HANDLER_PTR_value(gtr->skhands, i); diff --git a/crypto/mem_sec.c b/crypto/mem_sec.c index cc4c4e08f6..ebc0e557b5 100644 --- a/crypto/mem_sec.c +++ b/crypto/mem_sec.c @@ -132,7 +132,8 @@ void *CRYPTO_secure_malloc(size_t num, const char *file, int line) if (!secure_mem_initialized) { return CRYPTO_malloc(num, file, line); } - CRYPTO_THREAD_write_lock(sec_malloc_lock); + if (!CRYPTO_THREAD_write_lock(sec_malloc_lock)) + return NULL; ret = sh_malloc(num); actual_size = ret ? sh_actual_size(ret) : 0; secure_mem_used += actual_size; @@ -164,7 +165,8 @@ void CRYPTO_secure_free(void *ptr, const char *file, int line) CRYPTO_free(ptr, file, line); return; } - CRYPTO_THREAD_write_lock(sec_malloc_lock); + if (!CRYPTO_THREAD_write_lock(sec_malloc_lock)) + return; actual_size = sh_actual_size(ptr); CLEAR(ptr, actual_size); secure_mem_used -= actual_size; @@ -188,7 +190,8 @@ void CRYPTO_secure_clear_free(void *ptr, size_t num, CRYPTO_free(ptr, file, line); return; } - CRYPTO_THREAD_write_lock(sec_malloc_lock); + if (!CRYPTO_THREAD_write_lock(sec_malloc_lock)) + return; actual_size = sh_actual_size(ptr); CLEAR(ptr, actual_size); secure_mem_used -= actual_size; @@ -209,7 +212,8 @@ int CRYPTO_secure_allocated(const void *ptr) if (!secure_mem_initialized) return 0; - CRYPTO_THREAD_write_lock(sec_malloc_lock); + if (!CRYPTO_THREAD_write_lock(sec_malloc_lock)) + return 0; ret = sh_allocated(ptr); CRYPTO_THREAD_unlock(sec_malloc_lock); return ret; @@ -232,7 +236,8 @@ size_t CRYPTO_secure_actual_size(void *ptr) #ifndef OPENSSL_NO_SECURE_MEMORY size_t actual_size; - CRYPTO_THREAD_write_lock(sec_malloc_lock); + if (!CRYPTO_THREAD_write_lock(sec_malloc_lock)) + return 0; actual_size = sh_actual_size(ptr); CRYPTO_THREAD_unlock(sec_malloc_lock); return actual_size; diff --git a/crypto/objects/o_names.c b/crypto/objects/o_names.c index fbc756ebb8..372f65d1ff 100644 --- a/crypto/objects/o_names.c +++ b/crypto/objects/o_names.c @@ -86,7 +86,8 @@ int OBJ_NAME_new_index(unsigned long (*hash_func) (const char *), if (!OBJ_NAME_init()) return 0; - CRYPTO_THREAD_write_lock(obj_lock); + if (!CRYPTO_THREAD_write_lock(obj_lock)) + return 0; if (name_funcs_stack == NULL) name_funcs_stack = sk_NAME_FUNCS_new_null(); @@ -169,7 +170,8 @@ const char *OBJ_NAME_get(const char *name, int type) return NULL; if (!OBJ_NAME_init()) return NULL; - CRYPTO_THREAD_read_lock(obj_lock); + if (!CRYPTO_THREAD_read_lock(obj_lock)) + return NULL; alias = type & OBJ_NAME_ALIAS; type &= ~OBJ_NAME_ALIAS; @@ -207,17 +209,18 @@ int OBJ_NAME_add(const char *name, int type, const char *data) type &= ~OBJ_NAME_ALIAS; onp = OPENSSL_malloc(sizeof(*onp)); - if (onp == NULL) { - /* ERROR */ - goto unlock; - } + if (onp == NULL) + return 0; onp->name = name; onp->alias = alias; onp->type = type; onp->data = data; - CRYPTO_THREAD_write_lock(obj_lock); + if (!CRYPTO_THREAD_write_lock(obj_lock)) { + OPENSSL_free(onp); + return 0; + } ret = lh_OBJ_NAME_insert(names_lh, onp); if (ret != NULL) { @@ -256,7 +259,8 @@ int OBJ_NAME_remove(const char *name, int type) if (!OBJ_NAME_init()) return 0; - CRYPTO_THREAD_write_lock(obj_lock); + if (!CRYPTO_THREAD_write_lock(obj_lock)) + return 0; type &= ~OBJ_NAME_ALIAS; on.name = name; diff --git a/crypto/provider_core.c b/crypto/provider_core.c index 47eda52224..f5d54b22ec 100644 --- a/crypto/provider_core.c +++ b/crypto/provider_core.c @@ -205,7 +205,8 @@ int ossl_provider_disable_fallback_loading(OSSL_LIB_CTX *libctx) struct provider_store_st *store; if ((store = get_provider_store(libctx)) != NULL) { - CRYPTO_THREAD_write_lock(store->lock); + if (!CRYPTO_THREAD_write_lock(store->lock)) + return 0; store->use_fallbacks = 0; CRYPTO_THREAD_unlock(store->lock); return 1; @@ -233,7 +234,8 @@ OSSL_PROVIDER *ossl_provider_find(OSSL_LIB_CTX *libctx, const char *name, #endif tmpl.name = (char *)name; - CRYPTO_THREAD_write_lock(store->lock); + if (!CRYPTO_THREAD_write_lock(store->lock)) + return NULL; if ((i = sk_OSSL_PROVIDER_find(store->providers, &tmpl)) == -1 || (prov = sk_OSSL_PROVIDER_value(store->providers, i)) == NULL || !ossl_provider_up_ref(prov)) @@ -303,7 +305,8 @@ OSSL_PROVIDER *ossl_provider_new(OSSL_LIB_CTX *libctx, const char *name, if ((prov = provider_new(name, init_function)) == NULL) return NULL; - CRYPTO_THREAD_write_lock(store->lock); + if (!CRYPTO_THREAD_write_lock(store->lock)) + return NULL; if (!ossl_provider_up_ref(prov)) { /* +1 One reference for the store */ ossl_provider_free(prov); /* -1 Reference that was to be returned */ prov = NULL; @@ -486,7 +489,8 @@ static int provider_init(OSSL_PROVIDER *prov) * modifies a number of things in the provider structure that this * function needs to perform under lock anyway. */ - CRYPTO_THREAD_write_lock(prov->flag_lock); + if (!CRYPTO_THREAD_write_lock(prov->flag_lock)) + goto end; if (prov->flag_initialized) { ok = 1; goto end; @@ -671,7 +675,8 @@ static int provider_deactivate(OSSL_PROVIDER *prov) return 0; if (ref < 1) { - CRYPTO_THREAD_write_lock(prov->flag_lock); + if (!CRYPTO_THREAD_write_lock(prov->flag_lock)) + return 0; prov->flag_activated = 0; CRYPTO_THREAD_unlock(prov->flag_lock); } @@ -688,7 +693,8 @@ static int provider_activate(OSSL_PROVIDER *prov) return 0; if (provider_init(prov)) { - CRYPTO_THREAD_write_lock(prov->flag_lock); + if (!CRYPTO_THREAD_write_lock(prov->flag_lock)) + return 0; prov->flag_activated = 1; CRYPTO_THREAD_unlock(prov->flag_lock); @@ -705,7 +711,10 @@ int ossl_provider_activate(OSSL_PROVIDER *prov, int retain_fallbacks) return 0; if (provider_activate(prov)) { if (!retain_fallbacks) { - CRYPTO_THREAD_write_lock(prov->store->lock); + if (!CRYPTO_THREAD_write_lock(prov->store->lock)) { + provider_deactivate(prov); + return 0; + } prov->store->use_fallbacks = 0; CRYPTO_THREAD_unlock(prov->store->lock); } @@ -738,13 +747,15 @@ static void provider_activate_fallbacks(struct provider_store_st *store) int activated_fallback_count = 0; int i; - CRYPTO_THREAD_read_lock(store->lock); + if (!CRYPTO_THREAD_read_lock(store->lock)) + return; use_fallbacks = store->use_fallbacks; CRYPTO_THREAD_unlock(store->lock); if (!use_fallbacks) return; - CRYPTO_THREAD_write_lock(store->lock); + if (!CRYPTO_THREAD_write_lock(store->lock)) + return; /* Check again, just in case another thread changed it */ use_fallbacks = store->use_fallbacks; if (!use_fallbacks) { @@ -804,7 +815,8 @@ int ossl_provider_doall_activated(OSSL_LIB_CTX *ctx, * Under lock, grab a copy of the provider list and up_ref each * provider so that they don't disappear underneath us. */ - CRYPTO_THREAD_read_lock(store->lock); + if (!CRYPTO_THREAD_read_lock(store->lock)) + return 0; provs = sk_OSSL_PROVIDER_dup(store->providers); if (provs == NULL) { CRYPTO_THREAD_unlock(store->lock); @@ -973,7 +985,8 @@ int ossl_provider_set_operation_bit(OSSL_PROVIDER *provider, size_t bitnum) size_t byte = bitnum / 8; unsigned char bit = (1 << (bitnum % 8)) & 0xFF; - CRYPTO_THREAD_write_lock(provider->opbits_lock); + if (!CRYPTO_THREAD_write_lock(provider->opbits_lock)) + return 0; if (provider->operation_bits_sz <= byte) { unsigned char *tmp = OPENSSL_realloc(provider->operation_bits, byte + 1); @@ -1005,7 +1018,8 @@ int ossl_provider_test_operation_bit(OSSL_PROVIDER *provider, size_t bitnum, } *result = 0; - CRYPTO_THREAD_read_lock(provider->opbits_lock); + if (!CRYPTO_THREAD_read_lock(provider->opbits_lock)) + return 0; if (provider->operation_bits_sz > byte) *result = ((provider->operation_bits[byte] & bit) != 0); CRYPTO_THREAD_unlock(provider->opbits_lock); diff --git a/crypto/rand/rand_lib.c b/crypto/rand/rand_lib.c index e248d5753a..3cd34198c1 100644 --- a/crypto/rand/rand_lib.c +++ b/crypto/rand/rand_lib.c @@ -163,7 +163,8 @@ int RAND_set_rand_method(const RAND_METHOD *meth) if (!RUN_ONCE(&rand_init, do_rand_init)) return 0; - CRYPTO_THREAD_write_lock(rand_meth_lock); + if (!CRYPTO_THREAD_write_lock(rand_meth_lock)) + return 0; # ifndef OPENSSL_NO_ENGINE ENGINE_finish(funct_ref); funct_ref = NULL; @@ -180,7 +181,8 @@ const RAND_METHOD *RAND_get_rand_method(void) if (!RUN_ONCE(&rand_init, do_rand_init)) return NULL; - CRYPTO_THREAD_write_lock(rand_meth_lock); + if (!CRYPTO_THREAD_write_lock(rand_meth_lock)) + return NULL; if (default_RAND_meth == NULL) { # ifndef OPENSSL_NO_ENGINE ENGINE *e; @@ -220,7 +222,11 @@ int RAND_set_rand_engine(ENGINE *engine) return 0; } } - CRYPTO_THREAD_write_lock(rand_engine_lock); + if (!CRYPTO_THREAD_write_lock(rand_engine_lock)) { + ENGINE_finish(engine); + return 0; + } + /* This function releases any prior ENGINE so call it first */ RAND_set_rand_method(tmp_meth); funct_ref = engine; diff --git a/crypto/rsa/rsa_ossl.c b/crypto/rsa/rsa_ossl.c index b92137ee0a..9f98c037c8 100644 --- a/crypto/rsa/rsa_ossl.c +++ b/crypto/rsa/rsa_ossl.c @@ -163,7 +163,8 @@ static BN_BLINDING *rsa_get_blinding(RSA *rsa, int *local, BN_CTX *ctx) { BN_BLINDING *ret; - CRYPTO_THREAD_write_lock(rsa->lock); + if (!CRYPTO_THREAD_write_lock(rsa->lock)) + return NULL; if (rsa->blinding == NULL) { rsa->blinding = RSA_setup_blinding(rsa, ctx); diff --git a/crypto/store/store_register.c b/crypto/store/store_register.c index 51bd591790..5049671f52 100644 --- a/crypto/store/store_register.c +++ b/crypto/store/store_register.c @@ -194,7 +194,8 @@ int ossl_store_register_loader_int(OSSL_STORE_LOADER *loader) ERR_raise(ERR_LIB_OSSL_STORE, ERR_R_MALLOC_FAILURE); return 0; } - CRYPTO_THREAD_write_lock(registry_lock); + if (!CRYPTO_THREAD_write_lock(registry_lock)) + return 0; if (ossl_store_register_init() && (lh_OSSL_STORE_LOADER_insert(loader_register, loader) != NULL @@ -226,7 +227,8 @@ const OSSL_STORE_LOADER *ossl_store_get0_loader_int(const char *scheme) ERR_raise(ERR_LIB_OSSL_STORE, ERR_R_MALLOC_FAILURE); return NULL; } - CRYPTO_THREAD_write_lock(registry_lock); + if (!CRYPTO_THREAD_write_lock(registry_lock)) + return NULL; if (!ossl_store_register_init()) ERR_raise(ERR_LIB_OSSL_STORE, ERR_R_INTERNAL_ERROR); @@ -255,7 +257,8 @@ OSSL_STORE_LOADER *ossl_store_unregister_loader_int(const char *scheme) ERR_raise(ERR_LIB_OSSL_STORE, ERR_R_MALLOC_FAILURE); return NULL; } - CRYPTO_THREAD_write_lock(registry_lock); + if (!CRYPTO_THREAD_write_lock(registry_lock)) + return NULL; if (!ossl_store_register_init()) ERR_raise(ERR_LIB_OSSL_STORE, ERR_R_INTERNAL_ERROR); diff --git a/crypto/threads_none.c b/crypto/threads_none.c index 55d4b5f0f8..f0e0f7fba4 100644 --- a/crypto/threads_none.c +++ b/crypto/threads_none.c @@ -31,14 +31,14 @@ CRYPTO_RWLOCK *CRYPTO_THREAD_lock_new(void) return lock; } -int CRYPTO_THREAD_read_lock(CRYPTO_RWLOCK *lock) +__owur int CRYPTO_THREAD_read_lock(CRYPTO_RWLOCK *lock) { if (!ossl_assert(*(unsigned int *)lock == 1)) return 0; return 1; } -int CRYPTO_THREAD_write_lock(CRYPTO_RWLOCK *lock) +__owur int CRYPTO_THREAD_write_lock(CRYPTO_RWLOCK *lock) { if (!ossl_assert(*(unsigned int *)lock == 1)) return 0; diff --git a/crypto/threads_pthread.c b/crypto/threads_pthread.c index e81f3cf1ef..ecb53299c4 100644 --- a/crypto/threads_pthread.c +++ b/crypto/threads_pthread.c @@ -73,7 +73,7 @@ CRYPTO_RWLOCK *CRYPTO_THREAD_lock_new(void) return lock; } -int CRYPTO_THREAD_read_lock(CRYPTO_RWLOCK *lock) +__owur int CRYPTO_THREAD_read_lock(CRYPTO_RWLOCK *lock) { # ifdef USE_RWLOCK if (pthread_rwlock_rdlock(lock) != 0) @@ -88,7 +88,7 @@ int CRYPTO_THREAD_read_lock(CRYPTO_RWLOCK *lock) return 1; } -int CRYPTO_THREAD_write_lock(CRYPTO_RWLOCK *lock) +__owur int CRYPTO_THREAD_write_lock(CRYPTO_RWLOCK *lock) { # ifdef USE_RWLOCK if (pthread_rwlock_wrlock(lock) != 0) diff --git a/crypto/threads_win.c b/crypto/threads_win.c index 085ce9e96c..fdc32a2a54 100644 --- a/crypto/threads_win.c +++ b/crypto/threads_win.c @@ -57,7 +57,7 @@ CRYPTO_RWLOCK *CRYPTO_THREAD_lock_new(void) return lock; } -int CRYPTO_THREAD_read_lock(CRYPTO_RWLOCK *lock) +__owur int CRYPTO_THREAD_read_lock(CRYPTO_RWLOCK *lock) { # ifdef USE_RWLOCK CRYPTO_win_rwlock *rwlock = lock; @@ -69,7 +69,7 @@ int CRYPTO_THREAD_read_lock(CRYPTO_RWLOCK *lock) return 1; } -int CRYPTO_THREAD_write_lock(CRYPTO_RWLOCK *lock) +__owur int CRYPTO_THREAD_write_lock(CRYPTO_RWLOCK *lock) { # ifdef USE_RWLOCK CRYPTO_win_rwlock *rwlock = lock; diff --git a/crypto/trace.c b/crypto/trace.c index ba9b8dd742..12fe02acfc 100644 --- a/crypto/trace.c +++ b/crypto/trace.c @@ -468,7 +468,8 @@ BIO *OSSL_trace_begin(int category) prefix = trace_channels[category].prefix; if (channel != NULL) { - CRYPTO_THREAD_write_lock(trace_lock); + if (!CRYPTO_THREAD_write_lock(trace_lock)) + return NULL; current_channel = channel; switch (trace_channels[category].type) { case SIMPLE_CHANNEL: diff --git a/crypto/ui/ui_openssl.c b/crypto/ui/ui_openssl.c index 0a38658c72..42524d42a2 100644 --- a/crypto/ui/ui_openssl.c +++ b/crypto/ui/ui_openssl.c @@ -372,7 +372,8 @@ static int read_string_inner(UI *ui, UI_STRING *uis, int echo, int strip_nl) /* Internal functions to open, handle and close a channel to the console. */ static int open_console(UI *ui) { - CRYPTO_THREAD_write_lock(ui->lock); + if (!CRYPTO_THREAD_write_lock(ui->lock)) + return 0; is_a_tty = 1; # if defined(OPENSSL_SYS_VXWORKS) diff --git a/crypto/x509/by_dir.c b/crypto/x509/by_dir.c index 1e5e8469aa..6c0894796b 100644 --- a/crypto/x509/by_dir.c +++ b/crypto/x509/by_dir.c @@ -268,7 +268,8 @@ static int get_cert_by_subject_ex(X509_LOOKUP *xl, X509_LOOKUP_TYPE type, } if (type == X509_LU_CRL && ent->hashes) { htmp.hash = h; - CRYPTO_THREAD_read_lock(ctx->lock); + if (!CRYPTO_THREAD_read_lock(ctx->lock)) + goto finish; idx = sk_BY_DIR_HASH_find(ent->hashes, &htmp); if (idx >= 0) { hent = sk_BY_DIR_HASH_value(ent->hashes, idx); @@ -346,7 +347,8 @@ static int get_cert_by_subject_ex(X509_LOOKUP *xl, X509_LOOKUP_TYPE type, /* If a CRL, update the last file suffix added for this */ if (type == X509_LU_CRL) { - CRYPTO_THREAD_write_lock(ctx->lock); + if (!CRYPTO_THREAD_write_lock(ctx->lock)) + goto finish; /* * Look for entry again in case another thread added an entry * first. diff --git a/crypto/x509/pcy_cache.c b/crypto/x509/pcy_cache.c index cc7aaba1e9..22cf431895 100644 --- a/crypto/x509/pcy_cache.c +++ b/crypto/x509/pcy_cache.c @@ -188,7 +188,8 @@ const X509_POLICY_CACHE *policy_cache_set(X509 *x) { if (x->policy_cache == NULL) { - CRYPTO_THREAD_write_lock(x->lock); + if (!CRYPTO_THREAD_write_lock(x->lock)) + return NULL; policy_cache_new(x); CRYPTO_THREAD_unlock(x->lock); } diff --git a/crypto/x509/v3_purp.c b/crypto/x509/v3_purp.c index 1149e83780..ee2d3c7289 100644 --- a/crypto/x509/v3_purp.c +++ b/crypto/x509/v3_purp.c @@ -411,7 +411,8 @@ int x509v3_cache_extensions(X509 *x) return (x->ex_flags & EXFLAG_INVALID) == 0; #endif - CRYPTO_THREAD_write_lock(x->lock); + if (!CRYPTO_THREAD_write_lock(x->lock)) + return 0; if (x->ex_flags & EXFLAG_SET) { /* Cert has already been processed */ CRYPTO_THREAD_unlock(x->lock); return (x->ex_flags & EXFLAG_INVALID) == 0; diff --git a/crypto/x509/x_crl.c b/crypto/x509/x_crl.c index 4058202b77..4bb7a880a3 100644 --- a/crypto/x509/x_crl.c +++ b/crypto/x509/x_crl.c @@ -427,7 +427,8 @@ static int def_crl_lookup(X509_CRL *crl, * under a lock to avoid race condition. */ if (!sk_X509_REVOKED_is_sorted(crl->crl.revoked)) { - CRYPTO_THREAD_write_lock(crl->lock); + if (!CRYPTO_THREAD_write_lock(crl->lock)) + return 0; sk_X509_REVOKED_sort(crl->crl.revoked); CRYPTO_THREAD_unlock(crl->lock); } diff --git a/doc/internal/man3/ossl_namemap_new.pod b/doc/internal/man3/ossl_namemap_new.pod index d6f30a7047..ff247e87b0 100644 --- a/doc/internal/man3/ossl_namemap_new.pod +++ b/doc/internal/man3/ossl_namemap_new.pod @@ -85,7 +85,8 @@ ossl_namemap_new() and ossl_namemap_stored() return the pointer to a B<OSSL_NAMEMAP>, or NULL on error. ossl_namemap_empty() returns 1 if the B<OSSL_NAMEMAP> is NULL or -empty, or 0 if it's not empty. +empty, 0 if it's not empty, or -1 on internal error (such as inability +to lock). ossl_namemap_add_name() and ossl_namemap_add_name_n() return the number associated with the added string, or zero on error. diff --git a/include/openssl/crypto.h.in b/include/openssl/crypto.h.in index d7c4dd7426..adceb0c103 100644 --- a/include/openssl/crypto.h.in +++ b/include/openssl/crypto.h.in @@ -79,8 +79,8 @@ typedef struct { typedef void CRYPTO_RWLOCK; CRYPTO_RWLOCK *CRYPTO_THREAD_lock_new(void); -int CRYPTO_THREAD_read_lock(CRYPTO_RWLOCK *lock); -int CRYPTO_THREAD_write_lock(CRYPTO_RWLOCK *lock); +__owur int CRYPTO_THREAD_read_lock(CRYPTO_RWLOCK *lock); +__owur int CRYPTO_THREAD_write_lock(CRYPTO_RWLOCK *lock); int CRYPTO_THREAD_unlock(CRYPTO_RWLOCK *lock); void CRYPTO_THREAD_lock_free(CRYPTO_RWLOCK *lock); diff --git a/providers/fips/self_test.c b/providers/fips/self_test.c index b20af1bd6c..dbd744ff7c 100644 --- a/providers/fips/self_test.c +++ b/providers/fips/self_test.c @@ -11,6 +11,7 @@ #include <openssl/evp.h> #include <openssl/params.h> #include <openssl/crypto.h> +#include <internal/cryptlib.h> #include <openssl/fipskey.h> #include <openssl/err.h> #include <openssl/proverr.h> @@ -212,9 +213,10 @@ err: static void set_fips_state(int state) { - CRYPTO_THREAD_write_lock(fips_state_lock); - FIPS_state = state; - CRYPTO_THREAD_unlock(fips_state_lock); + if (ossl_assert(CRYPTO_THREAD_write_lock(fips_state_lock) != 0)) { + FIPS_state = state; + CRYPTO_THREAD_unlock(fips_state_lock); + } } /* This API is triggered either on loading of the FIPS module or on demand */ @@ -232,7 +234,8 @@ int SELF_TEST_post(SELF_TEST_POST_PARAMS *st, int on_demand_test) if (!RUN_ONCE(&fips_self_test_init, do_fips_self_test_init)) return 0; - CRYPTO_THREAD_read_lock(fips_state_lock); + if (!CRYPTO_THREAD_read_lock(fips_state_lock)) + return 0; loclstate = FIPS_state; CRYPTO_THREAD_unlock(fips_state_lock); @@ -244,8 +247,12 @@ int SELF_TEST_post(SELF_TEST_POST_PARAMS *st, int on_demand_test) return 0; } - CRYPTO_THREAD_write_lock(self_test_lock); - CRYPTO_THREAD_read_lock(fips_state_lock); + if (!CRYPTO_THREAD_write_lock(self_test_lock)) + return 0; + if (!CRYPTO_THREAD_read_lock(fips_state_lock)) { + CRYPTO_THREAD_unlock(self_test_lock); + return 0; + } if (FIPS_state == FIPS_STATE_RUNNING) { CRYPTO_THREAD_unlock(fips_state_lock); if (!on_demand_test) { diff --git a/ssl/ssl_lib.c b/ssl/ssl_lib.c index 348b507622..4cb40bd89b 100644 --- a/ssl/ssl_lib.c +++ b/ssl/ssl_lib.c @@ -893,7 +893,8 @@ int SSL_set_session_id_context(SSL *ssl, const unsigned char *sid_ctx, int SSL_CTX_set_generate_session_id(SSL_CTX *ctx, GEN_SESSION_CB cb) { - CRYPTO_THREAD_write_lock(ctx->lock); + if (!CRYPTO_THREAD_write_lock(ctx->lock)) + return 0; ctx->generate_session_id = cb; CRYPTO_THREAD_unlock(ctx->lock); return 1; @@ -901,7 +902,8 @@ int SSL_CTX_set_generate_session_id(SSL_CTX *ctx, GEN_SESSION_CB cb) int SSL_set_generate_session_id(SSL *ssl, GEN_SESSION_CB cb) { - CRYPTO_THREAD_write_lock(ssl->lock); + if (!CRYPTO_THREAD_write_lock(ssl->lock)) + return 0; ssl->generate_session_id = cb; CRYPTO_THREAD_unlock(ssl->lock); return 1; @@ -926,7 +928,8 @@ int SSL_has_matching_session_id(const SSL *ssl, const unsigned char *id, r.session_id_length = id_len; memcpy(r.session_id, id, id_len); - CRYPTO_THREAD_read_lock(ssl->session_ctx->lock); + if (!CRYPTO_THREAD_read_lock(ssl->session_ctx->lock)) + return 0; p = lh_SSL_SESSION_retrieve(ssl->session_ctx->sessions, &r); CRYPTO_THREAD_unlock(ssl->session_ctx->lock); return (p != NULL); diff --git a/ssl/ssl_sess.c b/ssl/ssl_sess.c index 2bb7609395..4e4f9aacea 100644 --- a/ssl/ssl_sess.c +++ b/ssl/ssl_sess.c @@ -44,7 +44,8 @@ SSL_SESSION *SSL_get1_session(SSL *ssl) * somebody doesn't free ssl->session between when we check it's non-null * and when we up the reference count. */ - CRYPTO_THREAD_read_lock(ssl->lock); + if (!CRYPTO_THREAD_read_lock(ssl->lock)) + return NULL; sess = ssl->session; if (sess) SSL_SESSION_up_ref(sess); @@ -319,8 +320,14 @@ int ssl_generate_session_id(SSL *s, SSL_SESSION *ss) } /* Choose which callback will set the session ID */ - CRYPTO_THREAD_read_lock(s->lock); - CRYPTO_THREAD_read_lock(s->session_ctx->lock); + if (!CRYPTO_THREAD_read_lock(s->lock)) + return 0; + if (!CRYPTO_THREAD_read_lock(s->session_ctx->lock)) { + CRYPTO_THREAD_unlock(s->lock); + SSLfatal(s, SSL_AD_INTERNAL_ERROR, + SSL_R_SESSION_ID_CONTEXT_UNINITIALIZED); + return 0; + } if (s->generate_session_id) cb = s->generate_session_id; else if (s->session_ctx->generate_session_id) @@ -428,7 +435,8 @@ SSL_SESSION *lookup_sess_in_cache(SSL *s, const unsigned char *sess_id, memcpy(data.session_id, sess_id, sess_id_len); data.session_id_length = sess_id_len; - CRYPTO_THREAD_read_lock(s->session_ctx->lock); + if (!CRYPTO_THREAD_read_lock(s->session_ctx->lock)) + return NULL; ret = lh_SSL_SESSION_retrieve(s->session_ctx->sessions, &data); if (ret != NULL) { /* don't allow other threads to steal it: */ @@ -643,7 +651,10 @@ int SSL_CTX_add_session(SSL_CTX *ctx, SSL_SESSION *c) * if session c is in already in cache, we take back the increment later */ - CRYPTO_THREAD_write_lock(ctx->lock); + if (!CRYPTO_THREAD_write_lock(ctx->lock)) { + SSL_SESSION_free(c); + return 0; + } s = lh_SSL_SESSION_insert(ctx->sessions, c); /* @@ -716,8 +727,10 @@ static int remove_session_lock(SSL_CTX *ctx, SSL_SESSION *c, int lck) int ret = 0; if ((c != NULL) && (c->session_id_length != 0)) { - if (lck) - CRYPTO_THREAD_write_lock(ctx->lock); + if (lck) { + if (!CRYPTO_THREAD_write_lock(ctx->lock)) + return 0; + } if ((r = lh_SSL_SESSION_retrieve(ctx->sessions, c)) != NULL) { ret = 1; r = lh_SSL_SESSION_delete(ctx->sessions, r); @@ -733,8 +746,7 @@ static int remove_session_lock(SSL_CTX *ctx, SSL_SESSION *c, int lck) if (ret) SSL_SESSION_free(r); - } else - ret = 0; + } return ret; } @@ -1068,7 +1080,8 @@ void SSL_CTX_flush_sessions(SSL_CTX *s, long t) if (tp.cache == NULL) return; tp.time = t; - CRYPTO_THREAD_write_lock(s->lock); + if (!CRYPTO_THREAD_write_lock(s->lock)) + return; i = lh_SSL_SESSION_get_down_load(s->sessions); lh_SSL_SESSION_set_down_load(s->sessions, 0); lh_SSL_SESSION_doall_TIMEOUT_PARAM(tp.cache, timeout_cb, &tp); diff --git a/test/namemap_internal_test.c b/test/namemap_internal_test.c index c1f77561e4..6c6095a40a 100644 --- a/test/namemap_internal_test.c +++ b/test/namemap_internal_test.c @@ -21,11 +21,11 @@ static int test_namemap_empty(void) OSSL_NAMEMAP *nm = NULL; int ok; - ok = TEST_true(ossl_namemap_empty(NULL)) + ok = TEST_int_eq(ossl_namemap_empty(NULL), 1) && TEST_ptr(nm = ossl_namemap_new()) - && TEST_true(ossl_namemap_empty(nm)) + && TEST_int_eq(ossl_namemap_empty(nm), 1) && TEST_int_ne(ossl_namemap_add_name(nm, 0, NAME1), 0) - && TEST_false(ossl_namemap_empty(nm)); + && TEST_int_eq(ossl_namemap_empty(nm), 0); ossl_namemap_free(nm); return ok; }