Unlock only when lock was successful

Addressing issue (openssl#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 <[email protected]> Reviewed-by: Tomas Mraz <[email protected]> Reviewed-by: Todd Short <[email protected]> (Merged from openssl#24779)
a-mair · Jul 3, 2024 · 3f4da93 · 3f4da93
1 parent e6174ca
commit 3f4da93
Show file tree

Hide file tree

Showing 3 changed files with 14 additions and 11 deletions.
diff --git 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
@@ -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
@@ -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;
  }