Changeset 199759 in webkit

Timestamp:

Apr 19, 2016 9:14:02 PM (8 years ago)

Author:

ggaren@apple.com

Message:

bmalloc: fix up overflow checks
​https://bugs.webkit.org/show_bug.cgi?id=156780

Reviewed by Mark Lam.

We used to try to avoid overflow in large object math by setting a very
high limit on the largest large object. But that's a bit error-prone
since the check is far away from the math that might overflow -- and
we were missing some cases.

This patch removes the limit and instead checks at each math site.

• bmalloc/Allocator.cpp:

(bmalloc::Allocator::tryAllocate):
(bmalloc::Allocator::allocate):
(bmalloc::Allocator::reallocate):
(bmalloc::Allocator::allocateSlowCase): Remove the limit. tryAllocateLarge
will check for overflow for us.

• bmalloc/Chunk.h: This ASSERT was just totally wrong.

• bmalloc/Heap.cpp:

(bmalloc::Heap::tryAllocateLarge): Check for overflow when adding.

• bmalloc/Sizes.h:

• bmalloc/VMAllocate.h:

(bmalloc::tryVMAllocate): Check for overflow when adding.

• bmalloc/VMHeap.cpp:

(bmalloc::VMHeap::tryAllocateLargeChunk): Check for overflow when adding.

Location:

trunk/Source/bmalloc

Files:

• ChangeLog (modified) (1 diff)
• bmalloc/Allocator.cpp (modified) (3 diffs)
• bmalloc/Chunk.h (modified) (1 diff)
• bmalloc/Heap.cpp (modified) (1 diff)
• bmalloc/Sizes.h (modified) (1 diff)
• bmalloc/VMAllocate.h (modified) (1 diff)
• bmalloc/VMHeap.cpp (modified) (1 diff)

trunk/Source/bmalloc/ChangeLog

@@ +1 @@
+2016-04-19  Geoffrey Garen  <ggaren@apple.com>
+
+        bmalloc: fix up overflow checks
+        https://bugs.webkit.org/show_bug.cgi?id=156780
+
+        Reviewed by Mark Lam.
+
+        We used to try to avoid overflow in large object math by setting a very
+        high limit on the largest large object. But that's a bit error-prone
+        since the check is far away from the math that might overflow -- and
+        we were missing some cases.
+
+        This patch removes the limit and instead checks at each math site.
+
+        * bmalloc/Allocator.cpp:
+        (bmalloc::Allocator::tryAllocate):
+        (bmalloc::Allocator::allocate):
+        (bmalloc::Allocator::reallocate):
+        (bmalloc::Allocator::allocateSlowCase): Remove the limit. tryAllocateLarge
+        will check for overflow for us.
+
+        * bmalloc/Chunk.h: This ASSERT was just totally wrong.
+
+        * bmalloc/Heap.cpp:
+        (bmalloc::Heap::tryAllocateLarge): Check for overflow when adding.
+
+        * bmalloc/Sizes.h:
+
+        * bmalloc/VMAllocate.h:
+        (bmalloc::tryVMAllocate): Check for overflow when adding.
+
+        * bmalloc/VMHeap.cpp:
+        (bmalloc::VMHeap::tryAllocateLargeChunk): Check for overflow when adding.
+
 2016-04-19  Geoffrey Garen  <ggaren@apple.com>
 

trunk/Source/bmalloc/bmalloc/Allocator.cpp

@@ -59 +59 @@
         return allocate(size);
 
-    if (size <= largeMax) {
-        std::lock_guard<StaticMutex> lock(PerProcess<Heap>::mutex());
-        return PerProcess<Heap>::getFastCase()->tryAllocateLarge(lock, alignment, size);
-    }
-
-    return nullptr;
+    std::lock_guard<StaticMutex> lock(PerProcess<Heap>::mutex());
+    return PerProcess<Heap>::getFastCase()->tryAllocateLarge(lock, alignment, size);
 }
 
@@ -84 +80 @@
         return allocate(roundUpToMultipleOf(alignment, size));
 
-    if (size <= largeMax && alignment <= largeMax / 2) {
-        std::lock_guard<StaticMutex> lock(PerProcess<Heap>::mutex());
-        return PerProcess<Heap>::getFastCase()->allocateLarge(lock, alignment, size);
-    }
-
-    BCRASH();
-    return nullptr;
+    std::lock_guard<StaticMutex> lock(PerProcess<Heap>::mutex());
+    return PerProcess<Heap>::getFastCase()->allocateLarge(lock, alignment, size);
 }
 
@@ -194 +185 @@
         return allocateLogSizeClass(size);
 
-    if (size <= largeMax)
-        return allocateLarge(size);
-
-    BCRASH();
-    return nullptr;
+    return allocateLarge(size);
 }
 

trunk/Source/bmalloc/bmalloc/Chunk.h

@@ -56 +56 @@
     std::array<SmallPage, chunkSize / smallPageSize> m_pages;
 };
-
-static_assert(sizeof(Chunk) + largeMax <= chunkSize, "largeMax is too big");
 
 struct ChunkHash {

trunk/Source/bmalloc/bmalloc/Heap.cpp

@@ -343 +343 @@
 void* Heap::tryAllocateLarge(std::lock_guard<StaticMutex>& lock, size_t alignment, size_t size)
 {
-    BASSERT(size <= largeMax);
-    BASSERT(alignment <= largeMax / 2);
     BASSERT(isPowerOfTwo(alignment));
 
-    size = size ? roundUpToMultipleOf(largeAlignment, size) : largeAlignment;
-    alignment = roundUpToMultipleOf<largeAlignment>(alignment);
+    size_t roundedSize = size ? roundUpToMultipleOf(largeAlignment, size) : largeAlignment;
+    if (roundedSize < size) // Check for overflow
+        return nullptr;
+    size = roundedSize;
+
+    size_t roundedAlignment = roundUpToMultipleOf<largeAlignment>(alignment);
+    if (roundedAlignment < alignment) // Check for overflow
+        return nullptr;
+    alignment = roundedAlignment;
 
     XLargeRange range = m_largeFree.remove(alignment, size);

trunk/Source/bmalloc/bmalloc/Sizes.h

@@ -65 +65 @@
     static const size_t largeAlignment = smallMax / pageSizeWasteFactor;
     static const size_t largeAlignmentMask = largeAlignment - 1;
-    static const size_t largeMax = std::numeric_limits<size_t>::max() - largeAlignment; // Make sure that rounding up to largeAlignment does not overflow.
 
     static const size_t deallocatorLogCapacity = 256;

trunk/Source/bmalloc/bmalloc/VMAllocate.h

@@ -146 +146 @@
     vmValidate(vmAlignment);
 
-    size_t mappedSize = vmAlignment - vmPageSize() + vmSize;
+    size_t mappedSize = vmAlignment + vmSize;
+    if (mappedSize < vmAlignment || mappedSize < vmSize) // Check for overflow
+        return nullptr;
+
     char* mapped = static_cast<char*>(tryVMAllocate(mappedSize));
     if (!mapped)

trunk/Source/bmalloc/bmalloc/VMHeap.cpp

@@ -34 +34 @@
     // We allocate VM in aligned multiples to increase the chances that
     // the OS will provide contiguous ranges that we can merge.
-    alignment = roundUpToMultipleOf<chunkSize>(alignment);
-    size = roundUpToMultipleOf<chunkSize>(size);
+    size_t roundedAlignment = roundUpToMultipleOf<chunkSize>(alignment);
+    if (roundedAlignment < alignment) // Check for overflow
+        return XLargeRange();
+    alignment = roundedAlignment;
+
+    size_t roundedSize = roundUpToMultipleOf<chunkSize>(size);
+    if (roundedSize < size) // Check for overflow
+        return XLargeRange();
+    size = roundedSize;
 
     void* memory = tryVMAllocate(alignment, size);