# HG changeset patch

# User Robert Relyea <rrelyea@redhat.com>

# Date 1516007838 -3600

#      Mon Jan 15 10:17:18 2018 +0100

# Node ID 33d9c969cd6548c335ce43fa8909b96ef323f670

# Parent  db32ef3be38eb06a91babbcbb48285284d704dbd

Bug 1054373, Crash in PK11_DoesMechanism due to race condition, r=rsleevi

diff --git a/lib/dev/devslot.c b/lib/dev/devslot.c

--- a/lib/dev/devslot.c

+++ b/lib/dev/devslot.c

@@ -33,6 +33,8 @@ nssSlot_Destroy(

         if (PR_ATOMIC_DECREMENT(&slot->base.refCount) == 0) {

             PK11_FreeSlot(slot->pk11slot);

             PZ_DestroyLock(slot->base.lock);

+            PZ_DestroyCondVar(slot->isPresentCondition);

+            PZ_DestroyLock(slot->isPresentLock);

             return nssArena_Destroy(slot->base.arena);

         }

     }

@@ -117,35 +119,61 @@ nssSlot_IsTokenPresent(

     nssSession *session;

     CK_SLOT_INFO slotInfo;

     void *epv;

+    PRBool isPresent = PR_FALSE;

+

     /* permanent slots are always present unless they're disabled */

     if (nssSlot_IsPermanent(slot)) {

         return !PK11_IsDisabled(slot->pk11slot);

     }

+

     /* avoid repeated calls to check token status within set interval */

+    PZ_Lock(slot->isPresentLock);

     if (within_token_delay_period(slot)) {

-        return ((slot->ckFlags & CKF_TOKEN_PRESENT) != 0);

+        CK_FLAGS ckFlags = slot->ckFlags;

+        PZ_Unlock(slot->isPresentLock);

+        return ((ckFlags & CKF_TOKEN_PRESENT) != 0);

     }

+    PZ_Unlock(slot->isPresentLock);

-    /* First obtain the slot info */

+    /* First obtain the slot epv before we set up the condition

+     * variable, so we can just return if we couldn't get it. */

     epv = slot->epv;

     if (!epv) {

         return PR_FALSE;

     }

+

+    /* set up condition so only one thread is active in this part of the code at a time */

+    PZ_Lock(slot->isPresentLock);

+    while (slot->inIsPresent) {

+        PR_WaitCondVar(slot->isPresentCondition, 0);

+    }

+    /* if we were one of multiple threads here, the first thread will have

+     * given us the answer, no need to make more queries of the token. */

+    if (within_token_delay_period(slot)) {

+        CK_FLAGS ckFlags = slot->ckFlags;

+        PZ_Unlock(slot->isPresentLock);

+        return ((ckFlags & CKF_TOKEN_PRESENT) != 0);

+    }

+    /* this is the winning thread, block all others until we've determined

+     * if the token is present and that it needs initialization. */

+    slot->inIsPresent = PR_TRUE;

+    PZ_Unlock(slot->isPresentLock);

+

     nssSlot_EnterMonitor(slot);

     ckrv = CKAPI(epv)->C_GetSlotInfo(slot->slotID, &slotInfo);

     nssSlot_ExitMonitor(slot);

     if (ckrv != CKR_OK) {

         slot->token->base.name[0] = 0; /* XXX */

-        slot->lastTokenPing = PR_IntervalNow();

-        return PR_FALSE;

+        isPresent = PR_FALSE;

+        goto done;

     }

     slot->ckFlags = slotInfo.flags;

     /* check for the presence of the token */

     if ((slot->ckFlags & CKF_TOKEN_PRESENT) == 0) {

         if (!slot->token) {

             /* token was never present */

-            slot->lastTokenPing = PR_IntervalNow();

-            return PR_FALSE;

+            isPresent = PR_FALSE;

+            goto done;

         }

         session = nssToken_GetDefaultSession(slot->token);

         if (session) {

@@ -167,15 +195,15 @@ nssSlot_IsTokenPresent(

         slot->token->base.name[0] = 0; /* XXX */

         /* clear the token cache */

         nssToken_Remove(slot->token);

-        slot->lastTokenPing = PR_IntervalNow();

-        return PR_FALSE;

+        isPresent = PR_FALSE;

+        goto done;

     }

     /* 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 isPresent = PR_FALSE;

+        PRBool tokenRemoved;

         nssSession_EnterMonitor(session);

         if (session->handle != CK_INVALID_SESSION) {

             CK_SESSION_INFO sessionInfo;

@@ -187,12 +215,12 @@ nssSlot_IsTokenPresent(

                 session->handle = CK_INVALID_SESSION;

             }

         }

-        isPresent = session->handle != CK_INVALID_SESSION;

+        tokenRemoved = (session->handle == CK_INVALID_SESSION);

         nssSession_ExitMonitor(session);

         /* token not removed, finished */

-        if (isPresent) {

-            slot->lastTokenPing = PR_IntervalNow();

-            return PR_TRUE;

+        if (!tokenRemoved) {

+            isPresent = PR_TRUE;

+            goto done;

         }

     }

     /* the token has been removed, and reinserted, or the slot contains

@@ -203,15 +231,27 @@ nssSlot_IsTokenPresent(

     nssToken_Remove(slot->token);

     /* token has been removed, need to refresh with new session */

     nssrv = nssSlot_Refresh(slot);

+    isPresent = PR_TRUE;

     if (nssrv != PR_SUCCESS) {

         slot->token->base.name[0] = 0; /* XXX */

         slot->ckFlags &= ~CKF_TOKEN_PRESENT;

-        /* TODO: insert a barrier here to avoid reordering of the assingments */

-        slot->lastTokenPing = PR_IntervalNow();

-        return PR_FALSE;

+        isPresent = PR_FALSE;

     }

+done:

+    /* Once we've set up the condition variable,

+     * Before returning, it's necessary to:

+     *  1) Set the lastTokenPing time so that any other threads waiting on this

+     *     initialization and any future calls within the initialization window

+     *     return the just-computed status.

+     *  2) Indicate we're complete, waking up all other threads that may still

+     *     be waiting on initialization can progress.

+     */

+    PZ_Lock(slot->isPresentLock);

     slot->lastTokenPing = PR_IntervalNow();

-    return PR_TRUE;

+    slot->inIsPresent = PR_FALSE;

+    PR_NotifyAllCondVar(slot->isPresentCondition);

+    PZ_Unlock(slot->isPresentLock);

+    return isPresent;

 }

 NSS_IMPLEMENT void *

@@ -229,7 +269,7 @@ nssSlot_GetToken(

     if (nssSlot_IsTokenPresent(slot)) {

         /* Even if a token should be present, check `slot->token` too as it

-	 * might be gone already. This would happen mostly on shutdown. */

+         * might be gone already. This would happen mostly on shutdown. */

         nssSlot_EnterMonitor(slot);

         if (slot->token)

             rvToken = nssToken_AddRef(slot->token);

diff --git a/lib/dev/devt.h b/lib/dev/devt.h

--- a/lib/dev/devt.h

+++ b/lib/dev/devt.h

@@ -81,6 +81,9 @@ struct NSSSlotStr {

     PZLock *lock;

     void *epv;

     PK11SlotInfo *pk11slot;

+    PZLock *isPresentLock;

+    PRCondVar *isPresentCondition;

+    PRBool inIsPresent;

 };

 struct nssSessionStr {

diff --git a/lib/pk11wrap/dev3hack.c b/lib/pk11wrap/dev3hack.c

--- a/lib/pk11wrap/dev3hack.c

+++ b/lib/pk11wrap/dev3hack.c

@@ -120,6 +120,9 @@ nssSlot_CreateFromPK11SlotInfo(NSSTrustD

     /* Grab the slot name from the PKCS#11 fixed-length buffer */

     rvSlot->base.name = nssUTF8_Duplicate(nss3slot->slot_name, td->arena);

     rvSlot->lock = (nss3slot->isThreadSafe) ? NULL : nss3slot->sessionLock;

+    rvSlot->isPresentLock = PZ_NewLock(nssiLockOther);

+    rvSlot->isPresentCondition = PR_NewCondVar(rvSlot->isPresentLock);

+    rvSlot->inIsPresent = PR_FALSE;

     return rvSlot;

 }