Bug 739469: fix an off-by-one error in PR_GetUniqueIdentity() when more
authorwtc%google.com
Tue, 03 Apr 2012 01:33:25 +0000
changeset 4382 05cb3460dd8caf533ccb02f6098736db4420a90f
parent 4381 a5039f93a3de7ad86fb3a0503b84b4da78ed09da
child 4383 b7eb2a33cb293622471b3bf5d669e6678f9a316c
push idunknown
push userunknown
push dateunknown
bugs739469
Bug 739469: fix an off-by-one error in PR_GetUniqueIdentity() when more than 16 identities are requested. Thanks to Eric Rescorla (ekr) for tracking down the bug. r=ekr.
pr/src/io/prlayer.c
--- a/pr/src/io/prlayer.c
+++ b/pr/src/io/prlayer.c
@@ -607,61 +607,68 @@ PR_IMPLEMENT(PRDescIdentity) PR_GetUniqu
             return PR_INVALID_IO_LAYER;
         }
         strcpy(name, layer_name);
     }
 
     /* this initial code runs unsafe */
 retry:
     PR_ASSERT(NULL == names);
+    /*
+     * In the initial round, both identity_cache.ident and
+     * identity_cache.length are 0, so (identity_cache.ident + 1) is greater
+     * than length.  In later rounds, identity_cache.ident is always less
+     * than length, so (identity_cache.ident + 1) can be equal to but cannot
+     * be greater than length.
+     */
     length = identity_cache.length;
-    if (length < (identity_cache.ident + 1))
+    if ((identity_cache.ident + 1) >= length)
     {
         length += ID_CACHE_INCREMENT;
         names = (char**)PR_CALLOC(length * sizeof(char*));
         if (NULL == names)
         {
             if (NULL != name) PR_DELETE(name);
             PR_SetError(PR_OUT_OF_MEMORY_ERROR, 0);
             return PR_INVALID_IO_LAYER;
         }
     }
 
     /* now we get serious about thread safety */
     PR_Lock(identity_cache.ml);
-    PR_ASSERT(identity_cache.ident <= identity_cache.length);
+    PR_ASSERT(identity_cache.length == 0 ||
+              identity_cache.ident < identity_cache.length);
     identity = identity_cache.ident + 1;
-    if (identity > identity_cache.length)  /* there's no room */
+    if (identity >= identity_cache.length)  /* there's no room */
     {
         /* we have to do something - hopefully it's already done */
-        if ((NULL != names) && (length >= identity))
+        if ((NULL != names) && (identity < length))
         {
             /* what we did is still okay */
             memcpy(
                 names, identity_cache.name,
                 identity_cache.length * sizeof(char*));
             old = identity_cache.name;
             identity_cache.name = names;
             identity_cache.length = length;
             names = NULL;
         }
         else
         {
-            PR_ASSERT(identity_cache.ident <= identity_cache.length);
             PR_Unlock(identity_cache.ml);
             if (NULL != names) PR_DELETE(names);
             goto retry;
         }
     }
     if (NULL != name) /* there's a name to be stored */
     {
         identity_cache.name[identity] = name;
     }
     identity_cache.ident = identity;
-    PR_ASSERT(identity_cache.ident <= identity_cache.length);
+    PR_ASSERT(identity_cache.ident < identity_cache.length);
     PR_Unlock(identity_cache.ml);
 
     if (NULL != old) PR_DELETE(old);
     if (NULL != names) PR_DELETE(names);
 
     return identity;
 }  /* PR_GetUniqueIdentity */