fix(ops): disallow memory slices as inputs to async ops (denoland#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 <[email protected]>

ops/lib.rs

@@ -209,7 +209,7 @@ fn codegen_v8_async(
  let rust_i0 = special_args.len();
  let args_head = special_args.into_iter().collect::<TokenStream2>();
 
- let (arg_decls, args_tail, argc) = codegen_args(core, f, rust_i0, 1);
+ let (arg_decls, args_tail, argc) = codegen_args(core, f, rust_i0, 1, true);
  let type_params = exclude_lifetime_params(&f.sig.generics.params);
 
  let (pre_result, mut result_fut) = match asyncness {
@@ -330,7 +330,7 @@ fn codegen_v8_sync(
  .collect::<Vec<_>>();
  let rust_i0 = special_args.len();
  let args_head = special_args.into_iter().collect::<TokenStream2>();
- let (arg_decls, args_tail, argc) = codegen_args(core, f, rust_i0, 0);
+ let (arg_decls, args_tail, argc) = codegen_args(core, f, rust_i0, 0, false);
  let ret = codegen_sync_ret(core, &f.sig.output);
  let type_params = exclude_lifetime_params(&f.sig.generics.params);
 
@@ -380,6 +380,7 @@ fn codegen_args(
  f: &syn::ItemFn,
  rust_i0: usize, // Index of first generic arg in rust
  v8_i0: usize, // Index of first generic arg in v8/js
+ asyncness: bool,
 ) -> ArgumentDecl {
  let inputs = &f.sig.inputs.iter().skip(rust_i0).enumerate();
  let ident_seq: TokenStream2 = inputs
@@ -392,7 +393,7 @@ fn codegen_args(
  let decls: TokenStream2 = inputs
  .clone()
  .map(|(i, arg)| {
- codegen_arg(core, arg, format!("arg_{i}").as_ref(), v8_i0 + i)
+ codegen_arg(core, arg, format!("arg_{i}").as_ref(), v8_i0 + i, asyncness)
  })
  .collect();
  (decls, ident_seq, inputs.len())
@@ -403,6 +404,7 @@ fn codegen_arg(
  arg: &syn::FnArg,
  name: &str,
  idx: usize,
+ asyncness: bool,
 ) -> TokenStream2 {
  let ident = quote::format_ident!("{name}");
  let (pat, ty) = match arg {
@@ -444,12 +446,14 @@ fn codegen_arg(
  match is_ref_slice(&**ty) {
  None => {}
  Some(SliceType::U32Mut) => {
+ assert!(!asyncness, "Memory slices are not allowed in async ops");
  let blck = codegen_u32_mut_slice(core, idx);
  return quote! {
  let #ident = #blck;
  };
  }
  Some(_) => {
+ assert!(!asyncness, "Memory slices are not allowed in async ops");
  let blck = codegen_u8_slice(core, idx);
  return quote! {
  let #ident = #blck;

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

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<RefCell<OpState>>,
- rid: ResourceId,
- buf: &mut [u8],
- ) -> Result<u32, Error> {}
+ async fn call(state: Rc<RefCell<OpState>>, rid: ResourceId) -> Result<u32, Error> {}
  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::<deno_core::v8::ArrayBuffer>::try_from(value) {
- Ok(b) => {
- let byte_length = b.byte_length();
- if let Some(data) = b.data() {
- let store = data.cast::<u8>().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::<u8>().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<deno_core::v8::Object>,
  __promise_id: i32,
  rid: ResourceId,
- buf: *const deno_core::v8::fast_api::FastApiTypedArray<u8>,
  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);

ops/optimizer_tests/async_result.rs

@@ -1,7 +1,6 @@
-async fn op_read(
+async fn op_async_result(
  state: Rc<RefCell<OpState>>,
  rid: ResourceId,
- buf: &mut [u8],
 ) -> Result<u32, Error> {
  // @test-attr:fast
 }

ops/tests/compile_fail/mem_slices.rs

@@ -0,0 +1,24 @@
+// Copyright 2018-2023 the Deno authors. All rights reserved. MIT license.
+
+use deno_ops::op;
+
+#[op]
+fn sync_test(slice: &mut [u32]) {
+ //
+}
+
+#[op]
+async fn async_test(slice: &[u8]) {
+ // Memory slices are not allowed in async ops.
+}
+
+#[op]
+fn async_test2(slice: &mut [u8]) -> impl Future<Output = ()> {
+ // Memory slices are not allowed in async ops, even when not implemented as an
+ // async function.
+ async {}
+}
+
+fn main() {
+ // pass
+}

ops/tests/compile_fail/mem_slices.stderr

@@ -0,0 +1,15 @@
+error: custom attribute panicked
+ --> tests/compile_fail/mem_slices.rs:10:1
+ |
+10 | #[op]
+ | ^^^^^
+ |
+ = help: message: Memory slices are not allowed in async ops
+
+error: custom attribute panicked
+ --> tests/compile_fail/mem_slices.rs:15:1
+ |
+15 | #[op]
+ | ^^^^^
+ |
+ = help: message: Memory slices are not allowed in async ops