From 1b4e9f1d4aea2ac1c29b821505e4c03872e46650 Mon Sep 17 00:00:00 2001 From: Doug Engert Date: Mon, 11 Jan 2021 12:47:12 -0600 Subject: [PATCH] C_Initialize may be called by multiple threads While trying to setup an OpenSC context, the global_locking and detect cards, it is possible that multiple threads may call C_Initialize. The current code tries to prevent this using "if (context == NULL)" but this is not a mutex, and multiple threads may endup overwrite contexts and global locking and cause additional problems, with pcsc and segfault. FireFox appears to do this see #2032 The PR adds a mutex or Critical section to make sure only one thread creates the context sets the global_locking and does the initial detect cards, etc. This allows the global_lock (if requested) to be setup which is then used for other calls. All but the first call to C_Initialize will return with CKR_OK, others will return CKR_CRYPTOKI_ALREADY_INITIALIZED. Date: Mon Jan 11 12:47:12 2021 -0600 Changes to be committed: modified: src/pkcs11/pkcs11-global.c --- src/pkcs11/pkcs11-global.c | 25 +++++++++++++++++++++++++ 1 file changed, 25 insertions(+) diff --git a/src/pkcs11/pkcs11-global.c b/src/pkcs11/pkcs11-global.c index a648b140..6ced6471 100644 --- a/src/pkcs11/pkcs11-global.c +++ b/src/pkcs11/pkcs11-global.c @@ -61,6 +61,11 @@ extern CK_FUNCTION_LIST_3_0 pkcs11_function_list_3_0; #if defined(HAVE_PTHREAD) +/* mutex used to control C_Initilize creation of mutexes */ +static pthread_mutex_t c_initialize_m = PTHREAD_MUTEX_INITIALIZER; +#define C_INITIALIZE_M_LOCK pthread_mutex_lock(&c_initialize_m); +#define C_INITIALIZE_M_UNLOCK pthread_mutex_unlock(&c_initialize_m); + CK_RV mutex_create(void **mutex) { pthread_mutex_t *m; @@ -101,6 +106,9 @@ static CK_C_INITIALIZE_ARGS _def_locks = { #define HAVE_OS_LOCKING #elif defined(_WIN32) +CRITICAL_SECTION c_initialize_cs = {0}; +#define C_INITIALIZE_M_LOCK EnterCriticalSection(&c_initialize_cs); +#define C_INITIALIZE_M_UNLOCK LeaveCriticalSection(&c_initialize_cs); CK_RV mutex_create(void **mutex) { @@ -141,6 +149,10 @@ static CK_C_INITIALIZE_ARGS _def_locks = { #endif +#else /* PKCS11_THREAD_LOCKING */ +#define C_INITIALIZE_M_LOCK +#define C_INITIALIZE_M_UNLOCK + #endif /* PKCS11_THREAD_LOCKING */ static CK_C_INITIALIZE_ARGS_PTR global_locking; @@ -221,6 +233,9 @@ __attribute__((constructor)) #endif int module_init() { +#ifdef _WIN32 + InitializeCriticalSection(&c_initialize_cs); +#endif sc_notify_init(); return 1; } @@ -236,6 +251,9 @@ int module_close() #endif #ifdef ENABLE_OPENPACE EAC_cleanup(); +#endif +#ifdef _WIN32 + DeleteCriticalSection(&c_initialize_cs); #endif return 1; } @@ -283,8 +301,12 @@ CK_RV C_Initialize(CK_VOID_PTR pInitArgs) in_finalize = 0; #endif + /* protect from multiple threads tryng to setup locking */ + C_INITIALIZE_M_LOCK + if (context != NULL) { sc_log(context, "C_Initialize(): Cryptoki already initialized\n"); + C_INITIALIZE_M_UNLOCK return CKR_CRYPTOKI_ALREADY_INITIALIZED; } @@ -336,6 +358,9 @@ out: sc_pkcs11_free_lock(); } + /* protect from multiple threads tryng to setup locking */ + C_INITIALIZE_M_UNLOCK + return rv; }