changeset 10403:560db669cfa6

6842872 Race condition in fork() and C_Initialize() causes deadlock in pkcs11 6862268 C_Initialize() does not correctly cleans resources when fails 6862202 token_session mutexes are not covered by at_fork handler 6862207 PKCS11 softtoken:C_Initialize() sets softtoken_initialized to TRUE also when it fails
author Zdenek Kotala <Zdenek.Kotala@Sun.COM>
date Fri, 28 Aug 2009 10:12:00 +0200
parents 5da1486d2c3a
children fa52ba964ce0
files usr/src/lib/pkcs11/libpkcs11/common/pkcs11General.c usr/src/lib/pkcs11/pkcs11_kernel/common/kernelGeneral.c usr/src/lib/pkcs11/pkcs11_softtoken/common/softGeneral.c usr/src/lib/pkcs11/pkcs11_softtoken/common/softKeystore.c usr/src/lib/pkcs11/pkcs11_softtoken/common/softSession.h usr/src/lib/pkcs11/pkcs11_softtoken/common/softSessionUtil.c
diffstat 6 files changed, 174 insertions(+), 130 deletions(-) [+]
line wrap: on
line diff
--- a/usr/src/lib/pkcs11/libpkcs11/common/pkcs11General.c	Fri Aug 28 13:38:17 2009 +0800
+++ b/usr/src/lib/pkcs11/libpkcs11/common/pkcs11General.c	Fri Aug 28 10:12:00 2009 +0200
@@ -19,12 +19,10 @@
  * CDDL HEADER END
  */
 /*
- * Copyright 2008 Sun Microsystems, Inc.  All rights reserved.
+ * Copyright 2009 Sun Microsystems, Inc.  All rights reserved.
  * Use is subject to license terms.
  */
 
-#pragma ident	"@(#)pkcs11General.c	1.10	08/08/12 SMI"
-
 #include <unistd.h>
 #include <string.h>
 #include <cryptoutil.h>
@@ -37,6 +35,7 @@
 #include "pkcs11Session.h"
 #include "metaGlobal.h"
 
+#pragma init(pkcs11_init)
 #pragma fini(pkcs11_fini)
 
 static struct CK_FUNCTION_LIST functionList = {
@@ -121,28 +120,38 @@
 static pthread_mutex_t globalmutex = PTHREAD_MUTEX_INITIALIZER;
 
 static CK_RV finalize_common(CK_VOID_PTR pReserved);
+static void pkcs11_init();
 static void pkcs11_fini();
 
 /*
  * Ensure that before a fork, all mutexes are taken.
+ * We cannot acquire globalmutex, because it can cause deadlock when
+ * atfork() and fork() are called in parallel. It can happen when
+ * C_Ininitialize() tries to dlopen() a provider. The dlopen() operation
+ * is protected by globalmutex and when another thread calls fork()
+ * pkcs11_fork_prepare cannot acquire the mutex again and thus it must wait.
+ * When a provider tries to register its atfork handler, atfork() must
+ * wait on fork(). See the comment in fork() libc function for more details.
+ *
  * Order:
- * 1. globalmutex
- * 2. slottable->st_mutex
- * 3. all slottable->st_slots' mutexes
+ * 1. slottable->st_mutex
+ * 2. all slottable->st_slots' mutexes
  */
 static void
 pkcs11_fork_prepare(void)
 {
 	int i;
-	(void) pthread_mutex_lock(&globalmutex);
-	if (slottable != NULL) {
-		(void) pthread_mutex_lock(&slottable->st_mutex);
+	if (pkcs11_initialized) {
+		if (slottable != NULL) {
+			(void) pthread_mutex_lock(&slottable->st_mutex);
 
-		/* Take the sl_mutex of all slots */
-		for (i = slottable->st_first; i <= slottable->st_last; i++) {
-			if (slottable->st_slots[i] != NULL) {
-				(void) pthread_mutex_lock(
-				    &slottable->st_slots[i]->sl_mutex);
+			/* Take the sl_mutex of all slots */
+			for (i = slottable->st_first;
+			    i <= slottable->st_last; i++) {
+				if (slottable->st_slots[i] != NULL) {
+					(void) pthread_mutex_lock(
+					    &slottable->st_slots[i]->sl_mutex);
+				}
 			}
 		}
 	}
@@ -157,39 +166,58 @@
 pkcs11_fork_parent(void)
 {
 	int i;
-	if (slottable != NULL) {
-		/* Release the sl_mutex of all slots */
-		for (i = slottable->st_first; i <= slottable->st_last; i++) {
-			if (slottable->st_slots[i] != NULL) {
-				(void) pthread_mutex_unlock(
-				    &slottable->st_slots[i]->sl_mutex);
+	if (pkcs11_initialized) {
+		if (slottable != NULL) {
+			/* Release the sl_mutex of all slots */
+			for (i = slottable->st_first;
+			    i <= slottable->st_last; i++) {
+				if (slottable->st_slots[i] != NULL) {
+					(void) pthread_mutex_unlock(
+					    &slottable->st_slots[i]->sl_mutex);
+				}
 			}
 		}
 		(void) pthread_mutex_unlock(&slottable->st_mutex);
 	}
-	(void) pthread_mutex_unlock(&globalmutex);
 }
 
 
 /*
  * Ensure that after a fork, in the child, all mutexes are released in opposite
  * order to pkcs11_fork_prepare() and cleanup is done.
+ * Because we need to handle fork correctly before library is initialized two
+ * handlers are necessary.
+ *
+ * 1) pkcs11_fork_child() - unlock mutexes
+ * 2) pkcs11_fork_child_fini() - cleanup library after fork, it is registered in
+ *                               C_Initialize() after providers initialization.
  */
 static void
 pkcs11_fork_child(void)
 {
 	int i;
-	if (slottable != NULL) {
-		/* Release the sl_mutex of all slots */
-		for (i = slottable->st_first; i <= slottable->st_last; i++) {
-			if (slottable->st_slots[i] != NULL) {
-				(void) pthread_mutex_unlock(
-				    &slottable->st_slots[i]->sl_mutex);
+	if (pkcs11_initialized) {
+		if (slottable != NULL) {
+			/* Release the sl_mutex of all slots */
+			for (i = slottable->st_first;
+			    i <= slottable->st_last; i++) {
+				if (slottable->st_slots[i] != NULL) {
+					(void) pthread_mutex_unlock(
+					    &slottable->st_slots[i]->sl_mutex);
+				}
 			}
 		}
 		(void) pthread_mutex_unlock(&slottable->st_mutex);
 	}
-	(void) pthread_mutex_unlock(&globalmutex);
+
+	(void) pthread_mutex_destroy(&globalmutex);
+	(void) pthread_mutex_init(&globalmutex, NULL);
+}
+
+/* Library cleanup have to be last afterfork child handler. */
+static void
+pkcs11_fork_child_fini(void)
+{
 	pkcs11_fini();
 }
 
@@ -298,10 +326,10 @@
 	pkcs11_pid = initialize_pid;
 	/* Children inherit parent's atfork handlers */
 	if (!pkcs11_atfork_initialized) {
-		(void) pthread_atfork(pkcs11_fork_prepare,
-		    pkcs11_fork_parent, pkcs11_fork_child);
+		(void) pthread_atfork(NULL, NULL, pkcs11_fork_child_fini);
 		pkcs11_atfork_initialized = B_TRUE;
 	}
+
 	(void) pthread_mutex_unlock(&globalmutex);
 
 	/* Cleanup data structures no longer needed */
@@ -404,6 +432,13 @@
 	return (rv);
 }
 
+static void
+pkcs11_init()
+{
+	(void) pthread_atfork(pkcs11_fork_prepare,
+	    pkcs11_fork_parent, pkcs11_fork_child);
+}
+
 /*
  * pkcs11_fini() function required to make sure complete cleanup
  * is done of plugins if the framework is ever unloaded without
--- a/usr/src/lib/pkcs11/pkcs11_kernel/common/kernelGeneral.c	Fri Aug 28 13:38:17 2009 +0800
+++ b/usr/src/lib/pkcs11/pkcs11_kernel/common/kernelGeneral.c	Fri Aug 28 10:12:00 2009 +0200
@@ -33,6 +33,7 @@
 #include "kernelSession.h"
 #include "kernelSlot.h"
 
+#pragma init(kernel_init)
 #pragma fini(kernel_fini)
 
 static struct CK_FUNCTION_LIST functionList = {
@@ -108,7 +109,6 @@
 };
 
 boolean_t kernel_initialized = B_FALSE;
-static boolean_t kernel_atfork_initialized = B_FALSE;
 static pid_t kernel_pid = 0;
 
 extern pthread_mutex_t mechhash_mutex;
@@ -125,10 +125,10 @@
 
 static void finalize_common();
 static void cleanup_library();
+static void kernel_init();
 static void kernel_fini();
 static void kernel_fork_prepare();
-static void kernel_fork_parent();
-static void kernel_fork_child();
+static void kernel_fork_after();
 
 CK_RV
 C_Initialize(CK_VOID_PTR pInitArgs)
@@ -246,13 +246,6 @@
 	kernel_initialized = B_TRUE;
 	kernel_pid = initialize_pid;
 
-	/* Children inherit parent's atfork handlers */
-	if (!kernel_atfork_initialized) {
-		(void) pthread_atfork(kernel_fork_prepare, kernel_fork_parent,
-		    kernel_fork_child);
-		kernel_atfork_initialized = B_TRUE;
-	}
-
 	(void) pthread_mutex_unlock(&globalmutex);
 
 	return (CKR_OK);
@@ -386,6 +379,13 @@
 	finalize_common();
 }
 
+static void
+kernel_init()
+{
+	(void) pthread_atfork(kernel_fork_prepare, kernel_fork_after,
+	    kernel_fork_after);
+}
+
 /*
  * kernel_fini() function required to make sure complete cleanup
  * is done if pkcs11_kernel is ever unloaded without
@@ -469,37 +469,31 @@
 kernel_fork_prepare()
 {
 	(void) pthread_mutex_lock(&globalmutex);
-	kernel_acquire_all_slots_mutexes();
-	(void) pthread_mutex_lock(
-	    &obj_delay_freed.obj_to_be_free_mutex);
-	(void) pthread_mutex_lock(
-	    &ses_delay_freed.ses_to_be_free_mutex);
-	(void) pthread_mutex_lock(&mechhash_mutex);
+	if (kernel_initialized) {
+		kernel_acquire_all_slots_mutexes();
+		(void) pthread_mutex_lock(
+		    &obj_delay_freed.obj_to_be_free_mutex);
+		(void) pthread_mutex_lock(
+		    &ses_delay_freed.ses_to_be_free_mutex);
+		(void) pthread_mutex_lock(&mechhash_mutex);
+	}
 }
 
-/* Release in opposite order to kernel_fork_prepare(). */
+/*
+ * Release in opposite order to kernel_fork_prepare().
+ * Function is used for parent and child.
+ */
 void
-kernel_fork_parent()
+kernel_fork_after()
 {
-	(void) pthread_mutex_unlock(&mechhash_mutex);
-	(void) pthread_mutex_unlock(
-	    &ses_delay_freed.ses_to_be_free_mutex);
-	(void) pthread_mutex_unlock(
-	    &obj_delay_freed.obj_to_be_free_mutex);
-	kernel_release_all_slots_mutexes();
-	(void) pthread_mutex_unlock(&globalmutex);
-}
-
-/* Release in opposite order to kernel_fork_prepare(). */
-void
-kernel_fork_child()
-{
-	(void) pthread_mutex_unlock(&mechhash_mutex);
-	(void) pthread_mutex_unlock(
-	    &ses_delay_freed.ses_to_be_free_mutex);
-	(void) pthread_mutex_unlock(
-	    &obj_delay_freed.obj_to_be_free_mutex);
-	kernel_release_all_slots_mutexes();
+	if (kernel_initialized) {
+		(void) pthread_mutex_unlock(&mechhash_mutex);
+		(void) pthread_mutex_unlock(
+		    &ses_delay_freed.ses_to_be_free_mutex);
+		(void) pthread_mutex_unlock(
+		    &obj_delay_freed.obj_to_be_free_mutex);
+		kernel_release_all_slots_mutexes();
+	}
 	(void) pthread_mutex_unlock(&globalmutex);
 }
 
--- a/usr/src/lib/pkcs11/pkcs11_softtoken/common/softGeneral.c	Fri Aug 28 13:38:17 2009 +0800
+++ b/usr/src/lib/pkcs11/pkcs11_softtoken/common/softGeneral.c	Fri Aug 28 10:12:00 2009 +0200
@@ -35,8 +35,11 @@
 #include "softKeystore.h"
 #include "softKeystoreUtil.h"
 
+#pragma init(softtoken_init)
 #pragma fini(softtoken_fini)
 
+extern soft_session_t token_session; /* for fork handler */
+
 static struct CK_FUNCTION_LIST functionList = {
 	{ 2, 20 },	/* version */
 	C_Initialize,
@@ -110,7 +113,6 @@
 };
 
 boolean_t softtoken_initialized = B_FALSE;
-static boolean_t softtoken_atfork_initialized = B_FALSE;
 
 static pid_t softtoken_pid = 0;
 
@@ -128,10 +130,10 @@
 pthread_mutex_t soft_giant_mutex = PTHREAD_MUTEX_INITIALIZER;
 
 static CK_RV finalize_common(boolean_t force, CK_VOID_PTR pReserved);
+static void softtoken_init();
 static void softtoken_fini();
 static void softtoken_fork_prepare();
-static void softtoken_fork_parent();
-static void softtoken_fork_child();
+static void softtoken_fork_after();
 
 CK_RV
 C_Initialize(CK_VOID_PTR pInitArgs)
@@ -213,9 +215,6 @@
 		return (CKR_CANT_LOCK);
 	}
 
-	softtoken_initialized = B_TRUE;
-	softtoken_pid = initialize_pid;
-
 	/*
 	 * token object related initialization
 	 */
@@ -225,12 +224,14 @@
 	soft_slot.keystore_load_status = KEYSTORE_UNINITIALIZED;
 
 	if ((rv = soft_init_token_session()) != CKR_OK) {
+		(void) pthread_mutex_destroy(&soft_sessionlist_mutex);
 		(void) pthread_mutex_unlock(&soft_giant_mutex);
 		return (rv);
 	}
 
 	/* Initialize the slot lock */
 	if (pthread_mutex_init(&soft_slot.slot_mutex, NULL) != 0) {
+		(void) pthread_mutex_destroy(&soft_sessionlist_mutex);
 		(void) soft_destroy_token_session();
 		(void) pthread_mutex_unlock(&soft_giant_mutex);
 		return (CKR_CANT_LOCK);
@@ -238,31 +239,47 @@
 
 	/* Initialize the keystore lock */
 	if (pthread_mutex_init(&soft_slot.keystore_mutex, NULL) != 0) {
+		(void) pthread_mutex_destroy(&soft_slot.slot_mutex);
+		(void) pthread_mutex_destroy(&soft_sessionlist_mutex);
+		(void) soft_destroy_token_session();
 		(void) pthread_mutex_unlock(&soft_giant_mutex);
 		return (CKR_CANT_LOCK);
 	}
 
-	/* Children inherit parent's atfork handlers */
-	if (!softtoken_atfork_initialized) {
-		(void) pthread_atfork(softtoken_fork_prepare,
-		    softtoken_fork_parent, softtoken_fork_child);
-		softtoken_atfork_initialized = B_TRUE;
+	/* Initialize the object_to_be_freed list */
+	if (pthread_mutex_init(&obj_delay_freed.obj_to_be_free_mutex, NULL)
+	    != 0) {
+		(void) pthread_mutex_destroy(&soft_slot.keystore_mutex);
+		(void) pthread_mutex_destroy(&soft_slot.slot_mutex);
+		(void) pthread_mutex_destroy(&soft_sessionlist_mutex);
+		(void) soft_destroy_token_session();
+		(void) pthread_mutex_unlock(&soft_giant_mutex);
+		return (CKR_CANT_LOCK);
 	}
-
-	(void) pthread_mutex_unlock(&soft_giant_mutex);
-
-	/* Initialize the object_to_be_freed list */
-	(void) pthread_mutex_init(&obj_delay_freed.obj_to_be_free_mutex, NULL);
 	obj_delay_freed.count = 0;
 	obj_delay_freed.first = NULL;
 	obj_delay_freed.last = NULL;
 
-	(void) pthread_mutex_init(&ses_delay_freed.ses_to_be_free_mutex, NULL);
+	if (pthread_mutex_init(&ses_delay_freed.ses_to_be_free_mutex, NULL)
+	    != 0) {
+		(void) pthread_mutex_destroy(
+		    &obj_delay_freed.obj_to_be_free_mutex);
+		(void) pthread_mutex_destroy(&soft_slot.keystore_mutex);
+		(void) pthread_mutex_destroy(&soft_slot.slot_mutex);
+		(void) pthread_mutex_destroy(&soft_sessionlist_mutex);
+		(void) soft_destroy_token_session();
+		(void) pthread_mutex_unlock(&soft_giant_mutex);
+		return (CKR_CANT_LOCK);
+	}
 	ses_delay_freed.count = 0;
 	ses_delay_freed.first = NULL;
 	ses_delay_freed.last = NULL;
+
+	softtoken_pid = initialize_pid;
+	softtoken_initialized = B_TRUE;
+	(void) pthread_mutex_unlock(&soft_giant_mutex);
+
 	return (CKR_OK);
-
 }
 
 /*
@@ -366,6 +383,14 @@
 	return (rv);
 }
 
+static void
+softtoken_init()
+{
+	/* Children inherit parent's atfork handlers */
+	(void) pthread_atfork(softtoken_fork_prepare,
+	    softtoken_fork_after, softtoken_fork_after);
+}
+
 /*
  * softtoken_fini() function required to make sure complete cleanup
  * is done if softtoken is ever unloaded without a C_Finalize() call.
@@ -446,59 +471,51 @@
 
 /*
  * Take out all mutexes before fork.
+ *
  * Order:
  * 1. soft_giant_mutex
  * 2. soft_sessionlist_mutex
  * 3. soft_slot.slot_mutex
  * 4. soft_slot.keystore_mutex
- * 5. all soft_session_list mutexs via soft_acquire_all_session_mutexes()
- * 6. obj_delay_freed.obj_to_be_free_mutex;
- * 7. ses_delay_freed.ses_to_be_free_mutex
+ * 5. token_session mutexes via soft_acquire_all_session_mutexes()
+ * 6. all soft_session_list mutexes via soft_acquire_all_session_mutexes()
+ * 7. obj_delay_freed.obj_to_be_free_mutex;
+ * 8. ses_delay_freed.ses_to_be_free_mutex
  */
 void
 softtoken_fork_prepare()
 {
 	(void) pthread_mutex_lock(&soft_giant_mutex);
-	(void) pthread_mutex_lock(&soft_sessionlist_mutex);
-	(void) pthread_mutex_lock(&soft_slot.slot_mutex);
-	(void) pthread_mutex_lock(&soft_slot.keystore_mutex);
-	soft_acquire_all_session_mutexes();
-	(void) pthread_mutex_lock(&obj_delay_freed.obj_to_be_free_mutex);
-	(void) pthread_mutex_lock(&ses_delay_freed.ses_to_be_free_mutex);
-}
-
-/* Release in opposite order to softtoken_fork_prepare(). */
-void
-softtoken_fork_parent()
-{
-	(void) pthread_mutex_unlock(&ses_delay_freed.ses_to_be_free_mutex);
-	(void) pthread_mutex_unlock(&obj_delay_freed.obj_to_be_free_mutex);
-	soft_release_all_session_mutexes();
-	(void) pthread_mutex_unlock(&soft_slot.keystore_mutex);
-	(void) pthread_mutex_unlock(&soft_slot.slot_mutex);
-	(void) pthread_mutex_unlock(&soft_sessionlist_mutex);
-	(void) pthread_mutex_unlock(&soft_giant_mutex);
+	if (softtoken_initialized) {
+		(void) pthread_mutex_lock(&soft_sessionlist_mutex);
+		(void) pthread_mutex_lock(&soft_slot.slot_mutex);
+		(void) pthread_mutex_lock(&soft_slot.keystore_mutex);
+		soft_acquire_all_session_mutexes(&token_session);
+		soft_acquire_all_session_mutexes(soft_session_list);
+		(void) pthread_mutex_lock(
+		    &obj_delay_freed.obj_to_be_free_mutex);
+		(void) pthread_mutex_lock(
+		    &ses_delay_freed.ses_to_be_free_mutex);
+	}
 }
 
 /*
  * Release in opposite order to softtoken_fork_prepare().
- *
- * Note: This never happens when softtoken is loaded by libpkcs11.so.1 because
- *       the libpkcs11 afterfork child handler is run first and it calls dlclose
- *       which removes the softtoken afterfork handler.  dlclose also causes
- *       softtoken_fini to be called at a time when all of the mutexes are
- *       already locked. Fortunately, the softtoken_fini is called from the same
- *       thread that locked them in the first place so the mutex_lock calls just
- *       return EDEADLK instead of blocking forever.
+ * Function is used for parent and child.
  */
 void
-softtoken_fork_child()
+softtoken_fork_after()
 {
-	(void) pthread_mutex_unlock(&ses_delay_freed.ses_to_be_free_mutex);
-	(void) pthread_mutex_unlock(&obj_delay_freed.obj_to_be_free_mutex);
-	soft_release_all_session_mutexes();
-	(void) pthread_mutex_unlock(&soft_slot.keystore_mutex);
-	(void) pthread_mutex_unlock(&soft_slot.slot_mutex);
-	(void) pthread_mutex_unlock(&soft_sessionlist_mutex);
+	if (softtoken_initialized) {
+		(void) pthread_mutex_unlock(
+		    &ses_delay_freed.ses_to_be_free_mutex);
+		(void) pthread_mutex_unlock(
+		    &obj_delay_freed.obj_to_be_free_mutex);
+		soft_release_all_session_mutexes(soft_session_list);
+		soft_release_all_session_mutexes(&token_session);
+		(void) pthread_mutex_unlock(&soft_slot.keystore_mutex);
+		(void) pthread_mutex_unlock(&soft_slot.slot_mutex);
+		(void) pthread_mutex_unlock(&soft_sessionlist_mutex);
+	}
 	(void) pthread_mutex_unlock(&soft_giant_mutex);
 }
--- a/usr/src/lib/pkcs11/pkcs11_softtoken/common/softKeystore.c	Fri Aug 28 13:38:17 2009 +0800
+++ b/usr/src/lib/pkcs11/pkcs11_softtoken/common/softKeystore.c	Fri Aug 28 10:12:00 2009 +0200
@@ -2172,6 +2172,8 @@
 	token_session.object_list = NULL;
 	token_session.ses_refcnt = 0;
 	token_session.ses_close_sync = 0;
+	token_session.next = NULL;
+	token_session.prev = NULL;
 
 	/* Initialize the lock for the token session */
 	if (pthread_mutex_init(&token_session.session_mutex, NULL) != 0) {
--- a/usr/src/lib/pkcs11/pkcs11_softtoken/common/softSession.h	Fri Aug 28 13:38:17 2009 +0800
+++ b/usr/src/lib/pkcs11/pkcs11_softtoken/common/softSession.h	Fri Aug 28 10:12:00 2009 +0200
@@ -199,8 +199,8 @@
 
 void soft_logout(void);
 
-void soft_acquire_all_session_mutexes();
-void soft_release_all_session_mutexes();
+void soft_acquire_all_session_mutexes(soft_session_t *session_p);
+void soft_release_all_session_mutexes(soft_session_t *session_p);
 
 #ifdef	__cplusplus
 }
--- a/usr/src/lib/pkcs11/pkcs11_softtoken/common/softSessionUtil.c	Fri Aug 28 13:38:17 2009 +0800
+++ b/usr/src/lib/pkcs11/pkcs11_softtoken/common/softSessionUtil.c	Fri Aug 28 10:12:00 2009 +0200
@@ -720,10 +720,8 @@
 }
 
 void
-soft_acquire_all_session_mutexes()
+soft_acquire_all_session_mutexes(soft_session_t *session_p)
 {
-	soft_session_t *session_p = soft_session_list;
-
 	/* Iterate through sessions acquiring all mutexes */
 	while (session_p) {
 		soft_object_t *object_p;
@@ -741,10 +739,8 @@
 }
 
 void
-soft_release_all_session_mutexes()
+soft_release_all_session_mutexes(soft_session_t *session_p)
 {
-	soft_session_t *session_p = soft_session_list;
-
 	/* Iterate through sessions releasing all mutexes */
 	while (session_p) {
 		/*