Avoid iterating through the common library layer's session list by assigning the session's own pointer as the handler.
This is similar to commit "Use pointer as session handler in API layer", although unlikely to cause segfaults due to tracking being done in the API layer. Signed-off-by: Klaus Heinrich Kiwi <[email protected]> --- usr/lib/pkcs11/common/globals.c | 2 +- usr/lib/pkcs11/common/h_extern.h | 2 +- usr/lib/pkcs11/common/host_defs.h | 4 ++++ usr/lib/pkcs11/common/new_host.c | 4 ++-- usr/lib/pkcs11/common/sess_mgr.c | 35 ++++++++++++++++++----------------- 5 files changed, 26 insertions(+), 21 deletions(-) diff --git a/usr/lib/pkcs11/common/globals.c b/usr/lib/pkcs11/common/globals.c index 7b1432f..47b1b7f 100755 --- a/usr/lib/pkcs11/common/globals.c +++ b/usr/lib/pkcs11/common/globals.c @@ -335,7 +335,7 @@ CK_STATE global_login_state = CKS_RO_PUBLIC_SESSION; LW_SHM_TYPE *global_shm; -CK_ULONG next_session_handle = 1; +//CK_ULONG next_session_handle = 1; CK_ULONG next_object_handle = 1; TOKEN_DATA *nv_token_data = NULL; diff --git a/usr/lib/pkcs11/common/h_extern.h b/usr/lib/pkcs11/common/h_extern.h index 4368902..63bd197 100755 --- a/usr/lib/pkcs11/common/h_extern.h +++ b/usr/lib/pkcs11/common/h_extern.h @@ -357,9 +357,9 @@ extern LW_SHM_TYPE *global_shm; extern TOKEN_DATA *nv_token_data; extern CK_SLOT_INFO slot_info; +// extern CK_ULONG next_session_handle; extern CK_ULONG ro_session_count; extern CK_ULONG next_object_handle; -extern CK_ULONG next_session_handle; extern CK_STATE global_login_state; diff --git a/usr/lib/pkcs11/common/host_defs.h b/usr/lib/pkcs11/common/host_defs.h index 3dddcda..dd33211 100755 --- a/usr/lib/pkcs11/common/host_defs.h +++ b/usr/lib/pkcs11/common/host_defs.h @@ -462,6 +462,10 @@ typedef struct _SESSION SIGN_VERIFY_CONTEXT verify_ctx; } SESSION; +/* TODO: + * Add compile-time checking that sizeof(void *) == sizeof(CK_SESSION_HANDLE) + * */ + typedef struct _DES_CONTEXT { diff --git a/usr/lib/pkcs11/common/new_host.c b/usr/lib/pkcs11/common/new_host.c index 394203f..70e1f6e 100755 --- a/usr/lib/pkcs11/common/new_host.c +++ b/usr/lib/pkcs11/common/new_host.c @@ -417,7 +417,7 @@ Fork_Initializer(void) // This should clear the entire session list out session_mgr_close_all_sessions(); - next_session_handle = 1; // Make is so sessions start with 1 + //next_session_handle = 1; // Make is so sessions start with 1 next_object_handle = 1; // Clean out the global login state variable @@ -1382,7 +1382,7 @@ CK_RV SC_OpenSession(CK_SLOT_ID sid, st_err_log(152, __FILE__, __LINE__); goto done; } - *phSession = sess->handle; + *phSession = (CK_SESSION_HANDLE_PTR) sess; // Set the correct slot ID here. Was hard coded to 1. - KEY sess->session_info.slotID = sid; done: diff --git a/usr/lib/pkcs11/common/sess_mgr.c b/usr/lib/pkcs11/common/sess_mgr.c index deedd39..3b8ceba 100755 --- a/usr/lib/pkcs11/common/sess_mgr.c +++ b/usr/lib/pkcs11/common/sess_mgr.c @@ -308,7 +308,7 @@ // session_mgr_find() // // search for the specified session. returning a pointer to the session -// is dangerous +// might be dangerous, but performs well // // Returns: SESSION * or NULL // @@ -319,22 +319,24 @@ session_mgr_find( CK_SESSION_HANDLE handle ) SESSION * result = NULL; CK_RV rc; + if (!handle) { + return NULL; + } + rc = MY_LockMutex( &sess_list_mutex ); if (rc != CKR_OK){ st_err_log(146, __FILE__, __LINE__); return NULL; } - node = sess_list; - while (node) { - SESSION *s = (SESSION *)node->data; + result = (SESSION *) handle; - if (s->handle == handle) { - result = s; - break; - } - - node = node->next; + // Try to dereference result and double-check by + // comparing the handle value. MAY segfault if + // handle is invalid, but that often means that + // the caller is buggy. + if ( result->handle != handle ) { + result = NULL; } MY_UnlockMutex( &sess_list_mutex ); @@ -385,13 +387,7 @@ session_mgr_new( CK_ULONG flags, SESSION **sess ) } pkcs_locked = TRUE; - do { - s = session_mgr_find( next_session_handle ); - if (s != NULL) - next_session_handle++; - else - new_session->handle = next_session_handle++; - } while (s != NULL); + new_session->handle = (CK_SESSION_HANDLE) new_session; MY_UnlockMutex( &pkcs_mutex ); pkcs_locked = FALSE; @@ -596,6 +592,9 @@ session_mgr_close_session( SESSION *sess ) ro_session_count--; } + // Make sure this address is now invalid + sess->handle = CK_INVALID_HANDLE; + if (sess->find_list) free( sess->find_list ); @@ -680,6 +679,8 @@ session_mgr_close_all_sessions( void ) object_mgr_purge_session_objects( sess, ALL ); + sess->handle = CK_INVALID_HANDLE; + if (sess->find_list) free( sess->find_list ); -- 1.6.6.1 ------------------------------------------------------------------------------ ThinkGeek and WIRED's GeekDad team up for the Ultimate GeekDad Father's Day Giveaway. ONE MASSIVE PRIZE to the lucky parental unit. See the prize list and enter to win: http://p.sf.net/sfu/thinkgeek-promo _______________________________________________ Opencryptoki-tech mailing list [email protected] https://lists.sourceforge.net/lists/listinfo/opencryptoki-tech
