Bug 1413096 - Use CheckedInt for overflow check in calloc. r?njn draft
authorMike Hommey <mh+mozilla@glandium.org>
Mon, 30 Oct 2017 11:28:17 +0900
changeset 689814 f6787a4f09218879132d80d6ee721f268bb77e4d
parent 689813 055b8b228054b08d491867f9c6e7f24893a224ff
child 689815 ab5b0c9e997593406359acb42b2de9d4b27c6867
push id87110
push userbmo:mh+mozilla@glandium.org
push dateWed, 01 Nov 2017 00:18:09 +0000
reviewersnjn
bugs1413096
milestone58.0a1
Bug 1413096 - Use CheckedInt for overflow check in calloc. r?njn Also use in in _recalloc.
memory/build/mozjemalloc.cpp
--- a/memory/build/mozjemalloc.cpp
+++ b/memory/build/mozjemalloc.cpp
@@ -103,16 +103,17 @@
 //   Huge : Each allocation is backed by a dedicated contiguous set of chunks.
 //          Metadata are stored in a separate red-black tree.
 //
 // *****************************************************************************
 
 #include "mozmemory_wrap.h"
 #include "mozjemalloc.h"
 #include "mozilla/Atomics.h"
+#include "mozilla/CheckedInt.h"
 #include "mozilla/DoublyLinkedList.h"
 #include "mozilla/GuardObjects.h"
 #include "mozilla/Likely.h"
 #include "mozilla/Sprintf.h"
 #include "mozilla/UniquePtr.h"
 #include "mozilla/Unused.h"
 
 // On Linux, we use madvise(MADV_DONTNEED) to release memory back to the
@@ -154,17 +155,16 @@ using namespace mozilla;
 #ifdef XP_WIN
 
 // Some defines from the CRT internal headers that we need here.
 #define _CRT_SPINCOUNT 5000
 #include <io.h>
 #include <windows.h>
 #include <intrin.h>
 
-#define SIZE_T_MAX SIZE_MAX
 #define STDERR_FILENO 2
 
 // Use MSVC intrinsics.
 #pragma intrinsic(_BitScanForward)
 static __forceinline int
 ffs(int x)
 {
   unsigned long i;
@@ -216,19 +216,16 @@ typedef long ssize_t;
 #include <sys/types.h>
 #if !defined(XP_SOLARIS) && !defined(ANDROID)
 #include <sys/sysctl.h>
 #endif
 #include <sys/uio.h>
 
 #include <errno.h>
 #include <limits.h>
-#ifndef SIZE_T_MAX
-#define SIZE_T_MAX SIZE_MAX
-#endif
 #include <pthread.h>
 #include <sched.h>
 #include <stdarg.h>
 #include <stdio.h>
 #include <stdbool.h>
 #include <stdint.h>
 #include <stdlib.h>
 #include <string.h>
@@ -4366,40 +4363,32 @@ BaseAllocator::memalign(size_t aAlignmen
 
   return ret;
 }
 
 inline void*
 BaseAllocator::calloc(size_t aNum, size_t aSize)
 {
   void* ret;
-  size_t num_size;
-
-  if (!malloc_init()) {
+
+  if (malloc_init()) {
+    CheckedInt<size_t> checkedSize = CheckedInt<size_t>(aNum) * aSize;
+    if (checkedSize.isValid()) {
+      size_t allocSize = checkedSize.value();
+      if (allocSize == 0) {
+        allocSize = 1;
+      }
+      ret = imalloc(allocSize, /* zero = */ true, mArena);
+    } else {
+      ret = nullptr;
+    }
+  } else {
     ret = nullptr;
-    goto RETURN;
-  }
-
-  num_size = aNum * aSize;
-  if (num_size == 0) {
-    num_size = 1;
-
-    // Try to avoid division here.  We know that it isn't possible to
-    // overflow during multiplication if neither operand uses any of the
-    // most significant half of the bits in a size_t.
-  } else if (((aNum | aSize) & (SIZE_T_MAX << (sizeof(size_t) << 2))) &&
-             (num_size / aSize != aNum)) {
-    // size_t overflow.
-    ret = nullptr;
-    goto RETURN;
-  }
-
-  ret = imalloc(num_size, /* zero = */ true, mArena);
-
-RETURN:
+  }
+
   if (!ret) {
     errno = ENOMEM;
   }
 
   return ret;
 }
 
 inline void*
@@ -5117,17 +5106,23 @@ MOZ_EXPORT void* (*__memalign_hook)(size
 #error "Interposing malloc is unsafe on this system without libc malloc hooks."
 #endif
 
 #ifdef XP_WIN
 void*
 _recalloc(void* aPtr, size_t aCount, size_t aSize)
 {
   size_t oldsize = aPtr ? isalloc(aPtr) : 0;
-  size_t newsize = aCount * aSize;
+  CheckedInt<size_t> checkedSize = CheckedInt<size_t>(aCount) * aSize;
+
+  if (!checkedSize.isValid()) {
+    return nullptr;
+  }
+
+  size_t newsize = checkedSize.value();
 
   // In order for all trailing bytes to be zeroed, the caller needs to
   // use calloc(), followed by recalloc().  However, the current calloc()
   // implementation only zeros the bytes requested, so if recalloc() is
   // to work 100% correctly, calloc() will need to change to zero
   // trailing bytes.
   aPtr = DefaultMalloc::realloc(aPtr, newsize);
   if (aPtr && oldsize < newsize) {