diff -up nss/lib/pk11wrap/dev3hack.c.init-token-race nss/lib/pk11wrap/dev3hack.c --- nss/lib/pk11wrap/dev3hack.c.init-token-race 2017-01-13 17:58:55.485868744 +0100 +++ nss/lib/pk11wrap/dev3hack.c 2017-01-13 18:02:27.126675831 +0100 @@ -231,6 +231,16 @@ nssSlot_Refresh(NSSSlot *slot) if (slot->token && slot->token->base.name[0] == 0) { doit = PR_TRUE; } + /* invalidate the session in the nss3slot if we haven't done an init + * token since we noticed that the token->default session is invalid. + * This works because the monitor lock and the token session lock are the + * same locks */ + PK11_EnterSlotMonitor(nss3slot); + if ((slot->token == NULL) || (slot->token->defaultSession == NULL) || + (slot->token->defaultSession->handle == CK_INVALID_SESSION)) { + nss3slot->session = CK_INVALID_SESSION; + } + PK11_ExitSlotMonitor(nss3slot); if (PK11_InitToken(nss3slot, PR_FALSE) != SECSuccess) { return PR_FAILURE; } @@ -238,7 +248,8 @@ nssSlot_Refresh(NSSSlot *slot) nssTrustDomain_UpdateCachedTokenCerts(slot->token->trustDomain, slot->token); } - return nssToken_Refresh(slot->token); + /* no need to call nssToken_Refresh since PK11_Init has already done so */ + return PR_SUCCESS; } NSS_IMPLEMENT PRStatus diff -up nss/lib/pk11wrap/pk11auth.c.init-token-race nss/lib/pk11wrap/pk11auth.c --- nss/lib/pk11wrap/pk11auth.c.init-token-race 2017-01-13 17:58:55.485868744 +0100 +++ nss/lib/pk11wrap/pk11auth.c 2017-01-13 18:05:07.650739842 +0100 @@ -73,8 +73,6 @@ pk11_CheckPassword(PK11SlotInfo *slot, C (unsigned char *)pw, len); slot->lastLoginCheck = 0; mustRetry = PR_FALSE; - if (!alreadyLocked) - PK11_ExitSlotMonitor(slot); switch (crv) { /* if we're already logged in, we're good to go */ case CKR_OK: @@ -101,7 +99,16 @@ pk11_CheckPassword(PK11SlotInfo *slot, C break; } if (retry++ == 0) { + /* we already know the this session is invalid */ + slot->session = CK_INVALID_SESSION; + /* can't enter PK11_InitToken holding the lock + * This is safe because the only places that tries to + * hold the slot monitor over this call pass their own + * session, which would have failed above. + * (session != slot->session) */ + PK11_ExitSlotMonitor(slot); rv = PK11_InitToken(slot, PR_FALSE); + PK11_EnterSlotMonitor(slot); if (rv == SECSuccess) { if (slot->session != CK_INVALID_SESSION) { session = slot->session; /* we should have @@ -119,6 +126,8 @@ pk11_CheckPassword(PK11SlotInfo *slot, C PORT_SetError(PK11_MapError(crv)); rv = SECFailure; /* some failure we can't fix by retrying */ } + if (!alreadyLocked) + PK11_ExitSlotMonitor(slot); } while (mustRetry); return rv; } @@ -465,14 +474,18 @@ done: slot->lastLoginCheck = 0; PK11_RestoreROSession(slot, rwsession); if (rv == SECSuccess) { + PK11_EnterSlotMonitor(slot); /* update our view of the world */ + if (slot->session != CK_INVALID_SESSION) { + PK11_GETTAB(slot)->C_CloseSession(slot->session); + slot->session = CK_INVALID_SESSION; + } + PK11_ExitSlotMonitor(slot); PK11_InitToken(slot, PR_TRUE); if (slot->needLogin) { - PK11_EnterSlotMonitor(slot); PK11_GETTAB(slot)->C_Login(slot->session, CKU_USER, (unsigned char *)userpw, len); slot->lastLoginCheck = 0; - PK11_ExitSlotMonitor(slot); } } return rv; @@ -520,7 +533,7 @@ PK11_ChangePW(PK11SlotInfo *slot, const PK11_RestoreROSession(slot, rwsession); /* update our view of the world */ - PK11_InitToken(slot, PR_TRUE); + /* PK11_InitToken(slot,PR_TRUE); */ return rv; } diff -up nss/lib/pk11wrap/pk11slot.c.init-token-race nss/lib/pk11wrap/pk11slot.c --- nss/lib/pk11wrap/pk11slot.c.init-token-race 2017-01-13 17:58:55.486868720 +0100 +++ nss/lib/pk11wrap/pk11slot.c 2017-01-13 18:12:50.869381900 +0100 @@ -1085,6 +1085,7 @@ PK11_ReadMechanismList(PK11SlotInfo *slo CK_ULONG count; CK_RV crv; PRUint32 i; + char mechanismBits[sizeof(slot->mechanismBits)]; if (slot->mechanismList) { PORT_Free(slot->mechanismList); @@ -1092,12 +1093,8 @@ PK11_ReadMechanismList(PK11SlotInfo *slo } slot->mechanismCount = 0; - if (!slot->isThreadSafe) - PK11_EnterSlotMonitor(slot); crv = PK11_GETTAB(slot)->C_GetMechanismList(slot->slotID, NULL, &count); if (crv != CKR_OK) { - if (!slot->isThreadSafe) - PK11_ExitSlotMonitor(slot); PORT_SetError(PK11_MapError(crv)); return SECFailure; } @@ -1105,14 +1102,10 @@ PK11_ReadMechanismList(PK11SlotInfo *slo slot->mechanismList = (CK_MECHANISM_TYPE *) PORT_Alloc(count * sizeof(CK_MECHANISM_TYPE)); if (slot->mechanismList == NULL) { - if (!slot->isThreadSafe) - PK11_ExitSlotMonitor(slot); return SECFailure; } crv = PK11_GETTAB(slot)->C_GetMechanismList(slot->slotID, slot->mechanismList, &count); - if (!slot->isThreadSafe) - PK11_ExitSlotMonitor(slot); if (crv != CKR_OK) { PORT_Free(slot->mechanismList); slot->mechanismList = NULL; @@ -1120,14 +1113,16 @@ PK11_ReadMechanismList(PK11SlotInfo *slo return SECSuccess; } slot->mechanismCount = count; - PORT_Memset(slot->mechanismBits, 0, sizeof(slot->mechanismBits)); + PORT_Memset(mechanismBits, 0, sizeof(slot->mechanismBits)); for (i = 0; i < count; i++) { CK_MECHANISM_TYPE mech = slot->mechanismList[i]; if (mech < 0x7ff) { - slot->mechanismBits[mech & 0xff] |= 1 << (mech >> 8); + mechanismBits[mech & 0xff] |= 1 << (mech >> 8); } } + PORT_Memcpy(slot->mechanismBits, mechanismBits, + sizeof(slot->mechanismBits)); return SECSuccess; } @@ -1144,14 +1139,20 @@ PK11_InitToken(PK11SlotInfo *slot, PRBoo CK_RV crv; SECStatus rv; PRStatus status; + CK_SESSION_HANDLE session; + + PK11_EnterSlotMonitor(slot); + if (slot->session != CK_INVALID_SESSION) { + /* The reason for doing an InitToken has already been satisfied by + * another thread. Just return */ + PK11_ExitSlotMonitor(slot); + return SECSuccess; + } /* set the slot flags to the current token values */ - if (!slot->isThreadSafe) - PK11_EnterSlotMonitor(slot); crv = PK11_GETTAB(slot)->C_GetTokenInfo(slot->slotID, &tokenInfo); - if (!slot->isThreadSafe) - PK11_ExitSlotMonitor(slot); if (crv != CKR_OK) { + PK11_ExitSlotMonitor(slot); PORT_SetError(PK11_MapError(crv)); return SECFailure; } @@ -1186,8 +1187,10 @@ PK11_InitToken(PK11SlotInfo *slot, PRBoo slot->defRWSession = (PRBool)((!slot->readOnly) && (tokenInfo.ulMaxSessionCount == 1)); rv = PK11_ReadMechanismList(slot); - if (rv != SECSuccess) - return rv; + if (rv != SECSuccess) { + PK11_ExitSlotMonitor(slot); + return rv; + } slot->hasRSAInfo = PR_FALSE; slot->RSAInfoFlags = 0; @@ -1202,56 +1205,23 @@ PK11_InitToken(PK11SlotInfo *slot, PRBoo slot->maxKeyCount = tokenInfo.ulMaxSessionCount / 2; } - /* Make sure our session handle is valid */ - if (slot->session == CK_INVALID_SESSION) { - /* we know we don't have a valid session, go get one */ - CK_SESSION_HANDLE session; - - /* session should be Readonly, serial */ - if (!slot->isThreadSafe) - PK11_EnterSlotMonitor(slot); - crv = PK11_GETTAB(slot)->C_OpenSession(slot->slotID, + /* we know we don't have a valid session, go get one */ + /* session should be Readonly, serial */ + crv = PK11_GETTAB(slot)->C_OpenSession(slot->slotID, (slot->defRWSession ? CKF_RW_SESSION : 0) | CKF_SERIAL_SESSION, slot, pk11_notify, &session); - if (!slot->isThreadSafe) - PK11_ExitSlotMonitor(slot); - if (crv != CKR_OK) { - PORT_SetError(PK11_MapError(crv)); - return SECFailure; - } - slot->session = session; - } else { - /* The session we have may be defunct (the token associated with it) - * has been removed */ - CK_SESSION_INFO sessionInfo; - - if (!slot->isThreadSafe) - PK11_EnterSlotMonitor(slot); - crv = PK11_GETTAB(slot)->C_GetSessionInfo(slot->session, &sessionInfo); - if (crv == CKR_DEVICE_ERROR) { - PK11_GETTAB(slot) - ->C_CloseSession(slot->session); - crv = CKR_SESSION_CLOSED; - } - if ((crv == CKR_SESSION_CLOSED) || (crv == CKR_SESSION_HANDLE_INVALID)) { - crv = PK11_GETTAB(slot)->C_OpenSession(slot->slotID, - (slot->defRWSession ? CKF_RW_SESSION : 0) | CKF_SERIAL_SESSION, - slot, pk11_notify, &slot->session); - if (crv != CKR_OK) { - PORT_SetError(PK11_MapError(crv)); - slot->session = CK_INVALID_SESSION; - if (!slot->isThreadSafe) - PK11_ExitSlotMonitor(slot); - return SECFailure; - } - } - if (!slot->isThreadSafe) - PK11_ExitSlotMonitor(slot); + if (crv != CKR_OK) { + PK11_ExitSlotMonitor(slot); + PORT_SetError(PK11_MapError(crv)); + return SECFailure; } + slot->session = session; status = nssToken_Refresh(slot->nssToken); - if (status != PR_SUCCESS) + if (status != PR_SUCCESS) { + PK11_ExitSlotMonitor(slot); return SECFailure; + } if (!(slot->isInternal) && (slot->hasRandom)) { /* if this slot has a random number generater, use it to add entropy @@ -1264,28 +1234,20 @@ PK11_InitToken(PK11SlotInfo *slot, PRBoo /* if this slot can issue random numbers, get some entropy from * that random number generater and give it to our internal token. */ - PK11_EnterSlotMonitor(slot); crv = PK11_GETTAB(slot)->C_GenerateRandom(slot->session, random_bytes, sizeof(random_bytes)); - PK11_ExitSlotMonitor(slot); if (crv == CKR_OK) { - PK11_EnterSlotMonitor(int_slot); PK11_GETTAB(int_slot) ->C_SeedRandom(int_slot->session, random_bytes, sizeof(random_bytes)); - PK11_ExitSlotMonitor(int_slot); } /* Now return the favor and send entropy to the token's random * number generater */ - PK11_EnterSlotMonitor(int_slot); crv = PK11_GETTAB(int_slot)->C_GenerateRandom(int_slot->session, random_bytes, sizeof(random_bytes)); - PK11_ExitSlotMonitor(int_slot); if (crv == CKR_OK) { - PK11_EnterSlotMonitor(slot); crv = PK11_GETTAB(slot)->C_SeedRandom(slot->session, random_bytes, sizeof(random_bytes)); - PK11_ExitSlotMonitor(slot); } PK11_FreeSlot(int_slot); } @@ -1318,6 +1280,7 @@ PK11_InitToken(PK11SlotInfo *slot, PRBoo ->C_CloseSession(session); } } + PK11_ExitSlotMonitor(slot); return SECSuccess; } @@ -1433,6 +1396,8 @@ PK11_InitSlot(SECMODModule *mod, CK_SLOT } /* if the token is present, initialize it */ if ((slotInfo.flags & CKF_TOKEN_PRESENT) != 0) { + /* session was initialized to CK_INVALID_SESSION when the slot + * was created */ rv = PK11_InitToken(slot, PR_TRUE); /* the only hard failures are on permanent devices, or function * verify failures... function verify failures are already handled @@ -1888,10 +1853,14 @@ PK11_DoesMechanism(PK11SlotInfo *slot, C return (slot->mechanismBits[type & 0xff] & (1 << (type >> 8))) ? PR_TRUE : PR_FALSE; } + PK11_EnterSlotMonitor(slot); for (i = 0; i < (int)slot->mechanismCount; i++) { - if (slot->mechanismList[i] == type) - return PR_TRUE; + if (slot->mechanismList[i] == type) { + PK11_ExitSlotMonitor(slot); + return PR_TRUE; + } } + PK11_ExitSlotMonitor(slot); return PR_FALSE; } diff -up nss/lib/pk11wrap/pk11util.c.init-token-race nss/lib/pk11wrap/pk11util.c --- nss/lib/pk11wrap/pk11util.c.init-token-race 2017-01-13 17:58:55.487868695 +0100 +++ nss/lib/pk11wrap/pk11util.c 2017-01-13 18:01:21.280291292 +0100 @@ -1624,6 +1624,11 @@ SECMOD_RestartModules(PRBool force) * older modules require it, and it doesn't hurt (compliant modules * will return CKR_NOT_INITIALIZED */ (void)PK11_GETTAB(mod)->C_Finalize(NULL); + /* finalize clears the session, mark them dead in the + * slot as well */ + for (i=0; i < mod->slotCount; i++) { + mod->slots[i]->session = CK_INVALID_SESSION; + } /* now initialize the module, this function reinitializes * a module in place, preserving existing slots (even if they * no longer exist) */ @@ -1643,17 +1648,18 @@ SECMOD_RestartModules(PRBool force) /* get new token sessions, bump the series up so that * we refresh other old sessions. This will tell much of * NSS to flush cached handles it may hold as well */ - rv = PK11_InitToken(mod->slots[i], PR_TRUE); + PK11SlotInfo *slot = mod->slots[i]; + rv = PK11_InitToken(slot,PR_TRUE); /* PK11_InitToken could fail if the slot isn't present. * If it is present, though, something is wrong and we should * disable the slot and let the caller know. */ - if (rv != SECSuccess && PK11_IsPresent(mod->slots[i])) { + if (rv != SECSuccess && PK11_IsPresent(slot)) { /* save the last error code */ lastError = PORT_GetError(); rrv = rv; /* disable the token */ - mod->slots[i]->disabled = PR_TRUE; - mod->slots[i]->reason = PK11_DIS_COULD_NOT_INIT_TOKEN; + slot->disabled = PR_TRUE; + slot->reason = PK11_DIS_COULD_NOT_INIT_TOKEN; } } }