Bug 1433642 - Do not free transferables in synthetic clone buffers, r=jorendorff
☠☠ backed out by 0bea67efaa0a ☠ ☠
authorSteve Fink <sfink@mozilla.com>
Thu, 19 Apr 2018 22:46:53 -0700
changeset 469024 6b7d7b8605ea2240498f01eab16c0940f6e0cc30
parent 469023 a31159fb68887f49dccc854cd0637dfe77ed6df6
child 469025 2692e7aaefbdb8d40612704abbe1422c60ce5b0d
push id9165
push userasasaki@mozilla.com
push dateThu, 26 Apr 2018 21:04:54 +0000
treeherdermozilla-beta@064c3804de2e [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjorendorff
bugs1433642
milestone61.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1433642 - Do not free transferables in synthetic clone buffers, r=jorendorff
js/public/StructuredClone.h
js/src/builtin/TestingFunctions.cpp
--- a/js/public/StructuredClone.h
+++ b/js/public/StructuredClone.h
@@ -466,16 +466,21 @@ class MOZ_NON_MEMMOVABLE JS_PUBLIC_API(J
             return AppendBytes(data, size);
         });
     }
 
     size_t SizeOfExcludingThis(mozilla::MallocSizeOf mallocSizeOf) {
         return bufList_.SizeOfExcludingThis(mallocSizeOf);
     }
 
+    // Temporary until the scope is moved into JSStructuredCloneData.
+    void IgnoreTransferables() {
+        ownTransferables_ = OwnTransferablePolicy::IgnoreTransferablesIfAny;
+    }
+
     void discardTransferables();
 };
 
 /**
  * Implements StructuredDeserialize and StructuredDeserializeWithTransfer.
  *
  * Note: If `data` contains transferable objects, it can be read only once.
  */
--- a/js/src/builtin/TestingFunctions.cpp
+++ b/js/src/builtin/TestingFunctions.cpp
@@ -2737,33 +2737,33 @@ static bool
 SetIonCheckGraphCoherency(JSContext* cx, unsigned argc, Value* vp)
 {
     CallArgs args = CallArgsFromVp(argc, vp);
     jit::JitOptions.checkGraphConsistency = ToBoolean(args.get(0));
     args.rval().setUndefined();
     return true;
 }
 
+// A JSObject that holds structured clone data, similar to the C++ class
+// JSAutoStructuredCloneBuffer.
 class CloneBufferObject : public NativeObject {
     static const JSPropertySpec props_[3];
 
     static const size_t DATA_SLOT = 0;
-    static const size_t LENGTH_SLOT = 1;
-    static const size_t SYNTHETIC_SLOT = 2;
-    static const size_t NUM_SLOTS = 3;
+    static const size_t SYNTHETIC_SLOT = 1;
+    static const size_t NUM_SLOTS = 2;
 
   public:
     static const Class class_;
 
     static CloneBufferObject* Create(JSContext* cx) {
         RootedObject obj(cx, JS_NewObject(cx, Jsvalify(&class_)));
         if (!obj)
             return nullptr;
         obj->as<CloneBufferObject>().setReservedSlot(DATA_SLOT, PrivateValue(nullptr));
-        obj->as<CloneBufferObject>().setReservedSlot(LENGTH_SLOT, Int32Value(0));
         obj->as<CloneBufferObject>().setReservedSlot(SYNTHETIC_SLOT, BooleanValue(false));
 
         if (!JS_DefineProperties(cx, obj, props_))
             return nullptr;
 
         return &obj->as<CloneBufferObject>();
     }
 
@@ -2788,24 +2788,25 @@ class CloneBufferObject : public NativeO
     bool isSynthetic() const {
         return getReservedSlot(SYNTHETIC_SLOT).toBoolean();
     }
 
     void setData(JSStructuredCloneData* aData, bool synthetic) {
         MOZ_ASSERT(!data());
         setReservedSlot(DATA_SLOT, PrivateValue(aData));
         setReservedSlot(SYNTHETIC_SLOT, BooleanValue(synthetic));
+
+        // Temporary until the scope is moved into JSStructuredCloneData.
+        if (synthetic)
+            aData->IgnoreTransferables();
     }
 
     // Discard an owned clone buffer.
     void discard() {
-        if (data()) {
-            JSAutoStructuredCloneBuffer clonebuf(JS::StructuredCloneScope::SameProcessSameThread, nullptr, nullptr);
-            clonebuf.adopt(Move(*data()));
-        }
+        js_delete(data());
         setReservedSlot(DATA_SLOT, PrivateValue(nullptr));
     }
 
     static bool
     setCloneBuffer_impl(JSContext* cx, const CallArgs& args) {
         Rooted<CloneBufferObject*> obj(cx, &args.thisv().toObject().as<CloneBufferObject>());
 
         uint8_t* data = nullptr;
@@ -3078,46 +3079,49 @@ Deserialize(JSContext* cx, unsigned argc
             if (!str)
                 return false;
             auto maybeScope = ParseCloneScope(cx, str);
             if (!maybeScope) {
                 JS_ReportErrorASCII(cx, "Invalid structured clone scope");
                 return false;
             }
 
+            if (fuzzingSafe && *maybeScope < scope) {
+                JS_ReportErrorASCII(cx, "Fuzzing builds must not set less restrictive scope "
+                                    "than the deserialized clone buffer's scope");
+                return false;
+            }
+
             scope = *maybeScope;
         }
     }
 
     // Clone buffer was already consumed?
     if (!obj->data()) {
         JS_ReportErrorASCII(cx, "deserialize given invalid clone buffer "
                             "(transferables already consumed?)");
         return false;
     }
 
     bool hasTransferable;
     if (!JS_StructuredCloneHasTransferables(*obj->data(), &hasTransferable))
         return false;
 
-    if (obj->isSynthetic() && scope != JS::StructuredCloneScope::DifferentProcess) {
-        JS_ReportErrorASCII(cx, "clone buffer data is synthetic but may contain pointers");
-        return false;
-    }
-
     RootedValue deserialized(cx);
     if (!JS_ReadStructuredClone(cx, *obj->data(),
                                 JS_STRUCTURED_CLONE_VERSION,
                                 scope,
                                 &deserialized, nullptr, nullptr))
     {
         return false;
     }
     args.rval().set(deserialized);
 
+    // Consume any clone buffer with transferables; throw an error if it is
+    // deserialized again.
     if (hasTransferable)
         obj->discard();
 
     return true;
 }
 
 static bool
 DetachArrayBuffer(JSContext* cx, unsigned argc, Value* vp)