summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorJ.C. Jones <jjones@mozilla.com>2019-12-06 15:36:44 +0100
committerwolfbeast <mcwerewolf@wolfbeast.com>2019-12-06 15:36:44 +0100
commit836e72e96caf47add72a13a715722fad030f196c (patch)
tree7a9a2aeb77ea0e04ac1fc2779dd3cea591a0623e
parentd558cf6647406bdbb87c852772ad211670eff223 (diff)
downloaduxp-836e72e96caf47add72a13a715722fad030f196c.tar.gz
[NSS] Bug 1508776 - Remove unneeded refcounting from SFTKSession
SFTKSession objects are only ever actually destroyed at PK11 session closure, as the session is always the final holder -- and asserting refCount == 1 shows that to be true. Because of that, NSC_CloseSession can just call `sftk_DestroySession` directly and leave `sftk_FreeSession` as a no-op to be removed in the future.
-rw-r--r--security/nss/coreconf/coreconf.dep1
-rw-r--r--security/nss/lib/softoken/pkcs11.c9
-rw-r--r--security/nss/lib/softoken/pkcs11i.h2
-rw-r--r--security/nss/lib/softoken/pkcs11u.c23
4 files changed, 11 insertions, 24 deletions
diff --git a/security/nss/coreconf/coreconf.dep b/security/nss/coreconf/coreconf.dep
index 590d1bfaee..5182f75552 100644
--- a/security/nss/coreconf/coreconf.dep
+++ b/security/nss/coreconf/coreconf.dep
@@ -10,4 +10,3 @@
*/
#error "Do not include this header file."
-
diff --git a/security/nss/lib/softoken/pkcs11.c b/security/nss/lib/softoken/pkcs11.c
index 7dd4f7ee9c..7f0adf82f0 100644
--- a/security/nss/lib/softoken/pkcs11.c
+++ b/security/nss/lib/softoken/pkcs11.c
@@ -2674,8 +2674,9 @@ sftk_CloseAllSessions(SFTKSlot *slot, PRBool logout)
} else {
SKIP_AFTER_FORK(PZ_Unlock(lock));
}
- if (session)
- sftk_FreeSession(session);
+ if (session) {
+ sftk_DestroySession(session);
+ }
} while (session != NULL);
}
return CKR_OK;
@@ -3910,8 +3911,6 @@ NSC_CloseSession(CK_SESSION_HANDLE hSession)
if (sftkqueue_is_queued(session, hSession, slot->head, slot->sessHashSize)) {
sessionFound = PR_TRUE;
sftkqueue_delete(session, hSession, slot->head, slot->sessHashSize);
- session->refCount--; /* can't go to zero while we hold the reference */
- PORT_Assert(session->refCount > 0);
}
PZ_Unlock(lock);
@@ -3932,6 +3931,8 @@ NSC_CloseSession(CK_SESSION_HANDLE hSession)
if (session->info.flags & CKF_RW_SESSION) {
(void)PR_ATOMIC_DECREMENT(&slot->rwSessionCount);
}
+ sftk_DestroySession(session);
+ session = NULL;
}
sftk_FreeSession(session);
diff --git a/security/nss/lib/softoken/pkcs11i.h b/security/nss/lib/softoken/pkcs11i.h
index 7e57dc5e5c..1ba0bdb82c 100644
--- a/security/nss/lib/softoken/pkcs11i.h
+++ b/security/nss/lib/softoken/pkcs11i.h
@@ -281,7 +281,6 @@ struct SFTKSessionStr {
SFTKSession *next;
SFTKSession *prev;
CK_SESSION_HANDLE handle;
- int refCount;
PZLock *objectLock;
int objectIDCount;
CK_SESSION_INFO info;
@@ -670,6 +669,7 @@ extern SFTKSlot *sftk_SlotFromSessionHandle(CK_SESSION_HANDLE handle);
extern CK_SLOT_ID sftk_SlotIDFromSessionHandle(CK_SESSION_HANDLE handle);
extern SFTKSession *sftk_SessionFromHandle(CK_SESSION_HANDLE handle);
extern void sftk_FreeSession(SFTKSession *session);
+extern void sftk_DestroySession(SFTKSession *session);
extern SFTKSession *sftk_NewSession(CK_SLOT_ID slotID, CK_NOTIFY notify,
CK_VOID_PTR pApplication, CK_FLAGS flags);
extern void sftk_update_state(SFTKSlot *slot, SFTKSession *session);
diff --git a/security/nss/lib/softoken/pkcs11u.c b/security/nss/lib/softoken/pkcs11u.c
index 7b5fe732f3..8cb5d2e702 100644
--- a/security/nss/lib/softoken/pkcs11u.c
+++ b/security/nss/lib/softoken/pkcs11u.c
@@ -1772,7 +1772,6 @@ sftk_NewSession(CK_SLOT_ID slotID, CK_NOTIFY notify, CK_VOID_PTR pApplication,
return NULL;
session->next = session->prev = NULL;
- session->refCount = 1;
session->enc_context = NULL;
session->hash_context = NULL;
session->sign_context = NULL;
@@ -1796,11 +1795,10 @@ sftk_NewSession(CK_SLOT_ID slotID, CK_NOTIFY notify, CK_VOID_PTR pApplication,
}
/* free all the data associated with a session. */
-static void
+void
sftk_DestroySession(SFTKSession *session)
{
SFTKObjectList *op, *next;
- PORT_Assert(session->refCount == 0);
/* clean out the attributes */
/* since no one is referencing us, it's safe to walk the chain
@@ -1844,31 +1842,20 @@ sftk_SessionFromHandle(CK_SESSION_HANDLE handle)
PZ_Lock(lock);
sftkqueue_find(session, handle, slot->head, slot->sessHashSize);
- if (session)
- session->refCount++;
PZ_Unlock(lock);
return (session);
}
/*
- * release a reference to a session handle
+ * release a reference to a session handle. This method of using SFTKSessions
+ * is deprecated, but the pattern should be retained until a future effort
+ * to refactor all SFTKSession users at once is completed.
*/
void
sftk_FreeSession(SFTKSession *session)
{
- PRBool destroy = PR_FALSE;
- SFTKSlot *slot = sftk_SlotFromSession(session);
- PZLock *lock = SFTK_SESSION_LOCK(slot, session->handle);
-
- PZ_Lock(lock);
- if (session->refCount == 1)
- destroy = PR_TRUE;
- session->refCount--;
- PZ_Unlock(lock);
-
- if (destroy)
- sftk_DestroySession(session);
+ return;
}
void