Move engine library over to using the new thread API

Remove usage of CRYPTO_LOCK_ENGINE
Reviewed-by: NRichard Levitte <levitte@openssl.org>

crypto/engine/eng_ctrl.c

@@ -182,9 +182,9 @@ int ENGINE_ctrl(ENGINE *e, int cmd, long i, void *p, void (*f) (void))
         ENGINEerr(ENGINE_F_ENGINE_CTRL, ERR_R_PASSED_NULL_PARAMETER);
         return 0;
     }
-    CRYPTO_w_lock(CRYPTO_LOCK_ENGINE);
+    CRYPTO_THREAD_write_lock(global_engine_lock);
     ref_exists = ((e->struct_ref > 0) ? 1 : 0);
-    CRYPTO_w_unlock(CRYPTO_LOCK_ENGINE);
+    CRYPTO_THREAD_unlock(global_engine_lock);
     ctrl_exists = ((e->ctrl == NULL) ? 0 : 1);
     if (!ref_exists) {
         ENGINEerr(ENGINE_F_ENGINE_CTRL, ENGINE_R_NO_REFERENCE);

crypto/engine/eng_dyn.c

@@ -217,7 +217,7 @@ static int dynamic_set_data_ctx(ENGINE *e, dynamic_data_ctx **ctx)
     c->DYNAMIC_F1 = "v_check";
     c->DYNAMIC_F2 = "bind_engine";
     c->dir_load = 1;
-    CRYPTO_w_lock(CRYPTO_LOCK_ENGINE);
+    CRYPTO_THREAD_write_lock(global_engine_lock);
     if ((*ctx = (dynamic_data_ctx *)ENGINE_get_ex_data(e,
                                                        dynamic_ex_data_idx))
         == NULL) {
@@ -226,7 +226,7 @@ static int dynamic_set_data_ctx(ENGINE *e, dynamic_data_ctx **ctx)
         *ctx = c;
         c = NULL;
     }
-    CRYPTO_w_unlock(CRYPTO_LOCK_ENGINE);
+    CRYPTO_THREAD_unlock(global_engine_lock);
     /*
      * If we lost the race to set the context, c is non-NULL and *ctx is the
      * context of the thread that won.
@@ -256,14 +256,14 @@ static dynamic_data_ctx *dynamic_get_data_ctx(ENGINE *e)
             ENGINEerr(ENGINE_F_DYNAMIC_GET_DATA_CTX, ENGINE_R_NO_INDEX);
             return NULL;
         }
-        CRYPTO_w_lock(CRYPTO_LOCK_ENGINE);
+        CRYPTO_THREAD_write_lock(global_engine_lock);
         /* Avoid a race by checking again inside this lock */
         if (dynamic_ex_data_idx < 0) {
             /* Good, someone didn't beat us to it */
             dynamic_ex_data_idx = new_idx;
             new_idx = -1;
         }
-        CRYPTO_w_unlock(CRYPTO_LOCK_ENGINE);
+        CRYPTO_THREAD_unlock(global_engine_lock);
         /*
          * In theory we could "give back" the index here if (new_idx>-1), but
          * it's not possible and wouldn't gain us much if it were.

crypto/engine/eng_init.c

@@ -101,10 +101,10 @@ int engine_unlocked_finish(ENGINE *e, int unlock_for_handlers)
     engine_ref_debug(e, 1, -1);
     if ((e->funct_ref == 0) && e->finish) {
         if (unlock_for_handlers)
-            CRYPTO_w_unlock(CRYPTO_LOCK_ENGINE);
+            CRYPTO_THREAD_unlock(global_engine_lock);
         to_return = e->finish(e);
         if (unlock_for_handlers)
-            CRYPTO_w_lock(CRYPTO_LOCK_ENGINE);
+            CRYPTO_THREAD_write_lock(global_engine_lock);
         if (!to_return)
             return 0;
     }
@@ -125,9 +125,10 @@ int ENGINE_init(ENGINE *e)
         ENGINEerr(ENGINE_F_ENGINE_INIT, ERR_R_PASSED_NULL_PARAMETER);
         return 0;
     }
-    CRYPTO_w_lock(CRYPTO_LOCK_ENGINE);
+    CRYPTO_THREAD_run_once(&engine_lock_init, do_engine_lock_init);
+    CRYPTO_THREAD_write_lock(global_engine_lock);
     ret = engine_unlocked_init(e);
-    CRYPTO_w_unlock(CRYPTO_LOCK_ENGINE);
+    CRYPTO_THREAD_unlock(global_engine_lock);
     return ret;
 }
 
@@ -138,9 +139,9 @@ int ENGINE_finish(ENGINE *e)
 
     if (e == NULL)
         return 1;
-    CRYPTO_w_lock(CRYPTO_LOCK_ENGINE);
+    CRYPTO_THREAD_write_lock(global_engine_lock);
     to_return = engine_unlocked_finish(e, 1);
-    CRYPTO_w_unlock(CRYPTO_LOCK_ENGINE);
+    CRYPTO_THREAD_unlock(global_engine_lock);
     if (!to_return) {
         ENGINEerr(ENGINE_F_ENGINE_FINISH, ENGINE_R_FINISH_FAILED);
         return 0;

crypto/engine/eng_int.h

@@ -65,12 +65,15 @@
 # define HEADER_ENGINE_INT_H
 
 # include "internal/cryptlib.h"
+# include "internal/threads.h"
 # include <internal/engine.h>
 
 #ifdef  __cplusplus
 extern "C" {
 #endif
 
+extern CRYPTO_RWLOCK *global_engine_lock;
+
 /*
  * If we compile with this symbol defined, then both reference counts in the
  * ENGINE structure will be monitored with a line of output on stderr for
@@ -97,7 +100,7 @@ extern "C" {
 /*
  * Any code that will need cleanup operations should use these functions to
  * register callbacks. ENGINE_cleanup() will call all registered callbacks in
- * order. NB: both the "add" functions assume CRYPTO_LOCK_ENGINE to already be
+ * order. NB: both the "add" functions assume the engine lock to already be
  * held (in "write" mode).
  */
 typedef void (ENGINE_CLEANUP_CB) (void);
@@ -144,7 +147,7 @@ void engine_table_doall(ENGINE_TABLE *table, engine_table_doall_cb *cb,
 /*
  * Internal versions of API functions that have control over locking. These
  * are used between C files when functionality needs to be shared but the
- * caller may already be controlling of the CRYPTO_LOCK_ENGINE lock.
+ * caller may already be controlling of the engine lock.
  */
 int engine_unlocked_init(ENGINE *e);
 int engine_unlocked_finish(ENGINE *e, int unlock_for_handlers);
@@ -167,6 +170,10 @@ void engine_set_all_null(ENGINE *e);
 void engine_pkey_meths_free(ENGINE *e);
 void engine_pkey_asn1_meths_free(ENGINE *e);
 
+/* Once initialisation function */
+extern CRYPTO_ONCE engine_lock_init;
+void do_engine_lock_init(void);
+
 /*
  * This is a structure for storing implementations of various crypto
  * algorithms and functions.

crypto/engine/eng_lib.c

@@ -59,12 +59,23 @@
 #include "eng_int.h"
 #include <openssl/rand.h>
 
+CRYPTO_RWLOCK *global_engine_lock;
+
+CRYPTO_ONCE engine_lock_init = CRYPTO_ONCE_STATIC_INIT;
+
 /* The "new"/"free" stuff first */
 
+void do_engine_lock_init(void)
+{
+    global_engine_lock = CRYPTO_THREAD_lock_new();
+}
+
 ENGINE *ENGINE_new(void)
 {
     ENGINE *ret;
 
+    CRYPTO_THREAD_run_once(&engine_lock_init, do_engine_lock_init);
+
     ret = OPENSSL_zalloc(sizeof(*ret));
     if (ret == NULL) {
         ENGINEerr(ENGINE_F_ENGINE_NEW, ERR_R_MALLOC_FAILURE);
@@ -108,7 +119,7 @@ int engine_free_util(ENGINE *e, int locked)
     if (e == NULL)
         return 1;
     if (locked)
-        i = CRYPTO_add(&e->struct_ref, -1, CRYPTO_LOCK_ENGINE);
+        CRYPTO_atomic_add(&e->struct_ref, -1, &i, global_engine_lock);
     else
         i = --e->struct_ref;
     engine_ref_debug(e, 0, -1)
@@ -201,6 +212,7 @@ void ENGINE_cleanup(void)
      * registering a cleanup callback.
      */
     RAND_set_rand_method(NULL);
+    CRYPTO_THREAD_lock_free(global_engine_lock);
 }
 
 /* Now the "ex_data" support */

crypto/engine/eng_list.c

@@ -96,7 +96,7 @@ static void engine_list_cleanup(void)
 
 /*
  * These static functions starting with a lower case "engine_" always take
- * place when CRYPTO_LOCK_ENGINE has been locked up.
+ * place when global_engine_lock has been locked up.
  */
 static int engine_list_add(ENGINE *e)
 {
@@ -184,13 +184,14 @@ ENGINE *ENGINE_get_first(void)
 {
     ENGINE *ret;
 
-    CRYPTO_w_lock(CRYPTO_LOCK_ENGINE);
+    CRYPTO_THREAD_run_once(&engine_lock_init, do_engine_lock_init);
+    CRYPTO_THREAD_write_lock(global_engine_lock);
     ret = engine_list_head;
     if (ret) {
         ret->struct_ref++;
         engine_ref_debug(ret, 0, 1);
     }
-    CRYPTO_w_unlock(CRYPTO_LOCK_ENGINE);
+    CRYPTO_THREAD_unlock(global_engine_lock);
     return ret;
 }
 
@@ -198,13 +199,14 @@ ENGINE *ENGINE_get_last(void)
 {
     ENGINE *ret;
 
-    CRYPTO_w_lock(CRYPTO_LOCK_ENGINE);
+    CRYPTO_THREAD_run_once(&engine_lock_init, do_engine_lock_init);
+    CRYPTO_THREAD_write_lock(global_engine_lock);
     ret = engine_list_tail;
     if (ret) {
         ret->struct_ref++;
         engine_ref_debug(ret, 0, 1);
     }
-    CRYPTO_w_unlock(CRYPTO_LOCK_ENGINE);
+    CRYPTO_THREAD_unlock(global_engine_lock);
     return ret;
 }
 
@@ -216,14 +218,14 @@ ENGINE *ENGINE_get_next(ENGINE *e)
         ENGINEerr(ENGINE_F_ENGINE_GET_NEXT, ERR_R_PASSED_NULL_PARAMETER);
         return 0;
     }
-    CRYPTO_w_lock(CRYPTO_LOCK_ENGINE);
+    CRYPTO_THREAD_write_lock(global_engine_lock);
     ret = e->next;
     if (ret) {
         /* Return a valid structural reference to the next ENGINE */
         ret->struct_ref++;
         engine_ref_debug(ret, 0, 1);
     }
-    CRYPTO_w_unlock(CRYPTO_LOCK_ENGINE);
+    CRYPTO_THREAD_unlock(global_engine_lock);
     /* Release the structural reference to the previous ENGINE */
     ENGINE_free(e);
     return ret;
@@ -236,14 +238,14 @@ ENGINE *ENGINE_get_prev(ENGINE *e)
         ENGINEerr(ENGINE_F_ENGINE_GET_PREV, ERR_R_PASSED_NULL_PARAMETER);
         return 0;
     }
-    CRYPTO_w_lock(CRYPTO_LOCK_ENGINE);
+    CRYPTO_THREAD_write_lock(global_engine_lock);
     ret = e->prev;
     if (ret) {
         /* Return a valid structural reference to the next ENGINE */
         ret->struct_ref++;
         engine_ref_debug(ret, 0, 1);
     }
-    CRYPTO_w_unlock(CRYPTO_LOCK_ENGINE);
+    CRYPTO_THREAD_unlock(global_engine_lock);
     /* Release the structural reference to the previous ENGINE */
     ENGINE_free(e);
     return ret;
@@ -261,12 +263,12 @@ int ENGINE_add(ENGINE *e)
         ENGINEerr(ENGINE_F_ENGINE_ADD, ENGINE_R_ID_OR_NAME_MISSING);
         return 0;
     }
-    CRYPTO_w_lock(CRYPTO_LOCK_ENGINE);
+    CRYPTO_THREAD_write_lock(global_engine_lock);
     if (!engine_list_add(e)) {
         ENGINEerr(ENGINE_F_ENGINE_ADD, ENGINE_R_INTERNAL_LIST_ERROR);
         to_return = 0;
     }
-    CRYPTO_w_unlock(CRYPTO_LOCK_ENGINE);
+    CRYPTO_THREAD_unlock(global_engine_lock);
     return to_return;
 }
 
@@ -278,12 +280,12 @@ int ENGINE_remove(ENGINE *e)
         ENGINEerr(ENGINE_F_ENGINE_REMOVE, ERR_R_PASSED_NULL_PARAMETER);
         return 0;
     }
-    CRYPTO_w_lock(CRYPTO_LOCK_ENGINE);
+    CRYPTO_THREAD_write_lock(global_engine_lock);
     if (!engine_list_remove(e)) {
         ENGINEerr(ENGINE_F_ENGINE_REMOVE, ENGINE_R_INTERNAL_LIST_ERROR);
         to_return = 0;
     }
-    CRYPTO_w_unlock(CRYPTO_LOCK_ENGINE);
+    CRYPTO_THREAD_unlock(global_engine_lock);
     return to_return;
 }
 
@@ -325,7 +327,8 @@ ENGINE *ENGINE_by_id(const char *id)
         ENGINEerr(ENGINE_F_ENGINE_BY_ID, ERR_R_PASSED_NULL_PARAMETER);
         return NULL;
     }
-    CRYPTO_w_lock(CRYPTO_LOCK_ENGINE);
+    CRYPTO_THREAD_run_once(&engine_lock_init, do_engine_lock_init);
+    CRYPTO_THREAD_write_lock(global_engine_lock);
     iterator = engine_list_head;
     while (iterator && (strcmp(id, iterator->id) != 0))
         iterator = iterator->next;
@@ -348,7 +351,7 @@ ENGINE *ENGINE_by_id(const char *id)
             engine_ref_debug(iterator, 0, 1);
         }
     }
-    CRYPTO_w_unlock(CRYPTO_LOCK_ENGINE);
+    CRYPTO_THREAD_unlock(global_engine_lock);
     if (iterator != NULL)
         return iterator;
     /*
@@ -377,10 +380,11 @@ ENGINE *ENGINE_by_id(const char *id)
 
 int ENGINE_up_ref(ENGINE *e)
 {
+    int i;
     if (e == NULL) {
         ENGINEerr(ENGINE_F_ENGINE_UP_REF, ERR_R_PASSED_NULL_PARAMETER);
         return 0;
     }
-    CRYPTO_add(&e->struct_ref, 1, CRYPTO_LOCK_ENGINE);
+    CRYPTO_atomic_add(&e->struct_ref, 1, &i, global_engine_lock);
     return 1;
 }

crypto/engine/eng_pkey.c

@@ -105,13 +105,13 @@ EVP_PKEY *ENGINE_load_private_key(ENGINE *e, const char *key_id,
                   ERR_R_PASSED_NULL_PARAMETER);
         return 0;
     }
-    CRYPTO_w_lock(CRYPTO_LOCK_ENGINE);
+    CRYPTO_THREAD_write_lock(global_engine_lock);
     if (e->funct_ref == 0) {
-        CRYPTO_w_unlock(CRYPTO_LOCK_ENGINE);
+        CRYPTO_THREAD_unlock(global_engine_lock);
         ENGINEerr(ENGINE_F_ENGINE_LOAD_PRIVATE_KEY, ENGINE_R_NOT_INITIALISED);
         return 0;
     }
-    CRYPTO_w_unlock(CRYPTO_LOCK_ENGINE);
+    CRYPTO_THREAD_unlock(global_engine_lock);
     if (!e->load_privkey) {
         ENGINEerr(ENGINE_F_ENGINE_LOAD_PRIVATE_KEY,
                   ENGINE_R_NO_LOAD_FUNCTION);
@@ -136,13 +136,13 @@ EVP_PKEY *ENGINE_load_public_key(ENGINE *e, const char *key_id,
                   ERR_R_PASSED_NULL_PARAMETER);
         return 0;
     }
-    CRYPTO_w_lock(CRYPTO_LOCK_ENGINE);
+    CRYPTO_THREAD_write_lock(global_engine_lock);
     if (e->funct_ref == 0) {
-        CRYPTO_w_unlock(CRYPTO_LOCK_ENGINE);
+        CRYPTO_THREAD_unlock(global_engine_lock);
         ENGINEerr(ENGINE_F_ENGINE_LOAD_PUBLIC_KEY, ENGINE_R_NOT_INITIALISED);
         return 0;
     }
-    CRYPTO_w_unlock(CRYPTO_LOCK_ENGINE);
+    CRYPTO_THREAD_unlock(global_engine_lock);
     if (!e->load_pubkey) {
         ENGINEerr(ENGINE_F_ENGINE_LOAD_PUBLIC_KEY, ENGINE_R_NO_LOAD_FUNCTION);
         return 0;
@@ -167,14 +167,14 @@ int ENGINE_load_ssl_client_cert(ENGINE *e, SSL *s,
                   ERR_R_PASSED_NULL_PARAMETER);
         return 0;
     }
-    CRYPTO_w_lock(CRYPTO_LOCK_ENGINE);
+    CRYPTO_THREAD_write_lock(global_engine_lock);
     if (e->funct_ref == 0) {
-        CRYPTO_w_unlock(CRYPTO_LOCK_ENGINE);
+        CRYPTO_THREAD_unlock(global_engine_lock);
         ENGINEerr(ENGINE_F_ENGINE_LOAD_SSL_CLIENT_CERT,
                   ENGINE_R_NOT_INITIALISED);
         return 0;
     }
-    CRYPTO_w_unlock(CRYPTO_LOCK_ENGINE);
+    CRYPTO_THREAD_unlock(global_engine_lock);
     if (!e->load_ssl_client_cert) {
         ENGINEerr(ENGINE_F_ENGINE_LOAD_SSL_CLIENT_CERT,
                   ENGINE_R_NO_LOAD_FUNCTION);

crypto/engine/eng_table.c

@@ -130,7 +130,7 @@ int engine_table_register(ENGINE_TABLE **table, ENGINE_CLEANUP_CB *cleanup,
 {
     int ret = 0, added = 0;
     ENGINE_PILE tmplate, *fnd;
-    CRYPTO_w_lock(CRYPTO_LOCK_ENGINE);
+    CRYPTO_THREAD_write_lock(global_engine_lock);
     if (!(*table))
         added = 1;
     if (!int_table_check(table, 1))
@@ -179,7 +179,7 @@ int engine_table_register(ENGINE_TABLE **table, ENGINE_CLEANUP_CB *cleanup,
     }
     ret = 1;
  end:
-    CRYPTO_w_unlock(CRYPTO_LOCK_ENGINE);
+    CRYPTO_THREAD_unlock(global_engine_lock);
     return ret;
 }
 
@@ -201,10 +201,10 @@ IMPLEMENT_LHASH_DOALL_ARG(ENGINE_PILE, ENGINE);
 
 void engine_table_unregister(ENGINE_TABLE **table, ENGINE *e)
 {
-    CRYPTO_w_lock(CRYPTO_LOCK_ENGINE);
+    CRYPTO_THREAD_write_lock(global_engine_lock);
     if (int_table_check(table, 0))
         lh_ENGINE_PILE_doall_ENGINE(&(*table)->piles, int_unregister_cb, e);
-    CRYPTO_w_unlock(CRYPTO_LOCK_ENGINE);
+    CRYPTO_THREAD_unlock(global_engine_lock);
 }
 
 static void int_cleanup_cb_doall(ENGINE_PILE *p)
@@ -219,13 +219,13 @@ static void int_cleanup_cb_doall(ENGINE_PILE *p)
 
 void engine_table_cleanup(ENGINE_TABLE **table)
 {
-    CRYPTO_w_lock(CRYPTO_LOCK_ENGINE);
+    CRYPTO_THREAD_write_lock(global_engine_lock);
     if (*table) {
         lh_ENGINE_PILE_doall(&(*table)->piles, int_cleanup_cb_doall);
         lh_ENGINE_PILE_free(&(*table)->piles);
         *table = NULL;
     }
-    CRYPTO_w_unlock(CRYPTO_LOCK_ENGINE);
+    CRYPTO_THREAD_unlock(global_engine_lock);
 }
 
 /* return a functional reference for a given 'nid' */
@@ -248,7 +248,7 @@ ENGINE *engine_table_select_tmp(ENGINE_TABLE **table, int nid, const char *f,
         return NULL;
     }
     ERR_set_mark();
-    CRYPTO_w_lock(CRYPTO_LOCK_ENGINE);
+    CRYPTO_THREAD_write_lock(global_engine_lock);
     /*
      * Check again inside the lock otherwise we could race against cleanup
      * operations. But don't worry about a fprintf(stderr).
@@ -319,7 +319,7 @@ ENGINE *engine_table_select_tmp(ENGINE_TABLE **table, int nid, const char *f,
         fprintf(stderr, "engine_table_dbg: %s:%d, nid=%d, caching "
                 "'no matching ENGINE'\n", f, l, nid);
 #endif
-    CRYPTO_w_unlock(CRYPTO_LOCK_ENGINE);
+    CRYPTO_THREAD_unlock(global_engine_lock);
     /*
      * Whatever happened, any failed init()s are not failures in this
      * context, so clear our error state.

crypto/engine/tb_asnmth.c

@@ -233,7 +233,9 @@ const EVP_PKEY_ASN1_METHOD *ENGINE_pkey_asn1_find_str(ENGINE **pe,
     fstr.ameth = NULL;
     fstr.str = str;
     fstr.len = len;
-    CRYPTO_w_lock(CRYPTO_LOCK_ENGINE);
+
+    CRYPTO_THREAD_run_once(&engine_lock_init, do_engine_lock_init);
+    CRYPTO_THREAD_write_lock(global_engine_lock);
     engine_table_doall(pkey_asn1_meth_table, look_str_cb, &fstr);
     /* If found obtain a structural reference to engine */
     if (fstr.e) {
@@ -241,6 +243,6 @@ const EVP_PKEY_ASN1_METHOD *ENGINE_pkey_asn1_find_str(ENGINE **pe,
         engine_ref_debug(fstr.e, 0, 1);
     }
     *pe = fstr.e;
-    CRYPTO_w_unlock(CRYPTO_LOCK_ENGINE);
+    CRYPTO_THREAD_unlock(global_engine_lock);
     return fstr.ameth;
 }