diff options
author | Rich Salz <rsalz@akamai.com> | 2021-02-18 15:31:56 -0500 |
---|---|---|
committer | Pauli <ppzgs1@gmail.com> | 2021-03-14 15:33:34 +1000 |
commit | cd3f8c1b11b0b9f4163bc8c62cbae38aec1b4030 (patch) | |
tree | de59d50b2ff9b2bd73a1ebf08eedf78d8ba44aa3 /crypto/engine | |
parent | f62846b703d163265176fe960ec7d087b4c3fa96 (diff) |
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.lontis@oracle.com>
Reviewed-by: Paul Dale <pauli@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/14238)
Diffstat (limited to 'crypto/engine')
-rw-r--r-- | crypto/engine/eng_ctrl.c | 3 | ||||
-rw-r--r-- | crypto/engine/eng_dyn.c | 15 | ||||
-rw-r--r-- | crypto/engine/eng_init.c | 9 | ||||
-rw-r--r-- | crypto/engine/eng_list.c | 25 | ||||
-rw-r--r-- | crypto/engine/eng_pkey.c | 25 | ||||
-rw-r--r-- | crypto/engine/eng_table.c | 14 | ||||
-rw-r--r-- | crypto/engine/tb_asnmth.c | 3 |
7 files changed, 59 insertions, 35 deletions
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) { |