1667 pkcs11 may deadlock when multi-threaded consumers fork

Reviewed by: Garrett D'Amore <garrett@damore.org>
Reviewed by: Saso Kiselkov <skiselkov.ml@gmail.com>
Approved by: Gordon Ross <gordon.ross@nexenta.com>

5 files changed, 65 insertions, 15 deletions

diff --git a/usr/src/lib/libcryptoutil/common/cryptoutil.h b/usr/src/lib/libcryptoutil/common/cryptoutil.h
index b5aad833f7..63a3df665f 100644
--- a/usr/src/lib/libcryptoutil/common/cryptoutil.h
+++ b/usr/src/lib/libcryptoutil/common/cryptoutil.h
@@ -22,6 +22,7 @@
  */
 /*
  * Copyright 2010 Nexenta Systems, Inc.  All rights reserved.
+ * Copyright 2014, OmniTI Computer Consulting, Inc. All rights reserved.
  */
 
 #ifndef _CRYPTOUTIL_H
@@ -216,9 +217,6 @@ extern int pkcs11_seed_urandom(void *sbuf, size_t slen);
 extern int pkcs11_get_random(void *dbuf, size_t dlen);
 extern int pkcs11_get_urandom(void *dbuf, size_t dlen);
 extern int pkcs11_get_nzero_urandom(void *dbuf, size_t dlen);
-extern void pkcs11_close_random(void);
-extern void pkcs11_close_urandom(void);
-extern void pkcs11_close_urandom_seed(void);
 extern int pkcs11_read_data(char *filename, void **dbuf, size_t *dlen);
 
 extern int open_nointr(const char *path, int oflag, ...);
diff --git a/usr/src/lib/libcryptoutil/common/mapfile-vers b/usr/src/lib/libcryptoutil/common/mapfile-vers
index 5d3c214b55..c7f2576f37 100644
--- a/usr/src/lib/libcryptoutil/common/mapfile-vers
+++ b/usr/src/lib/libcryptoutil/common/mapfile-vers
@@ -19,6 +19,7 @@
 # CDDL HEADER END
 #
 # Copyright (c) 2006, 2010, Oracle and/or its affiliates. All rights reserved.
+# Copyright 2014, OmniTI Computer Consulting Inc. All rights reserved.
 #
 
 #
@@ -54,9 +55,6 @@ SYMBOL_VERSION SUNWprivate {
 	get_pkcs11conf_info;
 	hexstr_to_bytes;
 	open_nointr;
-	pkcs11_close_random;
-	pkcs11_close_urandom;
-	pkcs11_close_urandom_seed;
 	pkcs11_default_token;
 	pkcs11_free_uri;
 	pkcs11_get_nzero_urandom;
diff --git a/usr/src/lib/libcryptoutil/common/random.c b/usr/src/lib/libcryptoutil/common/random.c
index 771112850a..ab07168409 100644
--- a/usr/src/lib/libcryptoutil/common/random.c
+++ b/usr/src/lib/libcryptoutil/common/random.c
@@ -21,6 +21,7 @@
 
 /*
  * Copyright (c) 2003, 2010, Oracle and/or its affiliates. All rights reserved.
+ * Copyright 2014, OmniTI Computer Consulting, Inc. All rights reserved.
  */
 
 #include <stdio.h>
@@ -33,6 +34,7 @@
 #include <cryptoutil.h>
 #include <pthread.h>
 
+#pragma init(pkcs11_random_init)
 
 static pthread_mutex_t	random_mutex = PTHREAD_MUTEX_INITIALIZER;
 static pthread_mutex_t	urandom_mutex = PTHREAD_MUTEX_INITIALIZER;
@@ -181,13 +183,13 @@ pkcs11_close_common(int *fd, pthread_mutex_t *mtx)
 	(void) pthread_mutex_unlock(mtx);
 }
 
-void
+static void
 pkcs11_close_random(void)
 {
 	pkcs11_close_common(&random_fd, &random_mutex);
 }
 
-void
+static void
 pkcs11_close_urandom(void)
 {
 	pkcs11_close_common(&urandom_fd, &urandom_mutex);
@@ -199,7 +201,7 @@ pkcs11_close_random_seed(void)
 	pkcs11_close_common(&random_seed_fd, &random_seed_mutex);
 }
 
-void
+static void
 pkcs11_close_urandom_seed(void)
 {
 	pkcs11_close_common(&urandom_seed_fd, &urandom_seed_mutex);
@@ -377,3 +379,45 @@ pkcs11_get_nzero_urandom(void *dbuf, size_t dlen)
 	}
 	return (0);
 }
+
+static void
+pkcs11_random_prepare(void)
+{
+	/*
+	 * NOTE - None of these are acquired more than one at a time.
+	 * I can therefore acquire all four without fear of deadlock.
+	 */
+	(void) pthread_mutex_lock(&random_mutex);
+	(void) pthread_mutex_lock(&urandom_mutex);
+	(void) pthread_mutex_lock(&random_seed_mutex);
+	(void) pthread_mutex_lock(&urandom_seed_mutex);
+}
+
+static void
+pkcs11_random_parent_post(void)
+{
+	/* Drop the mutexes and get back to work! */
+	(void) pthread_mutex_unlock(&urandom_seed_mutex);
+	(void) pthread_mutex_unlock(&random_seed_mutex);
+	(void) pthread_mutex_unlock(&urandom_mutex);
+	(void) pthread_mutex_unlock(&random_mutex);
+}
+
+static void
+pkcs11_random_child_post(void)
+{
+	pkcs11_random_parent_post();
+
+	/* Also, close the FDs, just in case. */
+	pkcs11_close_random();
+	pkcs11_close_urandom();
+	pkcs11_close_random_seed();
+	pkcs11_close_urandom_seed();
+}
+
+static void
+pkcs11_random_init(void)
+{
+	(void) pthread_atfork(pkcs11_random_prepare, pkcs11_random_parent_post,
+	    pkcs11_random_child_post);
+}
diff --git a/usr/src/lib/pkcs11/libpkcs11/common/metaGeneral.c b/usr/src/lib/pkcs11/libpkcs11/common/metaGeneral.c
index 5e5c339b03..32b00216da 100644
--- a/usr/src/lib/pkcs11/libpkcs11/common/metaGeneral.c
+++ b/usr/src/lib/pkcs11/libpkcs11/common/metaGeneral.c
@@ -21,8 +21,9 @@
 /*
  * Copyright 2009 Sun Microsystems, Inc.  All rights reserved.
  * Use is subject to license terms.
+ *
+ * Copyright 2014, OmniTI Computer Consulting, Inc. All rights reserved.
  */
-
 /*
  * General-Purpose Functions
  * (as defined in PKCS#11 spec section 11.4)
@@ -196,8 +197,12 @@ meta_Finalize(CK_VOID_PTR pReserved)
 
 	(void) pthread_mutex_lock(&initmutex);
 
-	pkcs11_close_urandom();
-	pkcs11_close_urandom_seed();
+	/*
+	 * There used to be calls to cleanup libcryptoutil here.  Given that
+	 * libcryptoutil can be linked and invoked independently of PKCS#11,
+	 * cleaning up libcryptoutil here makes no sense.  Decoupling these
+	 * two also prevent deadlocks and other artificial dependencies.
+	 */
 
 	meta_objectManager_finalize();
 
diff --git a/usr/src/lib/pkcs11/pkcs11_softtoken/common/softGeneral.c b/usr/src/lib/pkcs11/pkcs11_softtoken/common/softGeneral.c
index 396a3c5bf4..c44cbcb2a2 100644
--- a/usr/src/lib/pkcs11/pkcs11_softtoken/common/softGeneral.c
+++ b/usr/src/lib/pkcs11/pkcs11_softtoken/common/softGeneral.c
@@ -21,6 +21,8 @@
 /*
  * Copyright 2009 Sun Microsystems, Inc.  All rights reserved.
  * Use is subject to license terms.
+ *
+ * Copyright 2014, OmniTI Computer Consulting, Inc. All rights reserved.
  */
 
 #include <strings.h>
@@ -354,9 +356,12 @@ finalize_common(boolean_t force, CK_VOID_PTR pReserved) {
 	softtoken_initialized = B_FALSE;
 	softtoken_pid = 0;
 
-	pkcs11_close_urandom();
-	pkcs11_close_urandom_seed();
-	pkcs11_close_random();
+	/*
+	 * There used to be calls to cleanup libcryptoutil here.  Given that
+	 * libcryptoutil can be linked and invoked independently of PKCS#11,
+	 * cleaning up libcryptoutil here makes no sense.  Decoupling these
+	 * two also prevent deadlocks and other artificial dependencies.
+	 */
 
 	/* Destroy the session list lock here */
 	(void) pthread_mutex_destroy(&soft_sessionlist_mutex);