From: cchinchole <cpchinchole@gmail.com>
Date: Tue, 2 Jul 2024 01:16:03 +0000 (-0500)
Subject: Unlock only when lock was successful
X-Git-Tag: openssl-3.4.0-alpha1~412
X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=refs%2Fpull%2F24646%2Fhead;p=thirdparty%2Fopenssl.git

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)
---

diff --git a/crypto/bio/bio_addr.c b/crypto/bio/bio_addr.c
index 0a64d0749a2..abdf3523045 100644
--- a/crypto/bio/bio_addr.c
+++ b/crypto/bio/bio_addr.c
@@ -799,14 +799,12 @@ int BIO_lookup_ex(const char *host, const char *service, int lookup_type,
         if (!RUN_ONCE(&bio_lookup_init, do_bio_lookup_init)) {
             /* Should this be raised inside do_bio_lookup_init()? */
             ERR_raise(ERR_LIB_BIO, ERR_R_CRYPTO_LIB);
-            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 9dc3144bbfd..cb43e2d967f 100644
--- 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 e26f406f121..b8894f1db58 100644
--- a/doc/man3/CRYPTO_THREAD_run_once.pod
+++ b/doc/man3/CRYPTO_THREAD_run_once.pod
@@ -244,10 +244,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;
  }