From 1e857bc03c92796fa9b952b6c84926a92f7f04d3 Mon Sep 17 00:00:00 2001 From: Andreu Botella Date: Mon, 26 Dec 2022 08:00:13 -0800 Subject: [PATCH] chore(core): Make `OpCtx` instances be realm-specific (#17174) --- core/ops.rs | 4 ++- core/ops_builtin_v8.rs | 3 ++- core/runtime.rs | 58 ++++++++++++++++++++++++++++++++---------- 3 files changed, 50 insertions(+), 15 deletions(-) diff --git a/core/ops.rs b/core/ops.rs index 8694324adf..65b5115329 100644 --- a/core/ops.rs +++ b/core/ops.rs @@ -153,8 +153,10 @@ pub fn to_op_result( pub struct OpCtx { pub id: OpId, pub state: Rc>, - pub decl: OpDecl, + pub decl: Rc, pub runtime_state: Weak>, + // Index of the current realm into `JsRuntimeState::known_realms`. + pub realm_idx: usize, } /// Maintains the resources and ops inside a JS runtime. diff --git a/core/ops_builtin_v8.rs b/core/ops_builtin_v8.rs index 880a87c8d7..5ef185757c 100644 --- a/core/ops_builtin_v8.rs +++ b/core/ops_builtin_v8.rs @@ -9,6 +9,7 @@ use crate::ops_builtin::WasmStreamingResource; use crate::resolve_url_or_path; use crate::serde_v8::from_v8; use crate::source_map::apply_source_map as apply_source_map_; +use crate::JsRealm; use crate::JsRuntime; use crate::OpDecl; use crate::ZeroCopyBuf; @@ -782,7 +783,7 @@ fn op_dispatch_exception( #[op(v8)] fn op_op_names(scope: &mut v8::HandleScope) -> Vec { - let state_rc = JsRuntime::state(scope); + let state_rc = JsRealm::state_from_scope(scope); let state = state_rc.borrow(); state .op_ctxs diff --git a/core/runtime.rs b/core/runtime.rs index f735f55758..1874409eb1 100644 --- a/core/runtime.rs +++ b/core/runtime.rs @@ -151,6 +151,9 @@ pub type CompiledWasmModuleStore = CrossIsolateStore; #[derive(Default)] pub(crate) struct ContextState { pub(crate) js_build_custom_error_cb: Option>, + // We don't explicitly re-read this prop but need the slice to live alongside + // the context + pub(crate) op_ctxs: Box<[OpCtx]>, } /// Internal state for JsRuntime which is stored in one of v8::Isolate's @@ -178,9 +181,6 @@ pub struct JsRuntimeState { pub(crate) unrefed_ops: HashSet, pub(crate) have_unpolled_ops: bool, pub(crate) op_state: Rc>, - #[allow(dead_code)] - // We don't explicitly re-read this prop but need the slice to live alongside the isolate - pub(crate) op_ctxs: Box<[OpCtx]>, pub(crate) shared_array_buffer_store: Option, pub(crate) compiled_wasm_module_store: Option, /// The error that was passed to an `op_dispatch_exception` call. @@ -407,7 +407,6 @@ impl JsRuntime { dispatched_exceptions: Default::default(), // Some fields are initialized later after isolate is created inspector: None, - op_ctxs: vec![].into_boxed_slice(), global_realm: None, known_realms: Vec::with_capacity(1), })); @@ -420,7 +419,8 @@ impl JsRuntime { id, state: op_state.clone(), runtime_state: weak.clone(), - decl, + decl: Rc::new(decl), + realm_idx: 0, }) .collect::>() .into_boxed_slice(); @@ -522,9 +522,13 @@ impl JsRuntime { (isolate, snapshot_options) }; - global_context - .open(&mut isolate) - .set_slot(&mut isolate, Rc::>::default()); + global_context.open(&mut isolate).set_slot( + &mut isolate, + Rc::new(RefCell::new(ContextState { + js_build_custom_error_cb: None, + op_ctxs, + })), + ); op_state.borrow_mut().put(isolate_ptr); let inspector = if options.inspector { @@ -543,7 +547,6 @@ impl JsRuntime { { let mut state = state_rc.borrow_mut(); state.global_realm = Some(JsRealm(global_context.clone())); - state.op_ctxs = op_ctxs; state.inspector = inspector; state .known_realms @@ -631,6 +634,23 @@ impl JsRuntime { /// constructed. pub fn create_realm(&mut self) -> Result { let realm = { + let realm_idx = self.state.borrow().known_realms.len(); + + let op_ctxs: Box<[OpCtx]> = self + .global_realm() + .state(self.v8_isolate()) + .borrow() + .op_ctxs + .iter() + .map(|op_ctx| OpCtx { + id: op_ctx.id, + state: op_ctx.state.clone(), + decl: op_ctx.decl.clone(), + runtime_state: op_ctx.runtime_state.clone(), + realm_idx, + }) + .collect(); + // SAFETY: Having the scope tied to self's lifetime makes it impossible to // reference JsRuntimeState::op_ctxs while the scope is alive. Here we // turn it into an unbound lifetime, which is sound because 1. it only @@ -639,12 +659,15 @@ impl JsRuntime { let scope = &mut v8::HandleScope::new(unsafe { &mut *(self.v8_isolate() as *mut v8::OwnedIsolate) }); - let context = bindings::initialize_context( + let context = + bindings::initialize_context(scope, &op_ctxs, self.snapshot_options); + context.set_slot( scope, - &self.state.borrow().op_ctxs, - self.snapshot_options, + Rc::new(RefCell::new(ContextState { + js_build_custom_error_cb: None, + op_ctxs, + })), ); - context.set_slot(scope, Rc::>::default()); self .state @@ -2408,6 +2431,15 @@ pub fn queue_async_op( None => unreachable!(), }; + // An op's realm (as given by `OpCtx::realm_idx`) must match the realm in + // which it is invoked. Otherwise, we might have cross-realm object exposure. + // deno_core doesn't currently support such exposure, even though embedders + // can cause them, so we panic in debug mode (since the check is expensive). + debug_assert_eq!( + runtime_state.borrow().known_realms[ctx.realm_idx].to_local(scope), + Some(scope.get_current_context()) + ); + match OpCall::eager(op) { // This calls promise.resolve() before the control goes back to userland JS. It works something // along the lines of: