Title: [96424] trunk/Source/_javascript_Core
Revision
96424
Author
gga...@apple.com
Date
2011-09-30 14:15:04 -0700 (Fri, 30 Sep 2011)

Log Message

Crash due to out of bounds read/write in MarkedSpace
https://bugs.webkit.org/show_bug.cgi?id=69148
        
This was a case of being surprised by a poorly aritulcated cell size limit,
plus an incorrect ASSERT guarding the cell size limit.

Reviewed by Oliver Hunt.

* heap/MarkedSpace.h:
(JSC::MarkedSpace::sizeClassFor): Changed heap size ranges to be inclusive,
since it makes the ranges easier to understand.
        
Bumped up the max cell size to support the use case in this bug. Since the
atomSize is much bigger than it used to be, there isn't much accounting
cost to handling more size classes.
        
Switched to FixedArray, to help catch SizeClass indexing bugs in the future.

* heap/MarkedSpace.cpp:
(JSC::MarkedSpace::MarkedSpace):
(JSC::MarkedSpace::resetAllocator):
(JSC::MarkedSpace::canonicalizeCellLivenessData): Updated for size ranges
being inclusive.

Modified Paths

Diff

Modified: trunk/Source/_javascript_Core/ChangeLog (96423 => 96424)


--- trunk/Source/_javascript_Core/ChangeLog	2011-09-30 21:04:45 UTC (rev 96423)
+++ trunk/Source/_javascript_Core/ChangeLog	2011-09-30 21:15:04 UTC (rev 96424)
@@ -1,3 +1,29 @@
+2011-09-30  Geoffrey Garen  <gga...@apple.com>
+
+        Crash due to out of bounds read/write in MarkedSpace
+        https://bugs.webkit.org/show_bug.cgi?id=69148
+        
+        This was a case of being surprised by a poorly aritulcated cell size limit,
+        plus an incorrect ASSERT guarding the cell size limit.
+
+        Reviewed by Oliver Hunt.
+
+        * heap/MarkedSpace.h:
+        (JSC::MarkedSpace::sizeClassFor): Changed heap size ranges to be inclusive,
+        since it makes the ranges easier to understand.
+        
+        Bumped up the max cell size to support the use case in this bug. Since the
+        atomSize is much bigger than it used to be, there isn't much accounting
+        cost to handling more size classes.
+        
+        Switched to FixedArray, to help catch SizeClass indexing bugs in the future.
+
+        * heap/MarkedSpace.cpp:
+        (JSC::MarkedSpace::MarkedSpace):
+        (JSC::MarkedSpace::resetAllocator):
+        (JSC::MarkedSpace::canonicalizeCellLivenessData): Updated for size ranges
+        being inclusive.
+
 2011-09-30  Pierre Rossi  <pierre.ro...@gmail.com>
 
         [Qt] Build fix: Qt::escape is deprecated in Qt5

Modified: trunk/Source/_javascript_Core/heap/MarkedSpace.cpp (96423 => 96424)


--- trunk/Source/_javascript_Core/heap/MarkedSpace.cpp	2011-09-30 21:04:45 UTC (rev 96423)
+++ trunk/Source/_javascript_Core/heap/MarkedSpace.cpp	2011-09-30 21:15:04 UTC (rev 96424)
@@ -35,10 +35,10 @@
     , m_highWaterMark(0)
     , m_heap(heap)
 {
-    for (size_t cellSize = preciseStep; cellSize < preciseCutoff; cellSize += preciseStep)
+    for (size_t cellSize = preciseStep; cellSize <= preciseCutoff; cellSize += preciseStep)
         sizeClassFor(cellSize).cellSize = cellSize;
 
-    for (size_t cellSize = impreciseStep; cellSize < impreciseCutoff; cellSize += impreciseStep)
+    for (size_t cellSize = impreciseStep; cellSize <= impreciseCutoff; cellSize += impreciseStep)
         sizeClassFor(cellSize).cellSize = cellSize;
 }
 
@@ -64,19 +64,19 @@
 {
     m_waterMark = 0;
 
-    for (size_t cellSize = preciseStep; cellSize < preciseCutoff; cellSize += preciseStep)
+    for (size_t cellSize = preciseStep; cellSize <= preciseCutoff; cellSize += preciseStep)
         sizeClassFor(cellSize).resetAllocator();
 
-    for (size_t cellSize = impreciseStep; cellSize < impreciseCutoff; cellSize += impreciseStep)
+    for (size_t cellSize = impreciseStep; cellSize <= impreciseCutoff; cellSize += impreciseStep)
         sizeClassFor(cellSize).resetAllocator();
 }
 
 void MarkedSpace::canonicalizeCellLivenessData()
 {
-    for (size_t cellSize = preciseStep; cellSize < preciseCutoff; cellSize += preciseStep)
+    for (size_t cellSize = preciseStep; cellSize <= preciseCutoff; cellSize += preciseStep)
         sizeClassFor(cellSize).zapFreeList();
 
-    for (size_t cellSize = impreciseStep; cellSize < impreciseCutoff; cellSize += impreciseStep)
+    for (size_t cellSize = impreciseStep; cellSize <= impreciseCutoff; cellSize += impreciseStep)
         sizeClassFor(cellSize).zapFreeList();
 }
 

Modified: trunk/Source/_javascript_Core/heap/MarkedSpace.h (96423 => 96424)


--- trunk/Source/_javascript_Core/heap/MarkedSpace.h	2011-09-30 21:04:45 UTC (rev 96423)
+++ trunk/Source/_javascript_Core/heap/MarkedSpace.h	2011-09-30 21:15:04 UTC (rev 96424)
@@ -32,7 +32,7 @@
 #include <wtf/Noncopyable.h>
 #include <wtf/Vector.h>
 
-#define ASSERT_CLASS_FITS_IN_CELL(class) COMPILE_ASSERT(sizeof(class) < MarkedSpace::maxCellSize, class_fits_in_cell)
+#define ASSERT_CLASS_FITS_IN_CELL(class) COMPILE_ASSERT(sizeof(class) <= MarkedSpace::maxCellSize, class_fits_in_cell)
 
 namespace JSC {
 
@@ -45,7 +45,7 @@
 class MarkedSpace {
     WTF_MAKE_NONCOPYABLE(MarkedSpace);
 public:
-    static const size_t maxCellSize = 1024;
+    static const size_t maxCellSize = 2048;
 
     struct SizeClass {
         SizeClass();
@@ -78,19 +78,18 @@
     template<typename Functor> typename Functor::ReturnType forEachBlock();
 
 private:
-    // [ 8, 16... 128 )
+    // [ 32... 256 ]
     static const size_t preciseStep = MarkedBlock::atomSize;
-    static const size_t preciseCutoff = 128;
-    static const size_t maximumPreciseAllocationSize = preciseCutoff - preciseStep;
-    static const size_t preciseCount = preciseCutoff / preciseStep - 1;
+    static const size_t preciseCutoff = 256;
+    static const size_t preciseCount = preciseCutoff / preciseStep;
 
-    // [ 128, 256... 1024 )
+    // [ 512... 2048 ]
     static const size_t impreciseStep = preciseCutoff;
     static const size_t impreciseCutoff = maxCellSize;
-    static const size_t impreciseCount = impreciseCutoff / impreciseStep - 1;
+    static const size_t impreciseCount = impreciseCutoff / impreciseStep;
 
-    SizeClass m_preciseSizeClasses[preciseCount];
-    SizeClass m_impreciseSizeClasses[impreciseCount];
+    FixedArray<SizeClass, preciseCount> m_preciseSizeClasses;
+    FixedArray<SizeClass, impreciseCount> m_impreciseSizeClasses;
     size_t m_waterMark;
     size_t m_highWaterMark;
     Heap* m_heap;
@@ -113,8 +112,8 @@
 
 inline MarkedSpace::SizeClass& MarkedSpace::sizeClassFor(size_t bytes)
 {
-    ASSERT(bytes && bytes < maxCellSize);
-    if (bytes <= maximumPreciseAllocationSize)
+    ASSERT(bytes && bytes <= maxCellSize);
+    if (bytes <= preciseCutoff)
         return m_preciseSizeClasses[(bytes - 1) / preciseStep];
     return m_impreciseSizeClasses[(bytes - 1) / impreciseStep];
 }
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
http://lists.webkit.org/mailman/listinfo.cgi/webkit-changes

Reply via email to