From 1ec7eff1b2d761592df564d7ee92f65e08da4cd6 Mon Sep 17 00:00:00 2001 From: Rich Salz Date: Wed, 7 Jun 2017 11:23:37 -0400 Subject: Add a lock around the OBJ_NAME table Various initialization functions modify this table, which can cause heap corruption in the absence of external synchronization. Some stats are modified from OPENSSL_LH_retrieve, where callers aren't expecting to have to take out an exclusive lock. Switch to using atomic operations for those stats. Reviewed-by: Matt Caswell Reviewed-by: Rich Salz (Merged from https://github.com/openssl/openssl/pull/3525) (cherry picked from commit be606c013d31847718ceb5d97c567988a771c2e5) --- crypto/objects/o_names.c | 93 +++++++++++++++++++++++++++++++++--------------- 1 file changed, 65 insertions(+), 28 deletions(-) (limited to 'crypto/objects') diff --git a/crypto/objects/o_names.c b/crypto/objects/o_names.c index ed98df8c2f..15fe653d09 100644 --- a/crypto/objects/o_names.c +++ b/crypto/objects/o_names.c @@ -16,6 +16,7 @@ #include #include #include +#include #include "obj_lcl.h" /* @@ -44,6 +45,7 @@ static int obj_strcmp(const char *a, const char *b) */ static LHASH_OF(OBJ_NAME) *names_lh = NULL; static int names_type_num = OBJ_NAME_TYPE_NUM; +static CRYPTO_RWLOCK *lock = NULL; struct name_funcs_st { unsigned long (*hash_func) (const char *name); @@ -62,23 +64,33 @@ static STACK_OF(NAME_FUNCS) *name_funcs_stack; static unsigned long obj_name_hash(const OBJ_NAME *a); static int obj_name_cmp(const OBJ_NAME *a, const OBJ_NAME *b); -int OBJ_NAME_init(void) +static CRYPTO_ONCE init = CRYPTO_ONCE_STATIC_INIT; +DEFINE_RUN_ONCE_STATIC(o_names_init) { - if (names_lh != NULL) - return (1); CRYPTO_mem_ctrl(CRYPTO_MEM_CHECK_DISABLE); names_lh = lh_OBJ_NAME_new(obj_name_hash, obj_name_cmp); + lock = CRYPTO_THREAD_lock_new(); CRYPTO_mem_ctrl(CRYPTO_MEM_CHECK_ENABLE); - return (names_lh != NULL); + return names_lh != NULL && lock != NULL; +} + +int OBJ_NAME_init(void) +{ + return RUN_ONCE(&init, o_names_init); } int OBJ_NAME_new_index(unsigned long (*hash_func) (const char *), int (*cmp_func) (const char *, const char *), void (*free_func) (const char *, int, const char *)) { - int ret, i, push; + int ret = 0, i, push; NAME_FUNCS *name_funcs; + if (!OBJ_NAME_init()) + return 0; + + CRYPTO_THREAD_write_lock(lock); + if (name_funcs_stack == NULL) { CRYPTO_mem_ctrl(CRYPTO_MEM_CHECK_DISABLE); name_funcs_stack = sk_NAME_FUNCS_new_null(); @@ -86,7 +98,7 @@ int OBJ_NAME_new_index(unsigned long (*hash_func) (const char *), } if (name_funcs_stack == NULL) { /* ERROR */ - return (0); + goto out; } ret = names_type_num; names_type_num++; @@ -96,7 +108,8 @@ int OBJ_NAME_new_index(unsigned long (*hash_func) (const char *), CRYPTO_mem_ctrl(CRYPTO_MEM_CHECK_ENABLE); if (name_funcs == NULL) { OBJerr(OBJ_F_OBJ_NAME_NEW_INDEX, ERR_R_MALLOC_FAILURE); - return (0); + ret = 0; + goto out; } name_funcs->hash_func = OPENSSL_LH_strhash; name_funcs->cmp_func = obj_strcmp; @@ -108,7 +121,8 @@ int OBJ_NAME_new_index(unsigned long (*hash_func) (const char *), if (!push) { OBJerr(OBJ_F_OBJ_NAME_NEW_INDEX, ERR_R_MALLOC_FAILURE); OPENSSL_free(name_funcs); - return 0; + ret = 0; + goto out; } } name_funcs = sk_NAME_FUNCS_value(name_funcs_stack, ret); @@ -118,7 +132,10 @@ int OBJ_NAME_new_index(unsigned long (*hash_func) (const char *), name_funcs->cmp_func = cmp_func; if (free_func != NULL) name_funcs->free_func = free_func; - return (ret); + +out: + CRYPTO_THREAD_unlock(lock); + return ret; } static int obj_name_cmp(const OBJ_NAME *a, const OBJ_NAME *b) @@ -134,7 +151,7 @@ static int obj_name_cmp(const OBJ_NAME *a, const OBJ_NAME *b) } else ret = strcmp(a->name, b->name); } - return (ret); + return ret; } static unsigned long obj_name_hash(const OBJ_NAME *a) @@ -150,18 +167,20 @@ static unsigned long obj_name_hash(const OBJ_NAME *a) ret = OPENSSL_LH_strhash(a->name); } ret ^= a->type; - return (ret); + return ret; } const char *OBJ_NAME_get(const char *name, int type) { OBJ_NAME on, *ret; int num = 0, alias; + const char *value = NULL; if (name == NULL) - return (NULL); - if ((names_lh == NULL) && !OBJ_NAME_init()) - return (NULL); + return NULL; + if (!OBJ_NAME_init()) + return NULL; + CRYPTO_THREAD_read_lock(lock); alias = type & OBJ_NAME_ALIAS; type &= ~OBJ_NAME_ALIAS; @@ -172,24 +191,30 @@ const char *OBJ_NAME_get(const char *name, int type) for (;;) { ret = lh_OBJ_NAME_retrieve(names_lh, &on); if (ret == NULL) - return (NULL); + break; if ((ret->alias) && !alias) { if (++num > 10) - return (NULL); + break; on.name = ret->data; } else { - return (ret->data); + value = ret->data; + break; } } + + CRYPTO_THREAD_unlock(lock); + return value; } int OBJ_NAME_add(const char *name, int type, const char *data) { OBJ_NAME *onp, *ret; - int alias; + int alias, ok = 0; - if ((names_lh == NULL) && !OBJ_NAME_init()) - return (0); + if (!OBJ_NAME_init()) + return 0; + + CRYPTO_THREAD_write_lock(lock); alias = type & OBJ_NAME_ALIAS; type &= ~OBJ_NAME_ALIAS; @@ -197,7 +222,7 @@ int OBJ_NAME_add(const char *name, int type, const char *data) onp = OPENSSL_malloc(sizeof(*onp)); if (onp == NULL) { /* ERROR */ - return 0; + goto unlock; } onp->name = name; @@ -223,18 +248,26 @@ int OBJ_NAME_add(const char *name, int type, const char *data) if (lh_OBJ_NAME_error(names_lh)) { /* ERROR */ OPENSSL_free(onp); - return 0; + goto unlock; } } - return 1; + + ok = 1; + +unlock: + CRYPTO_THREAD_unlock(lock); + return ok; } int OBJ_NAME_remove(const char *name, int type) { OBJ_NAME on, *ret; + int ok = 0; - if (names_lh == NULL) - return (0); + if (!OBJ_NAME_init()) + return 0; + + CRYPTO_THREAD_write_lock(lock); type &= ~OBJ_NAME_ALIAS; on.name = name; @@ -253,9 +286,11 @@ int OBJ_NAME_remove(const char *name, int type) ret->data); } OPENSSL_free(ret); - return (1); - } else - return (0); + ok = 1; + } + + CRYPTO_THREAD_unlock(lock); + return ok; } typedef struct { @@ -363,8 +398,10 @@ void OBJ_NAME_cleanup(int type) if (type < 0) { lh_OBJ_NAME_free(names_lh); sk_NAME_FUNCS_pop_free(name_funcs_stack, name_funcs_free); + CRYPTO_THREAD_lock_free(lock); names_lh = NULL; name_funcs_stack = NULL; + lock = NULL; } else lh_OBJ_NAME_set_down_load(names_lh, down_load); } -- cgit v1.2.3