From 40e068d50687edbf9b6d6633563151ab59369747 Mon Sep 17 00:00:00 2001 From: Matt Caswell Date: Tue, 8 Mar 2016 16:44:34 +0000 Subject: [PATCH] Move engine library over to using the new thread API Remove usage of CRYPTO_LOCK_ENGINE Reviewed-by: Richard Levitte --- crypto/engine/eng_ctrl.c | 4 ++-- crypto/engine/eng_dyn.c | 8 ++++---- crypto/engine/eng_init.c | 13 +++++++------ crypto/engine/eng_int.h | 11 +++++++++-- crypto/engine/eng_lib.c | 14 +++++++++++++- crypto/engine/eng_list.c | 36 ++++++++++++++++++++---------------- crypto/engine/eng_pkey.c | 18 +++++++++--------- crypto/engine/eng_table.c | 16 ++++++++-------- crypto/engine/tb_asnmth.c | 6 ++++-- 9 files changed, 76 insertions(+), 50 deletions(-) diff --git a/crypto/engine/eng_ctrl.c b/crypto/engine/eng_ctrl.c index 2db9902822..c912c1d913 100644 --- a/crypto/engine/eng_ctrl.c +++ b/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); diff --git a/crypto/engine/eng_dyn.c b/crypto/engine/eng_dyn.c index 597151b087..bdfc00cce6 100644 --- a/crypto/engine/eng_dyn.c +++ b/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. diff --git a/crypto/engine/eng_init.c b/crypto/engine/eng_init.c index ddf552a537..f3c0de99f9 100644 --- a/crypto/engine/eng_init.c +++ b/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; diff --git a/crypto/engine/eng_int.h b/crypto/engine/eng_int.h index 0eee21163a..f6a54d8567 100644 --- a/crypto/engine/eng_int.h +++ b/crypto/engine/eng_int.h @@ -65,12 +65,15 @@ # define HEADER_ENGINE_INT_H # include "internal/cryptlib.h" +# include "internal/threads.h" # include #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. diff --git a/crypto/engine/eng_lib.c b/crypto/engine/eng_lib.c index 15c2c6ff68..dd4734202e 100644 --- a/crypto/engine/eng_lib.c +++ b/crypto/engine/eng_lib.c @@ -59,12 +59,23 @@ #include "eng_int.h" #include +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 */ diff --git a/crypto/engine/eng_list.c b/crypto/engine/eng_list.c index 8ca1f80041..4883d14fff 100644 --- a/crypto/engine/eng_list.c +++ b/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; } diff --git a/crypto/engine/eng_pkey.c b/crypto/engine/eng_pkey.c index e581ac3113..ee6ba28440 100644 --- a/crypto/engine/eng_pkey.c +++ b/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); diff --git a/crypto/engine/eng_table.c b/crypto/engine/eng_table.c index 22390d2b4d..9648c0cb94 100644 --- a/crypto/engine/eng_table.c +++ b/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. diff --git a/crypto/engine/tb_asnmth.c b/crypto/engine/tb_asnmth.c index b0d0d50974..28aa8e8cd4 100644 --- a/crypto/engine/tb_asnmth.c +++ b/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; } -- 2.34.1