commit 664b5f505f7fed95256bf5632ed0cd5ba3308fc6 Author: Jeff Walden jwalden@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; }