Fix infinite loops in secure memory allocation.

crypto/mem_sec.c

@@ -68,8 +68,12 @@ int CRYPTO_secure_malloc_init(size_t size, int minsize)
         sec_malloc_lock = CRYPTO_THREAD_lock_new();
         if (sec_malloc_lock == NULL)
             return 0;
-        ret = sh_init(size, minsize);
-        secure_mem_initialized = 1;
+        if ((ret = sh_init(size, minsize)) != 0) {
+            secure_mem_initialized = 1;
+        } else {
+            CRYPTO_THREAD_lock_free(sec_malloc_lock);
+            sec_malloc_lock = NULL;
+        }
     }
 
     return ret;
@@ -85,6 +89,7 @@ int CRYPTO_secure_malloc_done()
         sh_done();
         secure_mem_initialized = 0;
         CRYPTO_THREAD_lock_free(sec_malloc_lock);
+        sec_malloc_lock = NULL;
         return 1;
     }
 #endif /* IMPLEMENTED */
@@ -336,7 +341,8 @@ static void sh_remove_from_list(char *ptr)
 
 static int sh_init(size_t size, int minsize)
 {
-    int i, ret;
+    int ret;
+    size_t i;
     size_t pgsize;
     size_t aligned;
 
@@ -414,7 +420,6 @@ static int sh_init(size_t size, int minsize)
             close(fd);
         }
     }
-    OPENSSL_assert(sh.map_result != MAP_FAILED);
     if (sh.map_result == MAP_FAILED)
         goto err;
     sh.arena = (char *)(sh.map_result + pgsize);
@@ -482,6 +487,9 @@ static char *sh_malloc(size_t size)
     size_t i;
     char *chunk;
 
+    if (size > sh.arena_size)
+        return NULL;
+
     list = sh.freelist_size - 1;
     for (i = sh.minsize; i < size; i <<= 1)
         list--;

test/secmemtest.c

@@ -7,6 +7,7 @@
  * https://www.openssl.org/source/license.html
  */
 
+#include <stdio.h>
 #include <openssl/crypto.h>
 
 #define perror_line()    perror_line1(__LINE__)
@@ -90,6 +91,67 @@ int main(int argc, char **argv)
         perror_line();
         return 1;
     }
+
+    fprintf(stderr, "Possible infinite loop: allocate more than available\n");
+    if (!CRYPTO_secure_malloc_init(32768, 16)) {
+        perror_line();
+        return 1;
+    }
+    if (OPENSSL_secure_malloc((size_t)-1) != NULL) {
+        perror_line();
+        return 1;
+    }
+    if (!CRYPTO_secure_malloc_done()) {
+        perror_line();
+        return 1;
+    }
+
+    /*
+     * If init fails, then initialized should be false, if not, this
+     * could cause an infinite loop secure_malloc, but we don't test it
+     */
+    if (!CRYPTO_secure_malloc_init(16, 16) &&
+        CRYPTO_secure_malloc_initialized()) {
+        CRYPTO_secure_malloc_done();
+        perror_line();
+        return 1;
+    }
+
+    /*-
+     * There was also a possible infinite loop when the number of
+     * elements was 1<<31, as |int i| was set to that, which is a
+     * negative number. However, it requires minimum input values:
+     *
+     * CRYPTO_secure_malloc_init((size_t)1<<34, (size_t)1<<4);
+     *
+     * Which really only works on 64-bit systems, since it took 16 GB
+     * secure memory arena to trigger the problem. It naturally takes
+     * corresponding amount of available virtual and physical memory
+     * for test to be feasible/representative. Since we can't assume
+     * that every system is equipped with that much memory, the test
+     * remains disabled. If the reader of this comment really wants
+     * to make sure that infinite loop is fixed, they can enable the
+     * code below.
+     */
+# if 0
+    /*-
+     * On Linux and BSD this test has a chance to complete in minimal
+     * time and with minimum side effects, because mlock is likely to
+     * fail because of RLIMIT_MEMLOCK, which is customarily [much]
+     * smaller than 16GB. In other words Linux and BSD users can be
+     * limited by virtual space alone...
+     */
+    if (sizeof(size_t) > 4) {
+        fprintf(stderr, "Possible infinite loop: 1<<31 limit\n");
+        if (CRYPTO_secure_malloc_init((size_t)1<<34, (size_t)1<<4) == 0) {
+            perror_line();
+        } else if (!CRYPTO_secure_malloc_done()) {
+            perror_line();
+            return 1;
+        }
+    }
+# endif
+
     /* this can complete - it was not really secure */
     OPENSSL_secure_free(r);
 #else