commit dcabfe2d8940529a69e007660fa7bf6c15954ecc
author Bob Beck <bbe@google.com> Tue Feb 07 19:06:08 2023 -0700
committer Boringssl LUCI CQ <boringssl-scoped@luci-project-accounts.iam.gserviceaccount.com> Mon Feb 13 22:13:11 2023 +0000
tree 4cc7c4784b38ea9f7c25a322d342a5e24e8137b7
parent d5e93f521b3fd4f57049583a1584d285e5aab16c

Make OPENSSL_malloc push ERR_R_MALLOC_FAILURE on failure.

Remove all the other ERR_R_MALLOC_FAILURES from the
codebase.

Also changes cbb to push to the error stack, to correctly
report cbb failures instead of now only reporting
malloc failures. Previously it turned all cbb failures
into a malloc failure

Bug: 564

Change-Id: Ic13208bf9d9aaa470e83b2f15782fc94946bbc7b
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/57046
Auto-Submit: Bob Beck <bbe@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: David Benjamin <davidben@google.com>

crypto/mem.c

diff --git a/crypto/mem.c b/crypto/mem.c
index f75e89e..4905caf 100644
--- a/crypto/mem.c
+++ b/crypto/mem.c
@@ -227,13 +227,17 @@
 
 void *OPENSSL_malloc(size_t size) {
   if (should_fail_allocation()) {
-    return NULL;
+    goto err;
   }
 
   if (OPENSSL_memory_alloc != NULL) {
     assert(OPENSSL_memory_free != NULL);
     assert(OPENSSL_memory_get_size != NULL);
-    return OPENSSL_memory_alloc(size);
+    void *ptr = OPENSSL_memory_alloc(size);
+    if (ptr == NULL && size != 0) {
+      goto err;
+    }
+    return ptr;
   }
 
   if (size + OPENSSL_MALLOC_PREFIX < size) {
@@ -245,18 +249,23 @@
     // rare code path.
     uint8_t unused = *(volatile uint8_t *)kBoringSSLBinaryTag;
     (void) unused;
-    return NULL;
+    goto err;
   }
 
   void *ptr = malloc(size + OPENSSL_MALLOC_PREFIX);
   if (ptr == NULL) {
-    return NULL;
+    goto err;
   }
 
   *(size_t *)ptr = size;
 
   __asan_poison_memory_region(ptr, OPENSSL_MALLOC_PREFIX);
   return ((uint8_t *)ptr) + OPENSSL_MALLOC_PREFIX;
+
+ err:
+  // This only works because ERR does not call OPENSSL_malloc.
+  OPENSSL_PUT_ERROR(CRYPTO, ERR_R_MALLOC_FAILURE);
+  return NULL;
 }
 
 void OPENSSL_free(void *orig_ptr) {
@@ -289,10 +298,6 @@
 }
 
 void *OPENSSL_realloc(void *orig_ptr, size_t new_size) {
-  if (should_fail_allocation()) {
-    return NULL;
-  }
-
   if (orig_ptr == NULL) {
     return OPENSSL_malloc(new_size);
   }
@@ -505,24 +510,21 @@
   va_copy(args_copy, args);
   int ret = vsnprintf(candidate, candidate_len, format, args_copy);
   va_end(args_copy);
-  if (ret == INT_MAX || ret < 0) {
-    // Failed, or size not int representable.
+  if (ret < 0) {
     goto err;
   }
   if ((size_t)ret >= candidate_len) {
     // Too big to fit in allocation.
     char *tmp;
 
-    candidate_len = ret + 1;
+    candidate_len = (size_t)ret + 1;
     if ((tmp = reallocate(candidate, candidate_len)) == NULL) {
       goto err;
     }
     candidate = tmp;
-    va_copy(args_copy, args);
-    ret = vsnprintf(candidate, candidate_len, format, args_copy);
-    va_end(args_copy);
+    ret = vsnprintf(candidate, candidate_len, format, args);
   }
-  // At this point this can't happen unless vsnprintf is insane.
+  // At this point this should not happen unless vsnprintf is insane.
   if (ret < 0 || (size_t)ret >= candidate_len) {
     goto err;
   }
@@ -559,7 +561,6 @@
   }
   char *ret = OPENSSL_malloc(alloc_size);
   if (ret == NULL) {
-    OPENSSL_PUT_ERROR(CRYPTO, ERR_R_MALLOC_FAILURE);
     return NULL;
   }
 
@@ -598,7 +599,6 @@
 
   void *ret = OPENSSL_malloc(size);
   if (ret == NULL) {
-    OPENSSL_PUT_ERROR(CRYPTO, ERR_R_MALLOC_FAILURE);
     return NULL;
   }