[openssl-commits] [openssl] master update

kaduk at mit.edu kaduk at mit.edu
Wed Jan 31 18:47:19 UTC 2018


The branch master has been updated
       via  adeb4bc7a03aa61b6b26c3857dd91b05164daa60 (commit)
       via  63ab5ea13b671cb60dd4b7cfde2bcae9d14c5a60 (commit)
      from  94f1c9379c3ed4b088718b35d0dd807d619d50c5 (commit)


- Log -----------------------------------------------------------------
commit adeb4bc7a03aa61b6b26c3857dd91b05164daa60
Author: Benjamin Kaduk <bkaduk at akamai.com>
Date:   Fri Jan 26 09:32:40 2018 -0600

    Restore clearing of init_lock after free
    
    The behavior of resetting the init_lock value to NULL after
    freeing it during OPENSSL_cleanup() was added as part of the
    global lock commits that were just reverted, but there is desire
    to retain this behavior for clarity.
    
    It is unclear that the library would actually remain usable in
    any form after OPENSSL_cleanup(), since the required re-initialization
    occurs under a CRYPTO_ONCE check that cannot be reset at cleanup time.
    That said, a NULL dereference is probably more friendly behavior
    in these treacherous waters than using freed memory would be.
    
    Reviewed-by: Kurt Roeckx <kurt at roeckx.be>
    Reviewed-by: Matthias St. Pierre <Matthias.St.Pierre at ncp-e.com>
    (Merged from https://github.com/openssl/openssl/pull/5089)

commit 63ab5ea13b671cb60dd4b7cfde2bcae9d14c5a60
Author: Benjamin Kaduk <bkaduk at akamai.com>
Date:   Tue Jan 16 09:49:54 2018 -0600

    Revert the crypto "global lock" implementation
    
    Conceptually, this is a squashed version of:
    
        Revert "Address feedback"
    
        This reverts commit 75551e07bd2339dfea06ef1d31d69929e13a4495.
    
    and
    
        Revert "Add CRYPTO_thread_glock_new"
    
        This reverts commit ed6b2c7938ec6f07b15745d4183afc276e74c6dd.
    
    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 at roeckx.be>
    Reviewed-by: Matthias St. Pierre <Matthias.St.Pierre at ncp-e.com>
    (Merged from https://github.com/openssl/openssl/pull/5089)

-----------------------------------------------------------------------

Summary of changes:
 crypto/bio/b_addr.c           |  2 +-
 crypto/bio/bio_meth.c         |  2 +-
 crypto/engine/eng_lib.c       |  2 +-
 crypto/err/err.c              |  2 +-
 crypto/ex_data.c              |  2 +-
 crypto/init.c                 | 81 +------------------------------------------
 crypto/mem_dbg.c              |  9 +++--
 crypto/mem_sec.c              |  2 +-
 crypto/objects/o_names.c      |  2 +-
 crypto/rand/drbg_lib.c        | 18 ++++------
 crypto/rand/rand_lib.c        |  4 +--
 crypto/store/store_register.c |  2 +-
 include/openssl/crypto.h      |  1 -
 util/libcrypto.num            |  1 -
 14 files changed, 21 insertions(+), 109 deletions(-)

diff --git a/crypto/bio/b_addr.c b/crypto/bio/b_addr.c
index 6fb135a..ab8e7ff 100644
--- a/crypto/bio/b_addr.c
+++ b/crypto/bio/b_addr.c
@@ -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;
 }
 
diff --git a/crypto/bio/bio_meth.c b/crypto/bio/bio_meth.c
index 955be84..1c5d196 100644
--- a/crypto/bio/bio_meth.c
+++ b/crypto/bio/bio_meth.c
@@ -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;
 }
 
diff --git a/crypto/engine/eng_lib.c b/crypto/engine/eng_lib.c
index 8f65584..db7717f 100644
--- a/crypto/engine/eng_lib.c
+++ b/crypto/engine/eng_lib.c
@@ -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;
 }
 
diff --git a/crypto/err/err.c b/crypto/err/err.c
index c0bdbb8..a83da9b 100644
--- a/crypto/err/err.c
+++ b/crypto/err/err.c
@@ -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;
diff --git a/crypto/ex_data.c b/crypto/ex_data.c
index 78162b5..538fdb1 100644
--- a/crypto/ex_data.c
+++ b/crypto/ex_data.c
@@ -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;
 }
 
diff --git a/crypto/init.c b/crypto/init.c
index 909775a..1b94d07 100644
--- a/crypto/init.c
+++ b/crypto/init.c
@@ -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();
@@ -514,16 +502,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 +685,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 +699,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
diff --git a/crypto/mem_dbg.c b/crypto/mem_dbg.c
index b394de8..e113882 100644
--- a/crypto/mem_dbg.c
+++ b/crypto/mem_dbg.c
@@ -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);
diff --git a/crypto/mem_sec.c b/crypto/mem_sec.c
index 8f4ae9c..4c01045 100644
--- a/crypto/mem_sec.c
+++ b/crypto/mem_sec.c
@@ -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) {
diff --git a/crypto/objects/o_names.c b/crypto/objects/o_names.c
index d0e8e05..7367644 100644
--- a/crypto/objects/o_names.c
+++ b/crypto/objects/o_names.c
@@ -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;
 }
diff --git a/crypto/rand/drbg_lib.c b/crypto/rand/drbg_lib.c
index 5e6bdce..cb2f9e8 100644
--- a/crypto/rand/drbg_lib.c
+++ b/crypto/rand/drbg_lib.c
@@ -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;
diff --git a/crypto/rand/rand_lib.c b/crypto/rand/rand_lib.c
index ab03356..20ac583 100644
--- a/crypto/rand/rand_lib.c
+++ b/crypto/rand/rand_lib.c
@@ -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;
diff --git a/crypto/store/store_register.c b/crypto/store/store_register.c
index 987ca42..85c38b8 100644
--- a/crypto/store/store_register.c
+++ b/crypto/store/store_register.c
@@ -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;
 }
 
diff --git a/include/openssl/crypto.h b/include/openssl/crypto.h
index 5e9517d..478f9e2 100644
--- a/include/openssl/crypto.h
+++ b/include/openssl/crypto.h
@@ -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);
diff --git a/util/libcrypto.num b/util/libcrypto.num
index 536a3d2..a2d03c6 100644
--- a/util/libcrypto.num
+++ b/util/libcrypto.num
@@ -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:


More information about the openssl-commits mailing list