Revert the crypto "global lock" implementation
Conceptually, this is a squashed version of: Revert "Address feedback" This reverts commit75551e07bd
. and Revert "Add CRYPTO_thread_glock_new" This reverts commited6b2c7938
. But there were some intervening commits that made neither revert apply cleanly, so instead do it all as one shot. The crypto global locks were an attempt to cope with the awkward POSIX semantics for pthread_atfork(); its documentation (the "RATIONALE" section) indicates that the expected usage is to have the prefork handler lock all "global" locks, and the parent and child handlers release those locks, to ensure that forking happens with a consistent (lock) state. However, the set of functions available in the child process is limited to async-signal-safe functions, and pthread_mutex_unlock() is not on the list of async-signal-safe functions! The only synchronization primitives that are async-signal-safe are the semaphore primitives, which are not really appropriate for general-purpose usage. However, the state consistency problem that the global locks were attempting to solve is not actually a serious problem, particularly for OpenSSL. That is, we can consider four cases of forking application that might use OpenSSL: (1) Single-threaded, does not call into OpenSSL in the child (e.g., the child calls exec() immediately) For this class of process, no locking is needed at all, since there is only ever a single thread of execution and the only reentrancy is due to signal handlers (which are themselves limited to async-signal-safe operation and should not be doing much work at all). (2) Single-threaded, calls into OpenSSL after fork() The application must ensure that it does not fork() with an unexpected lock held (that is, one that would get unlocked in the parent but accidentally remain locked in the child and cause deadlock). Since OpenSSL does not expose any of its internal locks to the application and the application is single-threaded, the OpenSSL internal locks will be unlocked for the fork(), and the state will be consistent. (OpenSSL will need to reseed its PRNG in the child, but that is an orthogonal issue.) If the application makes use of locks from libcrypto, proper handling for those locks is the responsibility of the application, as for any other locking primitive that is available for application programming. (3) Multi-threaded, does not call into OpenSSL after fork() As for (1), the OpenSSL state is only relevant in the parent, so no particular fork()-related handling is needed. The internal locks are relevant, but there is no interaction with the child to consider. (4) Multi-threaded, calls into OpenSSL after fork() This is the case where the pthread_atfork() hooks to ensure that all global locks are in a known state across fork() would come into play, per the above discussion. However, these "calls into OpenSSL after fork()" are still subject to the restriction to async-signal-safe functions. Since OpenSSL uses all sorts of locking and libc functions that are not on the list of safe functions (e.g., malloc()), this case is not currently usable and is unlikely to ever be usable, independently of the locking situation. So, there is no need to go through contortions to attempt to support this case in the one small area of locking interaction with fork(). In light of the above analysis (thanks @davidben and @achernya), go back to the simpler implementation that does not need to distinguish "library-global" locks or to have complicated atfork handling for locks. Reviewed-by: Kurt Roeckx <kurt@roeckx.be> Reviewed-by: Matthias St. Pierre <Matthias.St.Pierre@ncp-e.com> (Merged from https://github.com/openssl/openssl/pull/5089)
This commit is contained in:
parent
94f1c9379c
commit
63ab5ea13b
14 changed files with 21 additions and 110 deletions
|
@ -603,7 +603,7 @@ static int addrinfo_wrap(int family, int socktype,
|
|||
DEFINE_RUN_ONCE_STATIC(do_bio_lookup_init)
|
||||
{
|
||||
OPENSSL_init_crypto(0, NULL);
|
||||
bio_lookup_lock = CRYPTO_THREAD_glock_new("bio_lookup");
|
||||
bio_lookup_lock = CRYPTO_THREAD_lock_new();
|
||||
return bio_lookup_lock != NULL;
|
||||
}
|
||||
|
||||
|
|
|
@ -15,7 +15,7 @@ static CRYPTO_ONCE bio_type_init = CRYPTO_ONCE_STATIC_INIT;
|
|||
|
||||
DEFINE_RUN_ONCE_STATIC(do_bio_type_init)
|
||||
{
|
||||
bio_type_lock = CRYPTO_THREAD_glock_new("bio_type");
|
||||
bio_type_lock = CRYPTO_THREAD_lock_new();
|
||||
return bio_type_lock != NULL;
|
||||
}
|
||||
|
||||
|
|
|
@ -21,7 +21,7 @@ CRYPTO_ONCE engine_lock_init = CRYPTO_ONCE_STATIC_INIT;
|
|||
DEFINE_RUN_ONCE(do_engine_lock_init)
|
||||
{
|
||||
OPENSSL_init_crypto(0, NULL);
|
||||
global_engine_lock = CRYPTO_THREAD_glock_new("global_engine");
|
||||
global_engine_lock = CRYPTO_THREAD_lock_new();
|
||||
return global_engine_lock != NULL;
|
||||
}
|
||||
|
||||
|
|
|
@ -266,7 +266,7 @@ static void ERR_STATE_free(ERR_STATE *s)
|
|||
DEFINE_RUN_ONCE_STATIC(do_err_strings_init)
|
||||
{
|
||||
OPENSSL_init_crypto(0, NULL);
|
||||
err_string_lock = CRYPTO_THREAD_glock_new("err_string");
|
||||
err_string_lock = CRYPTO_THREAD_lock_new();
|
||||
int_error_hash = lh_ERR_STRING_DATA_new(err_string_data_hash,
|
||||
err_string_data_cmp);
|
||||
return err_string_lock != NULL && int_error_hash != NULL;
|
||||
|
|
|
@ -38,7 +38,7 @@ static CRYPTO_ONCE ex_data_init = CRYPTO_ONCE_STATIC_INIT;
|
|||
DEFINE_RUN_ONCE_STATIC(do_ex_data_init)
|
||||
{
|
||||
OPENSSL_init_crypto(0, NULL);
|
||||
ex_data_lock = CRYPTO_THREAD_glock_new("ex_data");
|
||||
ex_data_lock = CRYPTO_THREAD_lock_new();
|
||||
return ex_data_lock != NULL;
|
||||
}
|
||||
|
||||
|
|
|
@ -27,15 +27,6 @@
|
|||
#include "internal/dso.h"
|
||||
#include "internal/store.h"
|
||||
|
||||
|
||||
typedef struct global_lock_st {
|
||||
CRYPTO_RWLOCK *lock;
|
||||
const char *name;
|
||||
struct global_lock_st *next;
|
||||
} GLOBAL_LOCK;
|
||||
|
||||
static GLOBAL_LOCK *global_locks;
|
||||
|
||||
static int stopped = 0;
|
||||
|
||||
static void ossl_init_thread_stop(struct thread_local_inits_st *locals);
|
||||
|
@ -72,8 +63,6 @@ struct ossl_init_stop_st {
|
|||
OPENSSL_INIT_STOP *next;
|
||||
};
|
||||
|
||||
static CRYPTO_RWLOCK *glock_lock = NULL;
|
||||
|
||||
static OPENSSL_INIT_STOP *stop_handlers = NULL;
|
||||
static CRYPTO_RWLOCK *init_lock = NULL;
|
||||
|
||||
|
@ -95,7 +84,6 @@ DEFINE_RUN_ONCE_STATIC(ossl_init_base)
|
|||
#ifndef OPENSSL_SYS_UEFI
|
||||
atexit(OPENSSL_cleanup);
|
||||
#endif
|
||||
/* Do not change this to glock's! */
|
||||
if ((init_lock = CRYPTO_THREAD_lock_new()) == NULL)
|
||||
return 0;
|
||||
OPENSSL_cpuid_setup();
|
||||
|
@ -433,7 +421,6 @@ void OPENSSL_cleanup(void)
|
|||
stop_handlers = NULL;
|
||||
|
||||
CRYPTO_THREAD_lock_free(init_lock);
|
||||
init_lock = NULL;
|
||||
|
||||
/*
|
||||
* We assume we are single-threaded for this function, i.e. no race
|
||||
|
@ -514,16 +501,6 @@ void OPENSSL_cleanup(void)
|
|||
obj_cleanup_int();
|
||||
err_cleanup();
|
||||
|
||||
/* Free list of global locks. */
|
||||
while (global_locks != NULL) {
|
||||
GLOBAL_LOCK *next = global_locks->next;
|
||||
|
||||
free(global_locks);
|
||||
global_locks = next;
|
||||
}
|
||||
CRYPTO_THREAD_lock_free(glock_lock);
|
||||
glock_lock = NULL;
|
||||
|
||||
base_inited = 0;
|
||||
}
|
||||
|
||||
|
@ -707,56 +684,7 @@ int OPENSSL_atexit(void (*handler)(void))
|
|||
return 1;
|
||||
}
|
||||
|
||||
#ifndef OPENSSL_SYS_UNIX
|
||||
CRYPTO_RWLOCK *CRYPTO_THREAD_glock_new(const char *name)
|
||||
{
|
||||
return CRYPTO_THREAD_lock_new();
|
||||
}
|
||||
|
||||
#else
|
||||
DEFINE_RUN_ONCE_STATIC(glock_init)
|
||||
{
|
||||
glock_lock = CRYPTO_THREAD_lock_new();
|
||||
return glock_lock != NULL;
|
||||
}
|
||||
|
||||
/*
|
||||
* Create a new global lock, return NULL on error.
|
||||
*/
|
||||
CRYPTO_RWLOCK *CRYPTO_THREAD_glock_new(const char *name)
|
||||
{
|
||||
static CRYPTO_ONCE glock_once = CRYPTO_ONCE_STATIC_INIT;
|
||||
GLOBAL_LOCK *newlock;
|
||||
|
||||
if (glock_lock == NULL && !RUN_ONCE(&glock_once, glock_init))
|
||||
return NULL;
|
||||
if ((newlock = malloc(sizeof(*newlock))) == NULL)
|
||||
return NULL;
|
||||
if ((newlock->lock = CRYPTO_THREAD_lock_new()) == NULL) {
|
||||
free(newlock);
|
||||
return NULL;
|
||||
}
|
||||
newlock->name = name;
|
||||
CRYPTO_THREAD_write_lock(glock_lock);
|
||||
newlock->next = global_locks;
|
||||
global_locks = newlock;
|
||||
CRYPTO_THREAD_unlock(glock_lock);
|
||||
return newlock->lock;
|
||||
}
|
||||
|
||||
/*
|
||||
* Unlock all global locks.
|
||||
*/
|
||||
static void unlock_all(void)
|
||||
{
|
||||
GLOBAL_LOCK *lp;
|
||||
|
||||
CRYPTO_THREAD_write_lock(glock_lock);
|
||||
for (lp = global_locks; lp != NULL; lp = lp->next)
|
||||
CRYPTO_THREAD_unlock(lp->lock);
|
||||
CRYPTO_THREAD_unlock(glock_lock);
|
||||
}
|
||||
|
||||
#ifdef OPENSSL_SYS_UNIX
|
||||
/*
|
||||
* The following three functions are for OpenSSL developers. This is
|
||||
* where we set/reset state across fork (called via pthread_atfork when
|
||||
|
@ -770,22 +698,14 @@ static void unlock_all(void)
|
|||
|
||||
void OPENSSL_fork_prepare(void)
|
||||
{
|
||||
GLOBAL_LOCK *lp;
|
||||
|
||||
CRYPTO_THREAD_write_lock(glock_lock);
|
||||
for (lp = global_locks; lp != NULL; lp = lp->next)
|
||||
CRYPTO_THREAD_write_lock(lp->lock);
|
||||
CRYPTO_THREAD_unlock(glock_lock);
|
||||
}
|
||||
|
||||
void OPENSSL_fork_parent(void)
|
||||
{
|
||||
unlock_all();
|
||||
}
|
||||
|
||||
void OPENSSL_fork_child(void)
|
||||
{
|
||||
unlock_all();
|
||||
rand_fork();
|
||||
}
|
||||
#endif
|
||||
|
|
|
@ -93,11 +93,10 @@ static CRYPTO_THREAD_ID disabling_threadid;
|
|||
|
||||
DEFINE_RUN_ONCE_STATIC(do_memdbg_init)
|
||||
{
|
||||
memdbg_lock = CRYPTO_THREAD_glock_new("malloc");
|
||||
long_memdbg_lock = CRYPTO_THREAD_glock_new("long_malloc");
|
||||
if (memdbg_lock == NULL
|
||||
|| long_memdbg_lock == NULL
|
||||
|| !CRYPTO_THREAD_init_local(&appinfokey, NULL)) {
|
||||
memdbg_lock = CRYPTO_THREAD_lock_new();
|
||||
long_memdbg_lock = CRYPTO_THREAD_lock_new();
|
||||
if (memdbg_lock == NULL || long_memdbg_lock == NULL
|
||||
|| !CRYPTO_THREAD_init_local(&appinfokey, NULL)) {
|
||||
CRYPTO_THREAD_lock_free(memdbg_lock);
|
||||
memdbg_lock = NULL;
|
||||
CRYPTO_THREAD_lock_free(long_memdbg_lock);
|
||||
|
|
|
@ -68,7 +68,7 @@ int CRYPTO_secure_malloc_init(size_t size, int minsize)
|
|||
int ret = 0;
|
||||
|
||||
if (!secure_mem_initialized) {
|
||||
sec_malloc_lock = CRYPTO_THREAD_glock_new("sec_malloc");
|
||||
sec_malloc_lock = CRYPTO_THREAD_lock_new();
|
||||
if (sec_malloc_lock == NULL)
|
||||
return 0;
|
||||
if ((ret = sh_init(size, minsize)) != 0) {
|
||||
|
|
|
@ -69,7 +69,7 @@ DEFINE_RUN_ONCE_STATIC(o_names_init)
|
|||
{
|
||||
CRYPTO_mem_ctrl(CRYPTO_MEM_CHECK_DISABLE);
|
||||
names_lh = lh_OBJ_NAME_new(obj_name_hash, obj_name_cmp);
|
||||
obj_lock = CRYPTO_THREAD_glock_new("obj");
|
||||
obj_lock = CRYPTO_THREAD_lock_new();
|
||||
CRYPTO_mem_ctrl(CRYPTO_MEM_CHECK_ENABLE);
|
||||
return names_lh != NULL && obj_lock != NULL;
|
||||
}
|
||||
|
|
|
@ -98,7 +98,7 @@ static const char ossl_pers_string[] = "OpenSSL NIST SP 800-90A DRBG";
|
|||
|
||||
static CRYPTO_ONCE rand_drbg_init = CRYPTO_ONCE_STATIC_INIT;
|
||||
|
||||
static RAND_DRBG *drbg_setup(const char *name, RAND_DRBG *parent);
|
||||
static RAND_DRBG *drbg_setup(RAND_DRBG *parent);
|
||||
static void drbg_cleanup(RAND_DRBG *drbg);
|
||||
|
||||
/*
|
||||
|
@ -666,24 +666,18 @@ void *RAND_DRBG_get_ex_data(const RAND_DRBG *drbg, int idx)
|
|||
/*
|
||||
* Allocates a new global DRBG on the secure heap (if enabled) and
|
||||
* initializes it with default settings.
|
||||
* A global lock for the DRBG is created with the given name.
|
||||
*
|
||||
* Returns a pointer to the new DRBG instance on success, NULL on failure.
|
||||
*/
|
||||
static RAND_DRBG *drbg_setup(const char *name, RAND_DRBG *parent)
|
||||
static RAND_DRBG *drbg_setup(RAND_DRBG *parent)
|
||||
{
|
||||
RAND_DRBG *drbg;
|
||||
|
||||
if (name == NULL) {
|
||||
RANDerr(RAND_F_DRBG_SETUP, ERR_R_INTERNAL_ERROR);
|
||||
return NULL;
|
||||
}
|
||||
|
||||
drbg = OPENSSL_secure_zalloc(sizeof(RAND_DRBG));
|
||||
if (drbg == NULL)
|
||||
return NULL;
|
||||
|
||||
drbg->lock = CRYPTO_THREAD_glock_new(name);
|
||||
drbg->lock = CRYPTO_THREAD_lock_new();
|
||||
if (drbg->lock == NULL) {
|
||||
RANDerr(RAND_F_DRBG_SETUP, RAND_R_FAILED_TO_CREATE_LOCK);
|
||||
goto err;
|
||||
|
@ -737,9 +731,9 @@ DEFINE_RUN_ONCE_STATIC(do_rand_drbg_init)
|
|||
if (!OPENSSL_init_crypto(0, NULL))
|
||||
return 0;
|
||||
|
||||
drbg_master = drbg_setup("drbg_master", NULL);
|
||||
drbg_public = drbg_setup("drbg_public", drbg_master);
|
||||
drbg_private = drbg_setup("drbg_private", drbg_master);
|
||||
drbg_master = drbg_setup(NULL);
|
||||
drbg_public = drbg_setup(drbg_master);
|
||||
drbg_private = drbg_setup(drbg_master);
|
||||
|
||||
if (drbg_master == NULL || drbg_public == NULL || drbg_private == NULL)
|
||||
return 0;
|
||||
|
|
|
@ -282,10 +282,10 @@ DEFINE_RUN_ONCE_STATIC(do_rand_init)
|
|||
int ret = 1;
|
||||
|
||||
#ifndef OPENSSL_NO_ENGINE
|
||||
rand_engine_lock = CRYPTO_THREAD_glock_new("rand_engine");
|
||||
rand_engine_lock = CRYPTO_THREAD_lock_new();
|
||||
ret &= rand_engine_lock != NULL;
|
||||
#endif
|
||||
rand_meth_lock = CRYPTO_THREAD_glock_new("rand_meth");
|
||||
rand_meth_lock = CRYPTO_THREAD_lock_new();
|
||||
ret &= rand_meth_lock != NULL;
|
||||
|
||||
return ret;
|
||||
|
|
|
@ -20,7 +20,7 @@ static CRYPTO_ONCE registry_init = CRYPTO_ONCE_STATIC_INIT;
|
|||
|
||||
DEFINE_RUN_ONCE_STATIC(do_registry_init)
|
||||
{
|
||||
registry_lock = CRYPTO_THREAD_glock_new("registry");
|
||||
registry_lock = CRYPTO_THREAD_lock_new();
|
||||
return registry_lock != NULL;
|
||||
}
|
||||
|
||||
|
|
|
@ -67,7 +67,6 @@ typedef struct {
|
|||
typedef void CRYPTO_RWLOCK;
|
||||
|
||||
CRYPTO_RWLOCK *CRYPTO_THREAD_lock_new(void);
|
||||
CRYPTO_RWLOCK *CRYPTO_THREAD_glock_new(const char *name);
|
||||
int CRYPTO_THREAD_read_lock(CRYPTO_RWLOCK *lock);
|
||||
int CRYPTO_THREAD_write_lock(CRYPTO_RWLOCK *lock);
|
||||
int CRYPTO_THREAD_unlock(CRYPTO_RWLOCK *lock);
|
||||
|
|
|
@ -4388,7 +4388,6 @@ EVP_aria_256_ccm 4332 1_1_1 EXIST::FUNCTION:ARIA
|
|||
EVP_aria_128_gcm 4333 1_1_1 EXIST::FUNCTION:ARIA
|
||||
EVP_aria_128_ccm 4334 1_1_1 EXIST::FUNCTION:ARIA
|
||||
EVP_aria_192_gcm 4335 1_1_1 EXIST::FUNCTION:ARIA
|
||||
CRYPTO_THREAD_glock_new 4336 1_1_1 EXIST::FUNCTION:
|
||||
UI_get_result_length 4337 1_1_1 EXIST::FUNCTION:
|
||||
UI_set_result_ex 4338 1_1_1 EXIST::FUNCTION:
|
||||
UI_get_result_string_length 4339 1_1_1 EXIST::FUNCTION:
|
||||
|
|
Loading…
Reference in a new issue