feat: enable v8 sandboxed pointers (#34724)
* feat: enable v8 sandboxed pointers * update breaking-changes.md * update zero-fill patch benchmarks showed the function call was slower
This commit is contained in:
parent
0ee7f14190
commit
e5db178ab6
5 changed files with 41 additions and 165 deletions
|
@ -45,7 +45,3 @@ enable_cet_shadow_stack = false
|
||||||
# V8 in the browser process.
|
# V8 in the browser process.
|
||||||
# Ref: https://source.chromium.org/chromium/chromium/src/+/45fba672185aae233e75d6ddc81ea1e0b30db050:v8/BUILD.gn;l=281
|
# Ref: https://source.chromium.org/chromium/chromium/src/+/45fba672185aae233e75d6ddc81ea1e0b30db050:v8/BUILD.gn;l=281
|
||||||
is_cfi = false
|
is_cfi = false
|
||||||
|
|
||||||
# TODO(nornagon): this is disabled until node.js's internals can be made
|
|
||||||
# compatible with sandboxed pointers.
|
|
||||||
v8_enable_sandboxed_pointers = false
|
|
||||||
|
|
|
@ -14,6 +14,13 @@ This document uses the following convention to categorize breaking changes:
|
||||||
|
|
||||||
## Planned Breaking API Changes (20.0)
|
## Planned Breaking API Changes (20.0)
|
||||||
|
|
||||||
|
### Behavior Changed: V8 Memory Cage enabled
|
||||||
|
|
||||||
|
The V8 memory cage has been enabled, which has implications for native modules
|
||||||
|
which wrap non-V8 memory with `ArrayBuffer` or `Buffer`. See the [blog post
|
||||||
|
about the V8 memory cage](https://www.electronjs.org/blog/v8-memory-cage) for
|
||||||
|
more details.
|
||||||
|
|
||||||
### API Changed: `webContents.printToPDF()`
|
### API Changed: `webContents.printToPDF()`
|
||||||
|
|
||||||
`webContents.printToPDF()` has been modified to conform to [`Page.printToPDF`](https://chromedevtools.github.io/devtools-protocol/tot/Page/#method-printToPDF) in the Chrome DevTools Protocol. This has been changes in order to
|
`webContents.printToPDF()` has been modified to conform to [`Page.printToPDF`](https://chromedevtools.github.io/devtools-protocol/tot/Page/#method-printToPDF) in the Chrome DevTools Protocol. This has been changes in order to
|
||||||
|
|
|
@ -6,84 +6,27 @@ Subject: support V8 sandboxed pointers
|
||||||
This refactors several allocators to allocate within the V8 memory cage,
|
This refactors several allocators to allocate within the V8 memory cage,
|
||||||
allowing them to be compatible with the V8_SANDBOXED_POINTERS feature.
|
allowing them to be compatible with the V8_SANDBOXED_POINTERS feature.
|
||||||
|
|
||||||
diff --git a/lib/internal/bootstrap/pre_execution.js b/lib/internal/bootstrap/pre_execution.js
|
|
||||||
index 4c459b58b5a048d9d8a4f15f4011e7cce68089f4..6fb4c8d4567aee5b313ad621ea42699a196f18c7 100644
|
|
||||||
--- a/lib/internal/bootstrap/pre_execution.js
|
|
||||||
+++ b/lib/internal/bootstrap/pre_execution.js
|
|
||||||
@@ -14,7 +14,6 @@ const {
|
|
||||||
getOptionValue,
|
|
||||||
getEmbedderOptions,
|
|
||||||
} = require('internal/options');
|
|
||||||
-const { reconnectZeroFillToggle } = require('internal/buffer');
|
|
||||||
const {
|
|
||||||
defineOperation,
|
|
||||||
emitExperimentalWarning,
|
|
||||||
@@ -26,10 +25,6 @@ const { ERR_MANIFEST_ASSERT_INTEGRITY } = require('internal/errors').codes;
|
|
||||||
const assert = require('internal/assert');
|
|
||||||
|
|
||||||
function prepareMainThreadExecution(expandArgv1 = false) {
|
|
||||||
- // TODO(joyeecheung): this is also necessary for workers when they deserialize
|
|
||||||
- // this toggle from the snapshot.
|
|
||||||
- reconnectZeroFillToggle();
|
|
||||||
-
|
|
||||||
// Patch the process object with legacy properties and normalizations
|
|
||||||
patchProcessObject(expandArgv1);
|
|
||||||
setupTraceCategoryState();
|
|
||||||
diff --git a/lib/internal/buffer.js b/lib/internal/buffer.js
|
|
||||||
index bd38cf48a7fc6e8d61d8f11fa15c34aee182cbe3..1aa071cdc071dcdaf5c3b4bed0d3d76e5871731d 100644
|
|
||||||
--- a/lib/internal/buffer.js
|
|
||||||
+++ b/lib/internal/buffer.js
|
|
||||||
@@ -30,7 +30,7 @@ const {
|
|
||||||
hexWrite,
|
|
||||||
ucs2Write,
|
|
||||||
utf8Write,
|
|
||||||
- getZeroFillToggle
|
|
||||||
+ setZeroFillToggle
|
|
||||||
} = internalBinding('buffer');
|
|
||||||
const {
|
|
||||||
untransferable_object_private_symbol,
|
|
||||||
@@ -1055,24 +1055,15 @@ function markAsUntransferable(obj) {
|
|
||||||
// in C++.
|
|
||||||
// |zeroFill| can be undefined when running inside an isolate where we
|
|
||||||
// do not own the ArrayBuffer allocator. Zero fill is always on in that case.
|
|
||||||
-let zeroFill = getZeroFillToggle();
|
|
||||||
function createUnsafeBuffer(size) {
|
|
||||||
- zeroFill[0] = 0;
|
|
||||||
+ setZeroFillToggle(false);
|
|
||||||
try {
|
|
||||||
return new FastBuffer(size);
|
|
||||||
} finally {
|
|
||||||
- zeroFill[0] = 1;
|
|
||||||
+ setZeroFillToggle(true)
|
|
||||||
}
|
|
||||||
}
|
|
||||||
|
|
||||||
-// The connection between the JS land zero fill toggle and the
|
|
||||||
-// C++ one in the NodeArrayBufferAllocator gets lost if the toggle
|
|
||||||
-// is deserialized from the snapshot, because V8 owns the underlying
|
|
||||||
-// memory of this toggle. This resets the connection.
|
|
||||||
-function reconnectZeroFillToggle() {
|
|
||||||
- zeroFill = getZeroFillToggle();
|
|
||||||
-}
|
|
||||||
-
|
|
||||||
module.exports = {
|
|
||||||
FastBuffer,
|
|
||||||
addBufferPrototypeMethods,
|
|
||||||
@@ -1080,5 +1071,4 @@ module.exports = {
|
|
||||||
createUnsafeBuffer,
|
|
||||||
readUInt16BE,
|
|
||||||
readUInt32BE,
|
|
||||||
- reconnectZeroFillToggle
|
|
||||||
};
|
|
||||||
diff --git a/src/api/environment.cc b/src/api/environment.cc
|
diff --git a/src/api/environment.cc b/src/api/environment.cc
|
||||||
index 2abf5994405e8da2a04d1b23b75ccd3658398474..024d612a04d83583b397549589d994e32cf0107f 100644
|
index 2abf5994405e8da2a04d1b23b75ccd3658398474..b06e8529bb8ca2fa6d7f0735531bbbf39da6af12 100644
|
||||||
--- a/src/api/environment.cc
|
--- a/src/api/environment.cc
|
||||||
+++ b/src/api/environment.cc
|
+++ b/src/api/environment.cc
|
||||||
@@ -83,16 +83,16 @@ MaybeLocal<Value> PrepareStackTraceCallback(Local<Context> context,
|
@@ -80,19 +80,27 @@ MaybeLocal<Value> PrepareStackTraceCallback(Local<Context> context,
|
||||||
|
return result;
|
||||||
|
}
|
||||||
|
|
||||||
|
+NodeArrayBufferAllocator::NodeArrayBufferAllocator() {
|
||||||
|
+ zero_fill_field_ = static_cast<uint32_t*>(allocator_->Allocate(sizeof(*zero_fill_field_)));
|
||||||
|
+}
|
||||||
|
+
|
||||||
|
+NodeArrayBufferAllocator::~NodeArrayBufferAllocator() {
|
||||||
|
+ allocator_->Free(zero_fill_field_, sizeof(*zero_fill_field_));
|
||||||
|
+}
|
||||||
|
+
|
||||||
void* NodeArrayBufferAllocator::Allocate(size_t size) {
|
void* NodeArrayBufferAllocator::Allocate(size_t size) {
|
||||||
void* ret;
|
void* ret;
|
||||||
if (zero_fill_field_ || per_process::cli_options->zero_fill_all_buffers)
|
- if (zero_fill_field_ || per_process::cli_options->zero_fill_all_buffers)
|
||||||
- ret = UncheckedCalloc(size);
|
- ret = UncheckedCalloc(size);
|
||||||
|
+ if (*zero_fill_field_ || per_process::cli_options->zero_fill_all_buffers)
|
||||||
+ ret = allocator_->Allocate(size);
|
+ ret = allocator_->Allocate(size);
|
||||||
else
|
else
|
||||||
- ret = UncheckedMalloc(size);
|
- ret = UncheckedMalloc(size);
|
||||||
|
@ -99,7 +42,7 @@ index 2abf5994405e8da2a04d1b23b75ccd3658398474..024d612a04d83583b397549589d994e3
|
||||||
if (LIKELY(ret != nullptr))
|
if (LIKELY(ret != nullptr))
|
||||||
total_mem_usage_.fetch_add(size, std::memory_order_relaxed);
|
total_mem_usage_.fetch_add(size, std::memory_order_relaxed);
|
||||||
return ret;
|
return ret;
|
||||||
@@ -100,7 +100,7 @@ void* NodeArrayBufferAllocator::AllocateUninitialized(size_t size) {
|
@@ -100,7 +108,7 @@ void* NodeArrayBufferAllocator::AllocateUninitialized(size_t size) {
|
||||||
|
|
||||||
void* NodeArrayBufferAllocator::Reallocate(
|
void* NodeArrayBufferAllocator::Reallocate(
|
||||||
void* data, size_t old_size, size_t size) {
|
void* data, size_t old_size, size_t size) {
|
||||||
|
@ -108,7 +51,7 @@ index 2abf5994405e8da2a04d1b23b75ccd3658398474..024d612a04d83583b397549589d994e3
|
||||||
if (LIKELY(ret != nullptr) || UNLIKELY(size == 0))
|
if (LIKELY(ret != nullptr) || UNLIKELY(size == 0))
|
||||||
total_mem_usage_.fetch_add(size - old_size, std::memory_order_relaxed);
|
total_mem_usage_.fetch_add(size - old_size, std::memory_order_relaxed);
|
||||||
return ret;
|
return ret;
|
||||||
@@ -108,7 +108,7 @@ void* NodeArrayBufferAllocator::Reallocate(
|
@@ -108,7 +116,7 @@ void* NodeArrayBufferAllocator::Reallocate(
|
||||||
|
|
||||||
void NodeArrayBufferAllocator::Free(void* data, size_t size) {
|
void NodeArrayBufferAllocator::Free(void* data, size_t size) {
|
||||||
total_mem_usage_.fetch_sub(size, std::memory_order_relaxed);
|
total_mem_usage_.fetch_sub(size, std::memory_order_relaxed);
|
||||||
|
@ -209,65 +152,6 @@ index c431159e6f77f8c86844bcadb86012b056d03372..9f57ac58d826cb0aae422ddca54e2136
|
||||||
|
|
||||||
v8::Local<v8::ArrayBuffer> ToArrayBuffer(Environment* env);
|
v8::Local<v8::ArrayBuffer> ToArrayBuffer(Environment* env);
|
||||||
|
|
||||||
diff --git a/src/node_buffer.cc b/src/node_buffer.cc
|
|
||||||
index 215bd8003aabe17e43ac780c723cfe971b437eae..eb00eb6f592e20f3c17a529f30b09673774eb1c1 100644
|
|
||||||
--- a/src/node_buffer.cc
|
|
||||||
+++ b/src/node_buffer.cc
|
|
||||||
@@ -1175,33 +1175,14 @@ void SetBufferPrototype(const FunctionCallbackInfo<Value>& args) {
|
|
||||||
env->set_buffer_prototype_object(proto);
|
|
||||||
}
|
|
||||||
|
|
||||||
-void GetZeroFillToggle(const FunctionCallbackInfo<Value>& args) {
|
|
||||||
+void SetZeroFillToggle(const FunctionCallbackInfo<Value>& args) {
|
|
||||||
Environment* env = Environment::GetCurrent(args);
|
|
||||||
NodeArrayBufferAllocator* allocator = env->isolate_data()->node_allocator();
|
|
||||||
Local<ArrayBuffer> ab;
|
|
||||||
- // It can be a nullptr when running inside an isolate where we
|
|
||||||
- // do not own the ArrayBuffer allocator.
|
|
||||||
- if (allocator == nullptr) {
|
|
||||||
- // Create a dummy Uint32Array - the JS land can only toggle the C++ land
|
|
||||||
- // setting when the allocator uses our toggle. With this the toggle in JS
|
|
||||||
- // land results in no-ops.
|
|
||||||
- ab = ArrayBuffer::New(env->isolate(), sizeof(uint32_t));
|
|
||||||
- } else {
|
|
||||||
+ if (allocator != nullptr) {
|
|
||||||
uint32_t* zero_fill_field = allocator->zero_fill_field();
|
|
||||||
- std::unique_ptr<BackingStore> backing =
|
|
||||||
- ArrayBuffer::NewBackingStore(zero_fill_field,
|
|
||||||
- sizeof(*zero_fill_field),
|
|
||||||
- [](void*, size_t, void*) {},
|
|
||||||
- nullptr);
|
|
||||||
- ab = ArrayBuffer::New(env->isolate(), std::move(backing));
|
|
||||||
+ *zero_fill_field = args[0]->BooleanValue(env->isolate());
|
|
||||||
}
|
|
||||||
-
|
|
||||||
- ab->SetPrivate(
|
|
||||||
- env->context(),
|
|
||||||
- env->untransferable_object_private_symbol(),
|
|
||||||
- True(env->isolate())).Check();
|
|
||||||
-
|
|
||||||
- args.GetReturnValue().Set(Uint32Array::New(ab, 0, 1));
|
|
||||||
}
|
|
||||||
|
|
||||||
void DetachArrayBuffer(const FunctionCallbackInfo<Value>& args) {
|
|
||||||
@@ -1310,7 +1291,7 @@ void Initialize(Local<Object> target,
|
|
||||||
env->SetMethod(target, "ucs2Write", StringWrite<UCS2>);
|
|
||||||
env->SetMethod(target, "utf8Write", StringWrite<UTF8>);
|
|
||||||
|
|
||||||
- env->SetMethod(target, "getZeroFillToggle", GetZeroFillToggle);
|
|
||||||
+ env->SetMethod(target, "setZeroFillToggle", SetZeroFillToggle);
|
|
||||||
}
|
|
||||||
|
|
||||||
} // anonymous namespace
|
|
||||||
@@ -1350,7 +1331,7 @@ void RegisterExternalReferences(ExternalReferenceRegistry* registry) {
|
|
||||||
registry->Register(StringWrite<HEX>);
|
|
||||||
registry->Register(StringWrite<UCS2>);
|
|
||||||
registry->Register(StringWrite<UTF8>);
|
|
||||||
- registry->Register(GetZeroFillToggle);
|
|
||||||
+ registry->Register(SetZeroFillToggle);
|
|
||||||
|
|
||||||
registry->Register(DetachArrayBuffer);
|
|
||||||
registry->Register(CopyArrayBuffer);
|
|
||||||
diff --git a/src/node_i18n.cc b/src/node_i18n.cc
|
diff --git a/src/node_i18n.cc b/src/node_i18n.cc
|
||||||
index c537a247f55ff070da1988fc8b7309b5692b5c18..59bfb597849cd5a94800d6c83b238ef77245243e 100644
|
index c537a247f55ff070da1988fc8b7309b5692b5c18..59bfb597849cd5a94800d6c83b238ef77245243e 100644
|
||||||
--- a/src/node_i18n.cc
|
--- a/src/node_i18n.cc
|
||||||
|
@ -282,12 +166,26 @@ index c537a247f55ff070da1988fc8b7309b5692b5c18..59bfb597849cd5a94800d6c83b238ef7
|
||||||
return ret;
|
return ret;
|
||||||
|
|
||||||
diff --git a/src/node_internals.h b/src/node_internals.h
|
diff --git a/src/node_internals.h b/src/node_internals.h
|
||||||
index d37be23cd63e82d4040777bd0e17ed449ec0b15b..0b66996f11c66800a7e21ee84fa101450b856227 100644
|
index d37be23cd63e82d4040777bd0e17ed449ec0b15b..eb84760593ff5fb5aa6a8104e8714099f24a67a0 100644
|
||||||
--- a/src/node_internals.h
|
--- a/src/node_internals.h
|
||||||
+++ b/src/node_internals.h
|
+++ b/src/node_internals.h
|
||||||
@@ -118,6 +118,8 @@ class NodeArrayBufferAllocator : public ArrayBufferAllocator {
|
@@ -97,7 +97,9 @@ bool InitializePrimordials(v8::Local<v8::Context> context);
|
||||||
|
|
||||||
|
class NodeArrayBufferAllocator : public ArrayBufferAllocator {
|
||||||
|
public:
|
||||||
|
- inline uint32_t* zero_fill_field() { return &zero_fill_field_; }
|
||||||
|
+ NodeArrayBufferAllocator();
|
||||||
|
+ ~NodeArrayBufferAllocator() override;
|
||||||
|
+ inline uint32_t* zero_fill_field() { return zero_fill_field_; }
|
||||||
|
|
||||||
|
void* Allocate(size_t size) override; // Defined in src/node.cc
|
||||||
|
void* AllocateUninitialized(size_t size) override;
|
||||||
|
@@ -116,8 +118,10 @@ class NodeArrayBufferAllocator : public ArrayBufferAllocator {
|
||||||
|
}
|
||||||
|
|
||||||
private:
|
private:
|
||||||
uint32_t zero_fill_field_ = 1; // Boolean but exposed as uint32 to JS land.
|
- uint32_t zero_fill_field_ = 1; // Boolean but exposed as uint32 to JS land.
|
||||||
|
+ uint32_t* zero_fill_field_ = nullptr; // Boolean but exposed as uint32 to JS land.
|
||||||
std::atomic<size_t> total_mem_usage_ {0};
|
std::atomic<size_t> total_mem_usage_ {0};
|
||||||
+
|
+
|
||||||
+ std::unique_ptr<v8::ArrayBuffer::Allocator> allocator_{v8::ArrayBuffer::Allocator::NewDefaultAllocator()};
|
+ std::unique_ptr<v8::ArrayBuffer::Allocator> allocator_{v8::ArrayBuffer::Allocator::NewDefaultAllocator()};
|
||||||
|
|
|
@ -10,4 +10,3 @@ revert_fix_cppgc_removed_deleted_cstors_in_cppheapcreateparams.patch
|
||||||
revert_runtime_dhceck_terminating_exception_in_microtasks.patch
|
revert_runtime_dhceck_terminating_exception_in_microtasks.patch
|
||||||
chore_disable_is_execution_terminating_dcheck.patch
|
chore_disable_is_execution_terminating_dcheck.patch
|
||||||
build_remove_legacy_oom_error_callback.patch
|
build_remove_legacy_oom_error_callback.patch
|
||||||
force_disable_v8_sandboxed_pointers.patch
|
|
||||||
|
|
|
@ -1,24 +0,0 @@
|
||||||
From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
|
|
||||||
From: Jeremy Rose <japthorp@slack-corp.com>
|
|
||||||
Date: Thu, 23 Jun 2022 16:49:28 -0700
|
|
||||||
Subject: force disable v8 sandboxed pointers
|
|
||||||
|
|
||||||
temporarily, until we can fix node to be compatible
|
|
||||||
|
|
||||||
diff --git a/BUILD.gn b/BUILD.gn
|
|
||||||
index 71ee04624868f23a2e204b5ff1e973b45e2f1202..6aa33fdd3c767eb0045db918d2535fe35c98ca4b 100644
|
|
||||||
--- a/BUILD.gn
|
|
||||||
+++ b/BUILD.gn
|
|
||||||
@@ -506,9 +506,9 @@ if (v8_enable_sandbox == "") {
|
|
||||||
}
|
|
||||||
|
|
||||||
# Enable sandboxed pointers when the sandbox is enabled.
|
|
||||||
-if (v8_enable_sandbox) {
|
|
||||||
- v8_enable_sandboxed_pointers = true
|
|
||||||
-}
|
|
||||||
+#if (v8_enable_sandbox) {
|
|
||||||
+# v8_enable_sandboxed_pointers = true
|
|
||||||
+#}
|
|
||||||
|
|
||||||
# Enable all available sandbox features if sandbox future is enabled.
|
|
||||||
if (v8_enable_sandbox_future) {
|
|
Loading…
Reference in a new issue