[openssl-commits] [openssl] master update

Andy Polyakov appro at openssl.org
Wed Jul 25 14:39:39 UTC 2018


The branch master has been updated
       via  80ae7285e1994d35c84519bf9e038b11d9942875 (commit)
       via  ceb8e32cbc9f6ddd17c5639a721f5314eb1f3acc (commit)
       via  9e4a1c3f65863b0175ddc534e232e63c4f82ea5c (commit)
       via  b86d57bb0b23253c720db38ab18ca97cb888f701 (commit)
      from  f529b5cf05139c20f298f553446122123c012317 (commit)


- Log -----------------------------------------------------------------
commit 80ae7285e1994d35c84519bf9e038b11d9942875
Author: Andy Polyakov <appro at openssl.org>
Date:   Fri Jul 20 13:23:42 2018 +0200

    crypto/init.c: use destructor_key even as guard in OPENSSL_thread_stop.
    
    Problem was that Windows threads that were terminating before libcrypto
    was initialized were referencing uninitialized or possibly even
    unrelated thread local storage index.
    
    Reviewed-by: Kurt Roeckx <kurt at roeckx.be>
    (Merged from https://github.com/openssl/openssl/pull/6752)

commit ceb8e32cbc9f6ddd17c5639a721f5314eb1f3acc
Author: Andy Polyakov <appro at openssl.org>
Date:   Fri Jul 20 13:22:24 2018 +0200

    crypto/dllmain.c: remove unused OPENSSL_NONPIC_relocated variable.
    
    Reviewed-by: Kurt Roeckx <kurt at roeckx.be>
    (Merged from https://github.com/openssl/openssl/pull/6752)

commit 9e4a1c3f65863b0175ddc534e232e63c4f82ea5c
Author: Andy Polyakov <appro at openssl.org>
Date:   Fri Jul 20 13:19:11 2018 +0200

    crypto/cryptlib.c: resolve possible race in OPENSSL_isservice.
    
    Reviewed-by: Kurt Roeckx <kurt at roeckx.be>
    (Merged from https://github.com/openssl/openssl/pull/6752)

commit b86d57bb0b23253c720db38ab18ca97cb888f701
Author: Andy Polyakov <appro at openssl.org>
Date:   Fri Jul 20 13:15:48 2018 +0200

    crypto/cryptlib.c: make OPENSS_cpuid_setup safe to use as constructor.
    
    Reviewed-by: Kurt Roeckx <kurt at roeckx.be>
    (Merged from https://github.com/openssl/openssl/pull/6752)

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

Summary of changes:
 crypto/cryptlib.c           | 108 +++++++++++++++++++++++++++++++++++---------
 crypto/dllmain.c            |  15 ------
 crypto/init.c               |  54 ++++++++++++++--------
 include/internal/cryptlib.h |   1 -
 4 files changed, 122 insertions(+), 56 deletions(-)

diff --git a/crypto/cryptlib.c b/crypto/cryptlib.c
index 0470597..b1e535a 100644
--- a/crypto/cryptlib.c
+++ b/crypto/cryptlib.c
@@ -19,29 +19,97 @@
 extern unsigned int OPENSSL_ia32cap_P[4];
 
 # if defined(OPENSSL_CPUID_OBJ) && !defined(OPENSSL_NO_ASM) && !defined(I386_ONLY)
-#include <stdio.h>
+
+/*
+ * Purpose of these minimalistic and character-type-agnostic subroutines
+ * is to break dependency on MSVCRT (on Windows) and locale. This makes
+ * OPENSSL_cpuid_setup safe to use as "constructor". "Character-type-
+ * agnostic" means that they work with either wide or 8-bit characters,
+ * exploiting the fact that first 127 characters can be simply casted
+ * between the sets, while the rest would be simply rejected by ossl_is*
+ * subroutines.
+ */
+#  ifdef _WIN32
+typedef WCHAR variant_char;
+
+static variant_char *ossl_getenv(const char *name)
+{
+    /*
+     * Since we pull only one environment variable, it's simpler to
+     * to just ignore |name| and use equivalent wide-char L-literal.
+     * As well as to ignore excessively long values...
+     */
+    static WCHAR value[48];
+    DWORD len = GetEnvironmentVariableW(L"OPENSSL_ia32cap", value, 48);
+
+    return (len > 0 && len < 48) ? value : NULL;
+}
+#  else
+typedef char variant_char;
+#   define ossl_getenv getenv
+#  endif
+
+#  include "internal/ctype.h"
+
+static int todigit(variant_char c)
+{
+    if (ossl_isdigit(c))
+        return c - '0';
+    else if (ossl_isxdigit(c))
+        return ossl_tolower(c) - 'a' + 10;
+
+    /* return largest base value to make caller terminate the loop */
+    return 16;
+}
+
+static uint64_t ossl_strtouint64(const variant_char *str)
+{
+    uint64_t ret = 0;
+    unsigned int digit, base = 10;
+
+    if (*str == '0') {
+        base = 8, str++;
+        if (ossl_tolower(*str) == 'x')
+            base = 16, str++;
+    }
+
+    while((digit = todigit(*str++)) < base)
+        ret = ret * base + digit;
+
+    return ret;
+}
+
+static variant_char *ossl_strchr(const variant_char *str, char srch)
+{   variant_char c;
+
+    while((c = *str)) {
+        if (c == srch)
+	    return (variant_char *)str;
+        str++;
+    }
+
+    return NULL;
+}
+
 #  define OPENSSL_CPUID_SETUP
 typedef uint64_t IA32CAP;
+
 void OPENSSL_cpuid_setup(void)
 {
     static int trigger = 0;
     IA32CAP OPENSSL_ia32_cpuid(unsigned int *);
     IA32CAP vec;
-    char *env;
+    const variant_char *env;
 
     if (trigger)
         return;
 
     trigger = 1;
-    if ((env = getenv("OPENSSL_ia32cap"))) {
+    if ((env = ossl_getenv("OPENSSL_ia32cap")) != NULL) {
         int off = (env[0] == '~') ? 1 : 0;
-#  if defined(_WIN32)
-        if (!sscanf(env + off, "%I64i", &vec))
-            vec = strtoul(env + off, NULL, 0);
-#  else
-        if (!sscanf(env + off, "%lli", (long long *)&vec))
-            vec = strtoul(env + off, NULL, 0);
-#  endif
+
+        vec = ossl_strtouint64(env + off);
+
         if (off) {
             IA32CAP mask = vec;
             vec = OPENSSL_ia32_cpuid(OPENSSL_ia32cap_P) & ~mask;
@@ -60,17 +128,12 @@ void OPENSSL_cpuid_setup(void)
             vec = OPENSSL_ia32_cpuid(OPENSSL_ia32cap_P);
         }
 
-        if ((env = strchr(env, ':'))) {
+        if ((env = ossl_strchr(env, ':')) != NULL) {
             IA32CAP vecx;
+
             env++;
             off = (env[0] == '~') ? 1 : 0;
-#  if defined(_WIN32)
-            if (!sscanf(env + off, "%I64i", &vecx))
-                vecx = strtoul(env + off, NULL, 0);
-#  else
-            if (!sscanf(env + off, "%lli", (long long *)&vecx))
-                vecx = strtoul(env + off, NULL, 0);
-#  endif
+            vecx = ossl_strtouint64(env + off);
             if (off) {
                 OPENSSL_ia32cap_P[2] &= ~(unsigned int)vecx;
                 OPENSSL_ia32cap_P[3] &= ~(unsigned int)(vecx >> 32);
@@ -98,7 +161,6 @@ void OPENSSL_cpuid_setup(void)
 unsigned int OPENSSL_ia32cap_P[4];
 # endif
 #endif
-int OPENSSL_NONPIC_relocated = 0;
 #if !defined(OPENSSL_CPUID_SETUP) && !defined(OPENSSL_CPUID_OBJ)
 void OPENSSL_cpuid_setup(void)
 {
@@ -142,10 +204,14 @@ int OPENSSL_isservice(void)
 
     if (_OPENSSL_isservice.p == NULL) {
         HANDLE mod = GetModuleHandle(NULL);
+        FARPROC f;
+
         if (mod != NULL)
-            _OPENSSL_isservice.f = GetProcAddress(mod, "_OPENSSL_isservice");
-        if (_OPENSSL_isservice.p == NULL)
+            f = GetProcAddress(mod, "_OPENSSL_isservice");
+        if (f == NULL)
             _OPENSSL_isservice.p = (void *)-1;
+        else
+            _OPENSSL_isservice.f = f;
     }
 
     if (_OPENSSL_isservice.p != (void *)-1)
diff --git a/crypto/dllmain.c b/crypto/dllmain.c
index 81bcb2d..c23b06b 100644
--- a/crypto/dllmain.c
+++ b/crypto/dllmain.c
@@ -31,21 +31,6 @@ BOOL WINAPI DllMain(HINSTANCE hinstDLL, DWORD fdwReason, LPVOID lpvReserved)
     switch (fdwReason) {
     case DLL_PROCESS_ATTACH:
         OPENSSL_cpuid_setup();
-# if defined(_WIN32_WINNT)
-        {
-            IMAGE_DOS_HEADER *dos_header = (IMAGE_DOS_HEADER *) hinstDLL;
-            IMAGE_NT_HEADERS *nt_headers;
-
-            if (dos_header->e_magic == IMAGE_DOS_SIGNATURE) {
-                nt_headers = (IMAGE_NT_HEADERS *) ((char *)dos_header
-                                                   + dos_header->e_lfanew);
-                if (nt_headers->Signature == IMAGE_NT_SIGNATURE &&
-                    hinstDLL !=
-                    (HINSTANCE) (nt_headers->OptionalHeader.ImageBase))
-                    OPENSSL_NONPIC_relocated = 1;
-            }
-        }
-# endif
         break;
     case DLL_THREAD_ATTACH:
         break;
diff --git a/crypto/init.c b/crypto/init.c
index 2c8b48f..7b69927 100644
--- a/crypto/init.c
+++ b/crypto/init.c
@@ -30,11 +30,25 @@
 
 static int stopped = 0;
 
-static void ossl_init_thread_stop(struct thread_local_inits_st *locals);
+/*
+ * Since per-thread-specific-data destructors are not universally
+ * available, i.e. not on Windows, only below CRYPTO_THREAD_LOCAL key
+ * is assumed to have destructor associated. And then an effort is made
+ * to call this single destructor on non-pthread platform[s].
+ *
+ * Initial value is "impossible". It is used as guard value to shortcut
+ * destructor for threads terminating before libcrypto is initialized or
+ * after it's de-initialized. Access to the key doesn't have to be
+ * serialized for the said threads, because they didn't use libcrypto
+ * and it doesn't matter if they pick "impossible" or derefernce real
+ * key value and pull NULL past initialization in the first thread that
+ * intends to use libcrypto.
+ */
+static CRYPTO_THREAD_LOCAL destructor_key = (CRYPTO_THREAD_LOCAL)-1;
 
-static CRYPTO_THREAD_LOCAL threadstopkey;
+static void ossl_init_thread_stop(struct thread_local_inits_st *locals);
 
-static void ossl_init_thread_stop_wrap(void *local)
+static void ossl_init_thread_destructor(void *local)
 {
     ossl_init_thread_stop((struct thread_local_inits_st *)local);
 }
@@ -42,17 +56,17 @@ static void ossl_init_thread_stop_wrap(void *local)
 static struct thread_local_inits_st *ossl_init_get_thread_local(int alloc)
 {
     struct thread_local_inits_st *local =
-        CRYPTO_THREAD_get_local(&threadstopkey);
+        CRYPTO_THREAD_get_local(&destructor_key);
 
-    if (local == NULL && alloc) {
-        local = OPENSSL_zalloc(sizeof(*local));
-        if (local != NULL && !CRYPTO_THREAD_set_local(&threadstopkey, local)) {
+    if (alloc) {
+        if (local == NULL
+            && (local = OPENSSL_zalloc(sizeof(*local))) != NULL
+            && !CRYPTO_THREAD_set_local(&destructor_key, local)) {
             OPENSSL_free(local);
             return NULL;
         }
-    }
-    if (!alloc) {
-        CRYPTO_THREAD_set_local(&threadstopkey, NULL);
+    } else {
+        CRYPTO_THREAD_set_local(&destructor_key, NULL);
     }
 
     return local;
@@ -71,17 +85,15 @@ static CRYPTO_ONCE base = CRYPTO_ONCE_STATIC_INIT;
 static int base_inited = 0;
 DEFINE_RUN_ONCE_STATIC(ossl_init_base)
 {
+    CRYPTO_THREAD_LOCAL key;
+
 #ifdef OPENSSL_INIT_DEBUG
     fprintf(stderr, "OPENSSL_INIT: ossl_init_base: Setting up stop handlers\n");
 #endif
 #ifndef OPENSSL_NO_CRYPTO_MDEBUG
     ossl_malloc_setup_failures();
 #endif
-    /*
-     * We use a dummy thread local key here. We use the destructor to detect
-     * when the thread is going to stop (where that feature is available)
-     */
-    if (!CRYPTO_THREAD_init_local(&threadstopkey, ossl_init_thread_stop_wrap))
+    if (!CRYPTO_THREAD_init_local(&key, ossl_init_thread_destructor))
         return 0;
     if ((init_lock = CRYPTO_THREAD_lock_new()) == NULL)
         goto err;
@@ -91,6 +103,7 @@ DEFINE_RUN_ONCE_STATIC(ossl_init_base)
 #endif
     OPENSSL_cpuid_setup();
 
+    destructor_key = key;
     base_inited = 1;
     return 1;
 
@@ -101,7 +114,7 @@ err:
     CRYPTO_THREAD_lock_free(init_lock);
     init_lock = NULL;
 
-    CRYPTO_THREAD_cleanup_local(&threadstopkey);
+    CRYPTO_THREAD_cleanup_local(&key);
     return 0;
 }
 
@@ -396,8 +409,8 @@ static void ossl_init_thread_stop(struct thread_local_inits_st *locals)
 
 void OPENSSL_thread_stop(void)
 {
-    ossl_init_thread_stop(
-        (struct thread_local_inits_st *)ossl_init_get_thread_local(0));
+    if (destructor_key != (CRYPTO_THREAD_LOCAL)-1)
+        ossl_init_thread_stop(ossl_init_get_thread_local(0));
 }
 
 int ossl_init_thread_start(uint64_t opts)
@@ -442,6 +455,7 @@ int ossl_init_thread_start(uint64_t opts)
 void OPENSSL_cleanup(void)
 {
     OPENSSL_INIT_STOP *currhandler, *lasthandler;
+    CRYPTO_THREAD_LOCAL key;
 
     /* If we've not been inited then no need to deinit */
     if (!base_inited)
@@ -501,7 +515,9 @@ void OPENSSL_cleanup(void)
         err_free_strings_int();
     }
 
-    CRYPTO_THREAD_cleanup_local(&threadstopkey);
+    key = destructor_key;
+    destructor_key = (CRYPTO_THREAD_LOCAL)-1;
+    CRYPTO_THREAD_cleanup_local(&key);
 
 #ifdef OPENSSL_INIT_DEBUG
     fprintf(stderr, "OPENSSL_INIT: OPENSSL_cleanup: "
diff --git a/include/internal/cryptlib.h b/include/internal/cryptlib.h
index 2808fe7..a608735 100644
--- a/include/internal/cryptlib.h
+++ b/include/internal/cryptlib.h
@@ -78,7 +78,6 @@ DEFINE_LHASH_OF(MEM);
 void OPENSSL_cpuid_setup(void);
 extern unsigned int OPENSSL_ia32cap_P[];
 void OPENSSL_showfatal(const char *fmta, ...);
-extern int OPENSSL_NONPIC_relocated;
 void crypto_cleanup_all_ex_data_int(void);
 int openssl_init_fork_handlers(void);
 


More information about the openssl-commits mailing list