From e70f861ebb6e3e417a0e5e17d74b377dfa4ef902 Mon Sep 17 00:00:00 2001 From: Than Ngo Date: Wed, 6 Apr 2022 12:09:45 +0200 Subject: [PATCH] upstream fixes - openssl cleanup for opencryptoki, Avoid deadlock when stopping event thread --- ...-deadlock-when-stopping-event-thread.patch | 64 +++++++++++++++ ...7.0-openssl-cleanup-for-opencryptoki.patch | 77 +++++++++++++++++++ opencryptoki.spec | 7 +- 3 files changed, 147 insertions(+), 1 deletion(-) create mode 100644 opencryptoki-3.17-avoid-deadlock-when-stopping-event-thread.patch create mode 100644 opencryptoki-3.17.0-openssl-cleanup-for-opencryptoki.patch diff --git a/opencryptoki-3.17-avoid-deadlock-when-stopping-event-thread.patch b/opencryptoki-3.17-avoid-deadlock-when-stopping-event-thread.patch new file mode 100644 index 0000000..d552737 --- /dev/null +++ b/opencryptoki-3.17-avoid-deadlock-when-stopping-event-thread.patch @@ -0,0 +1,64 @@ +commit fed25d1f2f3fe43eb8f55f66e39b7f4dfdad2226 +Author: Ingo Franzki +Date: Mon Feb 21 13:31:20 2022 +0100 + + API: Avoid deadlock when stopping event thread + + Avoid that the event thread writes trace messages while it is + enabled for thread cancellation. This might leave the trace mutex in + the locked state and cause subsequent trace calls to lock forever + (e.g in stop_event_thread() right after canceling the thread). + + Disable cancellation right at the beginning of the thread function, + and disable it before calling a trace function or leaving the loop. + + Also make sure that the cleanup handler is registered and the + cancellation type is set before initially enabling cancellation. + + Signed-off-by: Ingo Franzki + +diff --git a/usr/lib/api/socket_client.c b/usr/lib/api/socket_client.c +index cbe55dce..62a8ec20 100644 +--- a/usr/lib/api/socket_client.c ++++ b/usr/lib/api/socket_client.c +@@ -284,6 +284,8 @@ static void *event_thread(void *arg) + + UNUSED(arg); + ++ pthread_setcancelstate(PTHREAD_CANCEL_DISABLE, &oldstate); ++ + TRACE_DEVEL("Event thread %lu running\n", pthread_self()); + + if (anchor->socketfd < 0) { +@@ -303,13 +305,13 @@ static void *event_thread(void *arg) + #endif + + /* Enable cancellation */ +- pthread_setcancelstate(PTHREAD_CANCEL_ENABLE, &oldstate); +- pthread_setcanceltype(PTHREAD_CANCEL_DEFERRED, &oldtype); + cleanup.anchor = anchor; + #if OPENSSL_VERSION_PREREQ(3, 0) + cleanup.prev_libctx = prev_libctx; + #endif + pthread_cleanup_push(event_thread_cleanup, &cleanup); ++ pthread_setcanceltype(PTHREAD_CANCEL_DEFERRED, &oldtype); ++ pthread_setcancelstate(PTHREAD_CANCEL_ENABLE, &oldstate); + + pollfd.fd = anchor->socketfd; + pollfd.events = POLLIN | POLLHUP | POLLERR; +@@ -320,6 +322,7 @@ static void *event_thread(void *arg) + if (rc < 0) { + if (errno == EINTR) + continue; ++ pthread_setcancelstate(PTHREAD_CANCEL_DISABLE, &oldstate); + TRACE_ERROR("poll failed: %d\n", errno); + break; + } +@@ -328,6 +331,7 @@ static void *event_thread(void *arg) + continue; + + if (pollfd.revents & (POLLHUP | POLLERR)) { ++ pthread_setcancelstate(PTHREAD_CANCEL_DISABLE, &oldstate); + TRACE_ERROR("Error on socket, possibly closed by slot daemon\n"); + break; + } diff --git a/opencryptoki-3.17.0-openssl-cleanup-for-opencryptoki.patch b/opencryptoki-3.17.0-openssl-cleanup-for-opencryptoki.patch new file mode 100644 index 0000000..e341e47 --- /dev/null +++ b/opencryptoki-3.17.0-openssl-cleanup-for-opencryptoki.patch @@ -0,0 +1,77 @@ +commit 22c625eedbc1b993cf3e0caaaf0fe64ec5c1a15c +Author: Ingo Franzki +Date: Tue Apr 5 15:09:58 2022 +0200 + + API: Do not cleanup OpenSSL library context during library destructor + + Only cleanup OpenSSL library context and providers if we are not in the + library destructor. The library destructor calls C_Finalize if not + already finalized, but this may happen during at-exit handlers when the + program is terminating. At that point in time, the OpenSSL at-exit + handler may already have performed cleanup which will then cause + crashes when trying to cleanup the already freed library context here. + + We are leaking the library context and providers if one just unloads + the library without calling C_Finalize. However, OpenSSL cleanup will + clean up the context at program termination anyway + + Closes: https://github.com/opencryptoki/opencryptoki/issues/527 + + Signed-off-by: Ingo Franzki + +diff --git a/usr/lib/api/api_interface.c b/usr/lib/api/api_interface.c +index 15520db9..97b5471c 100644 +--- a/usr/lib/api/api_interface.c ++++ b/usr/lib/api/api_interface.c +@@ -272,6 +272,7 @@ int slot_loaded[NUMBER_SLOTS_MANAGED]; // Array of flags to indicate + // if the STDLL loaded + + CK_BBOOL in_child_fork_initializer = FALSE; ++CK_BBOOL in_destructor = FALSE; + + /* + * Ordered array of interfaces: If more than one interface matches +@@ -1705,14 +1706,27 @@ CK_RV C_Finalize(CK_VOID_PTR pReserved) + bt_destroy(&Anchor->sess_btree); + + #if OPENSSL_VERSION_PREREQ(3, 0) +- ERR_set_mark(); +- if (Anchor->openssl_default_provider != NULL) +- OSSL_PROVIDER_unload(Anchor->openssl_default_provider); +- if (Anchor->openssl_legacy_provider != NULL) +- OSSL_PROVIDER_unload(Anchor->openssl_legacy_provider); +- if (Anchor->openssl_libctx != NULL) +- OSSL_LIB_CTX_free(Anchor->openssl_libctx); +- ERR_pop_to_mark(); ++ /* ++ * Only cleanup OpenSSL library context and providers if we are not in the ++ * library destructor. The library destructor calls C_Finalize if not ++ * already finalized, but this may happen during at-exit handlers when the ++ * program is terminating. At that point in time, the OpenSSL at-exit ++ * handler may already have performed cleanup which will then cause ++ * crashes when trying to cleanup the already freed library context here. ++ * We are leaking the library context and providers if one just unloads ++ * the library without calling C_Finalize. However, OpenSSL cleanup will ++ * clean up the context at program termination anyway. ++ */ ++ if (in_destructor == FALSE) { ++ ERR_set_mark(); ++ if (Anchor->openssl_default_provider != NULL) ++ OSSL_PROVIDER_unload(Anchor->openssl_default_provider); ++ if (Anchor->openssl_legacy_provider != NULL) ++ OSSL_PROVIDER_unload(Anchor->openssl_legacy_provider); ++ if (Anchor->openssl_libctx != NULL) ++ OSSL_LIB_CTX_free(Anchor->openssl_libctx); ++ ERR_pop_to_mark(); ++ } + #endif + + detach_shared_memory(Anchor->SharedMemP); +@@ -5469,6 +5483,7 @@ void api_fini(void) __attribute__ ((destructor)); + void api_fini() + { + if (API_Initialized() == TRUE) { ++ in_destructor = TRUE; + Call_Finalize(); + } + } diff --git a/opencryptoki.spec b/opencryptoki.spec index 8743a02..4dd3e6a 100644 --- a/opencryptoki.spec +++ b/opencryptoki.spec @@ -1,7 +1,7 @@ Name: opencryptoki Summary: Implementation of the PKCS#11 (Cryptoki) specification v3.0 Version: 3.17.0 -Release: 4%{?dist} +Release: 5%{?dist} License: CPL URL: https://github.com/opencryptoki/opencryptoki Source0: https://github.com/opencryptoki/%{name}/archive/v%{version}/%{name}-%{version}.tar.gz @@ -19,6 +19,8 @@ Patch3: opencryptoki-3.17.0-covscan.patch # upstream patches # PIDfile below legacy directory /var/run/ Patch300: opencryptoki-pkcsslotd-pidfile.patch +Patch301: opencryptoki-3.17-avoid-deadlock-when-stopping-event-thread.patch +Patch302: opencryptoki-3.17.0-openssl-cleanup-for-opencryptoki.patch Requires(pre): coreutils Requires: (selinux-policy >= 34.9-1 if selinux-policy-targeted) @@ -328,6 +330,9 @@ fi %changelog +* Wed Apr 06 2022 Than Ngo - 3.17.0-5 +- upstream fixes - openssl cleanup for opencryptoki, Avoid deadlock when stopping event thread + * Thu Jan 20 2022 Fedora Release Engineering - 3.17.0-4 - Rebuilt for https://fedoraproject.org/wiki/Fedora_36_Mass_Rebuild