When we're just reading EX_CALLBACK data just get a read lock
authorMatt Caswell <matt@openssl.org>
Thu, 11 May 2023 10:25:07 +0000 (11:25 +0100)
committerMatt Caswell <matt@openssl.org>
Tue, 30 May 2023 16:26:02 +0000 (17:26 +0100)
The crypto_ex_data code was always obtaining a write lock in all functions
regardless of whether we were only reading EX_CALLBACK data or actually
changing it. Changes to the EX_CALLBACK data are rare, with many reads so
we should change to a read lock where we can.

We hit this every time we create or free any object that can have ex_data
associated with it (e.g. BIOs, SSL, etc)

Partially fixes #20286

Reviewed-by: Tomas Mraz <tomas@openssl.org>
Reviewed-by: Hugo Landau <hlandau@openssl.org>
Reviewed-by: Paul Dale <pauli@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/20943)

crypto/ex_data.c

index 26c225001c7980106aa2940cf1d0f12c8392b012..48321febe4c9e6a2ed4c3e80e13a815a41abcded 100644 (file)
@@ -26,8 +26,10 @@ int ossl_do_ex_data_init(OSSL_LIB_CTX *ctx)
  * Return the EX_CALLBACKS from the |ex_data| array that corresponds to
  * a given class.  On success, *holds the lock.*
  * The |global| parameter is assumed to be non null (checked by the caller).
+ * If |read| is 1 then a read lock is obtained. Otherwise it is a write lock.
  */
-static EX_CALLBACKS *get_and_lock(OSSL_EX_DATA_GLOBAL *global, int class_index)
+static EX_CALLBACKS *get_and_lock(OSSL_EX_DATA_GLOBAL *global, int class_index,
+                                  int read)
 {
     EX_CALLBACKS *ip;
 
@@ -44,8 +46,14 @@ static EX_CALLBACKS *get_and_lock(OSSL_EX_DATA_GLOBAL *global, int class_index)
          return NULL;
     }
 
-    if (!CRYPTO_THREAD_write_lock(global->ex_data_lock))
-        return NULL;
+    if (read) {
+        if (!CRYPTO_THREAD_read_lock(global->ex_data_lock))
+            return NULL;
+    } else {
+        if (!CRYPTO_THREAD_write_lock(global->ex_data_lock))
+            return NULL;
+    }
+
     ip = &global->ex_data[class_index];
     return ip;
 }
@@ -112,7 +120,7 @@ int ossl_crypto_free_ex_index_ex(OSSL_LIB_CTX *ctx, int class_index, int idx)
     if (global == NULL)
         return 0;
 
-    ip = get_and_lock(global, class_index);
+    ip = get_and_lock(global, class_index, 0);
     if (ip == NULL)
         return 0;
 
@@ -153,7 +161,7 @@ int ossl_crypto_get_ex_new_index_ex(OSSL_LIB_CTX *ctx, int class_index,
     if (global == NULL)
         return -1;
 
-    ip = get_and_lock(global, class_index);
+    ip = get_and_lock(global, class_index, 0);
     if (ip == NULL)
         return -1;
 
@@ -219,7 +227,7 @@ int ossl_crypto_new_ex_data_ex(OSSL_LIB_CTX *ctx, int class_index, void *obj,
     if (global == NULL)
         return 0;
 
-    ip = get_and_lock(global, class_index);
+    ip = get_and_lock(global, class_index, 1);
     if (ip == NULL)
         return 0;
 
@@ -280,7 +288,7 @@ int CRYPTO_dup_ex_data(int class_index, CRYPTO_EX_DATA *to,
     if (global == NULL)
         return 0;
 
-    ip = get_and_lock(global, class_index);
+    ip = get_and_lock(global, class_index, 1);
     if (ip == NULL)
         return 0;
 
@@ -367,7 +375,7 @@ void CRYPTO_free_ex_data(int class_index, void *obj, CRYPTO_EX_DATA *ad)
     if (global == NULL)
         goto err;
 
-    ip = get_and_lock(global, class_index);
+    ip = get_and_lock(global, class_index, 1);
     if (ip == NULL)
         goto err;
 
@@ -434,7 +442,7 @@ int ossl_crypto_alloc_ex_data_intern(int class_index, void *obj,
     if (global == NULL)
         return 0;
 
-    ip = get_and_lock(global, class_index);
+    ip = get_and_lock(global, class_index, 1);
     if (ip == NULL)
         return 0;
     f = sk_EX_CALLBACK_value(ip->meth, idx);