From 0916deba59cfd38526ce9beae25c4612061e62a4 Mon Sep 17 00:00:00 2001 From: "J.C. Jones" Date: Fri, 6 Dec 2019 15:36:44 +0100 Subject: [PATCH 1/4] [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. --- security/nss/lib/softoken/pkcs11.c | 9 +++++---- security/nss/lib/softoken/pkcs11i.h | 2 +- security/nss/lib/softoken/pkcs11u.c | 23 +++++------------------ 3 files changed, 11 insertions(+), 23 deletions(-) diff --git a/security/nss/lib/softoken/pkcs11.c b/security/nss/lib/softoken/pkcs11.c index 34f25a9d06..55a30e3de8 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 From 39c9c5849bf2e56603de95600413e694e792a60b Mon Sep 17 00:00:00 2001 From: Christian Holler Date: Fri, 6 Dec 2019 15:53:50 +0100 Subject: [PATCH 2/4] Check return value of PeerConnectionImpl::GetFingerprint. --- media/webrtc/signaling/src/peerconnection/PeerConnectionImpl.h | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/media/webrtc/signaling/src/peerconnection/PeerConnectionImpl.h b/media/webrtc/signaling/src/peerconnection/PeerConnectionImpl.h index 1dcc70519a..6229e1c95b 100644 --- a/media/webrtc/signaling/src/peerconnection/PeerConnectionImpl.h +++ b/media/webrtc/signaling/src/peerconnection/PeerConnectionImpl.h @@ -530,7 +530,8 @@ public: void GetFingerprint(nsAString& fingerprint) { char *tmp; - GetFingerprint(&tmp); + nsresult rv = GetFingerprint(&tmp); + NS_ENSURE_SUCCESS_VOID(rv); fingerprint.AssignASCII(tmp); delete[] tmp; } From b7f6024e345619b9fde4b20cb6a3c64a2ea17efe Mon Sep 17 00:00:00 2001 From: Craig Disselkoen Date: Fri, 6 Dec 2019 16:06:30 +0100 Subject: [PATCH 3/4] [NSS] Bug 1586176 - EncryptUpdate should use maxout not block size. --- security/nss/lib/softoken/pkcs11c.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/security/nss/lib/softoken/pkcs11c.c b/security/nss/lib/softoken/pkcs11c.c index 7eec3d7ee8..fc5144ad9a 100644 --- a/security/nss/lib/softoken/pkcs11c.c +++ b/security/nss/lib/softoken/pkcs11c.c @@ -1285,7 +1285,7 @@ NSC_EncryptUpdate(CK_SESSION_HANDLE hSession, } /* encrypt the current padded data */ rv = (*context->update)(context->cipherInfo, pEncryptedPart, - &padoutlen, context->blockSize, context->padBuf, + &padoutlen, maxout, context->padBuf, context->blockSize); if (rv != SECSuccess) { return sftk_MapCryptError(PORT_GetError()); From d4b247abb35f144636302408c8eeee2b54becceb Mon Sep 17 00:00:00 2001 From: wolfbeast Date: Fri, 6 Dec 2019 17:03:06 +0100 Subject: [PATCH 4/4] Add missing call to WaitForIsDebuggerRegistered --- dom/workers/WorkerPrivate.cpp | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/dom/workers/WorkerPrivate.cpp b/dom/workers/WorkerPrivate.cpp index 27eb570e90..c6ef21f2cd 100644 --- a/dom/workers/WorkerPrivate.cpp +++ b/dom/workers/WorkerPrivate.cpp @@ -2543,6 +2543,12 @@ WorkerPrivateParent::DisableDebugger() WorkerPrivate* self = ParentAsWorkerPrivate(); + // RegisterDebugger might have been dispatched but not completed. + // Wait for its execution to complete before unregistering. + if (!NS_IsMainThread()) { + self->WaitForIsDebuggerRegistered(true); + } + if (NS_FAILED(UnregisterWorkerDebugger(self))) { NS_WARNING("Failed to unregister worker debugger!"); }