Hide Forgot
+++ This bug was initially created as a clone of Bug #784672 +++ beginning with nss 3.13 there is code like this: SECStatus NSS_RegisterShutdown(NSS_ShutdownFunc sFunc, void *appData) { int i; PZ_Lock(nssInitLock); The problem is that nssInitLock is NULL until nss_Init() is called, PZ_Lock will crash if called with NULL, and there are some poorly written applications that may call NSS_RegisterShutdown() and other functions before calling nss_Init().
The correct thread-safe fix here would be to add the following code before references to PZ_Lock(nssInitLock): + /* make sure our lock and condition variable are initialized one and only + * one time */ + if (PR_CallOnce(&nssInitOnce, nss_doLockInit) != PR_SUCCESS) { + return SECFailure; + } + This code should be added to: NSS_RegisterShutdown() NSS_Shutdown() NSS_ShutdownContext() This problem exists in upstream NSS as well. bob
This causes crashes admin server in Red Hat Directory Server/389 because of mod_nss https://bugzilla.redhat.com/show_bug.cgi?id=618466 because mod_nss calls SSL_ClearSessionCache() during shutdown without checking to see if NSS_IsInitialized() is true. I can see where there is a chicken/egg problem here PZ_Lock(nssInitLock); if (!NSS_IsInitialized()) { you have to acquire the nssInitLock to make sure nssIsInitted isn't changed out from under you, but nssInitLock is NULL if nssIsInitted is 0. So one way to guard against using NULL nssInitLock would be to call: /* make sure our lock and condition variable are initialized one and only * one time */ if (PR_CallOnce(&nssInitOnce, nss_doLockInit) != PR_SUCCESS) { return SECFailure; } before the first attempt to use nssInitLock in any function that uses nssInitLock. That might be too heavyweight. another approach would be to use an atomic integer for nssIsInitted. NSPR provides pratom.h. That way you could be guaranteed that all accesses of nssIsInitted would be implicitly protected. The best approach would be to use static lock initialization like static pthread_mutex_t nssInitLock = PTHREAD_MUTEX_INITIALIZER; but I do not think NSPR has support for static PR_Lock initialization.
Created attachment 557514 [details] Patch: make sure the initLock has been initialized in every case we use it.
Clearly Rich and I reach the same conclusions (see Comment 2 and 3). As for using atomic integers: that's how PR_CallOnce() is implemented. It's meant to be relatively lightweight, and NSS uses it in cases that are far more common than init, shutdown, or register. bob
Since the problem described in this bug report should be resolved in a recent advisory, it has been closed with a resolution of ERRATA. For information on the advisory, and where to find the updated files, follow the link below. If the solution does not work for you, open a new bug report. http://rhn.redhat.com/errata/RHSA-2012-0973.html