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)

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) {