[openssl] master update

beldmit at gmail.com beldmit at gmail.com
Thu Dec 31 12:17:37 UTC 2020


The branch master has been updated
       via  ea08f8b294d129371536649463c76a81dc4d4e55 (commit)
       via  49fff26d674adb65f3532eec4f0f37369b41a594 (commit)
       via  db6bcc81ab86fca74730566f0b471a7c3757c95c (commit)
       via  d5e742de653954bfae88f0e5f6c8f0a7a5f6c437 (commit)
      from  30af356df487b2dad571be15574b454daf70743c (commit)


- Log -----------------------------------------------------------------
commit ea08f8b294d129371536649463c76a81dc4d4e55
Author: Matt Caswell <matt at openssl.org>
Date:   Wed Dec 23 11:35:54 2020 +0000

    Add a test for the new CRYPTO_atomic_* functions
    
    Also tests the older CRYPTO_atomic_add() which was without a test
    
    Reviewed-by: Dmitry Belyavskiy <beldmit at gmail.com>
    (Merged from https://github.com/openssl/openssl/pull/13733)

commit 49fff26d674adb65f3532eec4f0f37369b41a594
Author: Matt Caswell <matt at openssl.org>
Date:   Wed Dec 23 11:15:03 2020 +0000

    Add documentation for CRYPTO_atomic_or and CRYPTO_atomic_load
    
    Reviewed-by: Dmitry Belyavskiy <beldmit at gmail.com>
    (Merged from https://github.com/openssl/openssl/pull/13733)

commit db6bcc81ab86fca74730566f0b471a7c3757c95c
Author: Matt Caswell <matt at openssl.org>
Date:   Tue Dec 22 17:44:07 2020 +0000

    Optimise OPENSSL_init_crypto
    
    If everything has already been initialised we can check this with a
    single test at the beginning of OPENSSL_init_crypto() and therefore
    reduce the amount of time spent in this function. Since this is called
    via very many codepaths this should have significant performance benefits.
    
    Partially fixes #13725 and #13578
    
    Reviewed-by: Dmitry Belyavskiy <beldmit at gmail.com>
    (Merged from https://github.com/openssl/openssl/pull/13733)

commit d5e742de653954bfae88f0e5f6c8f0a7a5f6c437
Author: Matt Caswell <matt at openssl.org>
Date:   Tue Dec 22 17:43:07 2020 +0000

    Add some more CRYPTO_atomic functions
    
    We add an implementation for CRYPTO_atomic_or() and CRYPTO_atomic_load()
    
    Reviewed-by: Dmitry Belyavskiy <beldmit at gmail.com>
    (Merged from https://github.com/openssl/openssl/pull/13733)

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

Summary of changes:
 crypto/init.c                       | 38 +++++++++++++++++++
 crypto/threads_none.c               | 16 ++++++++
 crypto/threads_pthread.c            | 50 +++++++++++++++++++++++-
 crypto/threads_win.c                | 19 ++++++++--
 doc/man3/CRYPTO_THREAD_run_once.pod | 43 ++++++++++++++++-----
 include/openssl/crypto.h.in         |  3 ++
 test/threadstest.c                  | 76 +++++++++++++++++++++++++++++++++++++
 util/libcrypto.num                  |  2 +
 8 files changed, 233 insertions(+), 14 deletions(-)

diff --git a/crypto/init.c b/crypto/init.c
index f1100df169..50aec32c3d 100644
--- a/crypto/init.c
+++ b/crypto/init.c
@@ -34,6 +34,7 @@
 #include <openssl/trace.h>
 
 static int stopped = 0;
+static uint64_t optsdone = 0;
 
 typedef struct ossl_init_stop_st OPENSSL_INIT_STOP;
 struct ossl_init_stop_st {
@@ -464,6 +465,28 @@ void OPENSSL_cleanup(void)
  */
 int OPENSSL_init_crypto(uint64_t opts, const OPENSSL_INIT_SETTINGS *settings)
 {
+    uint64_t tmp;
+    int aloaddone = 0;
+
+    /*
+     * We ignore failures from this function. It is probably because we are
+     * on a platform that doesn't support lockless atomic loads (we may not
+     * have created init_lock yet so we can't use it). This is just an
+     * optimisation to skip the full checks in this function if we don't need
+     * to, so we carry on regardless in the event of failure.
+     *
+     * There could be a race here with other threads, so that optsdone has not
+     * been updated yet, even though the options have in fact been initialised.
+     * This doesn't matter - it just means we will run the full function
+     * unnecessarily - but all the critical code is contained in RUN_ONCE
+     * functions anyway so we are safe.
+     */
+    if (CRYPTO_atomic_load(&optsdone, &tmp, NULL)) {
+        if ((tmp & opts) == opts)
+            return 1;
+        aloaddone = 1;
+    }
+
     /*
      * TODO(3.0): This function needs looking at with a view to moving most/all
      * of this into OSSL_LIB_CTX.
@@ -492,6 +515,18 @@ int OPENSSL_init_crypto(uint64_t opts, const OPENSSL_INIT_SETTINGS *settings)
     if (opts & OPENSSL_INIT_BASE_ONLY)
         return 1;
 
+    /*
+     * init_lock should definitely be set up now, so we can now repeat the
+     * same check from above but be sure that it will work even on platforms
+     * without lockless CRYPTO_atomic_load
+     */
+    if (!aloaddone) {
+        if (!CRYPTO_atomic_load(&optsdone, &tmp, init_lock))
+            return 0;
+        if ((tmp & opts) == opts)
+            return 1;
+    }
+
     /*
      * Now we don't always set up exit handlers, the INIT_BASE_ONLY calls
      * should not have the side-effect of setting up exit handlers, and
@@ -614,6 +649,9 @@ int OPENSSL_init_crypto(uint64_t opts, const OPENSSL_INIT_SETTINGS *settings)
         return 0;
 #endif
 
+    if (!CRYPTO_atomic_or(&optsdone, opts, &tmp, init_lock))
+        return 0;
+
     return 1;
 }
 
diff --git a/crypto/threads_none.c b/crypto/threads_none.c
index c12d5610aa..55d4b5f0f8 100644
--- a/crypto/threads_none.c
+++ b/crypto/threads_none.c
@@ -133,6 +133,22 @@ int CRYPTO_atomic_add(int *val, int amount, int *ret, CRYPTO_RWLOCK *lock)
     return 1;
 }
 
+int CRYPTO_atomic_or(uint64_t *val, uint64_t op, uint64_t *ret,
+                     CRYPTO_RWLOCK *lock)
+{
+    *val |= op;
+    *ret  = *val;
+
+    return 1;
+}
+
+int CRYPTO_atomic_load(uint64_t *val, uint64_t *ret, CRYPTO_RWLOCK *lock)
+{
+    *ret  = *val;
+
+    return 1;
+}
+
 int openssl_init_fork_handlers(void)
 {
     return 0;
diff --git a/crypto/threads_pthread.c b/crypto/threads_pthread.c
index afc29b7961..22ba793161 100644
--- a/crypto/threads_pthread.c
+++ b/crypto/threads_pthread.c
@@ -185,7 +185,7 @@ int CRYPTO_atomic_add(int *val, int amount, int *ret, CRYPTO_RWLOCK *lock)
         return 1;
     }
 # endif
-    if (!CRYPTO_THREAD_write_lock(lock))
+    if (lock == NULL || !CRYPTO_THREAD_write_lock(lock))
         return 0;
 
     *val += amount;
@@ -197,6 +197,54 @@ int CRYPTO_atomic_add(int *val, int amount, int *ret, CRYPTO_RWLOCK *lock)
     return 1;
 }
 
+int CRYPTO_atomic_or(uint64_t *val, uint64_t op, uint64_t *ret,
+                     CRYPTO_RWLOCK *lock)
+{
+# if defined(__GNUC__) && defined(__ATOMIC_ACQ_REL)
+    if (__atomic_is_lock_free(sizeof(*val), val)) {
+        *ret = __atomic_or_fetch(val, op, __ATOMIC_ACQ_REL);
+        return 1;
+    }
+# elif defined(__sun) && (defined(__SunOS_5_10) || defined(__SunOS_5_11))
+    /* This will work for all future Solaris versions. */
+    if (ret != NULL) {
+        *ret = atomic_or_64_nv(val, op);
+        return 1;
+    }
+# endif
+    if (lock == NULL || !CRYPTO_THREAD_write_lock(lock))
+        return 0;
+    *val |= op;
+    *ret  = *val;
+
+    if (!CRYPTO_THREAD_unlock(lock))
+        return 0;
+
+    return 1;
+}
+
+int CRYPTO_atomic_load(uint64_t *val, uint64_t *ret, CRYPTO_RWLOCK *lock)
+{
+# if defined(__GNUC__) && defined(__ATOMIC_ACQUIRE)
+    if (__atomic_is_lock_free(sizeof(*val), val)) {
+        __atomic_load(val, ret, __ATOMIC_ACQUIRE);
+        return 1;
+    }
+# elif defined(__sun) && (defined(__SunOS_5_10) || defined(__SunOS_5_11))
+    /* This will work for all future Solaris versions. */
+    if (ret != NULL) {
+        *ret = atomic_or_64_nv(val, 0);
+        return 1;
+    }
+# endif
+    if (lock == NULL || !CRYPTO_THREAD_read_lock(lock))
+        return 0;
+    *ret  = *val;
+    if (!CRYPTO_THREAD_unlock(lock))
+        return 0;
+
+    return 1;
+}
 # ifndef FIPS_MODULE
 #  ifdef OPENSSL_SYS_UNIX
 
diff --git a/crypto/threads_win.c b/crypto/threads_win.c
index a008831a3e..ef68fe2d24 100644
--- a/crypto/threads_win.c
+++ b/crypto/threads_win.c
@@ -66,9 +66,9 @@ void CRYPTO_THREAD_lock_free(CRYPTO_RWLOCK *lock)
     return;
 }
 
-#  define ONCE_UNINITED     0
-#  define ONCE_ININIT       1
-#  define ONCE_DONE         2
+# define ONCE_UNINITED     0
+# define ONCE_ININIT       1
+# define ONCE_DONE         2
 
 /*
  * We don't use InitOnceExecuteOnce because that isn't available in WinXP which
@@ -159,6 +159,19 @@ int CRYPTO_atomic_add(int *val, int amount, int *ret, CRYPTO_RWLOCK *lock)
     return 1;
 }
 
+int CRYPTO_atomic_or(uint64_t *val, uint64_t op, uint64_t *ret,
+                     CRYPTO_RWLOCK *lock)
+{
+    *ret = (uint64_t)InterlockedOr64((LONG64 volatile *)val, (LONG64)op) | op;
+    return 1;
+}
+
+int CRYPTO_atomic_load(uint64_t *val, uint64_t *ret, CRYPTO_RWLOCK *lock)
+{
+    *ret = (uint64_t)InterlockedOr64((LONG64 volatile *)val, 0);
+    return 1;
+}
+
 int openssl_init_fork_handlers(void)
 {
     return 0;
diff --git a/doc/man3/CRYPTO_THREAD_run_once.pod b/doc/man3/CRYPTO_THREAD_run_once.pod
index 5cffc42026..a182359f47 100644
--- a/doc/man3/CRYPTO_THREAD_run_once.pod
+++ b/doc/man3/CRYPTO_THREAD_run_once.pod
@@ -5,7 +5,7 @@
 CRYPTO_THREAD_run_once,
 CRYPTO_THREAD_lock_new, CRYPTO_THREAD_read_lock, CRYPTO_THREAD_write_lock,
 CRYPTO_THREAD_unlock, CRYPTO_THREAD_lock_free,
-CRYPTO_atomic_add - OpenSSL thread support
+CRYPTO_atomic_add, CRYPTO_atomic_or, CRYPTO_atomic_load - OpenSSL thread support
 
 =head1 SYNOPSIS
 
@@ -21,6 +21,9 @@ CRYPTO_atomic_add - OpenSSL thread support
  void CRYPTO_THREAD_lock_free(CRYPTO_RWLOCK *lock);
 
  int CRYPTO_atomic_add(int *val, int amount, int *ret, CRYPTO_RWLOCK *lock);
+ int CRYPTO_atomic_or(uint64_t *val, uint64_t op, uint64_t *ret,
+                      CRYPTO_RWLOCK *lock);
+ int CRYPTO_atomic_load(uint64_t *val, uint64_t *ret, CRYPTO_RWLOCK *lock);
 
 =head1 DESCRIPTION
 
@@ -38,10 +41,10 @@ The following multi-threading function are provided:
 =item *
 
 CRYPTO_THREAD_run_once() can be used to perform one-time initialization.
-The B<once> argument must be a pointer to a static object of type
+The I<once> argument must be a pointer to a static object of type
 B<CRYPTO_ONCE> that was statically initialized to the value
 B<CRYPTO_ONCE_STATIC_INIT>.
-The B<init> argument is a pointer to a function that performs the desired
+The I<init> argument is a pointer to a function that performs the desired
 exactly once initialization.
 In particular, this can be used to allocate locks in a thread-safe manner,
 which can then be used with the locking functions below.
@@ -53,27 +56,47 @@ lock.
 
 =item *
 
-CRYPTO_THREAD_read_lock() locks the provided B<lock> for reading.
+CRYPTO_THREAD_read_lock() locks the provided I<lock> for reading.
 
 =item *
 
-CRYPTO_THREAD_write_lock() locks the provided B<lock> for writing.
+CRYPTO_THREAD_write_lock() locks the provided I<lock> for writing.
 
 =item *
 
-CRYPTO_THREAD_unlock() unlocks the previously locked B<lock>.
+CRYPTO_THREAD_unlock() unlocks the previously locked I<lock>.
 
 =item *
 
-CRYPTO_THREAD_lock_free() frees the provided B<lock>.
+CRYPTO_THREAD_lock_free() frees the provided I<lock>.
 
 =item *
 
-CRYPTO_atomic_add() atomically adds B<amount> to B<val> and returns the
-result of the operation in B<ret>. B<lock> will be locked, unless atomic
+CRYPTO_atomic_add() atomically adds I<amount> to I<*val> and returns the
+result of the operation in I<*ret>. I<lock> will be locked, unless atomic
 operations are supported on the specific platform. Because of this, if a
 variable is modified by CRYPTO_atomic_add() then CRYPTO_atomic_add() must
-be the only way that the variable is modified.
+be the only way that the variable is modified. If atomic operations are not
+supported and I<lock> is NULL, then the function will fail.
+
+=item *
+
+CRYPTO_atomic_or() performs an atomic bitwise or of I<op> and I<*val> and stores
+the result back in I<*val>. It also returns the result of the operation in
+I<*ret>. I<lock> will be locked, unless atomic operations are supported on the
+specific platform. Because of this, if a variable is modified by
+CRYPTO_atomic_or() or read by CRYPTO_atomic_load() then CRYPTO_atomic_or() must
+be the only way that the variable is modified. If atomic operations are not
+supported and I<lock> is NULL, then the function will fail.
+
+=item *
+
+CRYPTO_atomic_load() atomically loads the contents of I<*val> into I<*ret>.
+I<lock> will be locked, unless atomic operations are supported on the specific
+platform. Because of this, if a variable is modified by CRYPTO_atomic_or() or
+read by CRYPTO_atomic_load() then CRYPTO_atomic_load() must be the only way that
+the variable is read. If atomic operations are not supported and I<lock> is
+NULL, then the function will fail.
 
 =back
 
diff --git a/include/openssl/crypto.h.in b/include/openssl/crypto.h.in
index 0641db3a44..0b9aeefe04 100644
--- a/include/openssl/crypto.h.in
+++ b/include/openssl/crypto.h.in
@@ -86,6 +86,9 @@ int CRYPTO_THREAD_unlock(CRYPTO_RWLOCK *lock);
 void CRYPTO_THREAD_lock_free(CRYPTO_RWLOCK *lock);
 
 int CRYPTO_atomic_add(int *val, int amount, int *ret, CRYPTO_RWLOCK *lock);
+int CRYPTO_atomic_or(uint64_t *val, uint64_t op, uint64_t *ret,
+                     CRYPTO_RWLOCK *lock);
+int CRYPTO_atomic_load(uint64_t *val, uint64_t *ret, CRYPTO_RWLOCK *lock);
 
 /* No longer needed, so this is a no-op */
 #define OPENSSL_malloc_init() while(0) continue
diff --git a/test/threadstest.c b/test/threadstest.c
index 5f373fe75f..d7ed59781d 100644
--- a/test/threadstest.c
+++ b/test/threadstest.c
@@ -184,10 +184,86 @@ static int test_thread_local(void)
     return 1;
 }
 
+static int test_atomic(void)
+{
+    int val = 0, ret = 0, testresult = 0;
+    uint64_t val64 = 1, ret64 = 0;
+    CRYPTO_RWLOCK *lock = CRYPTO_THREAD_lock_new();
+
+    if (!TEST_ptr(lock))
+        return 0;
+
+    if (CRYPTO_atomic_add(&val, 1, &ret, NULL)) {
+        /* This succeeds therefore we're on a platform with lockless atomics */
+        if (!TEST_int_eq(val, 1) || !TEST_int_eq(val, ret))
+            goto err;
+    } else {
+        /* This failed therefore we're on a platform without lockless atomics */
+        if (!TEST_int_eq(val, 0) || !TEST_int_eq(val, ret))
+            goto err;
+    }
+    val = 0;
+    ret = 0;
+
+    if (!TEST_true(CRYPTO_atomic_add(&val, 1, &ret, lock)))
+        goto err;
+    if (!TEST_int_eq(val, 1) || !TEST_int_eq(val, ret))
+        goto err;
+
+    if (CRYPTO_atomic_or(&val64, 2, &ret64, NULL)) {
+        /* This succeeds therefore we're on a platform with lockless atomics */
+        if (!TEST_uint_eq((unsigned int)val64, 3)
+                || !TEST_uint_eq((unsigned int)val64, (unsigned int)ret64))
+            goto err;
+    } else {
+        /* This failed therefore we're on a platform without lockless atomics */
+        if (!TEST_uint_eq((unsigned int)val64, 1)
+                || !TEST_int_eq((unsigned int)ret64, 0))
+            goto err;
+    }
+    val64 = 1;
+    ret64 = 0;
+
+    if (!TEST_true(CRYPTO_atomic_or(&val64, 2, &ret64, lock)))
+        goto err;
+
+    if (!TEST_uint_eq((unsigned int)val64, 3)
+            || !TEST_uint_eq((unsigned int)val64, (unsigned int)ret64))
+        goto err;
+
+    ret64 = 0;
+    if (CRYPTO_atomic_load(&val64, &ret64, NULL)) {
+        /* This succeeds therefore we're on a platform with lockless atomics */
+        if (!TEST_uint_eq((unsigned int)val64, 3)
+                || !TEST_uint_eq((unsigned int)val64, (unsigned int)ret64))
+            goto err;
+    } else {
+        /* This failed therefore we're on a platform without lockless atomics */
+        if (!TEST_uint_eq((unsigned int)val64, 3)
+                || !TEST_int_eq((unsigned int)ret64, 0))
+            goto err;
+    }
+
+    ret64 = 0;
+    if (!TEST_true(CRYPTO_atomic_load(&val64, &ret64, lock)))
+        goto err;
+
+    if (!TEST_uint_eq((unsigned int)val64, 3)
+            || !TEST_uint_eq((unsigned int)val64, (unsigned int)ret64))
+        goto err;
+
+    testresult = 1;
+ err:
+
+    CRYPTO_THREAD_lock_free(lock);
+    return testresult;
+}
+
 int setup_tests(void)
 {
     ADD_TEST(test_lock);
     ADD_TEST(test_once);
     ADD_TEST(test_thread_local);
+    ADD_TEST(test_atomic);
     return 1;
 }
diff --git a/util/libcrypto.num b/util/libcrypto.num
index 93ca779831..289a6672f9 100644
--- a/util/libcrypto.num
+++ b/util/libcrypto.num
@@ -5284,3 +5284,5 @@ PEM_write_bio_PrivateKey_ex             ?	3_0_0	EXIST::FUNCTION:
 PEM_write_PUBKEY_ex                     ?	3_0_0	EXIST::FUNCTION:STDIO
 PEM_write_bio_PUBKEY_ex                 ?	3_0_0	EXIST::FUNCTION:
 EVP_PKEY_get_group_name                 ?	3_0_0	EXIST::FUNCTION:
+CRYPTO_atomic_or                        ?	3_0_0	EXIST::FUNCTION:
+CRYPTO_atomic_load                      ?	3_0_0	EXIST::FUNCTION:


More information about the openssl-commits mailing list