QUIC: Fix bugs where threading is disabled

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

diff --git a/crypto/threads_none.c b/crypto/threads_none.c
index b18587b9f21cc5098f62d1994119b2c12914bfdb..a2f4b1fde0e1ce4b3a32d4d9016fd95517f7139a 100644 (file)
--- a/crypto/threads_none.c
+++ b/crypto/threads_none.c
@@ -148,6 +148,13 @@ int CRYPTO_atomic_load(uint64_t *val, uint64_t *ret, CRYPTO_RWLOCK *lock)
     return 1;
 }
 
+int CRYPTO_atomic_load_int(int *val, int *ret, CRYPTO_RWLOCK *lock)
+{
+    *ret = *val;
+
+    return 1;
+}
+
 int openssl_init_fork_handlers(void)
 {
     return 0;

diff --git a/crypto/threads_pthread.c b/crypto/threads_pthread.c
index 4aeba50479d9033642c4c80b2abd544f8544b504..a511271c530ed0f8324d47ff10794b4a2c86b0a3 100644 (file)
--- a/crypto/threads_pthread.c
+++ b/crypto/threads_pthread.c
@@ -270,6 +270,30 @@ int CRYPTO_atomic_load(uint64_t *val, uint64_t *ret, CRYPTO_RWLOCK *lock)
 
     return 1;
 }
+
+int CRYPTO_atomic_load_int(int *val, int *ret, CRYPTO_RWLOCK *lock)
+{
+# if defined(__GNUC__) && defined(__ATOMIC_ACQUIRE) && !defined(BROKEN_CLANG_ATOMICS)
+    if (__atomic_is_lock_free(sizeof(*val), val)) {
+        __atomic_load(val, ret, __ATOMIC_ACQUIRE);
+        return 1;
+    }
+# elif defined(__sun) && (defined(__SunOS_5_10) || defined(__SunOS_5_11))
+    /* This will work for all future Solaris versions. */
+    if (ret != NULL) {
+        *ret = (int *)atomic_or_uint_nv((unsigned int *)val, 0);
+        return 1;
+    }
+# endif
+    if (lock == NULL || !CRYPTO_THREAD_read_lock(lock))
+        return 0;
+    *ret  = *val;
+    if (!CRYPTO_THREAD_unlock(lock))
+        return 0;
+
+    return 1;
+}
+
 # ifndef FIPS_MODULE
 int openssl_init_fork_handlers(void)
 {

diff --git a/crypto/threads_win.c b/crypto/threads_win.c
index 7b2e876540348fb1f30664a2637baa2d96a3df90..82137b530ae26465e4876ef9f841f76aaa6359ec 100644 (file)
--- a/crypto/threads_win.c
+++ b/crypto/threads_win.c
@@ -251,6 +251,22 @@ int CRYPTO_atomic_load(uint64_t *val, uint64_t *ret, CRYPTO_RWLOCK *lock)
 #endif
 }
 
+int CRYPTO_atomic_load_int(int *val, int *ret, CRYPTO_RWLOCK *lock)
+{
+#if (defined(NO_INTERLOCKEDOR64))
+    if (lock == NULL || !CRYPTO_THREAD_read_lock(lock))
+        return 0;
+    *ret = *val;
+    if (!CRYPTO_THREAD_unlock(lock))
+        return 0;
+
+    return 1;
+#else
+    *ret = (int)InterlockedOr((LONG volatile *)val, 0);
+    return 1;
+#endif
+}
+
 int openssl_init_fork_handlers(void)
 {
     return 0;

diff --git a/doc/man3/CRYPTO_THREAD_run_once.pod b/doc/man3/CRYPTO_THREAD_run_once.pod
index fd2d6a207f1dfb9df4f08aaf94b8e9abf80722c0..1badd193978f5f4fb408ce7c8693d421a356991f 100644 (file)
--- a/doc/man3/CRYPTO_THREAD_run_once.pod
+++ b/doc/man3/CRYPTO_THREAD_run_once.pod
@@ -6,6 +6,7 @@ CRYPTO_THREAD_run_once,
 CRYPTO_THREAD_lock_new, CRYPTO_THREAD_read_lock, CRYPTO_THREAD_write_lock,
 CRYPTO_THREAD_unlock, CRYPTO_THREAD_lock_free,
 CRYPTO_atomic_add, CRYPTO_atomic_or, CRYPTO_atomic_load,
+CRYPTO_atomic_load_int,
 OSSL_set_max_threads, OSSL_get_max_threads,
 OSSL_get_thread_support_flags - OpenSSL thread support
 
@@ -26,6 +27,7 @@ OSSL_get_thread_support_flags - OpenSSL thread support
  int CRYPTO_atomic_or(uint64_t *val, uint64_t op, uint64_t *ret,
                       CRYPTO_RWLOCK *lock);
  int CRYPTO_atomic_load(uint64_t *val, uint64_t *ret, CRYPTO_RWLOCK *lock);
+ int CRYPTO_atomic_load_int(int *val, int *ret, CRYPTO_RWLOCK *lock);
 
  int OSSL_set_max_threads(OSSL_LIB_CTX *ctx, uint64_t max_threads);
  uint64_t OSSL_get_max_threads(OSSL_LIB_CTX *ctx);
@@ -106,6 +108,11 @@ NULL, then the function will fail.
 
 =item *
 
+CRYPTO_atomic_load_int() works identically to CRYPTO_atomic_load() but operates
+on an I<int> value instead of a I<uint64_t> value.
+
+=item *
+
 OSSL_set_max_threads() sets the maximum number of threads to be used by the
 thread pool. If the argument is 0, thread pooling is disabled. OpenSSL will
 not create any threads and existing threads in the thread pool will be torn

diff --git a/include/internal/cryptlib.h b/include/internal/cryptlib.h
index 700f387531f9f3a09823c0f2d3711eb56335218c..5aeb4fe0f2b9a1ba86843e4da966ceefa56c5b62 100644 (file)
--- a/include/internal/cryptlib.h
+++ b/include/internal/cryptlib.h
@@ -159,4 +159,5 @@ char *ossl_ipaddr_to_asc(unsigned char *p, int len);
 char *ossl_buf2hexstr_sep(const unsigned char *buf, long buflen, char sep);
 unsigned char *ossl_hexstr2buf_sep(const char *str, long *buflen,
                                    const char sep);
+
 #endif

diff --git a/include/internal/refcount.h b/include/internal/refcount.h
index 15f33d8c02d6a57fcdc6f5dec4fb59b43fb7ddaa..b74c283ae5df78ed5dd6770656412dcec7ebcbcc 100644 (file)
--- a/include/internal/refcount.h
+++ b/include/internal/refcount.h
@@ -197,7 +197,7 @@ typedef int CRYPTO_REF_COUNT;
 
 # define CRYPTO_UP_REF(val, ret, lock) CRYPTO_atomic_add(val, 1, ret, lock)
 # define CRYPTO_DOWN_REF(val, ret, lock) CRYPTO_atomic_add(val, -1, ret, lock)
-# define CRYPTO_GET_REF(val, ret, lock) CRYPTO_atomic_load(val, ret, lock)
+# define CRYPTO_GET_REF(val, ret, lock) CRYPTO_atomic_load_int(val, ret, lock)
 
 # endif
 

diff --git a/include/openssl/crypto.h.in b/include/openssl/crypto.h.in
index 963bc635fb10299279bc7c692118624f7fe02c4c..fb672811337fd293ec86c6eeb11487a2f89bc608 100644 (file)
--- a/include/openssl/crypto.h.in
+++ b/include/openssl/crypto.h.in
@@ -89,6 +89,7 @@ int CRYPTO_atomic_add(int *val, int amount, int *ret, CRYPTO_RWLOCK *lock);
 int CRYPTO_atomic_or(uint64_t *val, uint64_t op, uint64_t *ret,
                      CRYPTO_RWLOCK *lock);
 int CRYPTO_atomic_load(uint64_t *val, uint64_t *ret, CRYPTO_RWLOCK *lock);
+int CRYPTO_atomic_load_int(int *val, int *ret, CRYPTO_RWLOCK *lock);
 
 /* No longer needed, so this is a no-op */
 #define OPENSSL_malloc_init() while(0) continue

diff --git a/ssl/quic/quic_impl.c b/ssl/quic/quic_impl.c
index 1f76232c49c524bceefc69268284608956b16816..ac0e9f0380618cafafedca698a71a62d07bfcfe8 100644 (file)
--- a/ssl/quic/quic_impl.c
+++ b/ssl/quic/quic_impl.c
@@ -258,14 +258,18 @@ static int ossl_unused expect_quic_conn_only(const SSL *s, QCTX *ctx)
  */
 static void quic_lock(QUIC_CONNECTION *qc)
 {
+#if defined(OPENSSL_THREADS)
     ossl_crypto_mutex_lock(qc->mutex);
+#endif
 }
 
 /* Precondition: Channel mutex is held (unchecked) */
 QUIC_NEEDS_LOCK
 static void quic_unlock(QUIC_CONNECTION *qc)
 {
+#if defined(OPENSSL_THREADS)
     ossl_crypto_mutex_unlock(qc->mutex);
+#endif
 }
 
 
@@ -304,11 +308,15 @@ SSL *ossl_quic_new(SSL_CTX *ctx)
     if (qc->tls == NULL || (sc = SSL_CONNECTION_FROM_SSL(qc->tls)) == NULL)
          goto err;
 
+#if defined(OPENSSL_THREADS)
     if ((qc->mutex = ossl_crypto_mutex_new()) == NULL)
         goto err;
+#endif
 
+#if !defined(OPENSSL_NO_QUIC_THREAD_ASSIST)
     qc->is_thread_assisted
         = (ssl_base->method == OSSL_QUIC_client_thread_method());
+#endif
 
     qc->as_server       = 0; /* TODO(QUIC): server support */
     qc->as_server_state = qc->as_server;
@@ -338,6 +346,9 @@ SSL *ossl_quic_new(SSL_CTX *ctx)
 
 err:
     if (qc != NULL) {
+#if defined(OPENSSL_THREADS)
+        ossl_crypto_mutex_free(qc->mutex);
+#endif
         ossl_quic_channel_free(qc->ch);
         SSL_free(qc->tls);
     }
@@ -418,10 +429,12 @@ void ossl_quic_free(SSL *s)
     /* Ensure we have no remaining XSOs. */
     assert(ctx.qc->num_xso == 0);
 
+#if !defined(OPENSSL_NO_QUIC_THREAD_ASSIST)
     if (ctx.qc->is_thread_assisted && ctx.qc->started) {
         ossl_quic_thread_assist_wait_stopped(&ctx.qc->thread_assist);
         ossl_quic_thread_assist_cleanup(&ctx.qc->thread_assist);
     }
+#endif
 
     ossl_quic_channel_free(ctx.qc->ch);
 
@@ -431,7 +444,9 @@ void ossl_quic_free(SSL *s)
     /* Note: SSL_free calls OPENSSL_free(qc) for us */
 
     SSL_free(ctx.qc->tls);
+#if defined(OPENSSL_THREADS)
     ossl_crypto_mutex_free(&ctx.qc->mutex); /* freed while still locked */
+#endif
 }
 
 /* SSL method init */
@@ -491,8 +506,10 @@ void ossl_quic_conn_force_assist_thread_wake(SSL *s)
     if (!expect_quic(s, &ctx))
         return;
 
+#if !defined(OPENSSL_NO_QUIC_THREAD_ASSIST)
     if (ctx.qc->is_thread_assisted && ctx.qc->started)
         ossl_quic_thread_assist_notify_deadline_changed(&ctx.qc->thread_assist);
+#endif
 }
 
 QUIC_NEEDS_LOCK
@@ -1121,9 +1138,11 @@ static int ensure_channel_started(QUIC_CONNECTION *qc)
             || !ossl_quic_channel_start(qc->ch))
             goto err;
 
+#if !defined(OPENSSL_NO_QUIC_THREAD_ASSIST)
         if (qc->is_thread_assisted)
             if (!ossl_quic_thread_assist_init_start(&qc->thread_assist, qc->ch))
                 goto err;
+#endif
     }
 
     qc->started = 1;

diff --git a/ssl/quic/quic_reactor.c b/ssl/quic/quic_reactor.c
index 853d62f94706e73824bc284268b77e4a4525aef6..4d65a408bbff1a58fe28e3e8235544e30cc178c3 100644 (file)
--- a/ssl/quic/quic_reactor.c
+++ b/ssl/quic/quic_reactor.c
@@ -175,8 +175,10 @@ static int poll_two_fds(int rfd, int rfd_want_read,
         /* Do not block forever; should not happen. */
         return 0;
 
+# if defined(OPENSSL_THREADS)
     if (mutex != NULL)
         ossl_crypto_mutex_unlock(mutex);
+# endif
 
     do {
         /*
@@ -200,8 +202,10 @@ static int poll_two_fds(int rfd, int rfd_want_read,
         pres = select(maxfd + 1, &rfd_set, &wfd_set, &efd_set, ptv);
     } while (pres == -1 && get_last_socket_error_is_eintr());
 
+# if defined(OPENSSL_THREADS)
     if (mutex != NULL)
         ossl_crypto_mutex_lock(mutex);
+# endif
 
     return pres < 0 ? 0 : 1;
 #else
@@ -232,8 +236,10 @@ static int poll_two_fds(int rfd, int rfd_want_read,
         /* Do not block forever; should not happen. */
         return 0;
 
+# if defined(OPENSSL_THREADS)
     if (mutex != NULL)
         ossl_crypto_mutex_unlock(mutex);
+# endif
 
     do {
         if (ossl_time_is_infinite(deadline)) {
@@ -247,8 +253,10 @@ static int poll_two_fds(int rfd, int rfd_want_read,
         pres = poll(pfds, npfd, timeout_ms);
     } while (pres == -1 && get_last_socket_error_is_eintr());
 
+# if defined(OPENSSL_THREADS)
     if (mutex != NULL)
         ossl_crypto_mutex_lock(mutex);
+# endif
 
     return pres < 0 ? 0 : 1;
 #endif

diff --git a/ssl/quic/quic_tserver.c b/ssl/quic/quic_tserver.c
index 6788851f29879dd8f274d2d2dd04bc44e0b27fa2..6fc341f3c4ecf2d58921454da308604435a1386e 100644 (file)
--- a/ssl/quic/quic_tserver.c
+++ b/ssl/quic/quic_tserver.c
@@ -67,8 +67,10 @@ QUIC_TSERVER *ossl_quic_tserver_new(const QUIC_TSERVER_ARGS *args,
 
     srv->args = *args;
 
+#if defined(OPENSSL_THREADS)
     if ((srv->mutex = ossl_crypto_mutex_new()) == NULL)
         goto err;
+#endif
 
     srv->ctx = SSL_CTX_new_ex(srv->args.libctx, srv->args.propq, TLS_method());
     if (srv->ctx == NULL)
@@ -106,7 +108,9 @@ QUIC_TSERVER *ossl_quic_tserver_new(const QUIC_TSERVER_ARGS *args,
 err:
     if (srv != NULL) {
         ossl_quic_channel_free(srv->ch);
+#if defined(OPENSSL_THREADS)
         ossl_crypto_mutex_free(&srv->mutex);
+#endif
     }
 
     OPENSSL_free(srv);
@@ -123,7 +127,9 @@ void ossl_quic_tserver_free(QUIC_TSERVER *srv)
     BIO_free(srv->args.net_wbio);
     SSL_free(srv->tls);
     SSL_CTX_free(srv->ctx);
+#if defined(OPENSSL_THREADS)
     ossl_crypto_mutex_free(&srv->mutex);
+#endif
     OPENSSL_free(srv);
 }
 

diff --git a/test/quic_multistream_test.c b/test/quic_multistream_test.c
index 851f45278a2ad439b2a0dc7bdb8d20de906efc6f..f846a412b513ac36181c90b96a9f4ca8f42b93b9 100644 (file)
--- a/test/quic_multistream_test.c
+++ b/test/quic_multistream_test.c
@@ -581,7 +581,10 @@ static int run_script_worker(struct helper *h, const struct script_op *script,
     size_t offset = 0;
     size_t op_idx = 0;
     const struct script_op *op = NULL;
-    int no_advance = 0, first = 1, end_wait_warning = 0;
+    int no_advance = 0, first = 1;
+#if defined(OPENSSL_THREADS)
+    int end_wait_warning = 0;
+#endif
     OSSL_TIME op_start_time = ossl_time_zero(), op_deadline = ossl_time_zero();
     struct helper_local hl;
 #define REPEAT_SLOTS 8
@@ -657,6 +660,7 @@ static int run_script_worker(struct helper *h, const struct script_op *script,
             if (!TEST_size_t_eq(repeat_stack_len, 0))
                 goto out;
 
+#if defined(OPENSSL_THREADS)
             if (thread_idx < 0) {
                 int done;
                 size_t i;
@@ -679,6 +683,7 @@ static int run_script_worker(struct helper *h, const struct script_op *script,
                     }
                 }
             }
+#endif
 
             TEST_info("script finished on thread %d", thread_idx);
             testresult = 1;
@@ -1190,7 +1195,12 @@ static int run_script_worker(struct helper *h, const struct script_op *script,
         case OPK_NEW_THREAD:
             {
 #if !defined(OPENSSL_THREADS)
-                TEST_error("threading not supported");
+                /*
+                 * If this test script requires threading and we do not have
+                 * support for it, skip the rest of it.
+                 */
+                TEST_skip("threading not supported, skipping");
+                testresult = 1;
                 goto out;
 #else
                 size_t i;
@@ -1261,8 +1271,10 @@ static int run_script(const struct script_op *script, int free_order)
     if (!TEST_true(run_script_worker(&h, script, -1)))
         goto out;
 
+#if defined(OPENSSL_THREADS)
     if (!TEST_true(join_threads(h.threads, h.num_threads)))
         goto out;
+#endif
 
     testresult = 1;
 out:

diff --git a/test/quic_tserver_test.c b/test/quic_tserver_test.c
index cc412eafd0f5524e524675ea5bf87ce9a101b078..ae25adc44437ad3aa7f5a122b5f9fcf4f8b6a42a 100644 (file)
--- a/test/quic_tserver_test.c
+++ b/test/quic_tserver_test.c
@@ -12,6 +12,7 @@
 #include "internal/common.h"
 #include "internal/sockets.h"
 #include "internal/quic_tserver.h"
+#include "internal/quic_thread_assist.h"
 #include "internal/quic_ssl.h"
 #include "internal/time.h"
 #include "testutil.h"
@@ -75,6 +76,13 @@ static int do_test(int use_thread_assist, int use_fake_time, int use_inject)
     OSSL_TIME (*now_cb)(void *arg) = use_fake_time ? fake_now : real_now;
     size_t limit_ms = 1000;
 
+#if defined(OPENSSL_NO_QUIC_THREAD_ASSIST)
+    if (use_thread_assist) {
+        TEST_skip("thread assisted mode not enabled");
+        return 1;
+    }
+#endif
+
     ina.s_addr = htonl(0x7f000001UL);
 
     /* Setup test server. */

diff --git a/util/libcrypto.num b/util/libcrypto.num
index d3298ab4c6860c86fc77c3d0514f580a364bf043..753d1348ba6df07f1d685a106f5a9ba428c5dfe0 100644 (file)
--- a/util/libcrypto.num
+++ b/util/libcrypto.num
@@ -5517,3 +5517,4 @@ EC_GROUP_to_params                      ? 3_2_0   EXIST::FUNCTION:EC
 X509_STORE_CTX_init_rpk                 ?      3_2_0   EXIST::FUNCTION:
 X509_STORE_CTX_get0_rpk                 ?      3_2_0   EXIST::FUNCTION:
 X509_STORE_CTX_set0_rpk                 ?      3_2_0   EXIST::FUNCTION:
+CRYPTO_atomic_load_int                  ?      3_2_0   EXIST::FUNCTION: