Fix out-of-memory crashes related to ArrayBuffer allocation

third_party/WebKit/Source/bindings/core/v8/ScriptValueSerializer.cpp

 // Copyright 2014 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "config.h"
 #include "bindings/core/v8/ScriptValueSerializer.h"
 
 #include "bindings/core/v8/V8ArrayBuffer.h"
 #include "bindings/core/v8/V8ArrayBufferView.h"
 #include "bindings/core/v8/V8Blob.h"
@@ skipping 233 matching lines @@
 void SerializedScriptValueWriter::writeArrayBuffer(const DOMArrayBuffer& arrayBuffer)
 {
     append(ArrayBufferTag);
     doWriteArrayBuffer(arrayBuffer);
 }
 
 void SerializedScriptValueWriter::writeArrayBufferView(const DOMArrayBufferView& arrayBufferView)
 {
     append(ArrayBufferViewTag);
 #if ENABLE(ASSERT)
-    ASSERT(static_cast<const uint8_t*>(arrayBufferView.bufferBase()->data()) + arrayBufferView.byteOffset() ==
+    ASSERT(static_cast<const uint8_t*>(arrayBufferView.bufferBaseOrNull()->data()) + arrayBufferView.byteOffset() ==

[haraken, 2015/10/29 18:58:37]

arrayBufferView.bufferBaseOrNull() can return nullptr. Is this OK to access
data()?

[Justin Novosad, 2015/11/05 00:17:51]

Done.

         static_cast<const uint8_t*>(arrayBufferView.baseAddress()));
 #endif
     DOMArrayBufferView::ViewType type = arrayBufferView.type();
 
     switch (type) {
     case DOMArrayBufferView::TypeInt8:
         append(ByteArrayTag);
         break;
     case DOMArrayBufferView::TypeUint8Clamped:
         append(UnsignedByteClampedArrayTag);
@@ skipping 704 matching lines @@
 {
     v8::Local<v8::RegExp> regExp = value.As<v8::RegExp>();
     m_writer.writeRegExp(regExp->GetSource(), regExp->GetFlags());
 }
 
 ScriptValueSerializer::StateBase* ScriptValueSerializer::writeAndGreyArrayBufferView(v8::Local<v8::Object> object, ScriptValueSerializer::StateBase* next)
 {
     ASSERT(!object.IsEmpty());
     DOMArrayBufferView* arrayBufferView = V8ArrayBufferView::toImpl(object);
     if (!arrayBufferView)
-        return 0;
-    if (!arrayBufferView->bufferBase())
+        return nullptr;
+    if (!arrayBufferView->bufferBaseOrNull())
         return handleError(DataCloneError, "An ArrayBuffer could not be cloned.", next);
-    v8::Local<v8::Value> underlyingBuffer = toV8(arrayBufferView->bufferBase(), m_scriptState->context()->Global(), isolate());
+    v8::Local<v8::Value> underlyingBuffer = toV8(arrayBufferView->bufferBaseOrNull(), m_scriptState->context()->Global(), isolate());
     if (underlyingBuffer.IsEmpty())
         return handleError(DataCloneError, "An ArrayBuffer could not be cloned.", next);
     StateBase* stateOut = doSerializeArrayBuffer(underlyingBuffer, next);
     if (stateOut)
         return stateOut;
     m_writer.writeArrayBufferView(*arrayBufferView);
     // This should be safe: we serialize something that we know to be a wrapper (see
     // the toV8 call above), so the call to doSerializeArrayBuffer should neither
     // cause the system stack to overflow nor should it have potential to reach
     // this ArrayBufferView again.
@@ skipping 532 matching lines @@
     uint32_t height;
     uint32_t pixelDataLength;
     if (!doReadUint32(&width))
         return false;
     if (!doReadUint32(&height))
         return false;
     if (!doReadUint32(&pixelDataLength))
         return false;
     if (m_position + pixelDataLength > m_length)
         return false;
-    ImageData* imageData = ImageData::create(IntSize(width, height));
+    // TODO(junov): crbug.com/536816
+    // Here we use a NonThorwableExceptionState in order to fail silently
+    // when ImageData allocation fails. It needs to be ascertained whether
+    // the call sites that depend on value deserialization agree with
+    // re-throwing a RangeError exception from here, which is what happens
+    // when the ArrayBuffer encapsulated in the ImageData fails to be
+    // allocated, as per the ECMAScript spec:
+    // http://ecma-international.org/ecma-262/6.0/#sec-createbytedatablock
+    // Before we decide to propagate the exception down to the script
+    // execution context, all the APIs that depend on this routine would
+    // need to have specifications stating that exceptions thrown by sub
+    // routines involved in the deserialization process are re-thrown.
+    NonThrowableExceptionState exceptionState;
+    ImageData* imageData = ImageData::create(IntSize(width, height), exceptionState);
+    if (exceptionState.hadException())
+        return false;
     DOMUint8ClampedArray* pixelArray = imageData->data();
     ASSERT(pixelArray);
     ASSERT(pixelArray->length() >= pixelDataLength);
     memcpy(pixelArray->data(), m_buffer + m_position, pixelDataLength);
     m_position += pixelDataLength;
     *value = toV8(imageData, m_scriptState->context()->Global(), isolate());
     return !value->IsEmpty();
 }
 
 bool SerializedScriptValueReader::readCompositorProxy(v8::Local<v8::Value>* value)
 {
     uint32_t attributes;
     uint64_t element;
     if (!doReadUint64(&element))
         return false;
     if (!doReadUint32(&attributes))
         return false;
 
     CompositorProxy* compositorProxy = CompositorProxy::create(element, attributes);
     *value = toV8(compositorProxy, m_scriptState->context()->Global(), isolate());
     return !value->IsEmpty();
 }
 
-PassRefPtr<DOMArrayBuffer> SerializedScriptValueReader::doReadArrayBuffer()
+PassRefPtr<DOMArrayBuffer> SerializedScriptValueReader::doReadArrayBufferOrNull()
 {
     uint32_t byteLength;
     if (!doReadUint32(&byteLength))
         return nullptr;
     if (m_position + byteLength > m_length)
         return nullptr;
     const void* bufferStart = m_buffer + m_position;
     m_position += byteLength;
-    return DOMArrayBuffer::create(bufferStart, byteLength);
+    return DOMArrayBuffer::createOrNull(bufferStart, byteLength);
 }
 
 bool SerializedScriptValueReader::readArrayBuffer(v8::Local<v8::Value>* value)
 {
-    RefPtr<DOMArrayBuffer> arrayBuffer = doReadArrayBuffer();
-    if (!arrayBuffer)
-        return false;
+    RefPtr<DOMArrayBuffer> arrayBuffer = doReadArrayBufferOrNull();
+    // TODO(junov): crbug.com/536816 Instead of the following assert, we should consider doing:
+    //   if (!arrayBuffer)
+    //       return false;
+    // To do that, we need to make sure that call sites would react correctly
+    // in this case, with the value not having been set.
+    RELEASE_ASSERT(arrayBuffer); // This is essentially an out of memory crash

[haraken, 2015/10/29 18:58:37]

You've added a bunch of RELEASE_ASSERT(arrayBuffer) that catches OOM. Maybe
would it be better to introduce a helper never-inline function like
arrayBufferOutOfMemory()? Then we can get more consistent crash reports for
array buffer OOM.

[Justin Novosad, 2015/11/05 00:17:51]

Done. Turns out that I made a mistake about this case, and I ended up reverting
changes to call site where we are extracting an array buffer from an array
buffer view. These operation never perform any buffer allocations, so they did
not need to be touched.

     *value = toV8(arrayBuffer.release(), m_scriptState->context()->Global(), isolate());
     return !value->IsEmpty();
 }
 
 bool SerializedScriptValueReader::readArrayBufferView(v8::Local<v8::Value>* value, ScriptValueCompositeCreator& creator)
 {
     ArrayBufferViewSubTag subTag;
     uint32_t byteOffset;
     uint32_t byteLength;
     RefPtr<DOMArrayBufferBase> arrayBuffer;
@@ skipping 571 matching lines @@
         return false;
     uint32_t objectReference = m_openCompositeReferenceStack[m_openCompositeReferenceStack.size() - 1];
     m_openCompositeReferenceStack.shrink(m_openCompositeReferenceStack.size() - 1);
     if (objectReference >= m_objectPool.size())
         return false;
     *object = m_objectPool[objectReference];
     return true;
 }
 
 } // namespace blink