From 05d57413471eaaa425913a06edc2ab33ad9b05bc Mon Sep 17 00:00:00 2001 From: Dan McDonald Date: Thu, 5 Jun 2014 00:43:20 -0400 Subject: [PATCH] 1667 pkcs11 may deadlock when multi-threaded consumers fork Reviewed by: Garrett D'Amore Reviewed by: Saso Kiselkov Approved by: Gordon Ross --- usr/src/lib/libcryptoutil/common/cryptoutil.h | 4 +- usr/src/lib/libcryptoutil/common/mapfile-vers | 4 +- usr/src/lib/libcryptoutil/common/random.c | 50 ++++++++++++++++++++-- usr/src/lib/pkcs11/libpkcs11/common/metaGeneral.c | 11 +++-- .../pkcs11/pkcs11_softtoken/common/softGeneral.c | 11 +++-- 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 @@ -33,6 +34,7 @@ #include #include +#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 @@ -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); -- 2.11.4.GIT