Switch to using ossl_crypto_mutex from CRYPTO_RWLOCK

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

include/internal/quic_channel.h

@@ -16,6 +16,7 @@
 # include "internal/quic_reactor.h"
 # include "internal/quic_statm.h"
 # include "internal/time.h"
+# include "internal/thread.h"
 
 # ifndef OPENSSL_NO_QUIC
 
@@ -103,8 +104,13 @@ typedef struct quic_channel_args_st {
      * channel. The instantiator of the channel is responsible for providing a
      * mutex as this makes it easier to handle instantiation and teardown of
      * channels in situations potentially requiring locking.
+     *
+     * Note that this is a MUTEX not a RWLOCK as it needs to be an OS mutex for
+     * compatibility with an OS's condition variable wait API, whereas RWLOCK
+     * may, depending on the build configuration, be implemented using an OS's
+     * mutex primitive or using its RW mutex primitive.
      */
-    CRYPTO_RWLOCK   *mutex;
+    CRYPTO_MUTEX    *mutex;
 } QUIC_CHANNEL_ARGS;
 
 typedef struct quic_channel_st QUIC_CHANNEL;
@@ -256,7 +262,7 @@ SSL *ossl_quic_channel_get0_ssl(QUIC_CHANNEL *ch);
  * time; it does not belong to the channel but rather is presumed to belong to
  * the owner of the channel.
  */
-CRYPTO_RWLOCK *ossl_quic_channel_get_mutex(QUIC_CHANNEL *ch);
+CRYPTO_MUTEX *ossl_quic_channel_get_mutex(QUIC_CHANNEL *ch);
 
 # endif
 

ssl/quic/quic_impl.c

@@ -112,14 +112,15 @@ static ossl_inline int expect_quic_conn(const QUIC_CONNECTION *qc)
  */
 static int quic_lock(QUIC_CONNECTION *qc)
 {
-    return CRYPTO_THREAD_write_lock(qc->mutex);
+    ossl_crypto_mutex_lock(qc->mutex);
+    return 1;
 }
 
 /* Precondition: Channel mutex is held (unchecked) */
 QUIC_NEEDS_LOCK
 static void quic_unlock(QUIC_CONNECTION *qc)
 {
-    CRYPTO_THREAD_unlock(qc->mutex);
+    ossl_crypto_mutex_unlock(qc->mutex);
 }
 
 
@@ -158,7 +159,7 @@ SSL *ossl_quic_new(SSL_CTX *ctx)
     if (qc->tls == NULL || (sc = SSL_CONNECTION_FROM_SSL(qc->tls)) == NULL)
          goto err;
 
-    if ((qc->mutex = CRYPTO_THREAD_lock_new()) == NULL)
+    if ((qc->mutex = ossl_crypto_mutex_new()) == NULL)
         goto err;
 
     qc->is_thread_assisted
@@ -200,7 +201,7 @@ void ossl_quic_free(SSL *s)
     /* Note: SSL_free calls OPENSSL_free(qc) for us */
 
     SSL_free(qc->tls);
-    CRYPTO_THREAD_lock_free(qc->mutex);
+    ossl_crypto_mutex_free(&qc->mutex);
 }
 
 /* SSL method init */
@@ -681,6 +682,7 @@ static int ensure_channel(QUIC_CONNECTION *qc)
     args.propq      = qc->ssl.ctx->propq;
     args.is_server  = 0;
     args.tls        = qc->tls;
+    args.mutex      = qc->mutex;
 
     qc->ch = ossl_quic_channel_new(&args);
     if (qc->ch == NULL)
@@ -697,21 +699,23 @@ static int ensure_channel(QUIC_CONNECTION *qc)
 QUIC_NEEDS_LOCK
 static int ensure_channel_and_start(QUIC_CONNECTION *qc)
 {
-    if (!ensure_channel(qc))
+    if (!qc->started) {
-        return 0;
+        if (!ensure_channel(qc))
+            return 0;
 
-    if (!configure_channel(qc)
+        if (!configure_channel(qc)
-        || !ossl_quic_channel_start(qc->ch))
+            || !ossl_quic_channel_start(qc->ch))
-        goto err;
-
-    qc->stream0 = ossl_quic_channel_get_stream_by_id(qc->ch, 0);
-    if (qc->stream0 == NULL)
-        goto err;
-
-    if (qc->is_thread_assisted)
-        if (!ossl_quic_thread_assist_init_start(&qc->thread_assist, qc->ch))
             goto err;
 
+        qc->stream0 = ossl_quic_channel_get_stream_by_id(qc->ch, 0);
+        if (qc->stream0 == NULL)
+            goto err;
+
+        if (qc->is_thread_assisted)
+            if (!ossl_quic_thread_assist_init_start(&qc->thread_assist, qc->ch))
+                goto err;
+    }
+
     qc->started = 1;
     return 1;
 

ssl/quic/quic_thread_assist.c

@@ -23,8 +23,7 @@ static unsigned int assist_thread_main(void *arg)
     CRYPTO_MUTEX *m = ossl_quic_channel_get_mutex(qta->ch);
     QUIC_REACTOR *rtor;
 
-    if (!CRYPTO_THREAD_write_lock(m))
+    ossl_crypto_mutex_lock(m);
-        return 0;
 
     rtor = ossl_quic_channel_get_reactor(qta->ch);
 
@@ -52,7 +51,7 @@ static unsigned int assist_thread_main(void *arg)
         ossl_quic_reactor_tick(rtor, QUIC_REACTOR_TICK_FLAG_CHANNEL_ONLY);
     }
 
-    CRYPTO_THREAD_unlock(m);
+    ossl_crypto_mutex_unlock(m);
     return 1;
 }
 
@@ -92,12 +91,10 @@ int ossl_quic_thread_assist_stop_async(QUIC_THREAD_ASSIST *qta)
     return 1;
 }
 
-static void ignore_res(int x) {}
-
 int ossl_quic_thread_assist_wait_stopped(QUIC_THREAD_ASSIST *qta)
 {
     CRYPTO_THREAD_RETVAL rv;
-    CRYPTO_RWLOCK *m = ossl_quic_channel_get_mutex(qta->ch);
+    CRYPTO_MUTEX *m = ossl_quic_channel_get_mutex(qta->ch);
 
     if (qta->joined)
         return 1;
@@ -105,18 +102,16 @@ int ossl_quic_thread_assist_wait_stopped(QUIC_THREAD_ASSIST *qta)
     if (!ossl_quic_thread_assist_stop_async(qta))
         return 0;
 
-    CRYPTO_THREAD_unlock(m);
+    ossl_crypto_mutex_unlock(m);
 
     if (!ossl_crypto_thread_native_join(qta->t, &rv)) {
-        ignore_res(CRYPTO_THREAD_write_lock(m)); /* best effort */
+        ossl_crypto_mutex_lock(m);
         return 0;
     }
 
     qta->joined = 1;
 
-    if (!CRYPTO_THREAD_write_lock(m))
+    ossl_crypto_mutex_lock(m);
-        return 0;
-
     return 1;
 }