[tbb-commits] [tor-browser/esr24] Bug 982974 - Be paranoid about neutering ArrayBuffer objects. r=sfink, a=dveditz

[mikeperry at torproject.org]

commit 664b5f505f7fed95256bf5632ed0cd5ba3308fc6
Author: Jeff Walden <jwalden at mit.edu>
Date:   Fri Mar 14 17:20:22 2014 -0700

    Bug 982974 - Be paranoid about neutering ArrayBuffer objects.  r=sfink, a=dveditz
---
 js/src/jstypedarray.cpp      |   50 ++++++++++++++++++++++++++++--------------
 js/src/jstypedarray.h        |   27 ++++++++++++++++++++++-
 js/src/jstypedarrayinlines.h |    6 -----
 js/src/vm/ObjectImpl.h       |    9 +++++++-
 4 files changed, 67 insertions(+), 25 deletions(-)

diff --git a/js/src/jstypedarray.cpp b/js/src/jstypedarray.cpp
index 1630c5a..9d02d06 100644
--- a/js/src/jstypedarray.cpp
+++ b/js/src/jstypedarray.cpp
@@ -623,41 +623,57 @@ bool
 ArrayBufferObject::stealContents(JSContext *cx, JSObject *obj, void **contents,
                                  uint8_t **data)
 {
+    MOZ_ASSERT(cx);
+
     ArrayBufferObject &buffer = obj->as<ArrayBufferObject>();
     JSObject *views = *GetViewList(&buffer);
-    js::ObjectElements *header = js::ObjectElements::fromElements((js::HeapSlot*)buffer.dataPointer());
-    if (buffer.hasDynamicElements() && !buffer.isAsmJSArrayBuffer()) {
+
+    uint32_t byteLen = buffer.byteLength();
+
+    js::ObjectElements *oldHeader = buffer.getElementsHeader();
+    js::ObjectElements *newHeader;
+
+    // If the ArrayBuffer's elements are transferrable, transfer ownership
+    // directly.  Otherwise we have to copy the data into new elements.
+    bool stolen = buffer.hasStealableContents();
+    if (stolen) {
+        newHeader = AllocateArrayBufferContents(cx, byteLen, NULL);
+        if (!newHeader)
+            return false;
+
         *GetViewList(&buffer) = NULL;
-        *contents = header;
+        *contents = oldHeader;
         *data = buffer.dataPointer();
 
-        buffer.setFixedElements();
-        header = js::ObjectElements::fromElements((js::HeapSlot*)buffer.dataPointer());
+        MOZ_ASSERT(!buffer.isAsmJSArrayBuffer(),
+                   "buffer won't be neutered by neuterAsmJSArrayBuffer");
+
+        buffer.elements = newHeader->elements();
     } else {
-        uint32_t length = buffer.byteLength();
-        js::ObjectElements *newheader =
-            AllocateArrayBufferContents(cx, length, buffer.dataPointer());
-        if (!newheader) {
-            js_ReportOutOfMemory(cx);
+        js::ObjectElements *headerCopy =
+            AllocateArrayBufferContents(cx, byteLen, buffer.dataPointer());
+        if (!headerCopy)
             return false;
-        }
 
-        ArrayBufferObject::setElementsHeader(newheader, length);
-        *contents = newheader;
-        *data = reinterpret_cast<uint8_t *>(newheader + 1);
+        *contents = headerCopy;
+        *data = reinterpret_cast<uint8_t *>(headerCopy + 1);
 
         if (buffer.isAsmJSArrayBuffer())
             ArrayBufferObject::neuterAsmJSArrayBuffer(buffer);
+
+        // Keep using the current elements.
+        newHeader = oldHeader;
     }
 
     // Neuter the donor ArrayBuffer and all views of it
-    uint32_t flags = header->flags;
-    ArrayBufferObject::setElementsHeader(header, 0);
-    header->flags = flags;
+    uint32_t flags = newHeader->flags;
+    ArrayBufferObject::setElementsHeader(newHeader, 0);
+    newHeader->flags = flags;
     GetViewList(&buffer)->init(views);
     for (JSObject *view = views; view; view = NextView(view))
         TypedArray::neuter(view);
 
+    newHeader->setIsNeuteredBuffer();
     return true;
 }
 
diff --git a/js/src/jstypedarray.h b/js/src/jstypedarray.h
index f666e52..4751d53 100644
--- a/js/src/jstypedarray.h
+++ b/js/src/jstypedarray.h
@@ -137,6 +137,24 @@ class ArrayBufferObject : public JSObject
     static bool saveArrayBufferList(JSCompartment *c, ArrayBufferVector &vector);
     static void restoreArrayBufferLists(ArrayBufferVector &vector);
 
+    bool hasStealableContents() const {
+        // Inline elements strictly adhere to the corresponding buffer.
+        if (!hasDynamicElements())
+            return false;
+
+        // asm.js buffer contents are transferred by copying, just like inline
+        // elements.
+        if (isAsmJSArrayBuffer())
+            return false;
+
+        // Neutered contents aren't transferrable because we want a neutered
+        // array's contents to be backed by zeroed memory equal in length to
+        // the original buffer contents.  Transferring these contents would
+        // allocate new ones based on the current byteLength, which is 0 for a
+        // neutered array -- not the original byteLength.
+        return !isNeutered();
+    }
+
     static bool stealContents(JSContext *cx, JSObject *obj, void **contents,
                               uint8_t **data);
 
@@ -164,10 +182,17 @@ class ArrayBufferObject : public JSObject
      */
     inline bool hasData() const;
 
-    inline bool isAsmJSArrayBuffer() const;
+    bool isAsmJSArrayBuffer() const {
+        return getElementsHeader()->isAsmJSArrayBuffer();
+    }
+
     static bool prepareForAsmJS(JSContext *cx, Handle<ArrayBufferObject*> buffer);
     static void neuterAsmJSArrayBuffer(ArrayBufferObject &buffer);
     static void releaseAsmJSArrayBuffer(FreeOp *fop, JSObject *obj);
+
+    bool isNeutered() const {
+        return getElementsHeader()->isNeuteredBuffer();
+    }
 };
 
 /*
diff --git a/js/src/jstypedarrayinlines.h b/js/src/jstypedarrayinlines.h
index 5af70f5..f26e9cc 100644
--- a/js/src/jstypedarrayinlines.h
+++ b/js/src/jstypedarrayinlines.h
@@ -56,12 +56,6 @@ ArrayBufferObject::hasData() const
     return getClass() == &class_;
 }
 
-inline bool
-ArrayBufferObject::isAsmJSArrayBuffer() const
-{
-    return getElementsHeader()->isAsmJSArrayBuffer();
-}
-
 static inline int32_t
 ClampIntForUint8Array(int32_t x)
 {
diff --git a/js/src/vm/ObjectImpl.h b/js/src/vm/ObjectImpl.h
index 980f935..8eba5da 100644
--- a/js/src/vm/ObjectImpl.h
+++ b/js/src/vm/ObjectImpl.h
@@ -979,10 +979,11 @@ class ObjectElements
     enum Flags {
         CONVERT_DOUBLE_ELEMENTS = 0x1,
         ASMJS_ARRAY_BUFFER = 0x2,
+        NEUTERED_BUFFER = 0x4,
 
         // Present only if these elements correspond to an array with
         // non-writable length; never present for non-arrays.
-        NONWRITABLE_ARRAY_LENGTH = 0x4
+        NONWRITABLE_ARRAY_LENGTH = 0x8
     };
 
   private:
@@ -1036,6 +1037,12 @@ class ObjectElements
     void setIsAsmJSArrayBuffer() {
         flags |= ASMJS_ARRAY_BUFFER;
     }
+    bool isNeuteredBuffer() const {
+        return flags & NEUTERED_BUFFER;
+    }
+    void setIsNeuteredBuffer() {
+        flags |= NEUTERED_BUFFER;
+    }
     bool hasNonwritableArrayLength() const {
         return flags & NONWRITABLE_ARRAY_LENGTH;
     }