From b4aaeb5768b0a92df6bcea779eb6aac54c340d73 Mon Sep 17 00:00:00 2001 From: David Garske Date: Fri, 11 Aug 2017 17:26:10 -0700 Subject: [PATCH] Fix for possible leak with multi-threading and curve cache. Note memory leak still possible with `--enable-fpecc` and async multithreading. Add voltaile on event `state` and `ret` to resolve possible multi-thread timing issue. Use define for `--enable-stacksize` init value. --- wolfcrypt/src/ecc.c | 20 ++++++++++++-------- wolfssl/test.h | 6 +++--- wolfssl/wolfcrypt/wolfevent.h | 4 ++-- 3 files changed, 17 insertions(+), 13 deletions(-) diff --git a/wolfcrypt/src/ecc.c b/wolfcrypt/src/ecc.c index 2e78d45d4..faec6ca3e 100755 --- a/wolfcrypt/src/ecc.c +++ b/wolfcrypt/src/ecc.c @@ -1117,12 +1117,23 @@ static int wc_ecc_curve_load(const ecc_set_type* dp, ecc_curve_spec** pCurve, if (x == ECC_CURVE_INVALID) return ECC_BAD_ARG_E; +#if !defined(SINGLE_THREADED) + ret = wc_LockMutex(&ecc_curve_cache_mutex); + if (ret != 0) { + return ret; + } +#endif + /* make sure cache has been allocated */ if (ecc_curve_spec_cache[x] == NULL) { ecc_curve_spec_cache[x] = (ecc_curve_spec*)XMALLOC( sizeof(ecc_curve_spec), NULL, DYNAMIC_TYPE_ECC); - if (ecc_curve_spec_cache[x] == NULL) + if (ecc_curve_spec_cache[x] == NULL) { + #if defined(ECC_CACHE_CURVE) && !defined(SINGLE_THREADED) + wc_UnLockMutex(&ecc_curve_cache_mutex); + #endif return MEMORY_E; + } XMEMSET(ecc_curve_spec_cache[x], 0, sizeof(ecc_curve_spec)); } @@ -1149,13 +1160,6 @@ static int wc_ecc_curve_load(const ecc_set_type* dp, ecc_curve_spec** pCurve, } curve->dp = dp; /* set dp info */ -#if defined(ECC_CACHE_CURVE) && !defined(SINGLE_THREADED) - ret = wc_LockMutex(&ecc_curve_cache_mutex); - if (ret != 0) { - return MEMORY_E; - } -#endif - /* determine items to load */ load_items = (~curve->load_mask & load_mask); curve->load_mask |= load_items; diff --git a/wolfssl/test.h b/wolfssl/test.h index 96e6fa2f3..a5b3961a8 100644 --- a/wolfssl/test.h +++ b/wolfssl/test.h @@ -1385,7 +1385,7 @@ static INLINE void CaCb(unsigned char* der, int sz, int type) #ifdef HAVE_STACK_SIZE typedef THREAD_RETURN WOLFSSL_THREAD (*thread_func)(void* args); - +#define STACK_CHECK_VAL 0x01 static INLINE int StackSizeCheck(func_args* args, thread_func tf) { @@ -1405,7 +1405,7 @@ static INLINE int StackSizeCheck(func_args* args, thread_func tf) if (ret != 0 || myStack == NULL) err_sys("posix_memalign failed\n"); - XMEMSET(myStack, 0x01, stackSize); + XMEMSET(myStack, STACK_CHECK_VAL, stackSize); ret = pthread_attr_init(&myAttr); if (ret != 0) @@ -1426,7 +1426,7 @@ static INLINE int StackSizeCheck(func_args* args, thread_func tf) err_sys("pthread_join failed"); for (i = 0; i < stackSize; i++) { - if (myStack[i] != 0x01) { + if (myStack[i] != STACK_CHECK_VAL) { break; } } diff --git a/wolfssl/wolfcrypt/wolfevent.h b/wolfssl/wolfcrypt/wolfevent.h index 7b49229e8..62c2199ee 100644 --- a/wolfssl/wolfcrypt/wolfevent.h +++ b/wolfssl/wolfcrypt/wolfevent.h @@ -76,10 +76,10 @@ struct WOLF_EVENT { #ifdef HAVE_CAVIUM CavReqId reqId; #endif - int ret; /* Async return code */ + volatile int ret; /* Async return code */ unsigned int flags; WOLF_EVENT_TYPE type; - WOLF_EVENT_STATE state; + volatile WOLF_EVENT_STATE state; }; enum WOLF_POLL_FLAGS {