engine: update to structure based atomics

Reviewed-by: Tomas Mraz <tomas@openssl.org>
Reviewed-by: Matt Caswell <matt@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/21260)

diff --git a/crypto/engine/eng_init.c b/crypto/engine/eng_init.c
index 2f4259367638617681632b729d6553d0b32df28b..ca1cd45e976fb664b6e937814cdbb21f748e8804 100644 (file)
--- a/crypto/engine/eng_init.c
+++ b/crypto/engine/eng_init.c
@@ -34,7 +34,7 @@ int engine_unlocked_init(ENGINE *e)
          * OK, we return a functional reference which is also a structural
          * reference.
          */
-        if (!CRYPTO_UP_REF(&e->struct_ref, &ref, e->refcnt_lock)) {
+        if (!CRYPTO_UP_REF(&e->struct_ref, &ref)) {
             e->finish(e);
             return 0;
         }

diff --git a/crypto/engine/eng_lib.c b/crypto/engine/eng_lib.c
index e80d77d6e3a77555dbc68fb7595c6d39949bf40c..3bb89111ff8cbe5310564bff48ab699105960e69 100644 (file)
--- a/crypto/engine/eng_lib.c
+++ b/crypto/engine/eng_lib.c
@@ -35,15 +35,13 @@ ENGINE *ENGINE_new(void)
     }
     if ((ret = OPENSSL_zalloc(sizeof(*ret))) == NULL)
         return NULL;
-    ret->struct_ref = 1;
-    ENGINE_REF_PRINT(ret, 0, 1);
-    ret->refcnt_lock = CRYPTO_THREAD_lock_new();
-    if (ret->refcnt_lock == NULL) {
+    if (!CRYPTO_NEW_REF(&ret->struct_ref, 1)) {
         OPENSSL_free(ret);
         return NULL;
     }
+    ENGINE_REF_PRINT(ret, 0, 1);
     if (!CRYPTO_new_ex_data(CRYPTO_EX_INDEX_ENGINE, ret, &ret->ex_data)) {
-        CRYPTO_THREAD_lock_free(ret->refcnt_lock);
+        CRYPTO_FREE_REF(&ret->struct_ref);
         OPENSSL_free(ret);
         return NULL;
     }
@@ -82,7 +80,7 @@ int engine_free_util(ENGINE *e, int not_locked)
 
     if (e == NULL)
         return 1;
-    CRYPTO_DOWN_REF(&e->struct_ref, &i, e->refcnt_lock);
+    CRYPTO_DOWN_REF(&e->struct_ref, &i);
     ENGINE_REF_PRINT(e, 0, -1);
     if (i > 0)
         return 1;
@@ -98,7 +96,7 @@ int engine_free_util(ENGINE *e, int not_locked)
         e->destroy(e);
     engine_remove_dynamic_id(e, not_locked);
     CRYPTO_free_ex_data(CRYPTO_EX_INDEX_ENGINE, e, &e->ex_data);
-    CRYPTO_THREAD_lock_free(e->refcnt_lock);
+    CRYPTO_FREE_REF(&e->struct_ref);
     OPENSSL_free(e);
     return 1;
 }

diff --git a/crypto/engine/eng_list.c b/crypto/engine/eng_list.c
index 11db85805688d7d641ea2bec319b326481560fd5..5a6238daf4c02db3f00a7c59afe5076083e5d561 100644 (file)
--- a/crypto/engine/eng_list.c
+++ b/crypto/engine/eng_list.c
@@ -77,7 +77,7 @@ static int engine_list_add(ENGINE *e)
     /*
      * Having the engine in the list assumes a structural reference.
      */
-    if (!CRYPTO_UP_REF(&e->struct_ref, &ref, e->refcnt_lock)) {
+    if (!CRYPTO_UP_REF(&e->struct_ref, &ref)) {
             ERR_raise(ERR_LIB_ENGINE, ENGINE_R_INTERNAL_LIST_ERROR);
             return 0;
     }
@@ -85,7 +85,7 @@ static int engine_list_add(ENGINE *e)
     if (engine_list_head == NULL) {
         /* We are adding to an empty list. */
         if (engine_list_tail != NULL) {
-            CRYPTO_DOWN_REF(&e->struct_ref, &ref, e->refcnt_lock);
+            CRYPTO_DOWN_REF(&e->struct_ref, &ref);
             ERR_raise(ERR_LIB_ENGINE, ENGINE_R_INTERNAL_LIST_ERROR);
             return 0;
         }
@@ -98,7 +98,7 @@ static int engine_list_add(ENGINE *e)
     } else {
         /* We are adding to the tail of an existing list. */
         if ((engine_list_tail == NULL) || (engine_list_tail->next != NULL)) {
-            CRYPTO_DOWN_REF(&e->struct_ref, &ref, e->refcnt_lock);
+            CRYPTO_DOWN_REF(&e->struct_ref, &ref);
             ERR_raise(ERR_LIB_ENGINE, ENGINE_R_INTERNAL_LIST_ERROR);
             return 0;
         }
@@ -238,7 +238,7 @@ ENGINE *ENGINE_get_first(void)
     if (ret) {
         int ref;
 
-        if (!CRYPTO_UP_REF(&ret->struct_ref, &ref, ret->refcnt_lock)) {
+        if (!CRYPTO_UP_REF(&ret->struct_ref, &ref)) {
             ERR_raise(ERR_LIB_ENGINE, ERR_R_CRYPTO_LIB);
             return NULL;
         }
@@ -264,7 +264,7 @@ ENGINE *ENGINE_get_last(void)
     if (ret) {
         int ref;
 
-        if (!CRYPTO_UP_REF(&ret->struct_ref, &ref, ret->refcnt_lock)) {
+        if (!CRYPTO_UP_REF(&ret->struct_ref, &ref)) {
             ERR_raise(ERR_LIB_ENGINE, ERR_R_CRYPTO_LIB);
             return NULL;
         }
@@ -289,7 +289,7 @@ ENGINE *ENGINE_get_next(ENGINE *e)
         int ref;
 
         /* Return a valid structural reference to the next ENGINE */
-        if (!CRYPTO_UP_REF(&ret->struct_ref, &ref, ret->refcnt_lock)) {
+        if (!CRYPTO_UP_REF(&ret->struct_ref, &ref)) {
             ERR_raise(ERR_LIB_ENGINE, ERR_R_CRYPTO_LIB);
             return NULL;
         }
@@ -315,7 +315,7 @@ ENGINE *ENGINE_get_prev(ENGINE *e)
         int ref;
 
         /* Return a valid structural reference to the next ENGINE */
-        if (!CRYPTO_UP_REF(&ret->struct_ref, &ref, ret->refcnt_lock)) {
+        if (!CRYPTO_UP_REF(&ret->struct_ref, &ref)) {
             ERR_raise(ERR_LIB_ENGINE, ERR_R_CRYPTO_LIB);
             return NULL;
         }
@@ -435,8 +435,7 @@ ENGINE *ENGINE_by_id(const char *id)
         } else {
             int ref;
 
-            if (!CRYPTO_UP_REF(&iterator->struct_ref, &ref,
-                               iterator->refcnt_lock)) {
+            if (!CRYPTO_UP_REF(&iterator->struct_ref, &ref)) {
                 CRYPTO_THREAD_unlock(global_engine_lock);
                 ERR_raise(ERR_LIB_ENGINE, ERR_R_CRYPTO_LIB);
                 return NULL;
@@ -477,6 +476,6 @@ int ENGINE_up_ref(ENGINE *e)
         ERR_raise(ERR_LIB_ENGINE, ERR_R_PASSED_NULL_PARAMETER);
         return 0;
     }
-    CRYPTO_UP_REF(&e->struct_ref, &i, global_engine_lock);
+    CRYPTO_UP_REF(&e->struct_ref, &i);
     return 1;
 }

diff --git a/crypto/engine/eng_local.h b/crypto/engine/eng_local.h
index a0e3a4734e44ee3ea8579b500ea19016e864ee4c..71d65cda6db1a803b8c016dbb94a4ccc8a003be5 100644 (file)
--- a/crypto/engine/eng_local.h
+++ b/crypto/engine/eng_local.h
@@ -23,8 +23,7 @@ extern CRYPTO_RWLOCK *global_engine_lock;
  * This prints the engine's pointer address, "struct" or "funct" to
  * indicate the reference type, the before and after reference count, and
  * the file:line-number pair. The "ENGINE_REF_PRINT" statements must come
- * *after* the change. Since this is for tracing only we do not concern
- * ourselves with using atomic primitives for reading the struct_ref
+ * *after* the change.
  */
 # define ENGINE_REF_PRINT(e, isfunct, diff)                             \
     OSSL_TRACE6(ENGINE_REF_COUNT,                                       \
@@ -32,8 +31,8 @@ extern CRYPTO_RWLOCK *global_engine_lock;
                (void *)(e), (isfunct ? "funct" : "struct"),             \
                ((isfunct)                                               \
                 ? ((e)->funct_ref - (diff))                             \
-                : ((e)->struct_ref - (diff))),                          \
-               ((isfunct) ? (e)->funct_ref : (e)->struct_ref),          \
+                : (eng_struct_ref(e) - (diff))),                        \
+               ((isfunct) ? (e)->funct_ref : eng_struct_ref(e)),        \
                (OPENSSL_FILE), (OPENSSL_LINE))
 
 /*
@@ -136,7 +135,6 @@ struct engine_st {
     int flags;
     /* reference count on the structure itself */
     CRYPTO_REF_COUNT struct_ref;
-    CRYPTO_RWLOCK *refcnt_lock;
     /*
      * reference count on usability of the engine type. NB: This controls the
      * loading and initialisation of any functionality required by this
@@ -160,4 +158,12 @@ typedef struct st_engine_pile ENGINE_PILE;
 
 DEFINE_LHASH_OF_EX(ENGINE_PILE);
 
+static ossl_unused ossl_inline int eng_struct_ref(ENGINE *e)
+{
+    int res;
+
+    CRYPTO_GET_REF(&e->struct_ref, &res);
+    return res;
+}
+
 #endif                          /* OSSL_CRYPTO_ENGINE_ENG_LOCAL_H */

diff --git a/crypto/engine/tb_asnmth.c b/crypto/engine/tb_asnmth.c
index c72bf9d22fe6f073e7c18667ead4f714249486c4..a436a1856d9e82e9ff9d3430f5e7cc7d56bcc436 100644 (file)
--- a/crypto/engine/tb_asnmth.c
+++ b/crypto/engine/tb_asnmth.c
@@ -208,7 +208,7 @@ const EVP_PKEY_ASN1_METHOD *ENGINE_pkey_asn1_find_str(ENGINE **pe,
     if (fstr.e != NULL) {
         int ref;
 
-        if (!CRYPTO_UP_REF(&fstr.e->struct_ref, &ref, fstr.e->refcnt_lock)) {
+        if (!CRYPTO_UP_REF(&fstr.e->struct_ref, &ref)) {
             CRYPTO_THREAD_unlock(global_engine_lock);
             ERR_raise(ERR_LIB_ENGINE, ERR_R_CRYPTO_LIB);
             return NULL;