Bug 1370866 - Check return value of PK11Slot_GetNSSToken. r=djackson

Differential Revision: https://phabricator.services.mozilla.com/D139420

2 files changed, 10 insertions, 21 deletions

diff --git a/lib/dev/devtoken.c b/lib/dev/devtoken.c
index fedc44b4b..a7dbffc1a 100644
--- a/lib/dev/devtoken.c
+++ b/lib/dev/devtoken.c
@@ -53,13 +53,6 @@ nssToken_Remove(
     nssTokenObjectCache_Clear(tok->cache);
 }
 
-NSS_IMPLEMENT void
-NSSToken_Destroy(
-    NSSToken *tok)
-{
-    (void)nssToken_Destroy(tok);
-}
-
 NSS_IMPLEMENT NSSToken *
 nssToken_AddRef(
     NSSToken *tok)
@@ -996,8 +989,9 @@ sha1_hash(NSSItem *input, NSSItem *output)
     NSSToken *token = PK11Slot_GetNSSToken(internal);
     ap = NSSAlgorithmAndParameters_CreateSHA1Digest(NULL);
     (void)nssToken_Digest(token, NULL, ap, input, output, NULL);
-    PK11_FreeSlot(token->pk11slot);
     nss_ZFreeIf(ap);
+    (void)nssToken_Destroy(token);
+    PK11_FreeSlot(internal);
 }
 
 static void
@@ -1008,8 +1002,9 @@ md5_hash(NSSItem *input, NSSItem *output)
     NSSToken *token = PK11Slot_GetNSSToken(internal);
     ap = NSSAlgorithmAndParameters_CreateMD5Digest(NULL);
     (void)nssToken_Digest(token, NULL, ap, input, output, NULL);
-    PK11_FreeSlot(token->pk11slot);
     nss_ZFreeIf(ap);
+    (void)nssToken_Destroy(token);
+    PK11_FreeSlot(internal);
 }
 
 static CK_TRUST
diff --git a/lib/dev/devutil.c b/lib/dev/devutil.c
index 7c30a0da2..302a6b562 100644
--- a/lib/dev/devutil.c
+++ b/lib/dev/devutil.c
@@ -56,7 +56,7 @@ nssCryptokiObject_Destroy(
     nssCryptokiObject *object)
 {
     if (object) {
-        nssToken_Destroy(object->token);
+        (void)nssToken_Destroy(object->token);
         nss_ZFreeIf(object->label);
         nss_ZFreeIf(object);
     }
@@ -150,20 +150,13 @@ nssTokenArray_Destroy(
     if (tokens) {
         NSSToken **tokenp;
         for (tokenp = tokens; *tokenp; tokenp++) {
-            nssToken_Destroy(*tokenp);
+            (void)nssToken_Destroy(*tokenp);
         }
         nss_ZFreeIf(tokens);
     }
 }
 
 NSS_IMPLEMENT void
-NSSTokenArray_Destroy(
-    NSSToken **tokens)
-{
-    nssTokenArray_Destroy(tokens);
-}
-
-NSS_IMPLEMENT void
 nssCryptokiObjectArray_Destroy(
     nssCryptokiObject **objects)
 {
@@ -365,7 +358,7 @@ create_object(
     /* The cache is tied to the token, and therefore the objects
      * in it should not hold references to the token.
      */
-    nssToken_Destroy(object->token);
+    (void)nssToken_Destroy(object->token);
     rvCachedObject->object = object;
     rvCachedObject->attributes = nss_ZNEWARRAY(arena, CK_ATTRIBUTE, numTypes);
     if (!rvCachedObject->attributes) {
@@ -568,7 +561,7 @@ get_token_objects_for_cache(
                                                      &numObjects,
                                                      &status);
     if (status != PR_SUCCESS) {
-        nss_ZFreeIf(objects);
+        nssCryptokiObjectArray_Destroy(objects);
         return status;
     }
     for (i = 0; i < numObjects; i++) {
@@ -584,7 +577,8 @@ get_token_objects_for_cache(
     } else {
         PRUint32 j;
         for (j = 0; j < i; j++) {
-            /* sigh */
+            /* Any token references that were removed in successful loop iterations
+             * need to be restored before we call nssCryptokiObjectArray_Destroy */
             nssToken_AddRef(cache->objects[objectType][j]->object->token);
             nssArena_Destroy(cache->objects[objectType][j]->arena);
         }