Bugzilla bug 238914: declare namesRunningOffset as unsigned long to avoid

overflow. Fixed a regression introduced by the previous checkin ('nss' is
allowed to be NULL).  Set slotStrings[i] to NULL after freeing to avoid
double-free. r=relyea,nelsonb. a=chofmann for Mozilla 1.7.
 Tag: MOZILLA_1_7_BRANCH

11 files changed, 209 insertions, 172 deletions

diff --git a/security/coreconf/nsinstall/Makefile b/security/coreconf/nsinstall/Makefile
index 1bb7d728f..f8eb829de 100644
--- a/security/coreconf/nsinstall/Makefile
+++ b/security/coreconf/nsinstall/Makefile
@@ -52,6 +52,7 @@ ifeq (,$(filter-out OS2 WIN%,$(OS_TARGET)))
 PROGRAM		=
 else
 TARGETS		= $(PROGRAM) $(PLSRCS:.pl=)
+INSTALL		= true
 endif
 
 ifdef NATIVE_CC
diff --git a/security/coreconf/rules.mk b/security/coreconf/rules.mk
index ff380d472..1fc34a259 100644
--- a/security/coreconf/rules.mk
+++ b/security/coreconf/rules.mk
@@ -301,7 +301,7 @@ get_objs:
 $(LIBRARY): $(OBJS)
 	@$(MAKE_OBJDIR)
 	rm -f $@
-ifeq (,$(filter-out WIN%,$(OS_TARGET)))
+ifeq (,$(filter-out _WIN%,$(NS_USE_GCC)_$(OS_TARGET)))
 	$(AR) $(subst /,\\,$(OBJS))
 else
 	$(AR) $(OBJS)
diff --git a/security/nss/lib/certdb/alg1485.c b/security/nss/lib/certdb/alg1485.c
index b95cec96a..1743bbb7a 100644
--- a/security/nss/lib/certdb/alg1485.c
+++ b/security/nss/lib/certdb/alg1485.c
@@ -707,6 +707,7 @@ AppendAVA(stringBuf *bufp, CERTAVA *ava)
 	avaValue->data[maxLen++] = '.'; 
 	avaValue->data[maxLen++] = '.';
 	avaValue->data[maxLen++] = '.';
+	avaValue->data[maxLen]   = 0;
 	avaValue->len = maxLen;
     }
 
@@ -728,7 +729,7 @@ AppendAVA(stringBuf *bufp, CERTAVA *ava)
 	    PORT_SetError(SEC_ERROR_LIBRARY_FAILURE);
 	    rv = SECFailure;
     	} else {
-	    PORT_Strcpy(tmpBuf+len, (char *)avaValue->data);
+	    PORT_Strncpy(tmpBuf+len, (char *)avaValue->data, avaValue->len + 1);
 	    rv = SECSuccess;
 	}
     } else 
diff --git a/security/nss/lib/certdb/genname.c b/security/nss/lib/certdb/genname.c
index 18957509c..e9cc41ebd 100644
--- a/security/nss/lib/certdb/genname.c
+++ b/security/nss/lib/certdb/genname.c
@@ -75,9 +75,11 @@ const SEC_ASN1Template CERT_NameConstraintSubtreeExcludedTemplate[] = {
 static const SEC_ASN1Template CERTNameConstraintsTemplate[] = {
     { SEC_ASN1_SEQUENCE, 0, NULL, sizeof(CERTNameConstraints) },
     { SEC_ASN1_OPTIONAL | SEC_ASN1_CONTEXT_SPECIFIC | 0, 
-          offsetof(CERTNameConstraints, DERPermited), CERT_NameConstraintSubtreeSubTemplate},
+          offsetof(CERTNameConstraints, DERPermited), 
+	  CERT_NameConstraintSubtreeSubTemplate},
     { SEC_ASN1_OPTIONAL | SEC_ASN1_CONTEXT_SPECIFIC | 1, 
-          offsetof(CERTNameConstraints, DERExcluded), CERT_NameConstraintSubtreeSubTemplate},
+          offsetof(CERTNameConstraints, DERExcluded), 
+	  CERT_NameConstraintSubtreeSubTemplate},
     { 0, }
 };
 
@@ -92,7 +94,7 @@ static const SEC_ASN1Template CERTOthNameTemplate[] = {
 };
 
 static const SEC_ASN1Template CERTOtherNameTemplate[] = {
-    { SEC_ASN1_CONTEXT_SPECIFIC | 0 ,
+    { SEC_ASN1_CONTEXT_SPECIFIC | SEC_ASN1_CONSTRUCTED | 0 ,
       offsetof(CERTGeneralName, name.OthName), CERTOthNameTemplate, 
       sizeof(CERTGeneralName) }
 };
@@ -120,7 +122,7 @@ static const SEC_ASN1Template CERT_DNSNameTemplate[] = {
 };
 
 static const SEC_ASN1Template CERT_X400AddressTemplate[] = {
-    { SEC_ASN1_CONTEXT_SPECIFIC | 3,
+    { SEC_ASN1_CONTEXT_SPECIFIC | SEC_ASN1_CONSTRUCTED | 3,
           offsetof(CERTGeneralName, name.other), SEC_AnyTemplate,
           sizeof (CERTGeneralName)}
 };
@@ -133,7 +135,7 @@ static const SEC_ASN1Template CERT_DirectoryNameTemplate[] = {
 
 
 static const SEC_ASN1Template CERT_EDIPartyNameTemplate[] = {
-    { SEC_ASN1_CONTEXT_SPECIFIC | 5,
+    { SEC_ASN1_CONTEXT_SPECIFIC | SEC_ASN1_CONSTRUCTED | 5,
           offsetof(CERTGeneralName, name.other), SEC_AnyTemplate,
           sizeof (CERTGeneralName)}
 };
diff --git a/security/nss/lib/fortcrypt/swfort/pkcs11/Makefile b/security/nss/lib/fortcrypt/swfort/pkcs11/Makefile
index 44e50063b..cc57d8c5f 100644
--- a/security/nss/lib/fortcrypt/swfort/pkcs11/Makefile
+++ b/security/nss/lib/fortcrypt/swfort/pkcs11/Makefile
@@ -63,7 +63,7 @@ EXTRA_LIBS = \
 	-lplc4 \
 	-lplds4 \
 	-lnspr4 \
-	-lws2_32 \
+	-lwsock32 \
 	-lwinmm \
 	$(NULL)
 else # ! NS_USE_GCC
diff --git a/security/nss/lib/freebl/sparcfix.c b/security/nss/lib/freebl/sparcfix.c
deleted file mode 100644
index 6ebc41cf0..000000000
--- a/security/nss/lib/freebl/sparcfix.c
+++ /dev/null
@@ -1,95 +0,0 @@
-/*
- * The contents of this file are subject to the Mozilla Public
- * License Version 1.1 (the "License"); you may not use this file
- * except in compliance with the License. You may obtain a copy of
- * the License at http://www.mozilla.org/MPL/
- * 
- * Software distributed under the License is distributed on an "AS
- * IS" basis, WITHOUT WARRANTY OF ANY KIND, either express or
- * implied. See the License for the specific language governing
- * rights and limitations under the License.
- * 
- * The Original Code is a program to modify v8+vis objects for linking.
- *
- * The Initial Developer of the Original Code is Sun Microsystems Inc.
- * Portions created by Sun Microsystems Inc. are 
- * Copyright (C) 1999-2000 Sun Microsystems Inc. All Rights Reserved.
- * 
- * Contributor(s):
- *	Netscape Communications Corporation
- * 
- * Alternatively, the contents of this file may be used under the
- * terms of the GNU General Public License Version 2 or later (the
- * "GPL"), in which case the provisions of the GPL are applicable 
- * instead of those above.	If you wish to allow use of your 
- * version of this file only under the terms of the GPL and not to
- * allow others to use your version of this file under the MPL,
- * indicate your decision by deleting the provisions above and
- * replace them with the notice and other provisions required by
- * the GPL.  If you do not delete the provisions above, a recipient
- * may use your version of this file under either the MPL or the
- * GPL.
- *  $Id$
- */
-#include <sys/types.h>
-#include <sys/stat.h>
-#include <fcntl.h>
-#if defined(SOLARIS2_6) || defined(SOLARIS2_7) || defined(SOLARIS2_8)
-#define NEW_SYSV_SPARC 1
-#include <gelf.h>
-#endif
-#include <libelf.h>
-#include <sys/elf_SPARC.h>
-
-int
-main(int argc, char *argv[])
-{
-	Elf *     elf;
-	off_t     size;
-	int       fd;
-	int       count;
-#if defined(NEW_SYSV_SPARC)
-	GElf_Ehdr  hdr;
-	GElf_Ehdr *ehdr = &hdr;
-#else
-	Elf32_Ehdr *ehdr;
-#endif
-
-
-	elf_version(EV_CURRENT);
-	fd = open(argv[1], O_RDWR);
-	if (fd < 0)
-	    goto loser;
-	elf = elf_begin(fd, ELF_C_RDWR, (Elf *)0);
-	if (!elf)
-	    goto loser;
-
-#if defined(NEW_SYSV_SPARC)
-	gelf_getehdr(elf, ehdr);
-#else
-        ehdr = elf32_getehdr(elf);
-	if (!ehdr)
-	    goto loser;
-#endif
-
-	if (ehdr->e_machine == EM_SPARC32PLUS) {
-	    ehdr->e_machine = EM_SPARC;
-	    ehdr->e_flags &= ~(EF_SPARC_32PLUS | EF_SPARC_SUN_US1);
-#if defined(NEW_SYSV_SPARC)
-	    count = gelf_update_ehdr(elf, ehdr);
-	    if (count < 0)
-		goto loser;
-#endif
-	    size = elf_update(elf, ELF_C_WRITE);
-	    if (size < 0)
-		goto loser;
-	}
-
-	do {
-	    count = elf_end(elf);
-	} while (count > 0);
-	return count;
-
-loser:
-	return 1;
-}
diff --git a/security/nss/lib/nss/nss.h b/security/nss/lib/nss/nss.h
index 7c13a86a1..d59454af2 100644
--- a/security/nss/lib/nss/nss.h
+++ b/security/nss/lib/nss/nss.h
@@ -49,7 +49,7 @@ SEC_BEGIN_PROTOS
  * The format of the version string should be
  *     "<major version>.<minor version>[.<patch level>] [<Beta>]"
  */
-#define NSS_VERSION  "3.9.1 Beta"
+#define NSS_VERSION  "3.9.1 Beta 2"
 #define NSS_VMAJOR   3
 #define NSS_VMINOR   9
 #define NSS_VPATCH   1
diff --git a/security/nss/lib/pk11wrap/pk11skey.c b/security/nss/lib/pk11wrap/pk11skey.c
index e1431b3d4..f59f43187 100644
--- a/security/nss/lib/pk11wrap/pk11skey.c
+++ b/security/nss/lib/pk11wrap/pk11skey.c
@@ -5370,9 +5370,12 @@ PK11_UnwrapPrivKey(PK11SlotInfo *slot, PK11SymKey *wrappingKey,
 
     if (newKey) {
 	if (perm) {
+	    /* Get RW Session will either lock the monitor if necessary, 
+	     *  or return a thread safe session handle. */ 
 	    rwsession = PK11_GetRWSession(slot);
 	} else {
 	    rwsession = slot->session;
+	    PK11_EnterSlotMonitor(slot);
 	}
 	crv = PK11_GETTAB(slot)->C_UnwrapKey(rwsession, &mechanism, 
 					 newKey->objectID,
@@ -5380,7 +5383,11 @@ PK11_UnwrapPrivKey(PK11SlotInfo *slot, PK11SymKey *wrappingKey,
 					 wrappedKey->len, keyTemplate, 
 					 templateCount, &privKeyID);
 
-	if (perm) PK11_RestoreROSession(slot, rwsession);
+	if (perm) {
+	    PK11_RestoreROSession(slot, rwsession);
+	} else {
+	    PK11_ExitSlotMonitor(slot);
+	}
 	PK11_FreeSymKey(newKey);
     } else {
 	crv = CKR_FUNCTION_NOT_SUPPORTED;
diff --git a/security/nss/lib/softoken/pk11db.c b/security/nss/lib/softoken/pk11db.c
index 220e897ba..b5ee79231 100644
--- a/security/nss/lib/softoken/pk11db.c
+++ b/security/nss/lib/softoken/pk11db.c
@@ -40,6 +40,7 @@
 #include "pkcs11i.h"
 #include "mcom_db.h"
 #include "cdbhdl.h"
+#include "secerr.h"
 
 #define FREE_CLEAR(p) if (p) { PORT_Free(p); p = NULL; }
 
@@ -510,6 +511,19 @@ secmod_FreeData(DBT *data)
     }
 }
 
+static void
+secmod_FreeSlotStrings(char **slotStrings, int count)
+{
+    int i;
+
+    for (i=0; i < count; i++) {
+	if (slotStrings[i]) {
+	    PR_smprintf_free(slotStrings[i]);
+	    slotStrings[i] = NULL;
+	}
+    }
+}
+
 /*
  * build a module from the data base entry.
  */
@@ -518,128 +532,229 @@ secmod_DecodeData(char *defParams, DBT *data, PRBool *retInternal)
 {
     secmodData *encoded;
     secmodSlotData *slots;
-    char *commonName = NULL,*dllName = NULL,*parameters = NULL;
+    PLArenaPool *arena;
+    char *commonName 		= NULL;
+    char *dllName    		= NULL;
+    char *parameters 		= NULL;
+    char *nss;
+    char *moduleSpec;
+    char **slotStrings 		= NULL;
     unsigned char *names;
-    unsigned short len;
     unsigned long slotCount;
-    unsigned short offset;
-    PRBool isOldVersion  = PR_FALSE;
-    PRBool internal, isFIPS, isModuleDB=PR_FALSE, isModuleDBOnly=PR_FALSE;
-    PRBool extended=PR_FALSE;
-    PRBool hasRootCerts=PR_FALSE,hasRootTrust=PR_FALSE;
-    unsigned long trustOrder=PK11_DEFAULT_TRUST_ORDER, 
-				cipherOrder=PK11_DEFAULT_CIPHER_ORDER;
-    unsigned long ssl0=0, ssl1=0;
-    char **slotStrings = NULL;
-    unsigned long slotID,defaultFlags,timeout;
-    char *nss,*moduleSpec;
+    unsigned long ssl0		=0;
+    unsigned long ssl1		=0;
+    unsigned long slotID;
+    unsigned long defaultFlags;
+    unsigned long timeout;
+    unsigned long trustOrder	=PK11_DEFAULT_TRUST_ORDER;
+    unsigned long cipherOrder	=PK11_DEFAULT_CIPHER_ORDER;
+    unsigned short len;
+    unsigned short namesOffset  = 0;	/* start of the names block */
+    unsigned long namesRunningOffset;	/* offset to name we are 
+					 * currently processing */
+    unsigned short slotOffset;
+    PRBool isOldVersion  	= PR_FALSE;
+    PRBool internal;
+    PRBool isFIPS;
+    PRBool isModuleDB    	=PR_FALSE;
+    PRBool isModuleDBOnly	=PR_FALSE;
+    PRBool extended      	=PR_FALSE;
     int i;
 
-    PLArenaPool *arena;
 
     arena = PORT_NewArena(SEC_ASN1_DEFAULT_ARENA_SIZE);
-    if (arena == NULL) return NULL;
+    if (arena == NULL) 
+    	return NULL;
+
+#define CHECK_SIZE(x) \
+    if ((unsigned int) data->size < (unsigned int)(x)) goto db_loser
+
+    /* -------------------------------------------------------------
+    ** Process the buffer header, which is the secmodData struct. 
+    ** It may be an old or new version.  Check the length for each. 
+    */
+
+    CHECK_SIZE( offsetof(secmodData, trustOrder[0]) );
 
     encoded = (secmodData *)data->data;
-    names = (unsigned char *)data->data;
-    offset = SECMOD_GETSHORT(encoded->slotOffset);
-    slots = (secmodSlotData *) (names + offset + 2);
-    slotCount = SECMOD_GETSHORT(names + offset);
-    names += SECMOD_GETSHORT(encoded->nameStart);
 
-    * retInternal = internal = (encoded->internal != 0) ? PR_TRUE: PR_FALSE;
-    isFIPS = (encoded->fips != 0) ? PR_TRUE: PR_FALSE;
-    len = SECMOD_GETSHORT(names);
+    internal = (encoded->internal != 0) ? PR_TRUE: PR_FALSE;
+    isFIPS   = (encoded->fips     != 0) ? PR_TRUE: PR_FALSE;
 
+    if (retInternal)
+	*retInternal = internal;
+    if (internal) {
+	parameters = PORT_ArenaStrdup(arena,defParams);
+	if (parameters == NULL) 
+	    goto loser;
+    }
     if (internal && (encoded->major == SECMOD_DB_NOUI_VERSION_MAJOR) &&
  	(encoded->minor <= SECMOD_DB_NOUI_VERSION_MINOR)) {
 	isOldVersion = PR_TRUE;
     }
-
     if ((encoded->major == SECMOD_DB_EXT1_VERSION_MAJOR) &&
 	(encoded->minor >= SECMOD_DB_EXT1_VERSION_MINOR)) {
-	trustOrder = SECMOD_GETLONG(encoded->trustOrder);
-	cipherOrder = SECMOD_GETLONG(encoded->cipherOrder);
-	isModuleDB = (encoded->isModuleDB != 0) ? PR_TRUE: PR_FALSE;
+	CHECK_SIZE( sizeof(secmodData));
+	trustOrder     = SECMOD_GETLONG(encoded->trustOrder);
+	cipherOrder    = SECMOD_GETLONG(encoded->cipherOrder);
+	isModuleDB     = (encoded->isModuleDB != 0) ? PR_TRUE: PR_FALSE;
 	isModuleDBOnly = (encoded->isModuleDBOnly != 0) ? PR_TRUE: PR_FALSE;
-	extended = PR_TRUE;
+	extended       = PR_TRUE;
     } 
-
     if (internal && !extended) {
 	trustOrder = 0;
 	cipherOrder = 100;
     }
+    /* decode SSL cipher enable flags */
+    ssl0 = SECMOD_GETLONG(encoded->ssl);
+    ssl1 = SECMOD_GETLONG(encoded->ssl + 4);
+
+    slotOffset  = SECMOD_GETSHORT(encoded->slotOffset);
+    namesOffset = SECMOD_GETSHORT(encoded->nameStart);
+
+
+    /*--------------------------------------------------------------
+    ** Now process the variable length set of names.                
+    ** The names have this structure:
+    ** struct {
+    **     BYTE  commonNameLen[ 2 ];
+    **     BYTE  commonName   [ commonNameLen ];
+    **     BTTE  libNameLen   [ 2 ];
+    **     BYTE  libName      [ libNameLen ];
+    ** If it is "extended" it also has these members:
+    **     BYTE  initStringLen[ 2 ];
+    **     BYTE  initString   [ initStringLen ];
+    ** }
+    */
+
+    namesRunningOffset = namesOffset;
+    /* copy the module's common name */
+    CHECK_SIZE( namesRunningOffset + 2);
+    names = (unsigned char *)data->data;
+    len   = SECMOD_GETSHORT(names+namesRunningOffset);
 
-    /* decode the common name */
+    CHECK_SIZE( namesRunningOffset + 2 + len);
     commonName = (char*)PORT_ArenaAlloc(arena,len+1);
-    if (commonName == NULL) {
-	PORT_FreeArena(arena,PR_TRUE);
-	return NULL;
-    }
-    PORT_Memcpy(commonName,&names[2],len);
+    if (commonName == NULL) 
+	goto loser;
+    PORT_Memcpy(commonName, names + namesRunningOffset + 2, len);
     commonName[len] = 0;
+    namesRunningOffset += len + 2;
 
-    /* decode the DLL name */
-    names += len+2;
-    len = SECMOD_GETSHORT(names);
+    /* copy the module's shared library file name. */
+    CHECK_SIZE( namesRunningOffset + 2);
+    len = SECMOD_GETSHORT(names + namesRunningOffset);
     if (len) {
+	CHECK_SIZE( namesRunningOffset + 2 + len);
 	dllName = (char*)PORT_ArenaAlloc(arena,len + 1);
-	if (dllName == NULL) {
-	    PORT_FreeArena(arena,PR_TRUE);
-	    return NULL;
-	}
-	PORT_Memcpy(dllName,&names[2],len);
+	if (dllName == NULL) 
+	    goto loser;
+	PORT_Memcpy(dllName, names + namesRunningOffset + 2, len);
 	dllName[len] = 0;
     }
+    namesRunningOffset += len + 2;
+
+    /* copy the module's initialization string, if present. */
     if (!internal && extended) {
-	names += len+2;
-	len = SECMOD_GETSHORT(names);
+	CHECK_SIZE( namesRunningOffset + 2);
+	len = SECMOD_GETSHORT(names+namesRunningOffset);
 	if (len) {
+	    CHECK_SIZE( namesRunningOffset + 2 + len );
 	    parameters = (char*)PORT_ArenaAlloc(arena,len + 1);
-	    if (parameters == NULL) {
-		PORT_FreeArena(arena,PR_TRUE);
-		return NULL;
-	    }
-	    PORT_Memcpy(parameters,&names[2],len);
+	    if (parameters == NULL) 
+		goto loser;
+	    PORT_Memcpy(parameters,names + namesRunningOffset + 2, len);
 	    parameters[len] = 0;
 	}
+	namesRunningOffset += len + 2;
+    }
+
+    /* 
+     * Consistency check: Make sure the slot and names blocks don't
+     * overlap. These blocks can occur in any order, so this check is made 
+     * in 2 parts. First we check the case where the slot block starts 
+     * after the name block. Later, when we have the slot block length,
+     * we check the case where slot block starts before the name block.
+     * NOTE: in most cases any overlap will likely be detected by invalid 
+     * data read from the blocks, but it's better to find out sooner 
+     * than later.
+     */
+    if (slotOffset >= namesOffset) { /* slot block starts after name block */
+	if (slotOffset < namesRunningOffset) {
+	    goto db_loser;
+	}
     }
-    if (internal) {
-	parameters = PORT_ArenaStrdup(arena,defParams);
+
+    /* ------------------------------------------------------------------
+    ** Part 3, process the slot table.
+    ** This part has this structure:
+    ** struct {
+    **     BYTE slotCount [ 2 ];
+    **     secmodSlotData [ slotCount ];
+    ** {
+    */
+
+    CHECK_SIZE( slotOffset + 2 );
+    slotCount = SECMOD_GETSHORT((unsigned char *)data->data + slotOffset);
+
+    /* 
+     * Consistency check: Part 2. We now have the slot block length, we can 
+     * check the case where the slotblock procedes the name block.
+     */
+    if (slotOffset < namesOffset) { /* slot block starts before name block */
+	if (namesOffset < slotOffset + 2 + slotCount*sizeof(secmodSlotData)) {
+	    goto db_loser;
+	}
     }
 
-    /* decode SSL cipher enable flags */
-    ssl0 = SECMOD_GETLONG(encoded->ssl);
-    ssl1 = SECMOD_GETLONG(&encoded->ssl[4]);
+    CHECK_SIZE( (slotOffset + 2 + slotCount * sizeof(secmodSlotData)));
+    slots = (secmodSlotData *) ((unsigned char *)data->data + slotOffset + 2);
 
     /*  slotCount; */
-    slotStrings = (char **)PORT_ArenaAlloc(arena, slotCount * sizeof(char *));
-    for (i=0; i < (int) slotCount; i++) {
-	slotID = SECMOD_GETLONG(slots[i].slotID);
-	defaultFlags = SECMOD_GETLONG(slots[i].defaultFlags);
+    slotStrings = (char **)PORT_ArenaZAlloc(arena, slotCount * sizeof(char *));
+    if (slotStrings == NULL)
+	goto loser;
+    for (i=0; i < (int) slotCount; i++, slots++) {
+	PRBool hasRootCerts	=PR_FALSE;
+	PRBool hasRootTrust	=PR_FALSE;
+	slotID       = SECMOD_GETLONG(slots->slotID);
+	defaultFlags = SECMOD_GETLONG(slots->defaultFlags);
+	timeout      = SECMOD_GETLONG(slots->timeout);
+	hasRootCerts = slots->hasRootCerts;
 	if (isOldVersion && internal && (slotID != 2)) {
 		unsigned long internalFlags=
 			pk11_argSlotFlags("slotFlags",SECMOD_SLOT_FLAGS);
 		defaultFlags |= internalFlags;
 	}
-	timeout = SECMOD_GETLONG(slots[i].timeout);
-	hasRootCerts = slots[i].hasRootCerts;
 	if (hasRootCerts && !extended) {
 	    trustOrder = 100;
 	}
 
 	slotStrings[i] = pk11_mkSlotString(slotID, defaultFlags, timeout, 
-	                                   (unsigned char)slots[i].askpw, 
+	                                   (unsigned char)slots->askpw, 
 	                                   hasRootCerts, hasRootTrust);
+	if (slotStrings[i] == NULL) {
+	    secmod_FreeSlotStrings(slotStrings,i);
+	    goto loser;
+	}
     }
 
     nss = pk11_mkNSS(slotStrings, slotCount, internal, isFIPS, isModuleDB, 
-	isModuleDBOnly, internal, trustOrder, cipherOrder, ssl0, ssl1);
+		     isModuleDBOnly, internal, trustOrder, cipherOrder, 
+		     ssl0, ssl1);
+    secmod_FreeSlotStrings(slotStrings,slotCount);
+    /* it's permissible (and normal) for nss to be NULL. it simply means
+     * there are no NSS specific parameters in the database */
     moduleSpec = pk11_mkNewModuleSpec(dllName,commonName,parameters,nss);
     PR_smprintf_free(nss);
     PORT_FreeArena(arena,PR_TRUE);
+    return moduleSpec;
 
-    return (moduleSpec);
+db_loser:
+    PORT_SetError(SEC_ERROR_BAD_DATABASE);
+loser:
+    PORT_FreeArena(arena,PR_TRUE);
+    return NULL;
 }
 
 
diff --git a/security/nss/lib/util/nsslocks.c b/security/nss/lib/util/nsslocks.c
index ce9072c68..8b158e025 100644
--- a/security/nss/lib/util/nsslocks.c
+++ b/security/nss/lib/util/nsslocks.c
@@ -60,7 +60,9 @@ __nss_InitLock(   PZLock    **ppLock, nssILockType ltype )
     while (!*ppLock) {
         PRInt32 myAttempt = PR_AtomicIncrement(&initializers);
         if (myAttempt == 1) {
-	    *ppLock = PZ_NewLock(ltype);
+            if (!*ppLock) {
+                *ppLock = PZ_NewLock(ltype);
+            }
             (void) PR_AtomicDecrement(&initializers);
             break;
         }
@@ -93,7 +95,9 @@ nss_InitMonitor(PZMonitor **ppMonitor, nssILockType ltype )
     while (!*ppMonitor) {
         PRInt32 myAttempt = PR_AtomicIncrement(&initializers);
         if (myAttempt == 1) {
-	    *ppMonitor = PZ_NewMonitor(ltype);
+            if (!*ppMonitor) {
+                *ppMonitor = PZ_NewMonitor(ltype);
+            }
             (void) PR_AtomicDecrement(&initializers);
             break;
         }
diff --git a/security/nss/lib/util/nssrwlk.c b/security/nss/lib/util/nssrwlk.c
index 33aaaa713..421d43d87 100644
--- a/security/nss/lib/util/nssrwlk.c
+++ b/security/nss/lib/util/nssrwlk.c
@@ -185,7 +185,9 @@ nssRWLock_AtomicCreate( NSSRWLock  ** prwlock,
     while (NULL == (rwlock = *prwlock)) {
         PRInt32 myAttempt = PR_AtomicIncrement(&initializers);
         if (myAttempt == 1) {
-	    *prwlock = rwlock = NSSRWLock_New(lock_rank, lock_name);
+            if (NULL == (rwlock = *prwlock)) {
+                *prwlock = rwlock = NSSRWLock_New(lock_rank, lock_name);
+            }
             (void) PR_AtomicDecrement(&initializers);
             break;
         }