diff --git a/src/internal.c b/src/internal.c index 6fa7d2a4..f3306b5e 100644 --- a/src/internal.c +++ b/src/internal.c @@ -12065,22 +12065,27 @@ int WP11_Object_Find(WP11_Session* session, CK_OBJECT_HANDLE objHandle, WP11_Object* obj; #ifdef WOLFPKCS11_NSS WP11_Session* scan; +#else + int isPrivate = 0; #endif int onToken = OBJ_HANDLE_ON_TOKEN(objHandle); - if (!onToken) { #ifdef WOLFPKCS11_NSS - /* The NSS cross-session walk below needs the slot lock so a concurrent - * remove-session can't reclaim a session node. Acquire it before the - * token lock to match the global lock order (slot then token) used by - * WP11_Session_RemoveObjectByHandle / WP11_Slot_CloseSessions; acquiring - * them in the opposite order would risk an AB-BA deadlock. */ + /* The NSS cross-session walk below needs the slot lock so a concurrent + * remove-session can't reclaim a session node. Acquire it before the + * token lock to match the global lock order (slot then token) used by + * WP11_Session_RemoveObjectByHandle / WP11_Slot_CloseSessions; acquiring + * them in the opposite order would risk an AB-BA deadlock. */ + if (!onToken) WP11_Lock_LockRO(&session->slot->lock); #endif - /* Hold the token lock across the session->object walk so a concurrent - * WP11_Session_RemoveObjectByHandle (which holds the same lock around - * the unlink) can't free a node we're stepping through. */ - WP11_Lock_LockRO(&session->slot->token.lock); + /* The token lock guards every object list on the slot, and + * WP11_Session_RemoveObjectByHandle holds it around the unlink. Hold it + * across the walk and every read of the matched object: once it is + * released a concurrent C_DestroyObject on the same handle (token objects + * are shared by all sessions) can unlink and free the object. */ + WP11_Lock_LockRO(&session->slot->token.lock); + if (!onToken) { obj = session->object; while (obj != NULL) { if (obj->handle == objHandle) { @@ -12103,14 +12108,9 @@ int WP11_Object_Find(WP11_Session* session, CK_OBJECT_HANDLE objHandle, } } } -#endif - WP11_Lock_UnlockRO(&session->slot->token.lock); -#ifdef WOLFPKCS11_NSS - WP11_Lock_UnlockRO(&session->slot->lock); #endif } else { - WP11_Lock_LockRO(&session->slot->token.lock); obj = session->slot->token.object; while (obj != NULL) { if (obj->handle == objHandle) { @@ -12119,15 +12119,26 @@ int WP11_Object_Find(WP11_Session* session, CK_OBJECT_HANDLE objHandle, } obj = obj->next; } - WP11_Lock_UnlockRO(&session->slot->token.lock); } +#ifndef WOLFPKCS11_NSS + if (ret == 0) + isPrivate = ((obj->opFlag & WP11_FLAG_PRIVATE) == WP11_FLAG_PRIVATE); +#endif + WP11_Lock_UnlockRO(&session->slot->token.lock); +#ifdef WOLFPKCS11_NSS + if (!onToken) + WP11_Lock_UnlockRO(&session->slot->lock); +#endif + /* obj may already be freed by a concurrent destroy: do not dereference it + * from here on. */ - if (ret == 0 && obj != NULL && (obj->handle == objHandle)) { + if (ret == 0) { #ifndef WOLFPKCS11_NSS /* Enforce CKA_PRIVATE: reject private objects from public sessions. * Skipped in NSS mode because NSS operates as the internal crypto - * module without calling C_Login. */ - if ((obj->opFlag & WP11_FLAG_PRIVATE) == WP11_FLAG_PRIVATE) { + * module without calling C_Login. The slot lock is only taken after + * the token lock is released, keeping the slot-then-token order. */ + if (isPrivate) { int loginState; WP11_Lock_LockRO(&session->slot->lock); loginState = session->slot->token.loginState;