pkcs11: Call C_Finalize() to cancel jobs waiting in C_WaitForSlotEvent()
authorTobias Brunner <tobias@strongswan.org>
Fri, 6 Oct 2017 12:51:37 +0000 (14:51 +0200)
committerTobias Brunner <tobias@strongswan.org>
Thu, 2 Nov 2017 09:15:32 +0000 (10:15 +0100)
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.

src/libstrongswan/plugins/pkcs11/pkcs11_manager.c

index 31bcb0d..c7dfe69 100644 (file)
@@ -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);