Bug 808217 - Sporadic failure of 'certutil' to convert ASCII cert request to binary, additional patch, r=wtcNSS_3_15_BETA5

5 files changed, 79 insertions, 4 deletions

diff --git a/cmd/bltest/blapitest.c b/cmd/bltest/blapitest.c
index e3233810a..476f54a9f 100644
--- a/cmd/bltest/blapitest.c
+++ b/cmd/bltest/blapitest.c
@@ -1033,8 +1033,7 @@ misalignBuffer(PLArenaPool *arena, bltestIO *io, int off)
     ptrdiff_t offset = (ptrdiff_t)io->buf.data % WORDSIZE;
     int length = io->buf.len;
     if (offset != off) {
-	SECITEM_ReallocItem(arena, &io->buf, length, length + 2*WORDSIZE);
-	io->buf.len = length + 2*WORDSIZE; /* why doesn't realloc do this? */
+	SECITEM_ReallocItemV2(arena, &io->buf, length + 2*WORDSIZE);
 	/* offset may have changed? */
 	offset = (ptrdiff_t)io->buf.data % WORDSIZE;
 	if (offset != off) {
diff --git a/cmd/lib/secutil.c b/cmd/lib/secutil.c
index 292ec7c48..c849770db 100644
--- a/cmd/lib/secutil.c
+++ b/cmd/lib/secutil.c
@@ -530,7 +530,7 @@ SECU_ReadDERFromFile(SECItem *der, PRFileDesc *inFile, PRBool ascii)
 	    }
 	} else {
 	    /* need one additional byte for zero terminator */
-	    rv = SECITEM_ReallocItem(NULL, &filedata, filedata.len, filedata.len+1);
+	    rv = SECITEM_ReallocItemV2(NULL, &filedata, filedata.len+1);
 	    if (rv != SECSuccess) {
 		PORT_Free(filedata.data);
 		return rv;
diff --git a/lib/util/nssutil.def b/lib/util/nssutil.def
index 59a4d0bea..86a0ad7e4 100644
--- a/lib/util/nssutil.def
+++ b/lib/util/nssutil.def
@@ -266,6 +266,7 @@ NSSUTIL_QuoteSize;
 SECITEM_AllocArray;
 SECITEM_DupArray;
 SECITEM_FreeArray;
+SECITEM_ReallocItemV2;
 SECITEM_ZfreeArray;
 ;+    local:
 ;+       *;
diff --git a/lib/util/secitem.c b/lib/util/secitem.c
index d3db77085..ca41ff5d3 100644
--- a/lib/util/secitem.c
+++ b/lib/util/secitem.c
@@ -115,6 +115,63 @@ SECITEM_ReallocItem(PLArenaPool *arena, SECItem *item, unsigned int oldlen,
     return SECSuccess;
 }
 
+SECStatus
+SECITEM_ReallocItemV2(PLArenaPool *arena, SECItem *item, unsigned int newlen)
+{
+    unsigned char *newdata = NULL;
+
+    PORT_Assert(item);
+    if (!item) {
+	PORT_SetError(SEC_ERROR_INVALID_ARGS);
+	return SECFailure;
+    }
+    
+    if (item->len == newlen) {
+	return SECSuccess;
+    }
+
+    if (!newlen) {
+	SECITEM_FreeItem(item, PR_FALSE);
+	return SECSuccess;
+    }
+    
+    if (!item->len) {
+	/* allocate fresh block of memory */
+	PORT_Assert(!item->data);
+	if (arena) {
+	    newdata = PORT_ArenaAlloc(arena, newlen);
+	} else {
+	    newdata = PORT_Alloc(newlen);
+	}
+    } else {
+	/* reallocate or adjust existing block of memory */
+	if (arena) {
+	    if (item->len > newlen) {
+		/* There's no need to realloc a shorter block from the arena,
+		 * because it would result in using even more memory!
+		 * Therefore we'll continue to use the old block and 
+		 * set the item to the shorter size.
+		 */
+		item->len = newlen;
+		return SECSuccess;
+	    } else {
+		newdata = PORT_ArenaGrow(arena, item->data, item->len, newlen);
+	    }
+	} else {
+	    newdata = PORT_Realloc(item->data, newlen);
+	}
+    }
+
+    if (!newdata) {
+	PORT_SetError(SEC_ERROR_NO_MEMORY);
+	return SECFailure;
+    }
+
+    item->len = newlen;
+    item->data = newdata;
+    return SECSuccess;
+}
+
 SECComparison
 SECITEM_CompareItem(const SECItem *a, const SECItem *b)
 {
diff --git a/lib/util/secitem.h b/lib/util/secitem.h
index 63b07d246..33c2ce4f8 100644
--- a/lib/util/secitem.h
+++ b/lib/util/secitem.h
@@ -36,6 +36,11 @@ extern SECItem *SECITEM_AllocItem(PLArenaPool *arena, SECItem *item,
 				  unsigned int len);
 
 /*
+** This is a legacy function containing bugs. It doesn't update item->len,
+** and it has other issues as described in bug 298649 and bug 298938.
+** However, the function is  kept unchanged for consumers that might depend 
+** on the broken behaviour. New code should call SECITEM_ReallocItemV2.
+**
 ** Reallocate the data for the specified "item".  If "arena" is not NULL,
 ** then reallocate from there, otherwise reallocate from the heap.
 ** In the case where oldlen is 0, the data is allocated (not reallocated).
@@ -43,10 +48,23 @@ extern SECItem *SECITEM_AllocItem(PLArenaPool *arena, SECItem *item,
 ** SECFailure is returned if it is not.  If the allocation succeeds,
 ** SECSuccess is returned.
 */
-extern SECStatus SECITEM_ReallocItem(PLArenaPool *arena, SECItem *item,
+extern SECStatus SECITEM_ReallocItem( /* deprecated function */
+				     PLArenaPool *arena, SECItem *item,
 				     unsigned int oldlen, unsigned int newlen);
 
 /*
+** Reallocate the data for the specified "item".  If "arena" is not NULL,
+** then reallocate from there, otherwise reallocate from the heap.
+** If the item already has at least the request new size,
+** then the item is kept unchanged and SECSuccess is returned.
+** In any case, "item" is expected to be a valid SECItem pointer;
+** SECFailure is returned if it is not, and the item will remain unchanged.
+** If the allocation succeeds, the item is updated and SECSuccess is returned.
+ */
+extern SECStatus SECITEM_ReallocItemV2(PLArenaPool *arena, SECItem *item,
+				       unsigned int newlen);
+
+/*
 ** Compare two items returning the difference between them.
 */
 extern SECComparison SECITEM_CompareItem(const SECItem *a, const SECItem *b);