Unlock only when lock was successful

Addressing issue (#24517):
Updated the example in CRYPTO_THREAD_run_once.pod to reflect that an unlock call should not be made if a write_lock failed.
Updated BIO_lookup_ex in bio_addr.c and ossl_engine_table_select in eng_table.c to not call unlock if the lock failed.

Reviewed-by: Neil Horman <nhorman@openssl.org>
Reviewed-by: Tomas Mraz <tomas@openssl.org>
Reviewed-by: Todd Short <todd.short@me.com>
(Merged from https://github.com/openssl/openssl/pull/24779)

(cherry picked from commit 3f4da93678497fe64d262d03c388932f7ecfe74e)

diff --git a/crypto/bio/bio_addr.c b/crypto/bio/bio_addr.c
index 20c2895b509ba1d7c8ed227f3646ff5451c13e0f..e25c7f5d211f5ede61a596b3921756adcb25681c 100644 (file)
--- a/crypto/bio/bio_addr.c
+++ b/crypto/bio/bio_addr.c
@@ -778,14 +778,12 @@ int BIO_lookup_ex(const char *host, const char *service, int lookup_type,
 
         if (!RUN_ONCE(&bio_lookup_init, do_bio_lookup_init)) {
             ERR_raise(ERR_LIB_BIO, ERR_R_MALLOC_FAILURE);
-            ret = 0;
-            goto err;
+            return 0;
         }
 
-        if (!CRYPTO_THREAD_write_lock(bio_lookup_lock)) {
-            ret = 0;
-            goto err;
-        }
+        if (!CRYPTO_THREAD_write_lock(bio_lookup_lock))
+            return 0;
+        
         he_fallback_address = INADDR_ANY;
         if (host == NULL) {
             he = &he_fallback;

diff --git a/crypto/engine/eng_table.c b/crypto/engine/eng_table.c
index 9dc3144bbfd7b610c1cd73f056a9c7a0ea862bc8..cb43e2d967fa41d4f6df1fbdc250e53b0cf5ff9e 100644 (file)
--- a/crypto/engine/eng_table.c
+++ b/crypto/engine/eng_table.c
@@ -215,9 +215,11 @@ ENGINE *ossl_engine_table_select(ENGINE_TABLE **table, int nid,
                    f, l, nid);
         return NULL;
     }
-    ERR_set_mark();
+
     if (!CRYPTO_THREAD_write_lock(global_engine_lock))
-        goto end;
+        return NULL;
+
+    ERR_set_mark();
     /*
      * Check again inside the lock otherwise we could race against cleanup
      * operations. But don't worry about a debug printout

diff --git a/doc/man3/CRYPTO_THREAD_run_once.pod b/doc/man3/CRYPTO_THREAD_run_once.pod
index adb1259f238919dc67d4ab2ed3458e1bd06c09f3..9fc44e84fbe3819c0188307704592b226379937f 100644 (file)
--- a/doc/man3/CRYPTO_THREAD_run_once.pod
+++ b/doc/man3/CRYPTO_THREAD_run_once.pod
@@ -164,10 +164,13 @@ This example safely initializes and uses a lock.
  {
      int ret = 0;
 
-     if (mylock()) {
-         /* Your code here, do not return without releasing the lock! */
-         ret = ... ;
+     if (!mylock()) {
+        /* Do not unlock unless the lock was successfully acquired. */
+        return 0;
      }
+
+     /* Your code here, do not return without releasing the lock! */
+     ret = ... ;
      myunlock();
      return ret;
  }