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

1 files changed, 14 insertions, 0 deletions

diff --git a/lib/dev/devslot.c b/lib/dev/devslot.c
index 9d730c5f1..3f4e54e06 100644
--- a/lib/dev/devslot.c
+++ b/lib/dev/devslot.c
@@ -188,6 +188,7 @@ nssSlot_IsTokenPresent(
         }
         session = nssToken_GetDefaultSession(slot->token);
         if (session) {
+            nssSlot_ExitMonitor(slot);
             nssSession_EnterMonitor(session);
             /* token is not present */
             if (session->handle != CK_INVALID_HANDLE) {
@@ -197,6 +198,12 @@ nssSlot_IsTokenPresent(
                 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 */
@@ -223,6 +230,7 @@ nssSlot_IsTokenPresent(
     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;
@@ -236,11 +244,17 @@ nssSlot_IsTokenPresent(
         }
         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