pkcs11: Call C_Finalize() to cancel jobs waiting in C_WaitForSlotEvent()

This is not ideal as the call to C_Finalize() should be the last one via
the PKCS#11 API.  Since the order in which jobs are canceled is undefined
we can't be sure there is no other thread still using the library (it could
even be the canceled job that still handles a previous slot event).
According to PKCS#11 the behavior of C_Finalize() is undefined while other
threads still make calls over the API.

However, canceling the thread, as done previously, could also be problematic
as PKCS#11 libraries could hold locks while in the C_WaitForSlotEvent() call,
which might not get released properly when the thread is just canceled,
and which then might cause later calls to other API functions to block.

Fixes #2437.

1 files changed, 14 insertions, 9 deletions

diff --git a/src/libstrongswan/plugins/pkcs11/pkcs11_manager.c b/src/libstrongswan/plugins/pkcs11/pkcs11_manager.c
index 31bcb0d25..c7dfe69d7 100644
--- a/src/libstrongswan/plugins/pkcs11/pkcs11_manager.c
+++ b/src/libstrongswan/plugins/pkcs11/pkcs11_manager.c
@@ -164,18 +164,13 @@ static void handle_slot(lib_entry_t *entry, CK_SLOT_ID slot, bool hot)
 	}
 }
 
-/**
- * Dispatch slot events
- */
-static job_requeue_t dispatch_slot_events(lib_entry_t *entry)
+CALLBACK(dispatch_slot_events, job_requeue_t,
+	lib_entry_t *entry)
 {
 	CK_SLOT_ID slot;
 	CK_RV rv;
-	bool old;
 
-	old = thread_cancelability(TRUE);
 	rv = entry->lib->f->C_WaitForSlotEvent(0, &slot, NULL);
-	thread_cancelability(old);
 	if (rv == CKR_FUNCTION_NOT_SUPPORTED || rv == CKR_NO_EVENT)
 	{
 		DBG1(DBG_CFG, "module '%s' does not support hot-plugging, cancelled",
@@ -195,6 +190,16 @@ static job_requeue_t dispatch_slot_events(lib_entry_t *entry)
 	return JOB_REQUEUE_DIRECT;
 }
 
+CALLBACK(cancel_events, bool,
+	lib_entry_t *entry)
+{
+	/* it's possible other threads still use the API after this call, but we
+	 * have no other way to return from C_WaitForSlotEvent() if we can't cancel
+	 * the thread because libraries hold locks they don't release */
+	entry->lib->f->C_Finalize(NULL);
+	return TRUE;
+}
+
 /**
  * Get the slot list of a library
  */
@@ -377,8 +382,8 @@ pkcs11_manager_t *pkcs11_manager_create(pkcs11_manager_token_event_t cb,
 	{
 		query_slots(entry);
 		lib->processor->queue_job(lib->processor,
-			(job_t*)callback_job_create_with_prio((void*)dispatch_slot_events,
-						entry, NULL, (void*)return_false, JOB_PRIO_CRITICAL));
+			(job_t*)callback_job_create_with_prio(dispatch_slot_events,
+						entry, NULL, cancel_events, JOB_PRIO_CRITICAL));
 	}
 	enumerator->destroy(enumerator);