Skip to content

Commit

Permalink
Merge pull request wolfSSL#8169 from douzzer/20241108-WOLFSSL_CLEANUP…
Browse files Browse the repository at this point in the history
…_THREADSAFE

20241108-WOLFSSL_CLEANUP_THREADSAFE
  • Loading branch information
dgarske authored Nov 13, 2024
2 parents 878cf3a + 0ebd86d commit c8f56f0
Show file tree
Hide file tree
Showing 9 changed files with 126 additions and 26 deletions.
6 changes: 4 additions & 2 deletions src/internal.c
Original file line number Diff line number Diff line change
Expand Up @@ -25672,7 +25672,9 @@ const char* wolfSSL_ERR_reason_error_string(unsigned long e)
}

/* pass to wolfCrypt */
if (error <= WC_FIRST_E && error >= WC_LAST_E) {
if ((error <= WC_SPAN1_FIRST_E && error >= WC_SPAN1_MIN_CODE_E) ||
(error <= WC_SPAN2_FIRST_E && error >= WC_SPAN2_MIN_CODE_E))
{
return wc_GetErrorString(error);
}

Expand All @@ -25684,7 +25686,7 @@ const char* wolfSSL_ERR_reason_error_string(unsigned long e)
#endif
}

switch ((enum wolfSSL_ErrorCodes)error) {
switch ((enum wolfSSL_ErrorCodes)error) { /* // NOLINT(clang-analyzer-optin.core.EnumCastOutOfRange) */

case UNSUPPORTED_SUITE :
return "unsupported cipher suite";
Expand Down
72 changes: 66 additions & 6 deletions src/ssl.c
Original file line number Diff line number Diff line change
Expand Up @@ -1037,7 +1037,7 @@ static volatile WOLFSSL_GLOBAL int initRefCount = 0;
static WOLFSSL_GLOBAL wolfSSL_Mutex inits_count_mutex
WOLFSSL_MUTEX_INITIALIZER_CLAUSE(inits_count_mutex);
#ifndef WOLFSSL_MUTEX_INITIALIZER
static WOLFSSL_GLOBAL int inits_count_mutex_valid = 0;
static WOLFSSL_GLOBAL volatile int inits_count_mutex_valid = 0;
#endif

/* Create a new WOLFSSL_CTX struct and return the pointer to created struct.
Expand Down Expand Up @@ -5641,12 +5641,48 @@ int AddCA(WOLFSSL_CERT_MANAGER* cm, DerBuffer** pDer, int type, int verify)
static int wolfSSL_RAND_InitMutex(void);
#endif

/* If we don't have static mutex initializers, but we do have static atomic
* initializers, activate WOLFSSL_CLEANUP_THREADSAFE_BY_ATOMIC_OPS to leverage
* the latter.
*
* See further explanation below in wolfSSL_Init().
*/
#ifndef WOLFSSL_CLEANUP_THREADSAFE_BY_ATOMIC_OPS
#if !defined(WOLFSSL_MUTEX_INITIALIZER) && !defined(SINGLE_THREADED) && \
defined(WOLFSSL_ATOMIC_OPS) && defined(WOLFSSL_ATOMIC_INITIALIZER)
#define WOLFSSL_CLEANUP_THREADSAFE_BY_ATOMIC_OPS 1
#else
#define WOLFSSL_CLEANUP_THREADSAFE_BY_ATOMIC_OPS 0
#endif
#elif defined(WOLFSSL_MUTEX_INITIALIZER) || defined(SINGLE_THREADED)
#undef WOLFSSL_CLEANUP_THREADSAFE_BY_ATOMIC_OPS
#define WOLFSSL_CLEANUP_THREADSAFE_BY_ATOMIC_OPS 0
#endif

#if WOLFSSL_CLEANUP_THREADSAFE_BY_ATOMIC_OPS
#ifndef WOLFSSL_ATOMIC_OPS
#error WOLFSSL_CLEANUP_THREADSAFE_BY_ATOMIC_OPS requires WOLFSSL_ATOMIC_OPS
#endif
#ifndef WOLFSSL_ATOMIC_INITIALIZER
#error WOLFSSL_CLEANUP_THREADSAFE_BY_ATOMIC_OPS requires WOLFSSL_ATOMIC_INITIALIZER
#endif
static wolfSSL_Atomic_Int inits_count_mutex_atomic_initing_flag =
WOLFSSL_ATOMIC_INITIALIZER(0);
#endif /* WOLFSSL_CLEANUP_THREADSAFE_BY_ATOMIC_OPS && !WOLFSSL_MUTEX_INITIALIZER */

#if defined(OPENSSL_EXTRA) && defined(HAVE_ATEXIT)
static void AtExitCleanup(void)
{
if (initRefCount > 0) {
initRefCount = 1;
(void)wolfSSL_Cleanup();
#if WOLFSSL_CLEANUP_THREADSAFE_BY_ATOMIC_OPS
if (inits_count_mutex_valid == 1) {
(void)wc_FreeMutex(&inits_count_mutex);
inits_count_mutex_valid = 0;
inits_count_mutex_atomic_initing_flag = 0;
}
#endif
}
}
#endif
Expand All @@ -5663,8 +5699,31 @@ int wolfSSL_Init(void)

#ifndef WOLFSSL_MUTEX_INITIALIZER
if (inits_count_mutex_valid == 0) {
#if WOLFSSL_CLEANUP_THREADSAFE_BY_ATOMIC_OPS

/* Without this mitigation, if two threads enter wolfSSL_Init() at the
* same time, and both see zero inits_count_mutex_valid, then both will
* run wc_InitMutex(&inits_count_mutex), leading to process corruption
* or (best case) a resource leak.
*
* When WOLFSSL_ATOMIC_INITIALIZER() is available, we can mitigate this
* by use an atomic counting int as a mutex.
*/

if (wolfSSL_Atomic_Int_FetchAdd(&inits_count_mutex_atomic_initing_flag,
1) != 0)
{
(void)wolfSSL_Atomic_Int_FetchSub(
&inits_count_mutex_atomic_initing_flag, 1);
return DEADLOCK_AVERTED_E;
}
#endif /* WOLFSSL_CLEANUP_THREADSAFE_BY_ATOMIC_OPS */
if (wc_InitMutex(&inits_count_mutex) != 0) {
WOLFSSL_MSG("Bad Init Mutex count");
#if WOLFSSL_CLEANUP_THREADSAFE_BY_ATOMIC_OPS
(void)wolfSSL_Atomic_Int_FetchSub(
&inits_count_mutex_atomic_initing_flag, 1);
#endif
return BAD_MUTEX_E;
}
else {
Expand Down Expand Up @@ -10422,7 +10481,8 @@ int wolfSSL_Cleanup(void)
#endif
#endif /* !NO_SESSION_CACHE */

#ifndef WOLFSSL_MUTEX_INITIALIZER
#if !defined(WOLFSSL_MUTEX_INITIALIZER) && \
!WOLFSSL_CLEANUP_THREADSAFE_BY_ATOMIC_OPS
if ((inits_count_mutex_valid == 1) &&
(wc_FreeMutex(&inits_count_mutex) != 0)) {
if (ret == WOLFSSL_SUCCESS)
Expand Down Expand Up @@ -15729,11 +15789,11 @@ int wolfSSL_ERR_GET_REASON(unsigned long err)
return ASN1_R_HEADER_TOO_LONG;
#endif

/* check if error value is in range of wolfSSL errors */
/* check if error value is in range of wolfCrypt or wolfSSL errors */
ret = 0 - ret; /* setting as negative value */
/* wolfCrypt range is less than MAX (-100)
wolfSSL range is MIN (-300) and lower */
if ((ret <= WC_FIRST_E && ret >= WC_LAST_E) ||

if ((ret <= WC_SPAN1_FIRST_E && ret >= WC_SPAN1_LAST_E) ||
(ret <= WC_SPAN2_FIRST_E && ret >= WC_SPAN2_LAST_E) ||
(ret <= WOLFSSL_FIRST_E && ret >= WOLFSSL_LAST_E))
{
return ret;
Expand Down
10 changes: 6 additions & 4 deletions support/gen-debug-trace-error-codes.sh
Original file line number Diff line number Diff line change
Expand Up @@ -20,10 +20,12 @@ BEGIN {
if ((errcode_a[1] == "MIN_CODE_E") ||
(errcode_a[1] == "MAX_CODE_E") ||
(errcode_a[1] == "WC_FIRST_E") ||
(errcode_a[1] == "WC_LAST_E") ||
(errcode_a[1] == "WOLFSSL_FIRST_E") ||
(errcode_a[1] == "WOLFSSL_LAST_E"))
(errcode_a[1] ~ "WC.*MIN_CODE_E") ||
(errcode_a[1] ~ "WC.*MAX_CODE_E") ||
(errcode_a[1] ~ "WC.*_FIRST_E") ||
(errcode_a[1] ~ "WC.*_LAST_E") ||
(errcode_a[1] ~ "WOLFSSL.*_FIRST_E") ||
(errcode_a[1] ~ "WOLFSSL.*_LAST_E"))
{
next;
}
Expand Down
12 changes: 6 additions & 6 deletions tests/api.c
Original file line number Diff line number Diff line change
Expand Up @@ -89866,9 +89866,9 @@ static int error_test(void)
{ -15, -17 },
{ -19, -19 },
{ -26, -27 },
{ -30, WC_FIRST_E+1 },
{ -30, WC_SPAN1_FIRST_E + 1 },
#else
{ -9, WC_FIRST_E+1 },
{ -9, WC_SPAN1_FIRST_E + 1 },
#endif
{ -124, -124 },
{ -166, -169 },
Expand All @@ -89879,14 +89879,15 @@ static int error_test(void)
{ -358, -358 },
{ -384, -384 },
{ -466, -499 },
{ WOLFSSL_LAST_E-1, WOLFSSL_LAST_E-1 }
{ WOLFSSL_LAST_E - 1, WC_SPAN2_FIRST_E + 1 },
{ WC_SPAN2_LAST_E - 1, MIN_CODE_E }
};

/* Check that all errors have a string and it's the same through the two
* APIs. Check that the values that are not errors map to the unknown
* string.
*/
for (i = 0; i >= WOLFSSL_LAST_E-1; i--) {
for (i = 0; i >= MIN_CODE_E; i--) {
int this_missing = 0;
for (j = 0; j < (int)XELEM_CNT(missing); ++j) {
if ((i <= missing[j].first) && (i >= missing[j].last)) {
Expand Down Expand Up @@ -89948,8 +89949,7 @@ static int test_wolfSSL_ERR_strings(void)
ExpectNotNull(err = wolfSSL_ERR_func_error_string(WC_NO_ERR_TRACE((word32)UNSUPPORTED_SUITE)));
ExpectIntEQ((*err == '\0'), 1);

/* The value -MIN_CODE_E+2 is PEM_R_PROBLEMS_GETTING_PASSWORD. */
ExpectNotNull(err = wolfSSL_ERR_lib_error_string(-MIN_CODE_E+2));
ExpectNotNull(err = wolfSSL_ERR_lib_error_string(-WOLFSSL_PEM_R_PROBLEMS_GETTING_PASSWORD_E));
ExpectIntEQ((*err == '\0'), 1);
#endif
#endif
Expand Down
6 changes: 4 additions & 2 deletions wolfcrypt/src/error.c
Original file line number Diff line number Diff line change
Expand Up @@ -642,11 +642,14 @@ const char* wc_GetErrorString(int error)
case PBKDF2_KAT_FIPS_E:
return "wolfCrypt FIPS PBKDF2 Known Answer Test Failure";

case DEADLOCK_AVERTED_E:
return "Deadlock averted -- retry the call";

case MAX_CODE_E:
case WC_SPAN1_MIN_CODE_E:
case MIN_CODE_E:
default:
return "unknown error number";

}
}

Expand All @@ -660,4 +663,3 @@ void wc_ErrorString(int error, char* buffer)
buffer[WOLFSSL_MAX_ERROR_SZ-1] = 0;
}
#endif /* !NO_ERROR_STRINGS */

6 changes: 4 additions & 2 deletions wolfcrypt/test/test.c
Original file line number Diff line number Diff line change
Expand Up @@ -2777,14 +2777,16 @@ WOLFSSL_TEST_SUBROUTINE wc_test_ret_t error_test(void)
int last;
} missing[] = {
{ -124, -124 },
{ -166, -169 }
{ -166, -169 },
{ WC_SPAN1_LAST_E - 1, WC_SPAN2_FIRST_E + 1 },
{ WC_SPAN2_LAST_E - 1, WC_SPAN2_MIN_CODE_E }
};

/* Check that all errors have a string and it's the same through the two
* APIs. Check that the values that are not errors map to the unknown
* string.
*/
for (i = WC_FIRST_E; i >= WC_LAST_E; i--) {
for (i = WC_SPAN1_FIRST_E; i >= WC_SPAN2_MIN_CODE_E; i--) {
int this_missing = 0;
for (j = 0; j < (int)XELEM_CNT(missing); ++j) {
if ((i <= missing[j].first) && (i >= missing[j].last)) {
Expand Down
4 changes: 4 additions & 0 deletions wolfssl/error-ssl.h
Original file line number Diff line number Diff line change
Expand Up @@ -233,8 +233,12 @@ enum wolfSSL_ErrorCodes {
WOLFSSL_EVP_R_PRIVATE_KEY_DECODE_ERROR = -515,

WOLFSSL_LAST_E = -515

/* codes -1000 to -1999 are reserved for wolfCrypt. */
};

wc_static_assert((int)WC_LAST_E <= (int)WOLFSSL_LAST_E);

/* I/O Callback default errors */
enum IOerrors {
WOLFSSL_CBIO_ERR_GENERAL = -1, /* general unexpected err */
Expand Down
29 changes: 25 additions & 4 deletions wolfssl/wolfcrypt/error-crypt.h
Original file line number Diff line number Diff line change
Expand Up @@ -47,8 +47,10 @@ enum wolfCrypt_ErrorCodes {
* reasons of backward compatibility.
*/

MAX_CODE_E = -96, /* errors -97 - -299 */
WC_FIRST_E = -97, /* errors -97 - -299 */
MAX_CODE_E = -96, /* WC_FIRST_E + 1, for backward compat. */
WC_FIRST_E = -97, /* First code used for wolfCrypt */

WC_SPAN1_FIRST_E = -97, /* errors -97 - -300 */

MP_MEM = -97, /* MP dynamic memory allocation failed. */
MP_VAL = -98, /* MP value passed is not able to be used. */
Expand Down Expand Up @@ -290,13 +292,32 @@ enum wolfCrypt_ErrorCodes {
SM4_GCM_AUTH_E = -298, /* SM4-GCM Authentication check failure */
SM4_CCM_AUTH_E = -299, /* SM4-CCM Authentication check failure */

WC_LAST_E = -299, /* Update this to indicate last error */
MIN_CODE_E = -300 /* errors -2 - -299 */
WC_SPAN1_LAST_E = -299, /* Last used code in span 1 */
WC_SPAN1_MIN_CODE_E = -300, /* Last usable code in span 1 */

WC_SPAN2_FIRST_E = -1000,

DEADLOCK_AVERTED_E = -1000, /* Deadlock averted -- retry the call */

WC_SPAN2_LAST_E = -1000, /* Update to indicate last used error code */
WC_SPAN2_MIN_CODE_E = -1999, /* Last usable code in span 2 */

WC_LAST_E = -1000, /* the last code used either here or in
* error-ssl.h
*/

MIN_CODE_E = -1999 /* the last code allocated either here or in
* error-ssl.h
*/

/* add new companion error id strings for any new error codes
wolfcrypt/src/error.c !!! */
};

wc_static_assert((int)WC_LAST_E <= (int)WC_SPAN2_LAST_E);
wc_static_assert((int)MIN_CODE_E <= (int)WC_LAST_E);
wc_static_assert((int)MIN_CODE_E <= (int)WC_SPAN2_MIN_CODE_E);

#ifdef NO_ERROR_STRINGS
#define wc_GetErrorString(error) "no support for error strings built in"
#define wc_ErrorString(err, buf) \
Expand Down
7 changes: 7 additions & 0 deletions wolfssl/wolfcrypt/wc_port.h
Original file line number Diff line number Diff line change
Expand Up @@ -320,6 +320,10 @@

#endif /* SINGLE_THREADED */

#ifdef WOLFSSL_TEST_NO_MUTEX_INITIALIZER
#undef WOLFSSL_MUTEX_INITIALIZER
#endif

#ifdef WOLFSSL_MUTEX_INITIALIZER
#define WOLFSSL_MUTEX_INITIALIZER_CLAUSE(lockname) = WOLFSSL_MUTEX_INITIALIZER(lockname)
#else
Expand All @@ -336,13 +340,15 @@
#if defined(__GNUC__) && defined(__ATOMIC_RELAXED)
/* C++ using direct calls to compiler built-in functions */
typedef volatile int wolfSSL_Atomic_Int;
#define WOLFSSL_ATOMIC_INITIALIZER(x) (x)
#define WOLFSSL_ATOMIC_OPS
#endif
#else
#ifdef WOLFSSL_HAVE_ATOMIC_H
/* Default C Implementation */
#include <stdatomic.h>
typedef atomic_int wolfSSL_Atomic_Int;
#define WOLFSSL_ATOMIC_INITIALIZER(x) (x)
#define WOLFSSL_ATOMIC_OPS
#endif /* WOLFSSL_HAVE_ATOMIC_H */
#endif
Expand All @@ -354,6 +360,7 @@
#include <intrin.h>
#endif
typedef volatile long wolfSSL_Atomic_Int;
#define WOLFSSL_ATOMIC_INITIALIZER(x) (x)
#define WOLFSSL_ATOMIC_OPS
#endif
#endif /* WOLFSSL_NO_ATOMICS */
Expand Down

0 comments on commit c8f56f0

Please sign in to comment.