Erikcorry/multi sandbox

src/workerd/api/blob.c++

@@ -129,7 +129,7 @@ jsg::BufferSource wrap(jsg::Lock& js, kj::Array<byte> data) {
   // the underlying v8::BackingStore is supposed to free the buffer when
   // it is done with it. Unfortunately ASAN complains about a leak that
   // will require more investigation.
-  // return jsg::BufferSource(js, jsg::BackingStore::from(kj::mv(data)));
+  // return jsg::BufferSource(js, jsg::BackingStore::from(js, kj::mv(data)));
 }
 
 kj::ArrayPtr<const kj::byte> getPtr(jsg::BufferSource& source) {

src/workerd/api/streams/standard-test.c++

@@ -24,16 +24,16 @@ void preamble(auto callback) {
 }
 
 v8::Local<v8::Value> toBytes(jsg::Lock& js, kj::String str) {
-  return jsg::BackingStore::from(str.asBytes().attach(kj::mv(str))).createHandle(js);
+  return jsg::BackingStore::from(js, str.asBytes().attach(kj::mv(str))).createHandle(js);
 }
 
 jsg::BufferSource toBufferSource(jsg::Lock& js, kj::String str) {
-  auto backing = jsg::BackingStore::from(str.asBytes().attach(kj::mv(str))).createHandle(js);
+  auto backing = jsg::BackingStore::from(js, str.asBytes().attach(kj::mv(str))).createHandle(js);
   return jsg::BufferSource(js, kj::mv(backing));
 }
 
 jsg::BufferSource toBufferSource(jsg::Lock& js, kj::Array<kj::byte> bytes) {
-  auto backing = jsg::BackingStore::from(kj::mv(bytes)).createHandle(js);
+  auto backing = jsg::BackingStore::from(js, kj::mv(bytes)).createHandle(js);
   return jsg::BufferSource(js, kj::mv(backing));
 }
 

src/workerd/jsg/buffersource-test.c++

@@ -48,7 +48,7 @@ struct BufferSourceContext: public jsg::Object, public jsg::ContextGlobal {
   }
 
   BufferSource makeBufferSource(jsg::Lock& js) {
-    return BufferSource(js, BackingStore::from(kj::arr<kj::byte>(1, 2, 3)));
+    return BufferSource(js, BackingStore::from(js, kj::arr<kj::byte>(1, 2, 3)));
   }
 
   BufferSource makeArrayBuffer(jsg::Lock& js) {

src/workerd/jsg/buffersource.h

@@ -69,19 +69,29 @@ using BufferSourceViewConstructor = v8::Local<v8::Value> (*)(Lock&, BackingStore
 // the byte length, offset, element size, and constructor type allowing the view to be
 // recreated.
 //
-// The BackingStore can be safely used outside of the isolate lock and can even be passed
-// into another isolate if necessary.
+// The BackingStore must be created in a particular isolate, but it can be
+// safely used outside of the isolate lock.  It cannot be passed to a different
+// isolate, unless they are in the same isolate group, which means they are in
+// the same sandbox.
 class BackingStore {
 public:
   template <BufferSourceType T = v8::Uint8Array>
-  static BackingStore from(kj::Array<kj::byte> data) {
+  static BackingStore from(Lock& js, kj::Array<kj::byte> data) {
     // Creates a new BackingStore that takes over ownership of the given kj::Array.
+    // The bytes may be moved if they are not inside the sandbox already.
     size_t size = data.size();
-    auto ptr = new kj::Array<byte>(kj::mv(data));
-    return BackingStore(
-        v8::ArrayBuffer::NewBackingStore(ptr->begin(), size,
-            [](void*, size_t, void* ptr) { delete reinterpret_cast<kj::Array<byte>*>(ptr); }, ptr),
-        size, 0, getBufferSourceElementSize<T>(), construct<T>, checkIsIntegerType<T>());
+    if (js.v8Isolate->GetGroup().SandboxContains(data.begin())) {
+      auto ptr = new kj::Array<byte>(kj::mv(data));
+      return BackingStore(
+          v8::ArrayBuffer::NewBackingStore(ptr->begin(), size,
+              [](void*, size_t, void* ptr) { delete reinterpret_cast<kj::Array<byte>*>(ptr); }, ptr),
+          size, 0, getBufferSourceElementSize<T>(), construct<T>, checkIsIntegerType<T>());
+    } else {
+      auto result = BackingStore(v8::ArrayBuffer::NewBackingStore(js.v8Isolate, size), size, 0,
+          getBufferSourceElementSize<T>(), construct<T>, checkIsIntegerType<T>());
+      memcpy(result.asArrayPtr().begin(), data.begin(), size);
+      return result;
+    }
   }
 
   // Creates a new BackingStore of the given size.
@@ -449,7 +459,7 @@ class BufferSourceWrapper {
 };
 
 inline BufferSource Lock::arrayBuffer(kj::Array<kj::byte> data) {
-  return BufferSource(*this, BackingStore::from<v8::ArrayBuffer>(kj::mv(data)));
+  return BufferSource(*this, BackingStore::from<v8::ArrayBuffer>(*this, kj::mv(data)));
 }
 
 }  // namespace workerd::jsg

src/workerd/jsg/jsg.h

@@ -2603,7 +2603,8 @@ class Lock {
   BufferSource bytes(kj::Array<kj::byte> data) KJ_WARN_UNUSED_RESULT;
 
   // Returns a jsg::BufferSource whose underlying JavaScript handle is an ArrayBuffer
-  // as opposed to the default Uint8Array.
+  // as opposed to the default Uint8Array.  May copy and move the bytes if they are
+  // not in the right sandbox.
   BufferSource arrayBuffer(kj::Array<kj::byte> data) KJ_WARN_UNUSED_RESULT;
 
   enum RegExpFlags {

src/workerd/jsg/jsvalue.c++

@@ -466,7 +466,7 @@ JsValue Lock::rangeError(kj::StringPtr message) {
 }
 
 BufferSource Lock::bytes(kj::Array<kj::byte> data) {
-  return BufferSource(*this, BackingStore::from(kj::mv(data)));
+  return BufferSource(*this, BackingStore::from(*this, kj::mv(data)));
 }
 
 JsSymbol Lock::symbol(kj::StringPtr str) {

src/workerd/jsg/value.h

@@ -916,28 +916,40 @@ class ArrayBufferWrapper {
     // v8::ArrayBuffer::NewBackingStore() that accepts a deleter callback, and arrange for it to
     // delete an Array<byte> placed on the heap.
     //
-    // TODO(perf): We could avoid an allocation here, perhaps, by decomposing the kj::Array<byte>
-    //   into its component pointer and disposer, and then pass the disposer pointer as the
-    //   "deleter_data" for NewBackingStore. However, KJ doesn't give us any way to decompose an
-    //   Array<T> this way, and it might not want to, as this could make it impossible to support
-    //   unifying Array<T> and Vector<T> in the future (i.e. making all Array<T>s growable). So
-    //   it may be best to stick with allocating an Array<byte> on the heap after all...
     size_t size = value.size();
     if (size == 0) {
       // BackingStore doesn't call custom deleter if begin is null, which it often is for empty
       // arrays.
       return v8::ArrayBuffer::New(isolate, 0);
     }
     byte* begin = value.begin();
+    if (isolate->GetGroup().SandboxContains(begin)) {
+      // TODO(perf): We could avoid an allocation here, perhaps, by decomposing the
+      //   kj::Array<byte> into its component pointer and disposer, and then pass the disposer
+      //   pointer as the "deleter_data" for NewBackingStore. However, KJ doesn't give us any way
+      //   to decompose an Array<T> this way, and it might not want to, as this could make it
+      //   impossible to support unifying Array<T> and Vector<T> in the future (i.e. making all
+      //   Array<T>s growable). So it may be best to stick with allocating an Array<byte> on the
+      //   heap after all...
+      auto ownerPtr = new kj::Array<byte>(kj::mv(value));
+
+      std::unique_ptr<v8::BackingStore> backing =
+          v8::ArrayBuffer::NewBackingStore(begin, size, [](void* begin, size_t size, void* ownerPtr) {
+        delete reinterpret_cast<kj::Array<byte>*>(ownerPtr);
+      }, ownerPtr);
+
+      return v8::ArrayBuffer::New(isolate, kj::mv(backing));
+    } else {
+      // The Array is not already inside the sandbox.  We have to make a copy and move it in.
+      // For performance reasons we might want to throw here and fix all callers to allocate
+      // inside the sandbox.
+      std::unique_ptr<v8::BackingStore> in_sandbox =
+          v8::ArrayBuffer::NewBackingStore(isolate, size, v8::BackingStoreInitializationMode::kUninitialized);
 
-    auto ownerPtr = new kj::Array<byte>(kj::mv(value));
-
-    std::unique_ptr<v8::BackingStore> backing =
-        v8::ArrayBuffer::NewBackingStore(begin, size, [](void* begin, size_t size, void* ownerPtr) {
-      delete reinterpret_cast<kj::Array<byte>*>(ownerPtr);
-    }, ownerPtr);
+      memcpy(in_sandbox->Data(), value.begin(), size);
 
-    return v8::ArrayBuffer::New(isolate, kj::mv(backing));
+      return v8::ArrayBuffer::New(isolate, kj::mv(in_sandbox));
+    }
   }
 
   v8::Local<v8::ArrayBuffer> wrap(v8::Local<v8::Context> context,