Fix data race on error queue state in wc_LoggingInit

[annihilatorq]

Description

In builds with the global error queue enabled (OPENSSL_EXTRA without ERROR_QUEUE_PER_THREAD), wc_LoggingInit() resets wc_errors, wc_errors_count, wc_last_node, and wc_current_node without acquiring the wc_error_mutex, whereas any other access to the queue (e.g. wc_ClearErrorNodes()) read/write the same variables while acquiring this mutex.

This may affect the application code because wolfSSL_CTX_new() implicitly calls wolfSSL_Init() internally when initRefCount == 0, and a call to wolfSSL_ERR_clear_error() may be made before wolfSSL_Init() is called (for example - asio does this). As a result:

Thread A:
1. ERR_clear_error()
2. clearErrorNodes()
3. wc_errors_count = 0 (under the wc_error_mutex)

Thread B:
1. SSL_CTX_new()
2. wolfSSL_Init()
3. wc_LoggingInit()
4. wc_errors_count, wc_errors, wc_current_node, wc_last_node = 0 or NULL (without acquiring the wc_error_mutex)

Found by TSan in the test-suite of my project that supports asio with OpenSSL-compatible wolfSSL (vcpkg wolfssl[asio], version 5.8.4 with global error queue, to be exact). In its constructor, asio::ssl::context first calls ERR_clear_error(), then SSL_CTX_new().

Fix: acquire the wc_error_mutex inside wc_LoggingInit while assigning new values to wc_errors_count, wc_errors, wc_current_node, and wc_last_node

Testing

./configure --enable-all --disable-error-queue-per-thread && make check and ./configure --enable-all && make check both pass (17 pass, 6 skipped).

Minimal repro. wolfSSL should be built with OPENSSL_EXTRA, disabled ERROR_QUEUE_PER_THREAD, and -fsanitize=thread:

#include <wolfssl/options.h>
#include <wolfssl/ssl.h>
#include <pthread.h>
#include <stdio.h>

static pthread_barrier_t go;

static void* clearer(void* arg)
{
    int i;
    (void)arg;
    pthread_barrier_wait(&go);
    for (i = 0; i < 2000; i++)
        wolfSSL_ERR_clear_error();
    return NULL;
}

int main(void)
{
    pthread_t t;
    WOLFSSL_CTX* ctx;

    pthread_barrier_init(&go, NULL, 2);
    pthread_create(&t, NULL, clearer, NULL);
    pthread_barrier_wait(&go);
    ctx = wolfSSL_CTX_new(wolfTLS_client_method()); /* -> wolfSSL_Init() */
    pthread_join(t, NULL);
    wolfSSL_CTX_free(ctx);
    wolfSSL_Cleanup();
    printf("done ctx=%p\n", (void*)ctx);
    return 0;
}

TSan report (unpatched)

==================
WARNING: ThreadSanitizer: data race (pid=154571)
  Write of size 4 at 0x572437fdc8c8 by main thread (mutexes: write M0):
    #0 wc_LoggingInit wolfcrypt/src/logging.c:1110
    #1 wolfCrypt_Init wolfcrypt/src/wc_port.c:669
    #2 wolfSSL_Init src/ssl.c:2463
    #3 wolfSSL_CTX_new_ex src/ssl.c:552
    #4 wolfSSL_CTX_new src/ssl.c:656
    #5 main race.c:29

  Previous write of size 4 at 0x572437fdc8c8 by thread T1 (mutexes: write M1):
    #0 clearErrorNodes wolfcrypt/src/logging.c:1418
    #1 wc_ClearErrorNodes wolfcrypt/src/logging.c:1433
    #2 wolfSSL_ERR_clear_error src/ssl_err.c:204
    #3 clearer race.c:17

  Location is global 'wc_errors_count' of size 4 at 0x572437fdc8c8

  Mutex M0 (0x572437fdc680) created at:
    #0 pthread_mutex_lock ../../../../src/libsanitizer/tsan/tsan_interceptors_posix.cpp:1424
    #1 wc_LockMutex wolfcrypt/src/wc_port.c:3185
    #2 wolfSSL_Init src/ssl.c:2447
    #3 wolfSSL_CTX_new_ex src/ssl.c:552
    #4 wolfSSL_CTX_new src/ssl.c:656
    #5 main race.c:29

  Mutex M1 (0x572437fdc900) created at:
    #0 pthread_mutex_lock ../../../../src/libsanitizer/tsan/tsan_interceptors_posix.cpp:1424
    #1 wc_LockMutex wolfcrypt/src/wc_port.c:3185
    #2 wc_ClearErrorNodes wolfcrypt/src/logging.c:1428
    #3 wolfSSL_ERR_clear_error src/ssl_err.c:204
    #4 clearer race.c:17

  Thread T1 (tid=154574, running) created by main thread at:
    #0 pthread_create ../../../../src/libsanitizer/tsan/tsan_interceptors_posix.cpp:1078
    #1 main race.c:27

SUMMARY: ThreadSanitizer: data race wolfcrypt/src/logging.c:1110 in wc_LoggingInit
==================

TSan report (patched): clean.

Checklist

• added tests
• updated/added doxygen
• updated appropriate READMEs
• Updated manual and documentation

No tests were added because the race cannot be deterministically asserted.

[wolfSSL-Bot]

Can one of the admins verify this patch?