TCMalloc: restrict maximum size of memory ranges

third_party/tcmalloc/chromium/src/tcmalloc.cc

 // Copyright (c) 2005, Google Inc.
 // All rights reserved.
 //
 // Redistribution and use in source and binary forms, with or without
 // modification, are permitted provided that the following conditions are
 // met:
 //
 //     * Redistributions of source code must retain the above copyright
 // notice, this list of conditions and the following disclaimer.
 //     * Redistributions in binary form must reproduce the above
@@ skipping 278 matching lines @@
   Log(kCrash, __FILE__, __LINE__,
       "Attempt to realloc invalid pointer", old_ptr);
   return 0;
 }
 
 size_t InvalidGetAllocatedSize(const void* ptr) {
   Log(kCrash, __FILE__, __LINE__,
       "Attempt to get the size of an invalid pointer", ptr);
   return 0;
 }
+
+// For security reasons, we want to limit the size of allocations.
+// See crbug.com/169327.
+inline bool IsAllocSizePermitted(size_t alloc_size) {
+  // Never allow an allocation larger than what can be indexed via an int.
+  // Remove kPageSize to account for various rounding, padding and to have a
+  // small margin.
+  return alloc_size <= ((std::numeric_limits<int>::max)() - kPageSize);
+}
+
 }  // unnamed namespace
 
 // Extract interesting stats
 struct TCMallocStats {
   uint64_t thread_bytes;      // Bytes in thread caches
   uint64_t central_bytes;     // Bytes in central cache
   uint64_t transfer_bytes;    // Bytes in central transfer cache
   uint64_t metadata_bytes;    // Bytes alloced for metadata
   uint64_t metadata_unmapped_bytes;    // Address space reserved for metadata
                                        // but is not committed.
@@ skipping 762 matching lines @@
   return result;
 }
 
 inline void* do_malloc(size_t size) {
   AddRoomForMark(&size);
 
   void* ret = NULL;
 
   // The following call forces module initialization
   ThreadCache* heap = ThreadCache::GetCache();
-  if (size <= kMaxSize) {
-    size_t cl = Static::sizemap()->SizeClass(size);
-    size = Static::sizemap()->class_to_size(cl);
+  // First, check if our security policy allows this size.
+  if (IsAllocSizePermitted(size)) {

[Chris Evans, 2013/01/15 02:13:32]

Move the test so it's in the "else" branch of "size <= kMaxSize"
It'll avoid a test/branch on the hot path.

[jln (very slow on Chromium), 2013/01/15 03:10:57]

In release mode, here is the assembly:

cmp     r14, 7FFFEFFFh
ja      loc_14F4
cmp     r14, 8000h
ja      loc_1206

As you can see, it does get inlined, but we don't issue branch prediction hints.
(Apparently modern Intel processors don't even honor such hints anymore).

If we really care about this I think we should rather give hints to the compiler
as to which one is the likely branch (and as you can see, as compiled, we
already don't take any branch anyways). But it seems very unlikely to have any
difference on a modern architecture. And if we care about that level of
optimization, we should optimize GetCache() above, which has more impact.

+    if (size <= kMaxSize) {
+      size_t cl = Static::sizemap()->SizeClass(size);
+      size = Static::sizemap()->class_to_size(cl);
 
-    // TODO(jar): If this has any detectable performance impact, it can be
-    // optimized by only tallying sizes if the profiler was activated to recall
-    // these tallies.  I don't think this is performance critical, but we really
-    // should measure it.
-    heap->AddToByteAllocatedTotal(size);  // Chromium profiling.
+      // TODO(jar): If this has any detectable performance impact, it can be
+      // optimized by only tallying sizes if the profiler was activated to
+      // recall these tallies.  I don't think this is performance critical, but
+      // we really should measure it.
+      heap->AddToByteAllocatedTotal(size);  // Chromium profiling.
 
-    if ((FLAGS_tcmalloc_sample_parameter > 0) && heap->SampleAllocation(size)) {
-      ret = DoSampledAllocation(size);
+      if ((FLAGS_tcmalloc_sample_parameter > 0) &&
+          heap->SampleAllocation(size)) {
+        ret = DoSampledAllocation(size);
+        MarkAllocatedRegion(ret);
+      } else {
+        // The common case, and also the simplest.  This just pops the
+        // size-appropriate freelist, after replenishing it if it's empty.
+        ret = CheckMallocResult(heap->Allocate(size, cl));
+      }
+    } else {
+      ret = do_malloc_pages(heap, size);
       MarkAllocatedRegion(ret);
-    } else {
-      // The common case, and also the simplest.  This just pops the
-      // size-appropriate freelist, after replenishing it if it's empty.
-      ret = CheckMallocResult(heap->Allocate(size, cl));
     }
-  } else {
-    ret = do_malloc_pages(heap, size);
-    MarkAllocatedRegion(ret);
   }
   if (ret == NULL) errno = ENOMEM;
   return ret;
 }
 
 inline void* do_calloc(size_t n, size_t elem_size) {
   // Overflow check
   const size_t size = n * elem_size;
   if (elem_size != 0 && size / elem_size != n) return NULL;
 
@@ skipping 114 matching lines @@
   AddRoomForMark(&new_size);
   // Get the size of the old entry
   const size_t old_size = GetSizeWithCallback(old_ptr, invalid_get_size_fn);
 
   // Reallocate if the new size is larger than the old size,
   // or if the new size is significantly smaller than the old size.
   // We do hysteresis to avoid resizing ping-pongs:
   //    . If we need to grow, grow to max(new_size, old_size * 1.X)
   //    . Don't shrink unless new_size < old_size * 0.Y
   // X and Y trade-off time for wasted space.  For now we do 1.25 and 0.5.
-  const int lower_bound_to_grow = old_size + old_size / 4;
-  const int upper_bound_to_shrink = old_size / 2;
+  const size_t lower_bound_to_grow = old_size + old_size / 4;

[jar (doing other things), 2013/01/15 20:06:00]

These two lines should be upstreamed to google3.

+  const size_t upper_bound_to_shrink = old_size / 2;
   if ((new_size > old_size) || (new_size < upper_bound_to_shrink)) {
     // Need to reallocate.
     void* new_ptr = NULL;
 
     if (new_size > old_size && new_size < lower_bound_to_grow) {
       new_ptr = do_malloc_or_cpp_alloc(lower_bound_to_grow);
     }
     ExcludeMarkFromSize(&new_size);  // do_malloc will add space if needed.
     if (new_ptr == NULL) {
       // Either new_size is not a tiny increment, or last do_malloc failed.
@@ skipping 606 matching lines @@
   *mark = ~allocated_mark;  //  Distinctively not allocated.
 }
 
 static void MarkAllocatedRegion(void* ptr) {
   if (ptr == NULL) return;
   MarkType* mark = GetMarkLocation(ptr);
   *mark = GetMarkValue(ptr, mark);
 }
 
 #endif  // TCMALLOC_VALIDATION