Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
Bug 1679290 - Don't hold slot lock when taking session lock r=bbeurdo…
…uche

[[ 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

--HG--
extra : moz-landing-system : lando
  • Loading branch information
Kevin Jacobs committed Dec 1, 2020
1 parent 856d672 commit e631126
Showing 1 changed file with 14 additions and 0 deletions.
14 changes: 14 additions & 0 deletions lib/dev/devslot.c
Expand Up @@ -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) {
Expand All @@ -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 */
Expand All @@ -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;
Expand All @@ -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
Expand Down

0 comments on commit e631126

Please sign in to comment.