From 90c03812720c328dcb68939400b265128b6eca8f Mon Sep 17 00:00:00 2001 From: Andreu Botella Date: Sat, 14 Jan 2023 23:40:01 -0800 Subject: fix(ops): disallow memory slices as inputs to async ops (#16738) In Rust, it is UB if a slice is mutated while borrowed except through the slice itself, and it is also UB if a mutable slice is read while borrowed. The op macro allows borrowing an `ArrayBuffer{,View}` as a memory slice for the duration of an op, but this is not sound for async ops, since the `ArrayBuffer` could be accessed from JS during the await points. This PR therefore disallows such automatic borrowing only for async ops. Co-authored-by: Divy Srivastava --- ops/optimizer_tests/async_result.expected | 6 +-- ops/optimizer_tests/async_result.out | 85 +++++-------------------------- ops/optimizer_tests/async_result.rs | 3 +- 3 files changed, 18 insertions(+), 76 deletions(-) (limited to 'ops/optimizer_tests') diff --git a/ops/optimizer_tests/async_result.expected b/ops/optimizer_tests/async_result.expected index be2e79ac0..87d46977d 100644 --- a/ops/optimizer_tests/async_result.expected +++ b/ops/optimizer_tests/async_result.expected @@ -3,9 +3,9 @@ returns_result: true has_ref_opstate: false has_rc_opstate: true has_fast_callback_option: false -needs_fast_callback_option: true +needs_fast_callback_option: false fast_result: None -fast_parameters: [V8Value, I32, U32, Uint8Array] -transforms: {2: Transform { kind: SliceU8(true), index: 2 }} +fast_parameters: [V8Value, I32, U32] +transforms: {} is_async: true fast_compatible: true diff --git a/ops/optimizer_tests/async_result.out b/ops/optimizer_tests/async_result.out index 3e4f9a49d..b812e4cef 100644 --- a/ops/optimizer_tests/async_result.out +++ b/ops/optimizer_tests/async_result.out @@ -1,13 +1,13 @@ #[allow(non_camel_case_types)] ///Auto-generated by `deno_ops`, i.e: `#[op]` /// -///Use `op_read::decl()` to get an op-declaration +///Use `op_async_result::decl()` to get an op-declaration ///you can include in a `deno_core::Extension`. -pub struct op_read; +pub struct op_async_result; #[doc(hidden)] -impl op_read { +impl op_async_result { pub fn name() -> &'static str { - stringify!(op_read) + stringify!(op_async_result) } pub fn v8_fn_ptr<'scope>() -> deno_core::v8::FunctionCallback { use deno_core::v8::MapFnTo; @@ -19,23 +19,19 @@ impl op_read { v8_fn_ptr: Self::v8_fn_ptr(), enabled: true, fast_fn: Some( - Box::new(op_read_fast { + Box::new(op_async_result_fast { _phantom: ::std::marker::PhantomData, }), ), is_async: true, is_unstable: false, is_v8: false, - argc: 2usize, + argc: 1usize, } } #[inline] #[allow(clippy::too_many_arguments)] - async fn call( - state: Rc>, - rid: ResourceId, - buf: &mut [u8], - ) -> Result {} + async fn call(state: Rc>, rid: ResourceId) -> Result {} pub fn v8_func<'scope>( scope: &mut deno_core::v8::HandleScope<'scope>, args: deno_core::v8::FunctionCallbackArguments, @@ -75,51 +71,6 @@ impl op_read { return deno_core::_ops::throw_type_error(scope, msg); } }; - let arg_1 = { - let value = args.get(2usize as i32); - match deno_core::v8::Local::::try_from(value) { - Ok(b) => { - let byte_length = b.byte_length(); - if let Some(data) = b.data() { - let store = data.cast::().as_ptr(); - unsafe { ::std::slice::from_raw_parts_mut(store, byte_length) } - } else { - &mut [] - } - } - Err(_) => { - if let Ok(view) - = deno_core::v8::Local::< - deno_core::v8::ArrayBufferView, - >::try_from(value) { - let len = view.byte_length(); - let offset = view.byte_offset(); - let buffer = match view.buffer(scope) { - Some(v) => v, - None => { - return deno_core::_ops::throw_type_error( - scope, - format!("Expected ArrayBufferView at position {}", 2usize), - ); - } - }; - if let Some(data) = buffer.data() { - let store = data.cast::().as_ptr(); - unsafe { - ::std::slice::from_raw_parts_mut(store.add(offset), len) - } - } else { - &mut [] - } - } else { - return deno_core::_ops::throw_type_error( - scope, - format!("Expected ArrayBufferView at position {}", 2usize), - ); - } - } - } - }; let get_class = { let state = ::std::cell::RefCell::borrow(&ctx.state); state.tracker.track_async(op_id); @@ -130,7 +81,7 @@ impl op_read { scope, false, async move { - let result = Self::call(ctx.state.clone(), arg_0, arg_1).await; + let result = Self::call(ctx.state.clone(), arg_0).await; ( realm_idx, promise_id, @@ -141,27 +92,26 @@ impl op_read { ); } } -struct op_read_fast { +struct op_async_result_fast { _phantom: ::std::marker::PhantomData<()>, } -impl<'scope> deno_core::v8::fast_api::FastFunction for op_read_fast { +impl<'scope> deno_core::v8::fast_api::FastFunction for op_async_result_fast { fn function(&self) -> *const ::std::ffi::c_void { - op_read_fast_fn as *const ::std::ffi::c_void + op_async_result_fast_fn as *const ::std::ffi::c_void } fn args(&self) -> &'static [deno_core::v8::fast_api::Type] { use deno_core::v8::fast_api::Type::*; use deno_core::v8::fast_api::CType; - &[V8Value, Int32, Uint32, TypedArray(CType::Uint8), CallbackOptions] + &[V8Value, Int32, Uint32, CallbackOptions] } fn return_type(&self) -> deno_core::v8::fast_api::CType { deno_core::v8::fast_api::CType::Void } } -fn op_read_fast_fn<'scope>( +fn op_async_result_fast_fn<'scope>( _: deno_core::v8::Local, __promise_id: i32, rid: ResourceId, - buf: *const deno_core::v8::fast_api::FastApiTypedArray, fast_api_callback_options: *mut deno_core::v8::fast_api::FastApiCallbackOptions, ) -> () { use deno_core::v8; @@ -174,14 +124,7 @@ fn op_read_fast_fn<'scope>( as *const _ops::OpCtx) }; let state = __ctx.state.clone(); - let buf = match unsafe { &*buf }.get_storage_if_aligned() { - Some(v) => v, - None => { - unsafe { &mut *fast_api_callback_options }.fallback = true; - return Default::default(); - } - }; - let result = op_read::call(state, rid, buf); + let result = op_async_result::call(state, rid); let __op_id = __ctx.id; let __state = ::std::cell::RefCell::borrow(&__ctx.state); __state.tracker.track_async(__op_id); diff --git a/ops/optimizer_tests/async_result.rs b/ops/optimizer_tests/async_result.rs index c1602ec49..54ac20d3c 100644 --- a/ops/optimizer_tests/async_result.rs +++ b/ops/optimizer_tests/async_result.rs @@ -1,7 +1,6 @@ -async fn op_read( +async fn op_async_result( state: Rc>, rid: ResourceId, - buf: &mut [u8], ) -> Result { // @test-attr:fast } -- cgit v1.2.3