]> git.ipfire.org Git - thirdparty/openssl.git/commitdiff
convert CRYPTO_THREAD_run_once to use InitOnceExecuteOnce api
authorNeil Horman <nhorman@openssl.org>
Fri, 29 May 2026 13:59:21 +0000 (09:59 -0400)
committerNeil Horman <nhorman@openssl.org>
Wed, 10 Jun 2026 19:46:53 +0000 (15:46 -0400)
Issue #22059 reported a race condition in CRYPTO_THREAD_run_once on
windows platforms.  The most correct fix for this is to convert the
windows run_once implementation to use the Win32 InitOnceRunOnce
interface.  Doing so requires at least Windows Vista/Windows Server 2008
to be available, and because WinXP hasn't built since 3.0 released, it
seems sensible to bump our minimal NT version to be 0x600 (Vista/2008)

Also, while we're at it, this change caught a bad programming practice
in the rio_notifier code, which attempts to reset the once variable
during shutdown.  The windows static initalizer macro for this api is
constructed such that attempting to do so causes a build break.  Since
once variables are not meant to be reset (since they are only triggered
once), remove that reset code to avoid the breakage.

Note that this problem was independently found and fixed in #30198.
We're taking the fix from this pr (as they are effectively identical),
and using that PR to add some much needed tests to the rio code.

Reviewed-by: Tomas Mraz <tomas@openssl.foundation>
Reviewed-by: Matt Caswell <matt@openssl.foundation>
Reviewed-by: Bob Beck <beck@openssl.org>
MergeDate: Wed Jun 10 19:44:19 2026
(Merged from https://github.com/openssl/openssl/pull/31339)

crypto/threads_win.c
include/internal/e_os.h
include/openssl/crypto.h.in
include/openssl/e_os2.h
providers/implementations/rands/seeding/rand_win.c
ssl/rio/rio_notifier.c
util/platform_symbols/windows-symbols.txt

index 4fedf24e90c0b8598567710d7b6185dd68e6ae25..d803b7221d9a69261e39059183b51d05690c4e3a 100644 (file)
@@ -512,32 +512,28 @@ void CRYPTO_THREAD_lock_free(CRYPTO_RWLOCK *lock)
     return;
 }
 
-#define ONCE_UNINITED 0
-#define ONCE_ININIT 1
-#define ONCE_DONE 2
+struct init_once_cb_info {
+    void (*init)(void);
+};
 
-/*
- * We don't use InitOnceExecuteOnce because that isn't available in WinXP which
- * we still have to support.
- */
-int CRYPTO_THREAD_run_once(CRYPTO_ONCE *once, void (*init)(void))
+static BOOL CALLBACK init_wrapper(PINIT_ONCE InitOnce, PVOID param, PVOID *context)
 {
-    LONG volatile *lock = (LONG *)once;
-    LONG result;
+    struct init_once_cb_info *info = (struct init_once_cb_info *)param;
 
-    if (*lock == ONCE_DONE)
-        return 1;
+    info->init();
+    return TRUE;
+}
 
-    do {
-        result = InterlockedCompareExchange(lock, ONCE_ININIT, ONCE_UNINITED);
-        if (result == ONCE_UNINITED) {
-            init();
-            *lock = ONCE_DONE;
-            return 1;
-        }
-    } while (result == ONCE_ININIT);
+int CRYPTO_THREAD_run_once(CRYPTO_ONCE *once, void (*init)(void))
+{
+    INIT_ONCE *myonce = (INIT_ONCE *)once;
+    struct init_once_cb_info info = { init };
+    BOOL bstatus;
 
-    return (*lock == ONCE_DONE);
+    bstatus = InitOnceExecuteOnce(myonce, init_wrapper, (void *)&info, NULL);
+    if (bstatus == TRUE)
+        return 1;
+    return 0;
 }
 
 int CRYPTO_THREAD_init_local(CRYPTO_THREAD_LOCAL *key, void (*cleanup)(void *))
index 3ea75209cf5ff36fd72e9d43b8489adb8c6dc252..f0de2f363ca0b83800453c4d279f8fdb5dff17ce 100644 (file)
  *     0x0603 // Windows 8.1
  *     0x0A00 // Windows 10
  */
-#define _WIN32_WINNT 0x0501
+#define _WIN32_WINNT 0x0600
 #endif
 #include <windows.h>
 #include <stdio.h>
index a9ade25b155498b6c95cedb6c73adbb45892579b..071b11b3bc24efdf367ecef0071c1a5ee9dfbf43 100644 (file)
@@ -543,8 +543,8 @@ void OPENSSL_INIT_free(OPENSSL_INIT_SETTINGS *settings);
 typedef DWORD CRYPTO_THREAD_LOCAL;
 typedef DWORD CRYPTO_THREAD_ID;
 
-typedef LONG CRYPTO_ONCE;
-#define CRYPTO_ONCE_STATIC_INIT 0
+typedef INIT_ONCE CRYPTO_ONCE;
+#define CRYPTO_ONCE_STATIC_INIT INIT_ONCE_STATIC_INIT
 #endif
 #else
 #if defined(__TANDEM) && defined(_SPT_MODEL_)
index bacc161bd6b1fdee8aaa49e1a0d9e0082ffd9d4c..698f2da26bf81fadef417ce61c10639724a0d758 100644 (file)
@@ -58,6 +58,7 @@ extern "C" {
 #define OPENSSL_SYS_WIN32_CYGWIN
 #else
 #if defined(_WIN32) || defined(OPENSSL_SYS_WIN32)
+#include "windows.h"
 #undef OPENSSL_SYS_UNIX
 #if !defined(OPENSSL_SYS_WIN32)
 #define OPENSSL_SYS_WIN32
index 63b523b729f37f0f6d17ba3f18a88ec2051b7e7e..5a0815042dc5c30803a336d95d7d3fadc9be86d8 100644 (file)
@@ -70,7 +70,7 @@ size_t ossl_pool_acquire_entropy(RAND_POOL *pool)
     buffer = ossl_rand_pool_add_begin(pool, bytes_needed);
     if (buffer != NULL) {
         size_t bytes = 0;
-        if (BCryptGenRandom(NULL, buffer, bytes_needed,
+        if (BCryptGenRandom(NULL, buffer, (ULONG)bytes_needed,
                 BCRYPT_USE_SYSTEM_PREFERRED_RNG)
             == STATUS_SUCCESS)
             bytes = bytes_needed;
index ab635aa7a0bee3f8e23c70fe484643b991dd2337..9d286144068c4c12cf4470d1600aa788a93d4924 100644 (file)
@@ -31,63 +31,21 @@ static int set_cloexec(int fd)
 #if defined(OPENSSL_SYS_WINDOWS)
 
 static CRYPTO_ONCE ensure_wsa_startup_once = CRYPTO_ONCE_STATIC_INIT;
-static CRYPTO_RWLOCK *wsa_lock;
-static int wsa_started;
-static int wsa_ref;
-
-static void ossl_wsa_cleanup(void)
-{
-    if (wsa_started) {
-        wsa_started = 0;
-        WSACleanup();
-    }
-
-    CRYPTO_THREAD_lock_free(wsa_lock);
-    wsa_lock = NULL;
-}
 
 DEFINE_RUN_ONCE_STATIC(do_wsa_startup)
 {
     WORD versionreq = 0x0202; /* Version 2.2 */
     WSADATA wsadata;
 
-    wsa_lock = CRYPTO_THREAD_lock_new();
-    if (wsa_lock == NULL)
+    if (WSAStartup(versionreq, &wsadata) != 0)
         return 0;
 
-    if (WSAStartup(versionreq, &wsadata) != 0) {
-        CRYPTO_THREAD_lock_free(wsa_lock);
-        wsa_lock = NULL;
-        return 0;
-    }
-    wsa_started = 1;
-
     return 1;
 }
 
 static ossl_inline int ensure_wsa_startup(void)
 {
-    int rv, unused;
-
-    rv = RUN_ONCE(&ensure_wsa_startup_once, do_wsa_startup);
-    if (rv != 0)
-        CRYPTO_atomic_add(&wsa_ref, 1, &unused, wsa_lock);
-
-    return rv;
-}
-
-static void wsa_done(void)
-{
-    int ref;
-
-    if (wsa_lock != NULL) {
-        CRYPTO_atomic_add(&wsa_ref, -1, &ref, wsa_lock);
-        if (ref == 0) {
-            ossl_wsa_cleanup();
-            ensure_wsa_startup_once = CRYPTO_ONCE_STATIC_INIT;
-            wsa_lock = NULL;
-        }
-    }
+    return RUN_ONCE(&ensure_wsa_startup_once, do_wsa_startup);
 }
 
 #endif
@@ -188,8 +146,6 @@ int ossl_rio_notifier_init(RIO_NOTIFIER *nfy)
     if (!ensure_wsa_startup()) {
         ERR_raise_data(ERR_LIB_SSL, ERR_R_INTERNAL_ERROR,
             "Cannot start Windows sockets");
-
-        wsa_done();
         return 0;
     }
 #endif
@@ -376,9 +332,6 @@ void ossl_rio_notifier_cleanup(RIO_NOTIFIER *nfy)
     BIO_closesocket(nfy->wfd);
     BIO_closesocket(nfy->rfd);
     nfy->rfd = nfy->wfd = -1;
-#if defined(OPENSSL_SYS_WINDOWS)
-    wsa_done();
-#endif
 }
 
 int ossl_rio_notifier_signal(RIO_NOTIFIER *nfy)
index 0f6cc11450cd03866dc3091a81eb18bfd242ec1a..7ae58a3f182a1e2bab7acc6c0a0d5a6a03dc3548 100644 (file)
@@ -1,5 +1,6 @@
 AcquireSRWLockExclusive
 AcquireSRWLockShared
+BCryptGenRandom
 CertCloseStore
 CertFindCertificateInStore
 CertFreeCertificateContext