From 8509dcc9f319190c565ab6baad7c88d37a951d1c Mon Sep 17 00:00:00 2001 From: Alessandro Ghedini Date: Wed, 2 Mar 2016 17:36:17 +0000 Subject: [PATCH] Convert ERR_STATE to new multi-threading API Reviewed-by: Matt Caswell Reviewed-by: Rich Salz --- crypto/err/err.c | 194 +++++--------------------------- crypto/err/err_lcl.h | 2 - crypto/init.c | 4 +- doc/crypto/ERR_remove_state.pod | 14 +-- doc/crypto/err.pod | 11 +- include/openssl/err.h | 5 +- util/libcrypto.num | 4 +- 7 files changed, 46 insertions(+), 188 deletions(-) delete mode 100644 crypto/err/err_lcl.h diff --git a/crypto/err/err.c b/crypto/err/err.c index 181882d04e..36ba9c74ef 100644 --- a/crypto/err/err.c +++ b/crypto/err/err.c @@ -112,13 +112,13 @@ #include #include #include +#include #include #include #include #include #include #include -#include "err_lcl.h" static void err_load_strings(int lib, ERR_STRING_DATA *str); @@ -231,25 +231,18 @@ static ERR_STRING_DATA ERR_str_reasons[] = { }; #endif +static CRYPTO_ONCE err_init = CRYPTO_ONCE_STATIC_INIT; +static CRYPTO_THREAD_LOCAL err_thread_local; + /* Predeclarations of the "err_defaults" functions */ static LHASH_OF(ERR_STRING_DATA) *get_hash(int create, int lockit); static ERR_STRING_DATA *int_err_get_item(const ERR_STRING_DATA *); -static LHASH_OF(ERR_STATE) *int_thread_get(int create, int lockit); -static void int_thread_release(LHASH_OF(ERR_STATE) **hash); -static ERR_STATE *int_thread_get_item(const ERR_STATE *); -static ERR_STATE *int_thread_set_item(ERR_STATE *); -static void int_thread_del_item(const ERR_STATE *); /* * The internal state */ -/* This is a struct so that REF_PRINT_COUNT works. */ -static struct refcount { - int references; -} refcount = { 0 }; static LHASH_OF(ERR_STRING_DATA) *int_error_hash = NULL; -static LHASH_OF(ERR_STATE) *int_thread_hash = NULL; static int int_err_library_number = ERR_LIB_USER; static unsigned long get_error_values(int inc, int top, const char **file, @@ -303,106 +296,6 @@ static ERR_STRING_DATA *int_err_get_item(const ERR_STRING_DATA *d) return p; } -static unsigned long err_state_hash(const ERR_STATE *a) -{ - return CRYPTO_THREADID_hash(&a->tid) * 13; -} - -static int err_state_cmp(const ERR_STATE *a, const ERR_STATE *b) -{ - return CRYPTO_THREADID_cmp(&a->tid, &b->tid); -} - -static LHASH_OF(ERR_STATE) *int_thread_get(int create, int lockit) -{ - LHASH_OF(ERR_STATE) *ret = NULL; - - if (lockit) - CRYPTO_w_lock(CRYPTO_LOCK_ERR); - if (!int_thread_hash && create) { - int_thread_hash = lh_ERR_STATE_new(err_state_hash, err_state_cmp); - } - if (int_thread_hash != NULL) { - refcount.references++; - ret = int_thread_hash; - } - if (lockit) - CRYPTO_w_unlock(CRYPTO_LOCK_ERR); - return ret; -} - -static void int_thread_release(LHASH_OF(ERR_STATE) **hash) -{ - int i; - - if (hash == NULL || *hash == NULL) - return; - - i = CRYPTO_add(&refcount.references, -1, CRYPTO_LOCK_ERR); - - REF_PRINT_COUNT(&refcount, "ERR"); - if (i > 0) - return; - REF_ASSERT_ISNT(i < 0); - *hash = NULL; -} - -static ERR_STATE *int_thread_get_item(const ERR_STATE *d) -{ - ERR_STATE *p = NULL; - LHASH_OF(ERR_STATE) *hash; - - CRYPTO_r_lock(CRYPTO_LOCK_ERR); - hash = int_thread_get(0, 0); - if (hash) - p = lh_ERR_STATE_retrieve(hash, d); - CRYPTO_r_unlock(CRYPTO_LOCK_ERR); - - int_thread_release(&hash); - return p; -} - -static ERR_STATE *int_thread_set_item(ERR_STATE *d) -{ - ERR_STATE *p = NULL; - LHASH_OF(ERR_STATE) *hash; - - CRYPTO_w_lock(CRYPTO_LOCK_ERR); - hash = int_thread_get(1, 0); - if (hash) - p = lh_ERR_STATE_insert(hash, d); - CRYPTO_w_unlock(CRYPTO_LOCK_ERR); - - int_thread_release(&hash); - return p; -} - -static void int_thread_del_item(const ERR_STATE *d) -{ - ERR_STATE *p = NULL; - LHASH_OF(ERR_STATE) *hash; - - CRYPTO_w_lock(CRYPTO_LOCK_ERR); - hash = int_thread_get(0, 0); - if (hash) { - p = lh_ERR_STATE_delete(hash, d); - /* If there are no other references, and we just removed the - * last item, delete the int_thread_hash */ - if (refcount.references == 1 - && int_thread_hash - && lh_ERR_STATE_num_items(int_thread_hash) == 0) { - refcount.references = 0; - lh_ERR_STATE_free(int_thread_hash); - int_thread_hash = NULL; - hash = NULL; - } - } - CRYPTO_w_unlock(CRYPTO_LOCK_ERR); - - int_thread_release(&hash); - ERR_STATE_free(p); -} - #ifndef OPENSSL_NO_ERR # define NUM_SYS_STR_REASONS 127 # define LEN_SYS_STR_REASON 32 @@ -768,7 +661,6 @@ void ERR_error_string_n(unsigned long e, char *buf, size_t len) } } -/* BAD for multi-threading: uses a local buffer if ret == NULL */ /* * ERR_error_string_n should be used instead for ret != NULL as * ERR_error_string cannot know how large the buffer is @@ -789,16 +681,6 @@ LHASH_OF(ERR_STRING_DATA) *ERR_get_string_table(void) return get_hash(0, 1); } -LHASH_OF(ERR_STATE) *ERR_get_err_state_table(void) -{ - return int_thread_get(0, 1); -} - -void ERR_release_err_state_table(LHASH_OF(ERR_STATE) **hash) -{ - int_thread_release(hash); -} - const char *ERR_lib_error_string(unsigned long e) { ERR_STRING_DATA d, *p; @@ -838,68 +720,52 @@ const char *ERR_reason_error_string(unsigned long e) return ((p == NULL) ? NULL : p->string); } -void ERR_remove_thread_state(const CRYPTO_THREADID *id) +void ERR_remove_thread_state(void) { - ERR_STATE tmp; + ERR_STATE *state = ERR_get_state(); + if (state == NULL) + return; - if (id) - CRYPTO_THREADID_cpy(&tmp.tid, id); - else - CRYPTO_THREADID_current(&tmp.tid); - /* - * thread_del_item automatically destroys the LHASH if the number of - * items reaches zero. - */ - int_thread_del_item(&tmp); + CRYPTO_THREAD_set_local(&err_thread_local, NULL); + ERR_STATE_free(state); } #if OPENSSL_API_COMPAT < 0x10000000L void ERR_remove_state(unsigned long pid) { - ERR_remove_thread_state(NULL); + ERR_remove_thread_state(); } #endif +static void err_do_init(void) +{ + CRYPTO_THREAD_init_local(&err_thread_local, NULL); +} + ERR_STATE *ERR_get_state(void) { - static ERR_STATE fallback; - ERR_STATE *ret, tmp, *tmpp = NULL; - int i; - CRYPTO_THREADID tid; + ERR_STATE *state = NULL; - CRYPTO_THREADID_current(&tid); - CRYPTO_THREADID_cpy(&tmp.tid, &tid); - ret = int_thread_get_item(&tmp); + CRYPTO_THREAD_run_once(&err_init, err_do_init); - /* ret == the error state, if NULL, make a new one */ - if (ret == NULL) { - ret = OPENSSL_malloc(sizeof(*ret)); - if (ret == NULL) - return (&fallback); - CRYPTO_THREADID_cpy(&ret->tid, &tid); - ret->top = 0; - ret->bottom = 0; - for (i = 0; i < ERR_NUM_ERRORS; i++) { - ret->err_data[i] = NULL; - ret->err_data_flags[i] = 0; + state = CRYPTO_THREAD_get_local(&err_thread_local); + + if (state == NULL) { + state = OPENSSL_zalloc(sizeof(*state)); + if (state == NULL) + return NULL; + + if (!CRYPTO_THREAD_set_local(&err_thread_local, state)) { + ERR_STATE_free(state); + return NULL; } - tmpp = int_thread_set_item(ret); - /* To check if insertion failed, do a get. */ - if (int_thread_get_item(ret) != ret) { - ERR_STATE_free(ret); /* could not insert it */ - return (&fallback); - } - /* - * If a race occurred in this function and we came second, tmpp is - * the first one that we just replaced. - */ - ERR_STATE_free(tmpp); /* Ignore failures from these */ OPENSSL_init_crypto(OPENSSL_INIT_LOAD_CRYPTO_STRINGS, NULL); ossl_init_thread_start(OPENSSL_INIT_THREAD_ERR_STATE); } - return ret; + + return state; } int ERR_get_next_error_library(void) diff --git a/crypto/err/err_lcl.h b/crypto/err/err_lcl.h deleted file mode 100644 index c9d24f111e..0000000000 --- a/crypto/err/err_lcl.h +++ /dev/null @@ -1,2 +0,0 @@ - -DEFINE_LHASH_OF(ERR_STATE); diff --git a/crypto/init.c b/crypto/init.c index 44acd4f4df..b0826ee41c 100644 --- a/crypto/init.c +++ b/crypto/init.c @@ -360,9 +360,9 @@ static void ossl_init_thread_stop(struct thread_local_inits_st *locals) if (locals->err_state) { #ifdef OPENSSL_INIT_DEBUG fprintf(stderr, "OPENSSL_INIT: ossl_init_thread_stop: " - "ERR_remove_thread_state(NULL)\n"); + "ERR_remove_thread_state()\n"); #endif - ERR_remove_thread_state(NULL); + ERR_remove_thread_state(); } OPENSSL_free(locals); diff --git a/doc/crypto/ERR_remove_state.pod b/doc/crypto/ERR_remove_state.pod index 55ded84c48..b011182c47 100644 --- a/doc/crypto/ERR_remove_state.pod +++ b/doc/crypto/ERR_remove_state.pod @@ -8,7 +8,7 @@ ERR_remove_thread_state, ERR_remove_state - free a thread's error queue #include - void ERR_remove_thread_state(const CRYPTO_THREADID *tid); + void ERR_remove_thread_state(void); Deprecated: @@ -18,17 +18,16 @@ Deprecated: =head1 DESCRIPTION -ERR_remove_thread_state() frees the error queue associated with thread B. -If B == B, the current thread will have its error queue removed. +ERR_remove_thread_state() frees the error queue associated with the current +thread. Since error queue data structures are allocated automatically for new threads, they must be freed when threads are terminated in order to avoid memory leaks. ERR_remove_state is deprecated and has been replaced by -ERR_remove_thread_state. Since threads in OpenSSL are no longer identified -by unsigned long values any argument to this function is ignored. Calling -ERR_remove_state is equivalent to B. +ERR_remove_thread_state. Any argument to this function is ignored and +calling ERR_remove_state is equivalent to B. =head1 RETURN VALUE @@ -41,7 +40,6 @@ L =head1 HISTORY ERR_remove_state() -was deprecated in OpenSSL 1.0.0 when ERR_remove_thread_state() was introduced -and thread IDs were introduced to identify threads instead of 'unsigned long'. +was deprecated in OpenSSL 1.0.0 when ERR_remove_thread_state() was introduced. =cut diff --git a/doc/crypto/err.pod b/doc/crypto/err.pod index 5fafbc5490..6847f2153c 100644 --- a/doc/crypto/err.pod +++ b/doc/crypto/err.pod @@ -22,7 +22,7 @@ err - error codes int ERR_GET_REASON(unsigned long e); void ERR_clear_error(void); - void ERR_remove_thread_state(const CRYPTO_THREADID *tid); + void ERR_remove_thread_state(void); char *ERR_error_string(unsigned long e, char *buf); const char *ERR_lib_error_string(unsigned long e); @@ -164,15 +164,14 @@ TBA more details =head1 INTERNALS -The error queues are stored in a hash table with one B -entry for each pid. ERR_get_state() returns the current thread's +The error queues are stored in a thread-local storage with one B +entry for each thread. ERR_get_state() returns the current thread's B. An B can hold up to B error codes. When more error codes are added, the old ones are overwritten, on the assumption that the most recent errors are most important. -Error strings are also stored in hash table. The hash tables can -be obtained by calling ERR_get_err_state_table(void) and -ERR_get_string_table(void) respectively. +Error strings are also stored in a hash table that can be obtained +by calling ERR_get_string_table(void). =head1 SEE ALSO diff --git a/include/openssl/err.h b/include/openssl/err.h index 0b12d927c2..4b6c663bb2 100644 --- a/include/openssl/err.h +++ b/include/openssl/err.h @@ -141,7 +141,6 @@ extern "C" { # define ERR_NUM_ERRORS 16 typedef struct err_state_st { - CRYPTO_THREADID tid; int err_flags[ERR_NUM_ERRORS]; unsigned long err_buffer[ERR_NUM_ERRORS]; char *err_data[ERR_NUM_ERRORS]; @@ -366,14 +365,12 @@ void ERR_load_ERR_strings(void); void ERR_free_strings(void); -void ERR_remove_thread_state(const CRYPTO_THREADID *tid); +void ERR_remove_thread_state(void); DEPRECATEDIN_1_0_0(void ERR_remove_state(unsigned long pid)) /* if zero we * look it up */ ERR_STATE *ERR_get_state(void); LHASH_OF(ERR_STRING_DATA) *ERR_get_string_table(void); -LHASH_OF(ERR_STATE) *ERR_get_err_state_table(void); -void ERR_release_err_state_table(LHASH_OF(ERR_STATE) **hash); int ERR_get_next_error_library(void); diff --git a/util/libcrypto.num b/util/libcrypto.num index 28e78e4efd..3ae4500258 100644 --- a/util/libcrypto.num +++ b/util/libcrypto.num @@ -1047,7 +1047,7 @@ X509at_get_attr_by_OBJ 1015 1_1_0 EXIST::FUNCTION: EVP_MD_CTX_copy_ex 1016 1_1_0 EXIST::FUNCTION: UI_dup_error_string 1017 1_1_0 EXIST::FUNCTION: lh_num_items 1018 1_1_0 EXIST::FUNCTION: -ERR_get_err_state_table 1019 1_1_0 EXIST::FUNCTION: +ERR_get_err_state_table 1019 1_1_0 NOEXIST::FUNCTION: ASN1_INTEGER_cmp 1020 1_1_0 EXIST::FUNCTION: X509_NAME_entry_count 1021 1_1_0 EXIST::FUNCTION: UI_method_set_closer 1022 1_1_0 EXIST::FUNCTION: @@ -3507,7 +3507,7 @@ i2d_PBE2PARAM 3389 1_1_0 EXIST::FUNCTION: EC_POINT_make_affine 3390 1_1_0 EXIST::FUNCTION:EC DSA_generate_parameters 3391 1_1_0 EXIST::FUNCTION:DEPRECATEDIN_0_9_8,DSA ASN1_BIT_STRING_num_asc 3392 1_1_0 EXIST::FUNCTION: -ERR_release_err_state_table 3393 1_1_0 EXIST::FUNCTION: +ERR_release_err_state_table 3393 1_1_0 NOEXIST::FUNCTION: X509_INFO_free 3394 1_1_0 EXIST::FUNCTION: d2i_PKCS8_PRIV_KEY_INFO_fp 3395 1_1_0 EXIST::FUNCTION:STDIO X509_OBJECT_retrieve_match 3396 1_1_0 EXIST::FUNCTION: