Mercurial > illumos > illumos-gate
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) { /*