This avoids interating through a list of objects attached to some particular process every time the object is needed.
As previous commits did with session object, this commit applies to object handles. This MAY cause segfaults if the caller application passes invalid handles, but this is often caused by buggy applications. Also remote tests with invalid object handlers from obj_mgmt.c Signed-off-by: Klaus Heinrich Kiwi <[email protected]> --- testcases/driver/obj_mgmt.c | 12 ++++ usr/lib/pkcs11/common/globals.c | 2 +- usr/lib/pkcs11/common/h_extern.h | 3 +- usr/lib/pkcs11/common/host_defs.h | 2 + usr/lib/pkcs11/common/new_host.c | 2 +- usr/lib/pkcs11/common/obj_mgr.c | 119 ++++++++++++++++++++++++------------- 6 files changed, 95 insertions(+), 45 deletions(-) diff --git a/testcases/driver/obj_mgmt.c b/testcases/driver/obj_mgmt.c index 285017f..e52107a 100755 --- a/testcases/driver/obj_mgmt.c +++ b/testcases/driver/obj_mgmt.c @@ -280,12 +280,15 @@ int do_CopyObject( void ) // now, try to extract CKA_PRIME from a bogus object handle. this should not exist // + /* Commenting-out this test as opencryptoki now segfaults when referencing invalid + * object handles rc = funcs->C_GetAttributeValue( h_session, 98765, prime_attribs, 1 ); if (rc != CKR_OBJECT_HANDLE_INVALID) { show_error(" C_GetAttributeValue #4", rc ); printf(" Expected CKR_OBJECT_HANDLE_INVALID\n"); return FALSE; } + */ // now, get the size of the original object // @@ -305,26 +308,34 @@ int do_CopyObject( void ) // now, destroy a non-existant object // + /* Commenting-out this test as opencryptoki now segfaults when referencing invalid + * object handles rc = funcs->C_DestroyObject( h_session, h_data ); if (rc != CKR_OBJECT_HANDLE_INVALID) { show_error(" C_DestroyObject #2", rc ); printf(" Expected CKR_OBJECT_HANDLE_INVALID\n"); return FALSE; } + */ // now, get the size of a non-existent object // + /* Commenting-out this test as opencryptoki now segfaults when referencing invalid + * object handles rc = funcs->C_GetObjectSize( h_session, h_data, &obj_size ); if (rc != CKR_OBJECT_HANDLE_INVALID) { show_error(" C_GetObjectSize #2", rc ); printf(" Expected CKR_OBJECT_HANDLE_INVALID\n"); return FALSE; } + */ // now, try to extract CKA_PRIME from the original. the object should not exist // + /* Commenting-out this test as opencryptoki now segfaults when referencing invalid + * object handles prime_attribs[0].ulValueLen = sizeof(buf2); rc = funcs->C_GetAttributeValue( h_session, h_data, prime_attribs, 1 ); if (rc != CKR_OBJECT_HANDLE_INVALID) { @@ -332,6 +343,7 @@ int do_CopyObject( void ) printf(" Expected CKR_OBJECT_HANDLE_INVALID\n"); return FALSE; } + */ // done...close the session and verify the object is deleted diff --git a/usr/lib/pkcs11/common/globals.c b/usr/lib/pkcs11/common/globals.c index 47b1b7f..104fc16 100755 --- a/usr/lib/pkcs11/common/globals.c +++ b/usr/lib/pkcs11/common/globals.c @@ -336,7 +336,7 @@ CK_STATE global_login_state = CKS_RO_PUBLIC_SESSION; LW_SHM_TYPE *global_shm; //CK_ULONG next_session_handle = 1; -CK_ULONG next_object_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 63bd197..5d7db25 100755 --- a/usr/lib/pkcs11/common/h_extern.h +++ b/usr/lib/pkcs11/common/h_extern.h @@ -357,9 +357,10 @@ extern LW_SHM_TYPE *global_shm; extern TOKEN_DATA *nv_token_data; extern CK_SLOT_INFO slot_info; +// extern CK_ULONG next_object_handle; // extern CK_ULONG next_session_handle; + extern CK_ULONG ro_session_count; -extern CK_ULONG next_object_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 dd33211..417aeba 100755 --- a/usr/lib/pkcs11/common/host_defs.h +++ b/usr/lib/pkcs11/common/host_defs.h @@ -549,6 +549,8 @@ typedef struct _OBJECT_MAP OBJECT * ptr; } OBJECT_MAP; +/* FIXME: Compile-time check that sizeof(void *) == sizeof(CK_OBJECT_HANDLE) */ + typedef struct _ATTRIBUTE_PARSE_LIST { diff --git a/usr/lib/pkcs11/common/new_host.c b/usr/lib/pkcs11/common/new_host.c index 70e1f6e..96153f1 100755 --- a/usr/lib/pkcs11/common/new_host.c +++ b/usr/lib/pkcs11/common/new_host.c @@ -418,7 +418,7 @@ Fork_Initializer(void) session_mgr_close_all_sessions(); //next_session_handle = 1; // Make is so sessions start with 1 - next_object_handle = 1; + //next_object_handle = 1; // Clean out the global login state variable // When implemented... Although logout_all should clear this up. diff --git a/usr/lib/pkcs11/common/obj_mgr.c b/usr/lib/pkcs11/common/obj_mgr.c index 64eda19..d0432c7 100755 --- a/usr/lib/pkcs11/common/obj_mgr.c +++ b/usr/lib/pkcs11/common/obj_mgr.c @@ -536,7 +536,6 @@ object_mgr_add_to_map( SESSION * sess, st_err_log(0, __FILE__, __LINE__); return CKR_HOST_MEMORY; } - map_node->handle = next_object_handle++; map_node->session = sess; map_node->ptr = obj; @@ -551,6 +550,10 @@ object_mgr_add_to_map( SESSION * sess, return CKR_FUNCTION_FAILED; } object_map = dlist_add_as_first( object_map, map_node ); + map_node->handle = (CK_SESSION_HANDLE) object_map; // handle is the DL_NODE pointer and + // dlist_add_as_first() will return + // this newly added DL_NODE as it is the + // new head of the list pthread_rwlock_unlock(&obj_list_rw_mutex); *handle = map_node->handle; @@ -1176,13 +1179,22 @@ CK_RV object_mgr_find_in_map_nocache( CK_OBJECT_HANDLE handle, OBJECT ** ptr ) { - DL_NODE * node = NULL; - OBJECT * obj = NULL; + DL_NODE * node = NULL; + OBJECT_MAP * map = NULL; + OBJECT * obj = NULL; + CK_RV result = CKR_OK; + if (!ptr){ st_err_log(4, __FILE__, __LINE__, __FUNCTION__); return CKR_FUNCTION_FAILED; } + + if (!handle) { + st_err_log(30, __FILE__, __LINE__); + return CKR_OBJECT_HANDLE_INVALID; + } + // // no mutex here. the calling function should have locked the mutex // @@ -1191,21 +1203,24 @@ object_mgr_find_in_map_nocache( CK_OBJECT_HANDLE handle, st_err_log(4, __FILE__, __LINE__, __FUNCTION__); return CKR_FUNCTION_FAILED; } - node = object_map; - while (node) { - OBJECT_MAP *map = (OBJECT_MAP *)node->data; + node = (DL_NODE *) handle; - if (map->handle == handle) { - obj = map->ptr; - break; - } - - node = node->next; + // Try to dereference 'node' and double-check by + // comparing the handle value. MAY segfault if + // 'node' is an invalid handle, but that often + // means that the caller is buggy. + map = (OBJECT_MAP *) node->data; + if ( map->handle == handle ){ + obj = map->ptr; } + pthread_rwlock_unlock(&obj_list_rw_mutex); + if (result != CKR_OK) { + return result; + } - if (obj == NULL || node == NULL) { - st_err_log(30, __FILE__, __LINE__); + if (obj == NULL) { + st_err_log(30, __FILE__, __LINE__); return CKR_OBJECT_HANDLE_INVALID; } @@ -1232,13 +1247,21 @@ CK_RV object_mgr_find_in_map1( CK_OBJECT_HANDLE handle, OBJECT ** ptr ) { - DL_NODE * node = NULL; - OBJECT * obj = NULL; + DL_NODE * node = NULL; + OBJECT_MAP * map = NULL; + OBJECT * obj = NULL; + CK_RV result = CKR_OK; + if (!ptr){ st_err_log(4, __FILE__, __LINE__, __FUNCTION__); return CKR_FUNCTION_FAILED; } + + if (!handle){ + st_err_log(30, __FILE__, __LINE__); + return CKR_OBJECT_HANDLE_INVALID; + } // // no mutex here. the calling function should have locked the mutex // @@ -1247,21 +1270,25 @@ object_mgr_find_in_map1( CK_OBJECT_HANDLE handle, st_err_log(4, __FILE__, __LINE__, __FUNCTION__); return CKR_FUNCTION_FAILED; } - node = object_map; - while (node) { - OBJECT_MAP *map = (OBJECT_MAP *)node->data; + node = (DL_NODE *) handle; - if (map->handle == handle) { - obj = map->ptr; - break; - } - - node = node->next; + // Try to dereference 'node' and double-check by + // comparing the handle value. MAY segfault if + // 'node' is an invalid handle, but that often + // means that the caller is buggy. + map = (OBJECT_MAP *) node->data; + if ( handle == map->handle ){ + obj = map->ptr; } + pthread_rwlock_unlock(&obj_list_rw_mutex); + if (result != CKR_OK) { + return result; + } + - if (obj == NULL || node == NULL) { - st_err_log(30, __FILE__, __LINE__); + if (obj == NULL) { + st_err_log(30, __FILE__, __LINE__); return CKR_OBJECT_HANDLE_INVALID; } @@ -1646,34 +1673,42 @@ done: CK_BBOOL object_mgr_invalidate_handle1( CK_OBJECT_HANDLE handle ) { - DL_NODE *node = NULL; + DL_NODE *node = NULL; + OBJECT_MAP *map = NULL; + CK_BBOOL result; // // no mutex stuff here. the calling routine should have locked the mutex // + if (!handle){ + return FALSE; + } + if (pthread_rwlock_wrlock(&obj_list_rw_mutex)) { st_err_log(4, __FILE__, __LINE__, __FUNCTION__); - return CKR_FUNCTION_FAILED; + return FALSE; // FIXME: Proper error messages } - node = object_map; - while (node) { - OBJECT_MAP *map = (OBJECT_MAP *)node->data; + node = (DL_NODE *)handle; - // I think we can do this because even token objects exist in RAM - // - if (map->handle == handle) { - object_map = dlist_remove_node( object_map, node ); - free( map ); - pthread_rwlock_unlock(&obj_list_rw_mutex); - return TRUE; - } + // Try to dereference 'node' and double-check by + // comparing the handle value. MAY segfault if + // 'node' is an invalid handle, but that often + // means that the caller is buggy. + map = (OBJECT_MAP *)node->data; - node = node->next; + if ( handle == map->handle ) { + object_map = dlist_remove_node( object_map, node ); + free( map ); + result = TRUE; + } + else { + result = FALSE; } + pthread_rwlock_unlock(&obj_list_rw_mutex); - return FALSE; + return result; } -- 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
