Bug 1767921 - check SFTKSlot head after acquiring session lock. r=rrelyea

Differential Revision: https://phabricator.services.mozilla.com/D147992

2 files changed, 32 insertions, 12 deletions

diff --git a/lib/softoken/pkcs11.c b/lib/softoken/pkcs11.c
index 28d219d8e..aceeaed48 100644
--- a/lib/softoken/pkcs11.c
+++ b/lib/softoken/pkcs11.c
@@ -2933,7 +2933,7 @@ sftk_CloseAllSessions(SFTKSlot *slot, PRBool logout)
         PZLock *lock = SFTK_SESSION_LOCK(slot, i);
         do {
             SKIP_AFTER_FORK(PZ_Lock(lock));
-            session = slot->head[i];
+            session = slot->head ? slot->head[i] : NULL;
             /* hand deque */
             /* this duplicates function of NSC_close session functions, but
              * because we know that we are freeing all the sessions, we can
@@ -3042,10 +3042,25 @@ SFTK_DestroySlotData(SFTKSlot *slot)
     }
     slot->sessObjHashSize = 0;
 
+    /* slot->head[i] is protected by slot->sessionLock[i], so we need to hold
+     * all of the session locks before we free slot->head[]. Note that as a
+     * consequence of this locking structure, one must always check that
+     * slot->head != NULL after acquiring a session lock.
+     */
+    for (i = 0; i < slot->numSessionLocks; i++) {
+        if (slot->sessionLock && slot->sessionLock[i]) {
+            PZ_Lock(slot->sessionLock[i]);
+        }
+    }
     if (slot->head) {
         PORT_Free(slot->head);
         slot->head = NULL;
     }
+    for (i = 0; i < slot->numSessionLocks; i++) {
+        if (slot->sessionLock && slot->sessionLock[i]) {
+            PZ_Unlock(slot->sessionLock[i]);
+        }
+    }
     slot->sessHashSize = 0;
 
     /* OK everything has been disassembled, now we can finally get rid
@@ -4203,13 +4218,16 @@ NSC_OpenSession(CK_SLOT_ID slotID, CK_FLAGS flags,
         } while (sessionID == CK_INVALID_HANDLE);
         lock = SFTK_SESSION_LOCK(slot, sessionID);
         PZ_Lock(lock);
-        sftkqueue_find(sameID, sessionID, slot->head, slot->sessHashSize);
-        if (sameID == NULL) {
-            session->handle = sessionID;
-            sftk_update_state(slot, session);
-            sftkqueue_add(session, sessionID, slot->head, slot->sessHashSize);
-        } else {
-            slot->sessionIDConflict++; /* for debugging */
+        sameID = NULL;
+        if (slot->head) {
+            sftkqueue_find(sameID, sessionID, slot->head, slot->sessHashSize);
+            if (sameID == NULL) {
+                session->handle = sessionID;
+                sftk_update_state(slot, session);
+                sftkqueue_add(session, sessionID, slot->head, slot->sessHashSize);
+            } else {
+                slot->sessionIDConflict++; /* for debugging */
+            }
         }
         PZ_Unlock(lock);
     } while (sameID != NULL);
@@ -4238,7 +4256,7 @@ NSC_CloseSession(CK_SESSION_HANDLE hSession)
     /* lock */
     lock = SFTK_SESSION_LOCK(slot, hSession);
     PZ_Lock(lock);
-    if (sftkqueue_is_queued(session, hSession, slot->head, slot->sessHashSize)) {
+    if (slot->head && sftkqueue_is_queued(session, hSession, slot->head, slot->sessHashSize)) {
         sessionFound = PR_TRUE;
         sftkqueue_delete(session, hSession, slot->head, slot->sessHashSize);
     }
diff --git a/lib/softoken/pkcs11u.c b/lib/softoken/pkcs11u.c
index 5299a700a..6c8fc723a 100644
--- a/lib/softoken/pkcs11u.c
+++ b/lib/softoken/pkcs11u.c
@@ -1852,7 +1852,7 @@ sftk_update_all_states(SFTKSlot *slot)
     for (i = 0; i < slot->sessHashSize; i++) {
         PZLock *lock = SFTK_SESSION_LOCK(slot, i);
         PZ_Lock(lock);
-        for (session = slot->head[i]; session; session = session->next) {
+        for (session = (slot->head ? slot->head[i] : NULL); session; session = session->next) {
             sftk_update_state(slot, session);
         }
         PZ_Unlock(lock);
@@ -1982,7 +1982,7 @@ SFTKSession *
 sftk_SessionFromHandle(CK_SESSION_HANDLE handle)
 {
     SFTKSlot *slot = sftk_SlotFromSessionHandle(handle);
-    SFTKSession *session;
+    SFTKSession *session = NULL;
     PZLock *lock;
 
     if (!slot)
@@ -1990,7 +1990,9 @@ sftk_SessionFromHandle(CK_SESSION_HANDLE handle)
     lock = SFTK_SESSION_LOCK(slot, handle);
 
     PZ_Lock(lock);
-    sftkqueue_find(session, handle, slot->head, slot->sessHashSize);
+    if (slot->head) {
+        sftkqueue_find(session, handle, slot->head, slot->sessHashSize);
+    }
     PZ_Unlock(lock);
 
     return (session);