# HG changeset patch
# User Kevin Jacobs <kjacobs@mozilla.com>
# Date 1606813429 0
# Node ID 19585ccc7a1f0f4e9a8d2b9c5ceeb408ea90acb9
# Parent  f1e48fbead3d9e69500d7aedc1ef6e4bf334f41e
Bug 1679290 - Don't hold slot lock when taking session lock r=bbeurdouche

[[ https://hg.mozilla.org/projects/nss/rev/0ed11a5835ac1556ff978362cd61069d48f4c5db | 0ed11a5835ac1556ff978362cd61069d48f4c5db ]] fixed a number of race conditions related to NSSSlot member accesses. Unfortunately the locking order that was imposed by that patch has been found to cause problems for at least one PKCS11 module, libnsspem.

This patch drops nested locking in favor of unlocking/re-locking. While this isn't perfect, the original problem in bug 1663661 was that `slot->token` could become NULL, which we can easily check after reacquiring.

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

diff --git a/lib/dev/devslot.c b/lib/dev/devslot.c
--- a/lib/dev/devslot.c
+++ b/lib/dev/devslot.c
@@ -183,25 +183,32 @@ nssSlot_IsTokenPresent(
     if ((slot->ckFlags & CKF_TOKEN_PRESENT) == 0) {
         if (!slot->token) {
             /* token was never present */
             isPresent = PR_FALSE;
             goto done; /* slot lock held */
         }
         session = nssToken_GetDefaultSession(slot->token);
         if (session) {
+            nssSlot_ExitMonitor(slot);
             nssSession_EnterMonitor(session);
             /* token is not present */
             if (session->handle != CK_INVALID_HANDLE) {
                 /* session is valid, close and invalidate it */
                 CKAPI(epv)
                     ->C_CloseSession(session->handle);
                 session->handle = CK_INVALID_HANDLE;
             }
             nssSession_ExitMonitor(session);
+            nssSlot_EnterMonitor(slot);
+            if (!slot->token) {
+                /* Check token presence after re-acquiring lock */
+                isPresent = PR_FALSE;
+                goto done; /* slot lock held */
+            }
         }
         if (slot->token->base.name[0] != 0) {
             /* notify the high-level cache that the token is removed */
             slot->token->base.name[0] = 0; /* XXX */
             nssToken_NotifyCertsNotVisible(slot->token);
         }
         slot->token->base.name[0] = 0; /* XXX */
         /* clear the token cache */
@@ -218,34 +225,41 @@ nssSlot_IsTokenPresent(
     }
 
     /* token is present, use the session info to determine if the card
      * has been removed and reinserted.
      */
     session = nssToken_GetDefaultSession(slot->token);
     if (session) {
         PRBool tokenRemoved;
+        nssSlot_ExitMonitor(slot);
         nssSession_EnterMonitor(session);
         if (session->handle != CK_INVALID_HANDLE) {
             CK_SESSION_INFO sessionInfo;
             ckrv = CKAPI(epv)->C_GetSessionInfo(session->handle, &sessionInfo);
             if (ckrv != CKR_OK) {
                 /* session is screwy, close and invalidate it */
                 CKAPI(epv)
                     ->C_CloseSession(session->handle);
                 session->handle = CK_INVALID_HANDLE;
             }
         }
         tokenRemoved = (session->handle == CK_INVALID_HANDLE);
         nssSession_ExitMonitor(session);
+        nssSlot_EnterMonitor(slot);
         /* token not removed, finished */
         if (!tokenRemoved) {
             isPresent = PR_TRUE;
             goto done; /* slot lock held */
         }
+        if (!slot->token) {
+            /* Check token presence after re-acquiring lock */
+            isPresent = PR_FALSE;
+            goto done; /* slot lock held */
+        }
     }
     /* the token has been removed, and reinserted, or the slot contains
      * a token it doesn't recognize. invalidate all the old
      * information we had on this token, if we can't refresh, clear
      * the present flag */
     nssToken_NotifyCertsNotVisible(slot->token);
     nssToken_Remove(slot->token);
     /* token has been removed, need to refresh with new session */