Add basic mutex support, including default ("OS") implementation using

POSIX threads.  Compiles, but no runtime testing done yet.

1 files changed, 482 insertions, 106 deletions

diff --git a/pkcs11.c b/pkcs11.c
index 5b7576f..fef9e0b 100644
--- a/pkcs11.c
+++ b/pkcs11.c
@@ -162,6 +162,20 @@
 #define PKCS15_KEYRING ".cryptech-pkcs11.p15"
 #endif
 
+/*
+ * Whether to include POSIX-specific features.
+ */
+
+#ifndef USE_POSIX
+#define USE_POSIX 1
+#endif
+
+#if USE_POSIX
+#include <unistd.h>
+#include <pthread.h>
+#include <errno.h>
+#endif
+
 
 
 /*
@@ -299,12 +313,41 @@ static CRYPT_DEVICE cryptlib_device = CRYPT_HANDLE_NONE;
 static char *database_filename = NULL;
 static char *keyring_filename = NULL;
 
+/*
+ * Mutex callbacks.
+ */
+
+static CK_CREATEMUTEX  mutex_cb_create;
+static CK_DESTROYMUTEX mutex_cb_destroy;
+static CK_LOCKMUTEX    mutex_cb_lock;
+static CK_UNLOCKMUTEX  mutex_cb_unlock;
+
+/*
+ * Global mutex.  We may want something finer grained later, but this
+ * will suffice to comply with the API requirements.
+ */
+
+static CK_VOID_PTR p11_global_mutex;
+
+/*
+ * (POSIX-specific) process which last called C_Initialize().
+ */
+
+#if USE_POSIX
+static pid_t initialized_pid;
+#endif
+
 
 
 /*
  * Syntactic sugar for functions returning CK_RV complex enough to
- * need cleanup actions on failure.  Also does very basic logging
- * for debug-by-printf().
+ * need cleanup actions on failure.  Also does very basic logging for
+ * debug-by-printf().
+ *
+ * NB: This uses a variable ("rv") and a goto target ("fail") which
+ * must be defined in the calling environment.  We could make these
+ * arguments to the macro, but doing so would make the code less
+ * readable without significantly reducing the voodoo factor.
  */
 
 #define lose(_ck_rv_code_)                                              \
@@ -386,6 +429,168 @@ static char *cf_pkcs15_keyring(void)
 
 
 /*
+ * Thread mutex utilities.  We need to handle three separate cases:
+ *
+ * 1) User doesn't care about mutexes;
+ * 2) User wants us to use "OS" mutexes;
+ * 3) User wants us to use user-specified mutexs.
+ *
+ * For "OS" mutexes, read POSIX Threads mutexes, at least for now.
+ *
+ * PKCS #11 sort of has a fourth case, but it's really just license
+ * for us to pick either the second or third case at whim.
+ *
+ * To simplify the rest of the API, we provide a POSIX-based
+ * implementation which uses the same API an user-provided mutex
+ * implementation would be required to use, use null function pointers
+ * to represent the case where the user doesn't need mutexes at all,
+ * and wrap the whole thing in trivial macros to insulate the rest of
+ * the code from the grotty details.
+ */
+
+/*
+ * Basic macros.
+ */
+
+#define mutex_create(_m_)   (mutex_cb_create  == NULL ? CKR_OK : mutex_cb_create(_m_))
+#define mutex_destroy(_m_)  (mutex_cb_destroy == NULL ? CKR_OK : mutex_cb_destroy(_m_))
+#define mutex_lock(_m_)     (mutex_cb_lock    == NULL ? CKR_OK : mutex_cb_lock(_m_))
+#define mutex_unlock(_m_)   (mutex_cb_unlock  == NULL ? CKR_OK : mutex_cb_unlock(_m_))
+
+/*
+ * Slightly higher-level macros for common operations.
+ */
+
+#define mutex_lock_or_fail(_m_)         \
+  do {                                  \
+    CK_RV _rv_ = mutex_lock(_m_);       \
+    if (_rv_ != CKR_OK)                 \
+      return _rv_;                      \
+  } while (0)
+
+#define mutex_unlock_with_rv(_rv_, _m_) \
+  ((_rv_) == CKR_OK ? mutex_unlock(_m_) : (mutex_unlock(_m_), (_rv_)))
+
+/*
+ * Mutex implementation using POSIX mutexes.
+ */
+
+#if USE_POSIX
+
+static CK_RV posix_mutex_create(CK_VOID_PTR_PTR ppMutex)
+{
+  pthread_mutex_t *m = NULL;
+  CK_RV rv;
+
+  if (ppMutex == NULL)
+    lose(CKR_GENERAL_ERROR);
+
+  if ((m = malloc(sizeof(*m))) == NULL)
+    lose(CKR_HOST_MEMORY);
+
+  switch (pthread_mutex_init(m, NULL)) {
+
+  case 0:
+    *ppMutex = m;
+    return CKR_OK;
+
+  case ENOMEM:
+    lose(CKR_HOST_MEMORY);
+
+  default:
+    lose(CKR_GENERAL_ERROR);
+  }  
+
+ fail:
+  if (m != NULL)
+    free(m);
+  return rv;
+}
+
+static CK_RV posix_mutex_destroy(CK_VOID_PTR pMutex)
+{
+  CK_RV rv;
+
+  if (pMutex == NULL)
+    lose(CKR_MUTEX_BAD);
+
+  switch (pthread_mutex_destroy(pMutex)) {
+
+  case 0:
+    free(pMutex);
+    return CKR_OK;
+
+  case EINVAL:
+    lose(CKR_MUTEX_BAD);
+
+  case EBUSY:
+    /*
+     * PKCS #11 mutex semantics are a bad match for POSIX here,
+     * leaving us only the nuclear option.  Feh.  Fall through.
+     */
+
+  default:
+    lose(CKR_GENERAL_ERROR);
+  }
+
+ fail:
+  return rv;
+}
+
+static CK_RV posix_mutex_lock(CK_VOID_PTR pMutex)
+{
+  CK_RV rv;
+
+  if (pMutex == NULL)
+    lose(CKR_MUTEX_BAD);
+
+  switch (pthread_mutex_lock(pMutex)) {
+
+  case 0:
+    return CKR_OK;
+
+  case EINVAL:
+    lose(CKR_MUTEX_BAD);
+
+  default:
+    lose(CKR_GENERAL_ERROR);
+  }
+
+ fail:
+  return rv;
+}
+
+static CK_RV posix_mutex_unlock(CK_VOID_PTR pMutex)
+{
+  CK_RV rv;
+
+  if (pMutex == NULL)
+    lose(CKR_MUTEX_BAD);
+
+  switch (pthread_mutex_unlock(pMutex)) {
+
+  case 0:
+    return CKR_OK;
+
+  case EINVAL:
+    lose(CKR_MUTEX_BAD);
+
+  case EPERM:
+    lose(CKR_MUTEX_NOT_LOCKED);
+
+  default:
+    lose(CKR_GENERAL_ERROR);
+  }
+
+ fail:
+  return rv;
+}
+
+#endif /* USE_POSIX */
+
+
+
+/*
  * Wrappers around some of Cryptlib's context functions, so that the
  * rest of the code can mostly ignore whether a particular algorithm
  * is implemented in hardware or not.  In theory, we could achieve
@@ -2240,36 +2445,97 @@ CK_RV C_Initialize(CK_VOID_PTR pInitArgs)
   CK_C_INITIALIZE_ARGS_PTR a = pInitArgs;
   CK_RV rv;
 
+  /*
+   * We'd like to detect the error of calling this method more than
+   * once in a single process without an intervening call to
+   * C_Finalize(), but there's no completely portable way to do that
+   * when faced with things like the POSIX fork() system call.  For
+   * the moment, we use a POSIX-specific check, but may need to
+   * generalize this for other platforms.
+   */
+
+#if USE_POSIX
+  if (initialized_pid == getpid())
+    lose(CKR_CRYPTOKI_ALREADY_INITIALIZED);
+#endif
+
+  /*
+   * Sort out what the user wants to do about mutexes.  Default is not
+   * to use mutexes at all.
+   *
+   * There's a chicken and egg problem here: setting up the global
+   * mutex and mutex function pointers creates a race condition, and
+   * there's no obvious action we can take which is robust in the face
+   * of pathological behavior by the caller such as simultaneous calls
+   * to this method with incompatible mutex primitives.
+   *
+   * Given that (a) it's an error to call this method more than once
+   * in the same process without an intervening F_Finalize() call, and
+   * given that (b) we haven't actually promised to do any kind of
+   * locking at all until this method returns CKR_OK, we punt
+   * responsibility for this pathological case back to the caller.
+   */
+
+  mutex_cb_create  = NULL;
+  mutex_cb_destroy = NULL;
+  mutex_cb_lock    = NULL;
+  mutex_cb_unlock  = NULL;
+
   if (a != NULL) {
-    int functions_provided = ((a->CreateMutex  != NULL) +
-                              (a->DestroyMutex != NULL) +
-                              (a->LockMutex    != NULL) +
-                              (a->UnlockMutex  != NULL));
+
+    const int functions_provided = ((a->CreateMutex  != NULL) +
+                                    (a->DestroyMutex != NULL) +
+                                    (a->LockMutex    != NULL) +
+                                    (a->UnlockMutex  != NULL));
 
     /*
      * Reserved is, um, reserved.
-     * Threading parameters must either all be present or all be absent.
+     * Mutex parameters must either all be present or all be absent.
      */
 
     if (a->pReserved != NULL || (functions_provided & 3) != 0)
       lose(CKR_ARGUMENTS_BAD);
 
     /*
-     * At present we don't support threads or locking.  This may be a
-     * problem for OpenDNSSEC.  Need to figure out what the "obvious"
-     * system threading mechanism is supposed to be, or make it
-     * configurable, or something.  For the moment, just return the
-     * correct error code to report that we're lame.
+     * If the user provided mutex functions, use them.  Otherwise, if
+     * the user wants locking, use POSIX mutexes or return an error
+     * depending on whether we have POSIX mutexes available.
+     * Otherwise, we don't need to use mutexes.
      */
 
-#warning Thread support check disabled, this needs to be fixed
-#if 0
-    if (functions_provided || (a->flags & CKF_OS_LOCKING_OK) != 0)
+    if (functions_provided) {
+      mutex_cb_create  = a->CreateMutex;
+      mutex_cb_destroy = a->DestroyMutex;
+      mutex_cb_lock    = a->LockMutex;
+      mutex_cb_unlock  = a->UnlockMutex;
+    }
+
+    else if ((a->flags & CKF_OS_LOCKING_OK) != 0) {
+#if USE_POSIX
+      mutex_cb_create  = posix_mutex_create;
+      mutex_cb_destroy = posix_mutex_destroy;
+      mutex_cb_lock    = posix_mutex_lock;
+      mutex_cb_unlock  = posix_mutex_unlock;
+#else
       lose(CKR_CANT_LOCK);
 #endif
+    }
   }
 
   /*
+   * Now that we know which mutex implementation to use, set up a
+   * global mutex.  We may want something finer grained later, but
+   * this is enough to preserve the basic API semantics.
+   *
+   * Open question whether we should lock at this point, given that
+   * until we return we haven't promised to do locking.  Skip for now
+   * as it's simpler, fix later if it turns out to be a problem.
+   */
+
+  if ((rv = mutex_create(&p11_global_mutex)) != CKR_OK)
+    goto fail;
+
+  /*
    * Initialize SQLite3, opening the database(s) and loading the
    * schema and views.
    */
@@ -2300,6 +2566,10 @@ CK_RV C_Initialize(CK_VOID_PTR pInitArgs)
     lose(CKR_GENERAL_ERROR);
 #endif
 
+#if USE_POSIX
+  initialized_pid = getpid();
+#endif
+
   return CKR_OK;
 
  fail:
@@ -2322,9 +2592,13 @@ CK_RV C_Initialize(CK_VOID_PTR pInitArgs)
 
 CK_RV C_Finalize(CK_VOID_PTR pReserved)
 {
+  CK_RV rv = CKR_OK;
+
   if (pReserved != NULL)
     return CKR_ARGUMENTS_BAD;
 
+  mutex_lock_or_fail(p11_global_mutex);
+
   /*
    * Destroy all current sessions.
    */
@@ -2336,7 +2610,7 @@ CK_RV C_Finalize(CK_VOID_PTR pReserved)
    */
 
   if (!sql_fini())
-    return CKR_GENERAL_ERROR;
+    lose(CKR_GENERAL_ERROR);
 
   /*
    * Shut down hardware device and exit cryptlib.  Is there any point
@@ -2350,7 +2624,20 @@ CK_RV C_Finalize(CK_VOID_PTR pReserved)
 #endif
 
   cryptEnd();
-  return CKR_OK;
+
+  /*
+   * By this point we're pretty well committed to shutting down, so
+   * there's not much to be done if these mutex operations fail.
+   */
+
+  rv =  mutex_unlock(p11_global_mutex);
+  (void) mutex_destroy(p11_global_mutex);
+  p11_global_mutex = NULL;
+  return rv;
+
+ fail:
+  (void) mutex_unlock(p11_global_mutex);
+  return rv;
 }
 
 CK_RV C_GetFunctionList(CK_FUNCTION_LIST_PTR_PTR ppFunctionList)
@@ -2358,6 +2645,8 @@ CK_RV C_GetFunctionList(CK_FUNCTION_LIST_PTR_PTR ppFunctionList)
   /*
    * Use pkcs11f.h to build dispatch vector for C_GetFunctionList().
    * This should be const, but that's not what PKCS #11 says, oh well.
+   *
+   * This doesn't touch anything requiring locks, nor should it.
    */
 
   static CK_FUNCTION_LIST ck_function_list = {
@@ -2381,6 +2670,7 @@ CK_RV C_GetSlotList(CK_BBOOL tokenPresent,
 {
   /*
    * We only have one slot, and it's hardwired.
+   * No locking required here as long as this holds.
    */
 
   if (pulCount == NULL)
@@ -2400,6 +2690,10 @@ CK_RV C_GetSlotList(CK_BBOOL tokenPresent,
 CK_RV C_GetTokenInfo(CK_SLOT_ID slotID,
                      CK_TOKEN_INFO_PTR pInfo)
 {
+  /*
+   * No locking required here as long as we're just returning constants.
+   */
+
   if (pInfo == NULL)
     return CKR_ARGUMENTS_BAD;
 
@@ -2496,6 +2790,8 @@ CK_RV C_OpenSession(CK_SLOT_ID slotID,
   p11_session_t *session = NULL;
   CK_RV rv;
 
+  mutex_lock_or_fail(p11_global_mutex);
+
   if (slotID != P11_ONE_AND_ONLY_SLOT)
     lose(CKR_SLOT_ID_INVALID);
 
@@ -2533,17 +2829,29 @@ CK_RV C_OpenSession(CK_SLOT_ID slotID,
 
   assert(p11_session_consistent_login());
 
+  if ((rv = mutex_unlock(p11_global_mutex)) != CKR_OK)
+    goto fail;
+
   *phSession = session->handle;
   return CKR_OK;
 
  fail:
   p11_session_free(session);
+  (void) mutex_unlock(p11_global_mutex);
   return rv;
 }
 
 CK_RV C_CloseSession(CK_SESSION_HANDLE hSession)
 {
-  return p11_session_delete(hSession);
+  CK_RV rv;
+
+  mutex_lock_or_fail(p11_global_mutex);
+
+  if ((rv = p11_session_delete(hSession)) != CKR_OK)
+    goto fail;
+
+ fail:
+  return mutex_unlock_with_rv(rv, p11_global_mutex);
 }
 
 CK_RV C_CloseAllSessions(CK_SLOT_ID slotID)
@@ -2551,9 +2859,11 @@ CK_RV C_CloseAllSessions(CK_SLOT_ID slotID)
   if (slotID != P11_ONE_AND_ONLY_SLOT)
     return CKR_SLOT_ID_INVALID;
 
+  mutex_lock_or_fail(p11_global_mutex);
+
   p11_session_delete_all();
 
-  return CKR_OK;
+  return mutex_unlock(p11_global_mutex);
 }
 
 CK_RV C_Login(CK_SESSION_HANDLE hSession,
@@ -2562,10 +2872,13 @@ CK_RV C_Login(CK_SESSION_HANDLE hSession,
               CK_ULONG ulPinLen)
 {
   p11_session_t *session;
+  CK_RV rv = CKR_OK;
   int crypt_cmd;
 
+  mutex_lock_or_fail(p11_global_mutex);
+
   if (pPin == NULL)
-    return CKR_ARGUMENTS_BAD;
+    lose(CKR_ARGUMENTS_BAD);
 
   /*
    * Mind, I don't really know why this function takes a session
@@ -2574,7 +2887,7 @@ CK_RV C_Login(CK_SESSION_HANDLE hSession,
    */
 
   if (p11_session_find(hSession) == NULL)
-    return CKR_SESSION_HANDLE_INVALID;
+    lose(CKR_SESSION_HANDLE_INVALID);
 
   /*
    * This is where the combination of pure-software and hardware
@@ -2596,7 +2909,7 @@ CK_RV C_Login(CK_SESSION_HANDLE hSession,
    */
 
   if (logged_in_as != not_logged_in)
-      return CKR_USER_ALREADY_LOGGED_IN;    
+    lose(CKR_USER_ALREADY_LOGGED_IN);    
 
   /*
    * We don't (yet?) support CKU_CONTEXT_SPECIFIC.
@@ -2605,8 +2918,8 @@ CK_RV C_Login(CK_SESSION_HANDLE hSession,
   switch (userType) {
   case CKU_USER:                crypt_cmd = CRYPT_DEVINFO_AUTHENT_USER;         break;
   case CKU_SO:                  crypt_cmd = CRYPT_DEVINFO_AUTHENT_SUPERVISOR;   break;
-  case CKU_CONTEXT_SPECIFIC:    return CKR_OPERATION_NOT_INITIALIZED;
-  default:                      return CKR_USER_TYPE_INVALID;
+  case CKU_CONTEXT_SPECIFIC:    lose(CKR_OPERATION_NOT_INITIALIZED);
+  default:                      lose(CKR_USER_TYPE_INVALID);
   }
 
   /*
@@ -2617,7 +2930,7 @@ CK_RV C_Login(CK_SESSION_HANDLE hSession,
   if (userType == CKU_SO)
     for (session = p11_sessions; session != NULL; session = session->link)
       if (session->state == CKS_RO_PUBLIC_SESSION)
-        return CKR_SESSION_READ_ONLY_EXISTS;
+        lose(CKR_SESSION_READ_ONLY_EXISTS);
 
   /*
    * Ask Cryptlib to log us in.  We may need to examine cryptlib error
@@ -2626,16 +2939,16 @@ CK_RV C_Login(CK_SESSION_HANDLE hSession,
 
 #if ENABLE_CRYPTLIB_DEVICE
   if (cryptSetAttributeString(cryptlib_device, crypt_cmd, pPin, ulPinLen) != CRYPT_OK)
-    return CKR_PIN_INCORRECT;
+    lose(CKR_PIN_INCORRECT);
 #endif
 
 #if ENABLE_CRYPTLIB_SOFTWARE
   {
     char *newpin;
     if (memchr(pPin, '\0', ulPinLen) != NULL)
-      return CKR_PIN_INCORRECT;
+      lose(CKR_PIN_INCORRECT);
     if ((newpin = malloc(ulPinLen + 1)) == NULL)
-      return CKR_HOST_MEMORY;
+      lose(CKR_HOST_MEMORY);
     memcpy(newpin, pPin, ulPinLen);
     newpin[ulPinLen] = '\0';
     if (pin != NULL)
@@ -2669,14 +2982,18 @@ CK_RV C_Login(CK_SESSION_HANDLE hSession,
 
   assert(p11_session_consistent_login());
 
-  return CKR_OK;
+ fail:
+  return mutex_unlock_with_rv(rv, p11_global_mutex);
 }
 
 CK_RV C_Logout(CK_SESSION_HANDLE hSession)
 {
   p11_session_t *session;
+  CK_RV rv = CKR_OK;
   int crypt_cmd;
 
+  mutex_lock_or_fail(p11_global_mutex);
+
   /*
    * Mind, I don't really know why this function takes a session
    * handle, given that the semantics don't seem to call upon us to do
@@ -2684,12 +3001,12 @@ CK_RV C_Logout(CK_SESSION_HANDLE hSession)
    */
 
   if (p11_session_find(hSession) == NULL)
-    return CKR_SESSION_HANDLE_INVALID;
+    lose(CKR_SESSION_HANDLE_INVALID);
 
   switch (logged_in_as) {
   case logged_in_as_user:       crypt_cmd = CRYPT_DEVINFO_AUTHENT_USER;         break;
   case logged_in_as_so:         crypt_cmd = CRYPT_DEVINFO_AUTHENT_SUPERVISOR;   break;
-  case not_logged_in:           return CKR_USER_NOT_LOGGED_IN;
+  case not_logged_in:           lose(CKR_USER_NOT_LOGGED_IN);
   }
 
   /*
@@ -2703,7 +3020,7 @@ CK_RV C_Logout(CK_SESSION_HANDLE hSession)
 
 #if ENABLE_CRYPTLIB_DEVICE
   if (cryptSetAttributeString(cryptlib_device, crypt_cmd, "", 0) != CRYPT_OK)
-    return CKR_FUNCTION_FAILED;
+    lose(CKR_FUNCTION_FAILED);
 #endif
 
 #if ENABLE_CRYPTLIB_SOFTWARE
@@ -2740,7 +3057,8 @@ CK_RV C_Logout(CK_SESSION_HANDLE hSession)
 
   assert(p11_session_consistent_login());
 
-  return CKR_OK;
+ fail:
+  return mutex_unlock_with_rv(rv, p11_global_mutex);
 }
 
 CK_RV C_DestroyObject(CK_SESSION_HANDLE hSession,
@@ -2757,10 +3075,14 @@ CK_RV C_DestroyObject(CK_SESSION_HANDLE hSession,
 
   const char *flavor = is_token_handle(hObject) ? "token" : "session";
 
-  p11_session_t *session = p11_session_find(hSession);
+  p11_session_t *session;
   sqlite3_stmt *q = NULL;
-  CK_RV rv = CKR_OK;
   sqlite3_int64 id;
+  CK_RV rv = CKR_OK;
+
+  mutex_lock_or_fail(p11_global_mutex);
+
+  session = p11_session_find(hSession);
 
   if ((rv = p11_object_check_rights(session, hObject, p11_object_access_write)) != CKR_OK)
     goto fail;
@@ -2804,7 +3126,7 @@ CK_RV C_DestroyObject(CK_SESSION_HANDLE hSession,
 
  fail:
   sqlite3_finalize(q);
-  return rv;
+  return mutex_unlock_with_rv(rv, p11_global_mutex);
 }
 
 CK_RV C_GetAttributeValue(CK_SESSION_HANDLE hSession,
@@ -2818,19 +3140,23 @@ CK_RV C_GetAttributeValue(CK_SESSION_HANDLE hSession,
 
   const char *flavor = is_token_handle(hObject) ? "token" : "session";
 
-  p11_session_t *session = p11_session_find(hSession);
+  p11_session_t *session;
   const p11_descriptor_t *descriptor = NULL;
   CK_BBOOL cka_sensitive, cka_extractable;
   CK_OBJECT_CLASS cka_class;
   CK_KEY_TYPE cka_key_type;
   int sensitive_object = 0;
   sqlite3_stmt *q = NULL;
-  CK_RV rv = CKR_OK;
+  CK_RV rv;
   int ret, i;
 
+  mutex_lock_or_fail(p11_global_mutex);
+
   if (pTemplate == NULL)
     lose(CKR_ARGUMENTS_BAD);
 
+  session = p11_session_find(hSession);
+
   if ((rv = p11_object_check_rights(session, hObject, p11_object_access_read)) != CKR_OK)
     goto fail;
 
@@ -2859,6 +3185,8 @@ CK_RV C_GetAttributeValue(CK_SESSION_HANDLE hSession,
       !sql_check_ok(sqlite3_bind_int64(q, 1, hObject)))
     lose(CKR_FUNCTION_FAILED);
 
+  rv = CKR_OK;
+
   for (i = 0; i < ulCount; i++) {
 
     if (sensitive_object && p11_attribute_is_sensitive(descriptor, pTemplate[i].type)) {
@@ -2893,7 +3221,7 @@ CK_RV C_GetAttributeValue(CK_SESSION_HANDLE hSession,
 
  fail:
   sqlite3_finalize(q);
-  return rv;
+  return mutex_unlock_with_rv(rv, p11_global_mutex);
 }
 
 CK_RV C_FindObjectsInit(CK_SESSION_HANDLE hSession,
@@ -2930,12 +3258,14 @@ CK_RV C_FindObjectsInit(CK_SESSION_HANDLE hSession,
   static const char select_format[] =
     " SELECT object_handle FROM findobjects_%lu NATURAL JOIN object ORDER BY object_id";
 
-  p11_session_t *session = p11_session_find(hSession);
+  p11_session_t *session;
   sqlite3_stmt *q1 = NULL, *q2 = NULL;
   CK_RV rv = CKR_OK;
   int i, ret;
 
-  if (session == NULL)
+  mutex_lock_or_fail(p11_global_mutex);
+
+  if ((session = p11_session_find(hSession)) == NULL)
     lose(CKR_SESSION_HANDLE_INVALID);
 
   if (ulCount > 0  && pTemplate == NULL)
@@ -3032,7 +3362,7 @@ CK_RV C_FindObjectsInit(CK_SESSION_HANDLE hSession,
  fail:
   sqlite3_finalize(q1);
   sqlite3_finalize(q2);
-  return rv;
+  return mutex_unlock_with_rv(rv, p11_global_mutex);
 }
 
 CK_RV C_FindObjects(CK_SESSION_HANDLE hSession,
@@ -3040,11 +3370,13 @@ CK_RV C_FindObjects(CK_SESSION_HANDLE hSession,
                     CK_ULONG ulMaxObjectCount,
                     CK_ULONG_PTR pulObjectCount)
 {
-  p11_session_t *session = p11_session_find(hSession);
+  p11_session_t *session;
   int i, ret = SQLITE_OK;
   CK_RV rv = CKR_OK;
 
-  if (session == NULL)
+  mutex_lock_or_fail(p11_global_mutex);
+
+  if ((session = p11_session_find(hSession)) == NULL)
     lose(CKR_SESSION_HANDLE_INVALID);
 
   if (session->find_query == NULL)
@@ -3083,7 +3415,7 @@ CK_RV C_FindObjects(CK_SESSION_HANDLE hSession,
   *pulObjectCount = i;
 
  fail:
-  return rv;
+  return mutex_unlock_with_rv(rv, p11_global_mutex);
 }
 
 CK_RV C_FindObjectsFinal(CK_SESSION_HANDLE hSession)
@@ -3091,11 +3423,13 @@ CK_RV C_FindObjectsFinal(CK_SESSION_HANDLE hSession)
   static const char drop_format[] =
     " DROP TABLE IF EXISTS findobjects_%lu";
 
-  p11_session_t *session = p11_session_find(hSession);
+  p11_session_t *session;
   sqlite3_stmt *q = NULL;
   CK_RV rv = CKR_OK;
   
-  if (session == NULL)
+  mutex_lock_or_fail(p11_global_mutex);
+
+  if ((session = p11_session_find(hSession)) == NULL)
     lose(CKR_SESSION_HANDLE_INVALID);
 
   if (session->find_query == NULL)
@@ -3114,32 +3448,34 @@ CK_RV C_FindObjectsFinal(CK_SESSION_HANDLE hSession)
 
  fail:
   sqlite3_finalize(q);
-  return rv;
+  return mutex_unlock_with_rv(rv, p11_global_mutex);
 }
 
 CK_RV C_DigestInit(CK_SESSION_HANDLE hSession,
                    CK_MECHANISM_PTR pMechanism)
 {
-  p11_session_t *session = p11_session_find(hSession);
+  p11_session_t *session;
   CRYPT_ALGO_TYPE algo;
-  unsigned hash_len;
-  CK_RV rv;
+  unsigned hash_len = 0;
+  CK_RV rv = CKR_OK;
 
-  if (session == NULL)
-    return CKR_SESSION_HANDLE_INVALID;
+  mutex_lock_or_fail(p11_global_mutex);
+
+  if ((session = p11_session_find(hSession)) == NULL)
+    lose(CKR_SESSION_HANDLE_INVALID);
 
   if (pMechanism == NULL)
-    return CKR_ARGUMENTS_BAD;
+    lose(CKR_ARGUMENTS_BAD);
 
   if (session->digest_context != CRYPT_HANDLE_NONE)
-    return CKR_OPERATION_ACTIVE;
+    lose(CKR_OPERATION_ACTIVE);
 
   switch (pMechanism->mechanism) {
   case CKM_SHA_1:       algo = CRYPT_ALGO_SHA1; break;
   case CKM_SHA256:      algo = CRYPT_ALGO_SHA2; hash_len = 256; break;
   case CKM_SHA384:      algo = CRYPT_ALGO_SHA2; hash_len = 384; break;
   case CKM_SHA512:      algo = CRYPT_ALGO_SHA2; hash_len = 512; break;
-  default:              return CKR_MECHANISM_INVALID;
+  default:              lose(CKR_MECHANISM_INVALID);
   }
 
   assert((hash_len & 7) == 0);
@@ -3151,13 +3487,14 @@ CK_RV C_DigestInit(CK_SESSION_HANDLE hSession,
       cryptSetAttribute(session->digest_context, CRYPT_CTXINFO_BLOCKSIZE, hash_len >> 3) != CRYPT_OK)
     lose(CKR_FUNCTION_FAILED);
 
-  return CKR_OK;
+  return mutex_unlock(p11_global_mutex);
 
  fail:
-  if (session->digest_context != CRYPT_HANDLE_NONE)
+  if (session != NULL && session->digest_context != CRYPT_HANDLE_NONE) {
     cryptDestroyContext(session->digest_context);
-  session->digest_context = CRYPT_HANDLE_NONE;
-  return rv;
+    session->digest_context = CRYPT_HANDLE_NONE;
+  }
+  return mutex_unlock_with_rv(rv, p11_global_mutex);
 }
 
 CK_RV C_Digest(CK_SESSION_HANDLE hSession,
@@ -3166,24 +3503,26 @@ CK_RV C_Digest(CK_SESSION_HANDLE hSession,
                CK_BYTE_PTR pDigest,
                CK_ULONG_PTR pulDigestLen)
 {
-  p11_session_t *session = p11_session_find(hSession);
-  CK_RV rv;
+  p11_session_t *session;
+  CK_RV rv = CKR_OK;
   int len;
 
-  if (session == NULL)
-    return CKR_SESSION_HANDLE_INVALID;
+  mutex_lock_or_fail(p11_global_mutex);
+
+  if ((session = p11_session_find(hSession)) == NULL)
+    lose(CKR_SESSION_HANDLE_INVALID);
 
   if (pData == NULL || pulDigestLen == NULL)
-    return CKR_ARGUMENTS_BAD;
+    lose(CKR_ARGUMENTS_BAD);
 
   if (session->digest_context == CRYPT_HANDLE_NONE)
-    return CKR_OPERATION_NOT_INITIALIZED;
+    lose(CKR_OPERATION_NOT_INITIALIZED);
 
   if (pDigest == NULL) {
     if (cryptGetAttribute(session->digest_context, CRYPT_CTXINFO_BLOCKSIZE, &len) != CRYPT_OK)
       lose(CKR_FUNCTION_FAILED);
     *pulDigestLen = len;
-    return CKR_OK;
+    return mutex_unlock(p11_global_mutex);
   }
 
   if (cryptEncrypt(session->digest_context, pData, ulDataLen) != CRYPT_OK)
@@ -3209,31 +3548,36 @@ CK_RV C_Digest(CK_SESSION_HANDLE hSession,
   rv = CKR_OK;                  /* Fall through */
 
  fail:
-  cryptDestroyContext(session->digest_context);
-  session->digest_context = CRYPT_HANDLE_NONE;
-  return rv;
+  if (session != NULL && session->digest_context != CRYPT_HANDLE_NONE) {
+    cryptDestroyContext(session->digest_context);
+    session->digest_context = CRYPT_HANDLE_NONE;
+  }
+  return mutex_unlock_with_rv(rv, p11_global_mutex);
 }
 
 CK_RV C_SignInit(CK_SESSION_HANDLE hSession,
                  CK_MECHANISM_PTR pMechanism,
                  CK_OBJECT_HANDLE hKey)
 {
-  p11_session_t *session = p11_session_find(hSession);
+  p11_session_t *session;
   char keyid[CRYPT_MAX_HASHSIZE * 2 + 1];
   CRYPT_ALGO_TYPE sign_algo, hash_algo;
   unsigned hash_size = 0;
+  int need_cleanup = 0;
   int key_algo;
-  CK_RV rv;
+  CK_RV rv = CKR_OK;
 
-  if (session == NULL)
-    return CKR_SESSION_HANDLE_INVALID;
+  mutex_lock_or_fail(p11_global_mutex);
+
+  if ((session = p11_session_find(hSession)) == NULL)
+    lose(CKR_SESSION_HANDLE_INVALID);
 
   if (pMechanism == NULL)
-    return CKR_ARGUMENTS_BAD;
+    lose(CKR_ARGUMENTS_BAD);
 
   if (session->sign_key_context    != CRYPT_HANDLE_NONE ||
       session->sign_digest_context != CRYPT_HANDLE_NONE)
-    return CKR_OPERATION_ACTIVE;
+    lose(CKR_OPERATION_ACTIVE);
 
   if ((rv = p11_object_check_rights(session, hKey, p11_object_access_read)) != CKR_OK)
     goto fail;
@@ -3249,6 +3593,8 @@ CK_RV C_SignInit(CK_SESSION_HANDLE hSession,
 
   assert((hash_size & 7) == 0);
 
+  need_cleanup = 1;
+
   if (!p11_object_get_keyid(hKey, keyid, sizeof(keyid)) ||
       cryptlib_load_key(&session->sign_key_context, keyid) != CRYPT_OK ||
       cryptGetAttribute(session->sign_key_context, CRYPT_CTXINFO_ALGO, &key_algo) != CRYPT_OK)
@@ -3266,18 +3612,24 @@ CK_RV C_SignInit(CK_SESSION_HANDLE hSession,
                         CRYPT_CTXINFO_BLOCKSIZE, hash_size >> 3) != CRYPT_OK)
     lose(CKR_FUNCTION_FAILED);
 
-  return CKR_OK;
+  need_cleanup = 0;
+
+  rv = CKR_OK;                  /* Fall through */
 
  fail:
-  if (session->sign_key_context != CRYPT_HANDLE_NONE)
+  assert(!need_cleanup || session != NULL);
+
+  if (need_cleanup && session->sign_key_context != CRYPT_HANDLE_NONE) {
     cryptDestroyContext(session->sign_key_context);
-  session->sign_key_context = CRYPT_HANDLE_NONE;
+    session->sign_key_context = CRYPT_HANDLE_NONE;
+  }
 
-  if (session->sign_digest_context != CRYPT_HANDLE_NONE)
+  if (need_cleanup && session->sign_digest_context != CRYPT_HANDLE_NONE) {
     cryptDestroyContext(session->sign_digest_context);
-  session->sign_digest_context = CRYPT_HANDLE_NONE;
+    session->sign_digest_context = CRYPT_HANDLE_NONE;
+  }
 
-  return rv;
+  return mutex_unlock_with_rv(rv, p11_global_mutex);
 }
 
 CK_RV C_Sign(CK_SESSION_HANDLE hSession,
@@ -3286,18 +3638,20 @@ CK_RV C_Sign(CK_SESSION_HANDLE hSession,
              CK_BYTE_PTR pSignature,
              CK_ULONG_PTR pulSignatureLen)
 {
-  p11_session_t *session = p11_session_find(hSession);
+  p11_session_t *session;
   int len, algo;
   CK_RV rv;
 
-  if (session == NULL)
-    return CKR_SESSION_HANDLE_INVALID;
+  mutex_lock_or_fail(p11_global_mutex);
+
+  if ((session = p11_session_find(hSession)) == NULL)
+    lose(CKR_SESSION_HANDLE_INVALID);
 
   if (pData == NULL || pulSignatureLen == NULL)
-    return CKR_ARGUMENTS_BAD;
+    lose(CKR_ARGUMENTS_BAD);
 
   if (session->sign_key_context == CRYPT_HANDLE_NONE)
-    return CKR_OPERATION_NOT_INITIALIZED;
+    lose(CKR_OPERATION_NOT_INITIALIZED);
 
   if (pSignature == NULL) {
     /*
@@ -3421,14 +3775,17 @@ CK_RV C_Sign(CK_SESSION_HANDLE hSession,
 
  fail:
 
-  if (session->sign_digest_context != CRYPT_HANDLE_NONE)
+  if (session != NULL && session->sign_digest_context != CRYPT_HANDLE_NONE) {
     cryptDestroyContext(session->sign_digest_context);
-  session->sign_digest_context = CRYPT_HANDLE_NONE;
+    session->sign_digest_context = CRYPT_HANDLE_NONE;
+  }
 
-  cryptDestroyContext(session->sign_key_context);
-  session->sign_key_context = CRYPT_HANDLE_NONE;
+  if (session != NULL && session->sign_key_context != CRYPT_HANDLE_NONE) {
+    cryptDestroyContext(session->sign_key_context);
+    session->sign_key_context = CRYPT_HANDLE_NONE;
+  }
 
-  return rv;
+  return mutex_unlock_with_rv(rv, p11_global_mutex);
 }
 
 /*
@@ -3436,6 +3793,7 @@ CK_RV C_Sign(CK_SESSION_HANDLE hSession,
  * More general use presumably wants this for things like generating
  * symmetric keys for later wrapping by asymmetric keys.
  */
+
 CK_RV C_GenerateKey(CK_SESSION_HANDLE hSession,
                     CK_MECHANISM_PTR pMechanism,
                     CK_ATTRIBUTE_PTR pTemplate,
@@ -3445,6 +3803,13 @@ CK_RV C_GenerateKey(CK_SESSION_HANDLE hSession,
   return CKR_FUNCTION_NOT_SUPPORTED;
 }
 
+/*
+ * If there's any method in this entire package which really needs a
+ * more complex mutex structure than the single global mutex, it's
+ * probably this one.  Key generation can take a looooong time.
+ * Drive off that bridge when we get to it.
+ */
+
 CK_RV C_GenerateKeyPair(CK_SESSION_HANDLE hSession,
                         CK_MECHANISM_PTR pMechanism,
                         CK_ATTRIBUTE_PTR pPublicKeyTemplate,
@@ -3454,40 +3819,51 @@ CK_RV C_GenerateKeyPair(CK_SESSION_HANDLE hSession,
                         CK_OBJECT_HANDLE_PTR phPublicKey,
                         CK_OBJECT_HANDLE_PTR phPrivateKey)
 {
-  p11_session_t *session = p11_session_find(hSession);
+  p11_session_t *session;
+  CK_RV rv;
 
-  if (session == NULL)
-    return CKR_SESSION_HANDLE_INVALID;
+  mutex_lock_or_fail(p11_global_mutex);
+
+  if ((session = p11_session_find(hSession)) == NULL)
+    lose(CKR_SESSION_HANDLE_INVALID);
 
   if (pMechanism          == NULL ||
       pPublicKeyTemplate  == NULL || phPublicKey  == NULL ||
       pPrivateKeyTemplate == NULL || phPrivateKey == NULL)
-    return CKR_ARGUMENTS_BAD;
+    lose(CKR_ARGUMENTS_BAD);
 
   switch (pMechanism->mechanism) {
+
   case CKM_RSA_PKCS_KEY_PAIR_GEN:
-    return generate_keypair_rsa_pkcs(session, pMechanism,
-                                     pPublicKeyTemplate, ulPublicKeyAttributeCount,
-                                     pPrivateKeyTemplate, ulPrivateKeyAttributeCount,
-                                     phPublicKey, phPrivateKey);
+    rv = generate_keypair_rsa_pkcs(session, pMechanism,
+                                   pPublicKeyTemplate, ulPublicKeyAttributeCount,
+                                   pPrivateKeyTemplate, ulPrivateKeyAttributeCount,
+                                   phPublicKey, phPrivateKey);
+    break;
+
   default:
-    return CKR_MECHANISM_INVALID;
+    lose(CKR_MECHANISM_INVALID);
   }
+
+ fail:
+  return mutex_unlock_with_rv(rv, p11_global_mutex);
 }
 
 CK_RV C_GenerateRandom(CK_SESSION_HANDLE hSession,
                        CK_BYTE_PTR RandomData,
                        CK_ULONG ulRandomLen)
 {
-  p11_session_t *session = p11_session_find(hSession);
+  p11_session_t *session;
   CRYPT_CONTEXT ctx = CRYPT_HANDLE_NONE;
   CK_RV rv = CKR_OK;
 
-  if (session == NULL)
-    return CKR_SESSION_HANDLE_INVALID;
+  mutex_lock_or_fail(p11_global_mutex);
+
+  if ((session = p11_session_find(hSession)) == NULL)
+    lose(CKR_SESSION_HANDLE_INVALID);
 
   if (RandomData == NULL)
-    return CKR_ARGUMENTS_BAD;
+    lose(CKR_ARGUMENTS_BAD);
 
   /*
    * Cryptlib doesn't expose the raw TRNG, but, per the manual, block
@@ -3508,7 +3884,7 @@ CK_RV C_GenerateRandom(CK_SESSION_HANDLE hSession,
   if (ctx != CRYPT_HANDLE_NONE)
     (void) cryptDestroyContext(ctx);
 
-  return rv;
+  return mutex_unlock_with_rv(rv, p11_global_mutex);
 }