From 22a583d5d4a98d1b5183823dcd10f9a87e65eb47 Mon Sep 17 00:00:00 2001 From: Maxwell Moyer-McKee Date: Thu, 30 Jul 2026 23:42:27 +0000 Subject: [PATCH 1/2] Minimize window logging thread holds logging_thread_mutex --- SymCryptProvider/src/p_scossl_keysinuse.c | 56 +++++++++++++---------- 1 file changed, 32 insertions(+), 24 deletions(-) diff --git a/SymCryptProvider/src/p_scossl_keysinuse.c b/SymCryptProvider/src/p_scossl_keysinuse.c index 58945a7a..0742fad1 100644 --- a/SymCryptProvider/src/p_scossl_keysinuse.c +++ b/SymCryptProvider/src/p_scossl_keysinuse.c @@ -837,7 +837,7 @@ static void p_scossl_keysinuse_log_common(int level, const char *message, va_lis int fd; for (int i = 0; i < 3; i++) { - fd = open(log_path, O_WRONLY | O_APPEND | O_CREAT, 0200); + fd = open(log_path, O_WRONLY | O_APPEND | O_CREAT | O_CLOEXEC, 0200); if (fd >= 0 || errno != EACCES) { break; @@ -1025,34 +1025,44 @@ static void *p_scossl_keysinuse_logging_thread_start(ossl_unused void *arg) p_scossl_keysinuse_log_error("Failed to lock keysinuse info stack,OPENSSL_%d", ERR_get_error()); } - // Log all pending usage events under lock. We need to lock in this section - // in case fork is called - if ((pthreadErr = pthread_mutex_lock(&logging_thread_mutex)) == 0) + // Log all pending usage events. logging_thread_mutex is held only around + // during the pKeysinuseInfo update to ensure the logging thread is not + // holding a lock during a fork. + while (sk_SCOSSL_PROV_KEYSINUSE_INFO_num(sk_keysinuse_info_pending) > 0) { - while (sk_SCOSSL_PROV_KEYSINUSE_INFO_num(sk_keysinuse_info_pending) > 0) + if ((pthreadErr = pthread_mutex_lock(&logging_thread_mutex)) != 0) { - pKeysinuseInfo = sk_SCOSSL_PROV_KEYSINUSE_INFO_pop(sk_keysinuse_info_pending); - if (CRYPTO_THREAD_write_lock(pKeysinuseInfo->lock)) - { - now = time(NULL); + p_scossl_keysinuse_log_error("Logging thread failed to accquire mutex,SYS_%d", pthreadErr); + goto cleanup; + } - pKeysinuseInfo->firstLogTime = pKeysinuseInfo->lastLogTime == 0 ? now : pKeysinuseInfo->firstLogTime; - pKeysinuseInfo->lastLogTime = now; - pKeysinuseInfo->logPending = FALSE; + pKeysinuseInfo = sk_SCOSSL_PROV_KEYSINUSE_INFO_pop(sk_keysinuse_info_pending); + if (pKeysinuseInfo != NULL && + CRYPTO_THREAD_write_lock(pKeysinuseInfo->lock)) + { + now = time(NULL); - keysinuseInfoTmp = *pKeysinuseInfo; + pKeysinuseInfo->firstLogTime = pKeysinuseInfo->lastLogTime == 0 ? now : pKeysinuseInfo->firstLogTime; + pKeysinuseInfo->lastLogTime = now; + pKeysinuseInfo->logPending = FALSE; - pKeysinuseInfo->decryptCounter = 0; - pKeysinuseInfo->signCounter = 0; + keysinuseInfoTmp = *pKeysinuseInfo; - CRYPTO_THREAD_unlock(pKeysinuseInfo->lock); - } - else - { - p_scossl_keysinuse_log_error("Failed to lock keysinuse info,OPENSSL_%d", ERR_get_error()); - keysinuseInfoTmp.refCount = -1; - } + pKeysinuseInfo->decryptCounter = 0; + pKeysinuseInfo->signCounter = 0; + + CRYPTO_THREAD_unlock(pKeysinuseInfo->lock); + } + else + { + p_scossl_keysinuse_log_error("Failed to lock keysinuse info,OPENSSL_%d", ERR_get_error()); + keysinuseInfoTmp.refCount = -1; + } + pthread_mutex_unlock(&logging_thread_mutex); + + if (pKeysinuseInfo != NULL) + { p_scossl_keysinuse_info_free(pKeysinuseInfo); if (keysinuseInfoTmp.refCount > 0) @@ -1065,8 +1075,6 @@ static void *p_scossl_keysinuse_logging_thread_start(ossl_unused void *arg) keysinuseInfoTmp.lastLogTime); } } - - pthread_mutex_unlock(&logging_thread_mutex); } } while (isLoggingThreadRunning); From 0df8c9cef205c01da6e1d1946045f381e0dd7da8 Mon Sep 17 00:00:00 2001 From: Maxwell Moyer-McKee Date: Thu, 6 Aug 2026 22:52:45 +0000 Subject: [PATCH 2/2] Remove unecessary null check --- SymCryptProvider/src/p_scossl_keysinuse.c | 21 +++++++++------------ 1 file changed, 9 insertions(+), 12 deletions(-) diff --git a/SymCryptProvider/src/p_scossl_keysinuse.c b/SymCryptProvider/src/p_scossl_keysinuse.c index 0742fad1..ee4cfa63 100644 --- a/SymCryptProvider/src/p_scossl_keysinuse.c +++ b/SymCryptProvider/src/p_scossl_keysinuse.c @@ -1061,19 +1061,16 @@ static void *p_scossl_keysinuse_logging_thread_start(ossl_unused void *arg) pthread_mutex_unlock(&logging_thread_mutex); - if (pKeysinuseInfo != NULL) - { - p_scossl_keysinuse_info_free(pKeysinuseInfo); + p_scossl_keysinuse_info_free(pKeysinuseInfo); - if (keysinuseInfoTmp.refCount > 0) - { - p_scossl_keysinuse_log_notice("%s,%d,%d,%ld,%ld", - keysinuseInfoTmp.keyIdentifier, - keysinuseInfoTmp.signCounter, - keysinuseInfoTmp.decryptCounter, - keysinuseInfoTmp.firstLogTime, - keysinuseInfoTmp.lastLogTime); - } + if (keysinuseInfoTmp.refCount > 0) + { + p_scossl_keysinuse_log_notice("%s,%d,%d,%ld,%ld", + keysinuseInfoTmp.keyIdentifier, + keysinuseInfoTmp.signCounter, + keysinuseInfoTmp.decryptCounter, + keysinuseInfoTmp.firstLogTime, + keysinuseInfoTmp.lastLogTime); } } }