From d2f26b7737330daa680f2b434bffbc2d025b554f Mon Sep 17 00:00:00 2001
From: Jeff Walden <jwalden@mit.edu>
Date: Mon, 18 Feb 2019 22:52:42 -0800
Subject: [PATCH] Bug 1529298 - Only pass BufferContents containing a non-null
 pointer to |ArrayBufferObject::createForContents|.  r=sfink

--HG--
extra : rebase_source : 9546122ddfcbe343dd074b2ca983207589aae1c3
---
 js/src/jsapi.h                  | 28 ++++++++++++++++++++++------
 js/src/vm/ArrayBufferObject.cpp | 23 ++++++++++++++++-------
 2 files changed, 38 insertions(+), 13 deletions(-)

diff --git a/js/src/jsapi.h b/js/src/jsapi.h
index c53741ff5992..7de208ad9cbc 100644
--- a/js/src/jsapi.h
+++ b/js/src/jsapi.h
@@ -2021,9 +2021,12 @@ JS_PUBLIC_API void JS_SetAllNonReservedSlotsToUndefined(JSContext* cx,
                                                         JSObject* objArg);
 
 /**
- * Create a new array buffer with the given contents. It must be legal to pass
- * these contents to JS_free(). On success, the ownership is transferred to the
- * new array buffer.
+ * Create a new array buffer with the given |contents|, which may be null only
+ * if |nbytes == 0|.  |contents| must be allocated compatible with deallocation
+ * by |JS_free|.
+ *
+ * If and only if an array buffer is successfully created and returned,
+ * ownership of |contents| is transferred to the new array buffer.
  */
 extern JS_PUBLIC_API JSObject* JS_NewArrayBufferWithContents(JSContext* cx,
                                                              size_t nbytes,
@@ -2066,9 +2069,22 @@ extern JS_PUBLIC_API JSObject* JS_NewExternalArrayBuffer(
     JS::BufferContentsFreeFunc freeFunc, void* freeUserData = nullptr);
 
 /**
- * Create a new array buffer with the given contents.  The array buffer does not
- * take ownership of contents.  JS_DetachArrayBuffer must be called before
- * the contents are disposed of by the user; this call will always succeed.
+ * Create a new ArrayBuffer with the given non-null |contents|.
+ *
+ * Ownership of |contents| remains with the caller: it isn't transferred to the
+ * returned ArrayBuffer.  Callers of this function *must* ensure that they
+ * perform these two steps, in this order, to properly relinquish ownership of
+ * |contents|:
+ *
+ *   1. Call |JS_DetachArrayBuffer| on the buffer returned by this function.
+ *      (|JS_DetachArrayBuffer| is generally fallible, but a call under these
+ *      circumstances is guaranteed to succeed.)
+ *   2. |contents| may be deallocated or discarded consistent with the manner
+ *      in which it was allocated.
+ *
+ * Do not simply allow the returned buffer to be garbage-collected before
+ * deallocating |contents|, because in general there is no way to know *when*
+ * an object is fully garbage-collected to the point where this would be safe.
  */
 extern JS_PUBLIC_API JSObject* JS_NewArrayBufferWithUserOwnedContents(
     JSContext* cx, size_t nbytes, void* contents);
diff --git a/js/src/vm/ArrayBufferObject.cpp b/js/src/vm/ArrayBufferObject.cpp
index bbb18c58607f..0257aa89efc0 100644
--- a/js/src/vm/ArrayBufferObject.cpp
+++ b/js/src/vm/ArrayBufferObject.cpp
@@ -1720,6 +1720,11 @@ JS_PUBLIC_API JSObject* JS_NewArrayBufferWithContents(JSContext* cx,
   CHECK_THREAD(cx);
   MOZ_ASSERT_IF(!data, nbytes == 0);
 
+  if (!data) {
+    // Don't pass nulled contents to |createForContents|.
+    return ArrayBufferObject::createZeroed(cx, 0);
+  }
+
   using BufferContents = ArrayBufferObject::BufferContents;
 
   BufferContents contents = BufferContents::createMalloced(data);
@@ -1736,9 +1741,10 @@ JS_PUBLIC_API JSObject* JS_NewExternalArrayBuffer(
   MOZ_ASSERT(data);
   MOZ_ASSERT(nbytes > 0);
 
-  ArrayBufferObject::BufferContents contents =
-      ArrayBufferObject::BufferContents::createExternal(data, freeFunc,
-                                                        freeUserData);
+  using BufferContents = ArrayBufferObject::BufferContents;
+
+  BufferContents contents =
+      BufferContents::createExternal(data, freeFunc, freeUserData);
   return ArrayBufferObject::createForContents(cx, nbytes, contents,
                                               ArrayBufferObject::OwnsData);
 }
@@ -1748,7 +1754,8 @@ JS_PUBLIC_API JSObject* JS_NewArrayBufferWithUserOwnedContents(JSContext* cx,
                                                                void* data) {
   AssertHeapIsIdle();
   CHECK_THREAD(cx);
-  MOZ_ASSERT_IF(!data, nbytes == 0);
+
+  MOZ_ASSERT(data);
 
   using BufferContents = ArrayBufferObject::BufferContents;
 
@@ -1829,9 +1836,11 @@ JS_PUBLIC_API JSObject* JS_NewMappedArrayBufferWithContents(JSContext* cx,
   CHECK_THREAD(cx);
 
   MOZ_ASSERT(data);
-  ArrayBufferObject::BufferContents contents =
-      ArrayBufferObject::BufferContents::create<ArrayBufferObject::MAPPED>(
-          data);
+
+  using BufferContents = ArrayBufferObject::BufferContents;
+
+  BufferContents contents =
+      BufferContents::create<ArrayBufferObject::MAPPED>(data);
   return ArrayBufferObject::createForContents(cx, nbytes, contents,
                                               ArrayBufferObject::OwnsData);
 }