From 85cc04df1458ed40042677348baceeab12bce3de Mon Sep 17 00:00:00 2001 From: Andrew Brown Date: Tue, 8 Nov 2022 11:03:14 -0800 Subject: [PATCH] wiggle: adapt Wiggle guest slices for `unsafe` shared use When multiple threads can concurrently modify a WebAssembly shared memory, the underlying data for a Wiggle `GuestSlice` and `GuestSliceMut` could change due to access from other threads. This breaks Rust guarantees when `&[T]` and `&mut [T]` slices are handed out. This change modifies `GuestPtr` to make `as_slice` and `as_slice_mut` return an `Option` which is `None` when the underlying WebAssembly memory is shared. But WASI implementations still need access to the underlying WebAssembly memory, both to read to it and write from it. This change adds new APIs: - `GuestPtr::to_vec` copies the bytes from WebAssembly memory (from which we can safely take a `&[T]`) - `GuestPtr::as_unsafe_slice_mut` returns a wrapper `struct` from which we can `unsafe`-ly return a mutable slice (users must accept the unsafety of concurrently modifying a `&mut [T]`) This approach allows us to maintain Wiggle's borrow-checking infrastructure, which enforces the guarantee that Wiggle will not modify overlapping regions, e.g. This is important because the underlying system calls may expect this. Though other threads may modify the same underlying region, this is impossible to prevent; at least Wiggle will not be able to do so. Finally, the changes to Wiggle's API are propagated to all WASI implementations in Wasmtime. For now, code locations that attempt to get a guest slice will panic if the underlying memory is shared. Note that Wiggle is not enabled for shared memory (that will come later in something like #5054), but when it is, these panics will be clear indicators of locations that must be re-implemented in a thread-safe way. --- crates/wasi-common/src/dir.rs | 1 - crates/wasi-common/src/snapshots/preview_0.rs | 48 ++++-- crates/wasi-common/src/snapshots/preview_1.rs | 88 ++++++---- .../wiggle_interfaces/asymmetric_common.rs | 30 +++- .../src/wiggle_interfaces/common.rs | 31 +++- .../src/wiggle_interfaces/key_exchange.rs | 3 +- .../src/wiggle_interfaces/signatures.rs | 15 +- .../src/wiggle_interfaces/symmetric.rs | 83 ++++++++-- crates/wasi-nn/src/impl.rs | 5 +- crates/wasi-nn/src/openvino.rs | 17 +- crates/wiggle/src/lib.rs | 155 ++++++++++++++---- crates/wiggle/tests/wasi.rs | 5 +- 12 files changed, 363 insertions(+), 118 deletions(-) diff --git a/crates/wasi-common/src/dir.rs b/crates/wasi-common/src/dir.rs index 56a8849db468..5783d284efdc 100644 --- a/crates/wasi-common/src/dir.rs +++ b/crates/wasi-common/src/dir.rs @@ -207,7 +207,6 @@ pub(crate) trait TableDirExt { fn get_dir(&self, fd: u32) -> Result<&DirEntry, Error>; fn is_preopen(&self, fd: u32) -> bool; } - impl TableDirExt for crate::table::Table { fn get_dir(&self, fd: u32) -> Result<&DirEntry, Error> { self.get(fd) diff --git a/crates/wasi-common/src/snapshots/preview_0.rs b/crates/wasi-common/src/snapshots/preview_0.rs index d04b471c1847..587be43aa6b6 100644 --- a/crates/wasi-common/src/snapshots/preview_0.rs +++ b/crates/wasi-common/src/snapshots/preview_0.rs @@ -468,14 +468,16 @@ impl wasi_unstable::WasiUnstable for WasiCtx { .get_file_mut(u32::from(fd))? .get_cap_mut(FileCaps::READ)?; - let mut guest_slices: Vec> = iovs - .iter() - .map(|iov_ptr| { - let iov_ptr = iov_ptr?; - let iov: types::Iovec = iov_ptr.read()?; - Ok(iov.buf.as_array(iov.buf_len).as_slice_mut()?) - }) - .collect::>()?; + let mut guest_slices: Vec> = + iovs.iter() + .map(|iov_ptr| { + let iov_ptr = iov_ptr?; + let iov: types::Iovec = iov_ptr.read()?; + Ok(iov.buf.as_array(iov.buf_len).as_slice_mut()?.expect( + "cannot use with shared memories; use `as_unsafe_slice_mut` instead", + )) + }) + .collect::>()?; let mut ioslices: Vec = guest_slices .iter_mut() @@ -497,14 +499,16 @@ impl wasi_unstable::WasiUnstable for WasiCtx { .get_file_mut(u32::from(fd))? .get_cap_mut(FileCaps::READ | FileCaps::SEEK)?; - let mut guest_slices: Vec> = iovs - .iter() - .map(|iov_ptr| { - let iov_ptr = iov_ptr?; - let iov: types::Iovec = iov_ptr.read()?; - Ok(iov.buf.as_array(iov.buf_len).as_slice_mut()?) - }) - .collect::>()?; + let mut guest_slices: Vec> = + iovs.iter() + .map(|iov_ptr| { + let iov_ptr = iov_ptr?; + let iov: types::Iovec = iov_ptr.read()?; + Ok(iov.buf.as_array(iov.buf_len).as_slice_mut()?.expect( + "cannot use with shared memories; use `as_unsafe_slice_mut` instead", + )) + }) + .collect::>()?; let mut ioslices: Vec = guest_slices .iter_mut() @@ -530,7 +534,11 @@ impl wasi_unstable::WasiUnstable for WasiCtx { .map(|iov_ptr| { let iov_ptr = iov_ptr?; let iov: types::Ciovec = iov_ptr.read()?; - Ok(iov.buf.as_array(iov.buf_len).as_slice()?) + Ok(iov + .buf + .as_array(iov.buf_len) + .as_slice()? + .expect("cannot use with shared memories; use `to_vec` instead")) }) .collect::>()?; @@ -559,7 +567,11 @@ impl wasi_unstable::WasiUnstable for WasiCtx { .map(|iov_ptr| { let iov_ptr = iov_ptr?; let iov: types::Ciovec = iov_ptr.read()?; - Ok(iov.buf.as_array(iov.buf_len).as_slice()?) + Ok(iov + .buf + .as_array(iov.buf_len) + .as_slice()? + .expect("cannot use with shared memories; use `to_vec` instead")) }) .collect::>()?; diff --git a/crates/wasi-common/src/snapshots/preview_1.rs b/crates/wasi-common/src/snapshots/preview_1.rs index ca7dde519d10..be601652bf2a 100644 --- a/crates/wasi-common/src/snapshots/preview_1.rs +++ b/crates/wasi-common/src/snapshots/preview_1.rs @@ -521,14 +521,16 @@ impl wasi_snapshot_preview1::WasiSnapshotPreview1 for WasiCtx { .get_file_mut(u32::from(fd))? .get_cap_mut(FileCaps::READ)?; - let mut guest_slices: Vec> = iovs - .iter() - .map(|iov_ptr| { - let iov_ptr = iov_ptr?; - let iov: types::Iovec = iov_ptr.read()?; - Ok(iov.buf.as_array(iov.buf_len).as_slice_mut()?) - }) - .collect::>()?; + let mut guest_slices: Vec> = + iovs.iter() + .map(|iov_ptr| { + let iov_ptr = iov_ptr?; + let iov: types::Iovec = iov_ptr.read()?; + Ok(iov.buf.as_array(iov.buf_len).as_slice_mut()?.expect( + "cannot use with shared memories; use `as_unsafe_slice_mut` instead", + )) + }) + .collect::>()?; let mut ioslices: Vec = guest_slices .iter_mut() @@ -550,14 +552,16 @@ impl wasi_snapshot_preview1::WasiSnapshotPreview1 for WasiCtx { .get_file_mut(u32::from(fd))? .get_cap_mut(FileCaps::READ | FileCaps::SEEK)?; - let mut guest_slices: Vec> = iovs - .iter() - .map(|iov_ptr| { - let iov_ptr = iov_ptr?; - let iov: types::Iovec = iov_ptr.read()?; - Ok(iov.buf.as_array(iov.buf_len).as_slice_mut()?) - }) - .collect::>()?; + let mut guest_slices: Vec> = + iovs.iter() + .map(|iov_ptr| { + let iov_ptr = iov_ptr?; + let iov: types::Iovec = iov_ptr.read()?; + Ok(iov.buf.as_array(iov.buf_len).as_slice_mut()?.expect( + "cannot use with shared memories; use `as_unsafe_slice_mut` instead", + )) + }) + .collect::>()?; let mut ioslices: Vec = guest_slices .iter_mut() @@ -583,7 +587,11 @@ impl wasi_snapshot_preview1::WasiSnapshotPreview1 for WasiCtx { .map(|iov_ptr| { let iov_ptr = iov_ptr?; let iov: types::Ciovec = iov_ptr.read()?; - Ok(iov.buf.as_array(iov.buf_len).as_slice()?) + Ok(iov + .buf + .as_array(iov.buf_len) + .as_slice()? + .expect("cannot use with shared memories; use `to_vec` instead")) }) .collect::>()?; @@ -612,7 +620,11 @@ impl wasi_snapshot_preview1::WasiSnapshotPreview1 for WasiCtx { .map(|iov_ptr| { let iov_ptr = iov_ptr?; let iov: types::Ciovec = iov_ptr.read()?; - Ok(iov.buf.as_array(iov.buf_len).as_slice()?) + Ok(iov + .buf + .as_array(iov.buf_len) + .as_slice()? + .expect("cannot use with shared memories; use `to_vec` instead")) }) .collect::>()?; @@ -654,7 +666,10 @@ impl wasi_snapshot_preview1::WasiSnapshotPreview1 for WasiCtx { if path_len < path_max_len as usize { return Err(Error::name_too_long()); } - let mut p_memory = path.as_array(path_len as u32).as_slice_mut()?; + let mut p_memory = path + .as_array(path_len as u32) + .as_slice_mut()? + .expect("cannot use with shared memories; use `as_unsafe_slice_mut` instead"); p_memory.copy_from_slice(path_bytes); Ok(()) } else { @@ -948,7 +963,10 @@ impl wasi_snapshot_preview1::WasiSnapshotPreview1 for WasiCtx { if link_len > buf_len as usize { return Err(Error::range()); } - let mut buf = buf.as_array(link_len as u32).as_slice_mut()?; + let mut buf = buf + .as_array(link_len as u32) + .as_slice_mut()? + .expect("cannot use with shared memories; use `as_unsafe_slice_mut` instead"); buf.copy_from_slice(link_bytes); Ok(link_len as types::Size) } @@ -1236,7 +1254,10 @@ impl wasi_snapshot_preview1::WasiSnapshotPreview1 for WasiCtx { buf: &GuestPtr<'a, u8>, buf_len: types::Size, ) -> Result<(), Error> { - let mut buf = buf.as_array(buf_len).as_slice_mut()?; + let mut buf = buf + .as_array(buf_len) + .as_slice_mut()? + .expect("cannot use with shared memories; use `as_unsafe_slice_mut` instead"); self.random.try_fill_bytes(buf.deref_mut())?; Ok(()) } @@ -1273,14 +1294,17 @@ impl wasi_snapshot_preview1::WasiSnapshotPreview1 for WasiCtx { .get_file_mut(u32::from(fd))? .get_cap_mut(FileCaps::READ)?; - let mut guest_slices: Vec> = ri_data - .iter() - .map(|iov_ptr| { - let iov_ptr = iov_ptr?; - let iov: types::Iovec = iov_ptr.read()?; - Ok(iov.buf.as_array(iov.buf_len).as_slice_mut()?) - }) - .collect::>()?; + let mut guest_slices: Vec> = + ri_data + .iter() + .map(|iov_ptr| { + let iov_ptr = iov_ptr?; + let iov: types::Iovec = iov_ptr.read()?; + Ok(iov.buf.as_array(iov.buf_len).as_slice_mut()?.expect( + "cannot use with shared memories; use `as_unsafe_slice_mut` instead", + )) + }) + .collect::>()?; let mut ioslices: Vec = guest_slices .iter_mut() @@ -1307,7 +1331,11 @@ impl wasi_snapshot_preview1::WasiSnapshotPreview1 for WasiCtx { .map(|iov_ptr| { let iov_ptr = iov_ptr?; let iov: types::Ciovec = iov_ptr.read()?; - Ok(iov.buf.as_array(iov.buf_len).as_slice()?) + Ok(iov + .buf + .as_array(iov.buf_len) + .as_slice()? + .expect("cannot use with shared memories; use `to_vec` instead")) }) .collect::>()?; diff --git a/crates/wasi-crypto/src/wiggle_interfaces/asymmetric_common.rs b/crates/wasi-crypto/src/wiggle_interfaces/asymmetric_common.rs index 2d5c25a1f991..a7751de83b2c 100644 --- a/crates/wasi-crypto/src/wiggle_interfaces/asymmetric_common.rs +++ b/crates/wasi-crypto/src/wiggle_interfaces/asymmetric_common.rs @@ -39,7 +39,10 @@ impl super::wasi_ephemeral_crypto_asymmetric_common::WasiEphemeralCryptoAsymmetr kp_id_ptr: &wiggle::GuestPtr<'_, u8>, kp_id_max_len: guest_types::Size, ) -> Result<(), guest_types::CryptoErrno> { - let key_id_buf = &mut *kp_id_ptr.as_array(kp_id_max_len).as_slice_mut()?; + let key_id_buf = &mut *kp_id_ptr + .as_array(kp_id_max_len) + .as_slice_mut()? + .expect("cannot use with shared memories; use `as_unsafe_slice_mut` instead"); Ok((&*self).keypair_store_managed( secrets_manager_handle.into(), kp_handle.into(), @@ -69,7 +72,10 @@ impl super::wasi_ephemeral_crypto_asymmetric_common::WasiEphemeralCryptoAsymmetr kp_id_len: guest_types::Size, kp_version: guest_types::Version, ) -> Result { - let kp_id = &*kp_id_ptr.as_array(kp_id_len).as_slice()?; + let kp_id = &*kp_id_ptr + .as_array(kp_id_len) + .as_slice()? + .expect("cannot use with shared memories; use `to_vec` instead"); Ok((&*self) .keypair_from_id(secrets_manager_handle.into(), kp_id, Version(kp_version))? .into()) @@ -102,7 +108,10 @@ impl super::wasi_ephemeral_crypto_asymmetric_common::WasiEphemeralCryptoAsymmetr encoding: guest_types::KeypairEncoding, ) -> Result { let alg_str = &*alg_str.as_str()?; - let encoded = &*encoded_ptr.as_array(encoded_len).as_slice()?; + let encoded = &*encoded_ptr + .as_array(encoded_len) + .as_slice()? + .expect("cannot use with shared memories; use `to_vec` instead"); Ok((&*self) .keypair_import(alg_type.into(), alg_str, encoded, encoding.into())? .into()) @@ -114,7 +123,10 @@ impl super::wasi_ephemeral_crypto_asymmetric_common::WasiEphemeralCryptoAsymmetr kp_id_ptr: &wiggle::GuestPtr<'_, u8>, kp_id_max_len: guest_types::Size, ) -> Result<(guest_types::Size, guest_types::Version), guest_types::CryptoErrno> { - let kp_id_buf = &mut *kp_id_ptr.as_array(kp_id_max_len as _).as_slice_mut()?; + let kp_id_buf = &mut *kp_id_ptr + .as_array(kp_id_max_len as _) + .as_slice_mut()? + .expect("cannot use with shared memories; use `as_unsafe_slice_mut` instead"); let (kp_id, version) = (&*self).keypair_id(kp_handle.into())?; ensure!(kp_id.len() <= kp_id_buf.len(), CryptoError::Overflow.into()); kp_id_buf.copy_from_slice(&kp_id); @@ -156,7 +168,10 @@ impl super::wasi_ephemeral_crypto_asymmetric_common::WasiEphemeralCryptoAsymmetr encoding: guest_types::PublickeyEncoding, ) -> Result { let alg_str = &*alg_str.as_str()?; - let encoded = &*encoded_ptr.as_array(encoded_len).as_slice()?; + let encoded = &*encoded_ptr + .as_array(encoded_len) + .as_slice()? + .expect("cannot use with shared memories; use `to_vec` instead"); Ok((&*self) .publickey_import(alg_type.into(), alg_str, encoded, encoding.into())? .into()) @@ -204,7 +219,10 @@ impl super::wasi_ephemeral_crypto_asymmetric_common::WasiEphemeralCryptoAsymmetr encoding: guest_types::SecretkeyEncoding, ) -> Result { let alg_str = &*alg_str.as_str()?; - let encoded = &*encoded_ptr.as_array(encoded_len).as_slice()?; + let encoded = &*encoded_ptr + .as_array(encoded_len) + .as_slice()? + .expect("cannot use with shared memories; use `to_vec` instead"); Ok((&*self) .secretkey_import(alg_type.into(), alg_str, encoded, encoding.into())? .into()) diff --git a/crates/wasi-crypto/src/wiggle_interfaces/common.rs b/crates/wasi-crypto/src/wiggle_interfaces/common.rs index 8c5ed13cbde1..f91371d928c8 100644 --- a/crates/wasi-crypto/src/wiggle_interfaces/common.rs +++ b/crates/wasi-crypto/src/wiggle_interfaces/common.rs @@ -28,7 +28,12 @@ impl super::wasi_ephemeral_crypto_common::WasiEphemeralCryptoCommon for WasiCryp value_len: guest_types::Size, ) -> Result<(), guest_types::CryptoErrno> { let name_str: &str = &*name_str.as_str()?; - let value: &[u8] = { &*value_ptr.as_array(value_len).as_slice()? }; + let value: &[u8] = { + &*value_ptr + .as_array(value_len) + .as_slice()? + .expect("cannot use with shared memories; use `to_vec` instead") + }; Ok((&*self).options_set(options_handle.into(), name_str, value)?) } @@ -40,8 +45,14 @@ impl super::wasi_ephemeral_crypto_common::WasiEphemeralCryptoCommon for WasiCryp buffer_len: guest_types::Size, ) -> Result<(), guest_types::CryptoErrno> { let name_str: &str = &*name_str.as_str()?; - let buffer: &'static mut [u8] = - unsafe { std::mem::transmute(&mut *buffer_ptr.as_array(buffer_len).as_slice_mut()?) }; + let buffer: &'static mut [u8] = unsafe { + std::mem::transmute( + &mut *buffer_ptr + .as_array(buffer_len) + .as_slice_mut()? + .expect("cannot use with shared memories; use `as_unsafe_slice_mut` instead"), + ) + }; Ok((&*self).options_set_guest_buffer(options_handle.into(), name_str, buffer)?) } @@ -72,7 +83,12 @@ impl super::wasi_ephemeral_crypto_common::WasiEphemeralCryptoCommon for WasiCryp buf_ptr: &wiggle::GuestPtr<'_, u8>, buf_len: guest_types::Size, ) -> Result { - let buf: &mut [u8] = { &mut *buf_ptr.as_array(buf_len).as_slice_mut()? }; + let buf: &mut [u8] = { + &mut *buf_ptr + .as_array(buf_len) + .as_slice_mut()? + .expect("cannot use with shared memories; use `as_unsafe_slice_mut` instead") + }; Ok((&*self) .array_output_pull(array_output_handle.into(), buf)? .try_into()?) @@ -107,7 +123,12 @@ impl super::wasi_ephemeral_crypto_common::WasiEphemeralCryptoCommon for WasiCryp key_id_len: guest_types::Size, key_version: guest_types::Version, ) -> Result<(), guest_types::CryptoErrno> { - let key_id: &[u8] = { &*key_id_ptr.as_array(key_id_len).as_slice()? }; + let key_id: &[u8] = { + &*key_id_ptr + .as_array(key_id_len) + .as_slice()? + .expect("cannot use with shared memories; use `to_vec` instead") + }; Ok((&*self).secrets_manager_invalidate( secrets_manager_handle.into(), key_id, diff --git a/crates/wasi-crypto/src/wiggle_interfaces/key_exchange.rs b/crates/wasi-crypto/src/wiggle_interfaces/key_exchange.rs index 462d76864581..4217f9201a0f 100644 --- a/crates/wasi-crypto/src/wiggle_interfaces/key_exchange.rs +++ b/crates/wasi-crypto/src/wiggle_interfaces/key_exchange.rs @@ -31,7 +31,8 @@ impl super::wasi_ephemeral_crypto_kx::WasiEphemeralCryptoKx for WasiCryptoCtx { ) -> Result { let encapsulated_secret = &*encapsulated_secret_ptr .as_array(encapsulated_secret_len) - .as_slice()?; + .as_slice()? + .expect("cannot use with shared memories; use `to_vec` instead"); Ok((&*self) .kx_decapsulate(sk_handle.into(), encapsulated_secret)? .into()) diff --git a/crates/wasi-crypto/src/wiggle_interfaces/signatures.rs b/crates/wasi-crypto/src/wiggle_interfaces/signatures.rs index ff4db33e0a14..d689efc0e34b 100644 --- a/crates/wasi-crypto/src/wiggle_interfaces/signatures.rs +++ b/crates/wasi-crypto/src/wiggle_interfaces/signatures.rs @@ -23,7 +23,10 @@ impl super::wasi_ephemeral_crypto_signatures::WasiEphemeralCryptoSignatures for encoding: guest_types::SignatureEncoding, ) -> Result { let alg_str = &*alg_str.as_str()?; - let encoded = &*encoded_ptr.as_array(encoded_len).as_slice()?; + let encoded = &*encoded_ptr + .as_array(encoded_len) + .as_slice()? + .expect("cannot use with shared memories; use `to_vec` instead"); Ok((&*self) .signature_import(alg_str, encoded, encoding.into())? .into()) @@ -42,7 +45,10 @@ impl super::wasi_ephemeral_crypto_signatures::WasiEphemeralCryptoSignatures for input_ptr: &wiggle::GuestPtr<'_, u8>, input_len: guest_types::Size, ) -> Result<(), guest_types::CryptoErrno> { - let input = &*input_ptr.as_array(input_len).as_slice()?; + let input = &*input_ptr + .as_array(input_len) + .as_slice()? + .expect("cannot use with shared memories; use `to_vec` instead"); Ok((&*self).signature_state_update(state_handle.into(), input)?) } @@ -77,7 +83,10 @@ impl super::wasi_ephemeral_crypto_signatures::WasiEphemeralCryptoSignatures for input_ptr: &wiggle::GuestPtr<'_, u8>, input_len: guest_types::Size, ) -> Result<(), guest_types::CryptoErrno> { - let input: &[u8] = &*input_ptr.as_array(input_len).as_slice()?; + let input: &[u8] = &*input_ptr + .as_array(input_len) + .as_slice()? + .expect("cannot use with shared memories; use `to_vec` instead"); Ok( (&*self) .signature_verification_state_update(verification_state_handle.into(), input)?, diff --git a/crates/wasi-crypto/src/wiggle_interfaces/symmetric.rs b/crates/wasi-crypto/src/wiggle_interfaces/symmetric.rs index 881c3f5d65fc..bf1e7de4fbad 100644 --- a/crates/wasi-crypto/src/wiggle_interfaces/symmetric.rs +++ b/crates/wasi-crypto/src/wiggle_interfaces/symmetric.rs @@ -35,7 +35,8 @@ impl super::wasi_ephemeral_crypto_symmetric::WasiEphemeralCryptoSymmetric for Wa ) -> Result<(), guest_types::CryptoErrno> { let key_id_buf = &mut *symmetric_key_id_ptr .as_array(symmetric_key_id_max_len) - .as_slice_mut()?; + .as_slice_mut()? + .expect("cannot use with shared memories; use `as_unsafe_slice_mut` instead"); Ok((&*self).symmetric_key_store_managed( secrets_manager_handle.into(), symmetric_key_handle.into(), @@ -67,7 +68,8 @@ impl super::wasi_ephemeral_crypto_symmetric::WasiEphemeralCryptoSymmetric for Wa ) -> Result { let symmetric_key_id = &*symmetric_key_id_ptr .as_array(symmetric_key_id_len) - .as_slice()?; + .as_slice()? + .expect("cannot use with shared memories; use `to_vec` instead"); Ok((&*self) .symmetric_key_from_id( secrets_manager_handle.into(), @@ -101,7 +103,10 @@ impl super::wasi_ephemeral_crypto_symmetric::WasiEphemeralCryptoSymmetric for Wa raw_len: guest_types::Size, ) -> Result { let alg_str = &*alg_str.as_str()?; - let raw = &*raw_ptr.as_array(raw_len).as_slice()?; + let raw = &*raw_ptr + .as_array(raw_len) + .as_slice()? + .expect("cannot use with shared memories; use `to_vec` instead"); Ok((&*self).symmetric_key_import(alg_str, raw)?.into()) } @@ -122,7 +127,8 @@ impl super::wasi_ephemeral_crypto_symmetric::WasiEphemeralCryptoSymmetric for Wa ) -> Result<(guest_types::Size, guest_types::Version), guest_types::CryptoErrno> { let key_id_buf = &mut *symmetric_key_id_ptr .as_array(symmetric_key_id_max_len) - .as_slice_mut()?; + .as_slice_mut()? + .expect("cannot use with shared memories; use `as_unsafe_slice_mut` instead"); let (key_id, version) = (&*self).symmetric_key_id(symmetric_key_handle.into())?; ensure!( key_id.len() <= key_id_buf.len(), @@ -173,7 +179,9 @@ impl super::wasi_ephemeral_crypto_symmetric::WasiEphemeralCryptoSymmetric for Wa value_max_len: guest_types::Size, ) -> Result { let name_str: &str = &*name_str.as_str()?; - let value = &mut *value_ptr.as_array(value_max_len).as_slice_mut()?; + let value = &mut *value_ptr.as_array(value_max_len).as_slice_mut()?.expect( + "cannot use with shared memories; use `as_unsafe_slice_mut` instead", + ); Ok((&*self) .options_get(symmetric_state_handle.into(), name_str, value)? .try_into()?) @@ -201,7 +209,10 @@ impl super::wasi_ephemeral_crypto_symmetric::WasiEphemeralCryptoSymmetric for Wa data_ptr: &wiggle::GuestPtr<'_, u8>, data_len: guest_types::Size, ) -> Result<(), guest_types::CryptoErrno> { - let data = &*data_ptr.as_array(data_len).as_slice()?; + let data = &*data_ptr + .as_array(data_len) + .as_slice()? + .expect("cannot use with shared memories; use `to_vec` instead"); Ok((&*self).symmetric_state_absorb(symmetric_state_handle.into(), data)?) } @@ -211,7 +222,10 @@ impl super::wasi_ephemeral_crypto_symmetric::WasiEphemeralCryptoSymmetric for Wa out_ptr: &wiggle::GuestPtr<'_, u8>, out_len: guest_types::Size, ) -> Result<(), guest_types::CryptoErrno> { - let out = &mut *out_ptr.as_array(out_len).as_slice_mut()?; + let out = &mut *out_ptr + .as_array(out_len) + .as_slice_mut()? + .expect("cannot use with shared memories; use `as_unsafe_slice_mut` instead"); Ok((&*self).symmetric_state_squeeze(symmetric_state_handle.into(), out)?) } @@ -252,8 +266,14 @@ impl super::wasi_ephemeral_crypto_symmetric::WasiEphemeralCryptoSymmetric for Wa data_ptr: &wiggle::GuestPtr<'_, u8>, data_len: guest_types::Size, ) -> Result { - let out = &mut *out_ptr.as_array(out_len).as_slice_mut()?; - let data = &*data_ptr.as_array(data_len).as_slice()?; + let out = &mut *out_ptr + .as_array(out_len) + .as_slice_mut()? + .expect("cannot use with shared memories; use `as_unsafe_slice_mut` instead"); + let data = &*data_ptr + .as_array(data_len) + .as_slice()? + .expect("cannot use with shared memories; use `to_vec` instead"); Ok((&*self) .symmetric_state_encrypt(symmetric_state_handle.into(), out, data)? .try_into()?) @@ -267,8 +287,14 @@ impl super::wasi_ephemeral_crypto_symmetric::WasiEphemeralCryptoSymmetric for Wa data_ptr: &wiggle::GuestPtr<'_, u8>, data_len: guest_types::Size, ) -> Result { - let out = &mut *out_ptr.as_array(out_len).as_slice_mut()?; - let data = &*data_ptr.as_array(data_len).as_slice()?; + let out = &mut *out_ptr + .as_array(out_len) + .as_slice_mut()? + .expect("cannot use with shared memories; use `as_unsafe_slice_mut` instead"); + let data = &*data_ptr + .as_array(data_len) + .as_slice()? + .expect("cannot use with shared memories; use `to_vec` instead"); Ok((&*self) .symmetric_state_encrypt_detached(symmetric_state_handle.into(), out, data)? .into()) @@ -282,8 +308,14 @@ impl super::wasi_ephemeral_crypto_symmetric::WasiEphemeralCryptoSymmetric for Wa data_ptr: &wiggle::GuestPtr<'_, u8>, data_len: guest_types::Size, ) -> Result { - let out = &mut *out_ptr.as_array(out_len).as_slice_mut()?; - let data = &*data_ptr.as_array(data_len).as_slice()?; + let out = &mut *out_ptr + .as_array(out_len) + .as_slice_mut()? + .expect("cannot use with shared memories; use `as_unsafe_slice_mut` instead"); + let data = &*data_ptr + .as_array(data_len) + .as_slice()? + .expect("cannot use with shared memories; use `to_vec` instead"); Ok((&*self) .symmetric_state_decrypt(symmetric_state_handle.into(), out, data)? .try_into()?) @@ -299,9 +331,18 @@ impl super::wasi_ephemeral_crypto_symmetric::WasiEphemeralCryptoSymmetric for Wa raw_tag_ptr: &wiggle::GuestPtr<'_, u8>, raw_tag_len: guest_types::Size, ) -> Result { - let out = &mut *out_ptr.as_array(out_len).as_slice_mut()?; - let data = &*data_ptr.as_array(data_len).as_slice()?; - let raw_tag: &[u8] = &*raw_tag_ptr.as_array(raw_tag_len).as_slice()?; + let out = &mut *out_ptr + .as_array(out_len) + .as_slice_mut()? + .expect("cannot use with shared memories; use `as_unsafe_slice_mut` instead"); + let data = &*data_ptr + .as_array(data_len) + .as_slice()? + .expect("cannot use with shared memories; use `to_vec` instead"); + let raw_tag: &[u8] = &*raw_tag_ptr + .as_array(raw_tag_len) + .as_slice()? + .expect("cannot use with shared memories; use `to_vec` instead"); Ok((&*self) .symmetric_state_decrypt_detached(symmetric_state_handle.into(), out, data, raw_tag)? .try_into()?) @@ -331,7 +372,10 @@ impl super::wasi_ephemeral_crypto_symmetric::WasiEphemeralCryptoSymmetric for Wa buf_ptr: &wiggle::GuestPtr<'_, u8>, buf_len: guest_types::Size, ) -> Result { - let buf = &mut *buf_ptr.as_array(buf_len).as_slice_mut()?; + let buf = &mut *buf_ptr + .as_array(buf_len) + .as_slice_mut()? + .expect("cannot use with shared memories; use `as_unsafe_slice_mut` instead"); Ok((&*self) .symmetric_tag_pull(symmetric_tag_handle.into(), buf)? .try_into()?) @@ -343,7 +387,10 @@ impl super::wasi_ephemeral_crypto_symmetric::WasiEphemeralCryptoSymmetric for Wa expected_raw_ptr: &wiggle::GuestPtr<'_, u8>, expected_raw_len: guest_types::Size, ) -> Result<(), guest_types::CryptoErrno> { - let expected_raw = &*expected_raw_ptr.as_array(expected_raw_len).as_slice()?; + let expected_raw = &*expected_raw_ptr + .as_array(expected_raw_len) + .as_slice()? + .expect("cannot use with shared memories; use `to_vec` instead"); Ok((&*self).symmetric_tag_verify(symmetric_tag_handle.into(), expected_raw)?) } diff --git a/crates/wasi-nn/src/impl.rs b/crates/wasi-nn/src/impl.rs index 85be8dd97c08..511109ed2a87 100644 --- a/crates/wasi-nn/src/impl.rs +++ b/crates/wasi-nn/src/impl.rs @@ -80,8 +80,11 @@ impl<'a> WasiEphemeralNn for WasiNnCtx { out_buffer: &GuestPtr<'_, u8>, out_buffer_max_size: u32, ) -> Result { - let mut destination = out_buffer.as_array(out_buffer_max_size).as_slice_mut()?; if let Some(exec_context) = self.executions.get_mut(exec_context_id) { + let mut destination = out_buffer + .as_array(out_buffer_max_size) + .as_slice_mut()? + .expect("cannot use with shared memories; use `as_unsafe_slice_mut` instead"); Ok(exec_context.get_output(index, &mut destination)?) } else { Err(UsageError::InvalidGraphHandle.into()) diff --git a/crates/wasi-nn/src/openvino.rs b/crates/wasi-nn/src/openvino.rs index 89f043455ce5..168abbc6ce24 100644 --- a/crates/wasi-nn/src/openvino.rs +++ b/crates/wasi-nn/src/openvino.rs @@ -31,8 +31,15 @@ impl Backend for OpenvinoBackend { // Read the guest array. let builders = builders.as_ptr(); - let xml = builders.read()?.as_slice()?; - let weights = builders.add(1)?.read()?.as_slice()?; + let xml = builders + .read()? + .as_slice()? + .expect("cannot use with shared memories; use `to_vec` instead"); + let weights = builders + .add(1)? + .read()? + .as_slice()? + .expect("cannot use with shared memories; use `to_vec` instead"); // Construct OpenVINO graph structures: `cnn_network` contains the graph // structure, `exec_network` can perform inference. @@ -78,6 +85,7 @@ impl BackendExecutionContext for OpenvinoExecutionContext { let dimensions = tensor .dimensions .as_slice()? + .expect("cannot use with shared memories; use `to_vec` instead") .iter() .map(|d| *d as usize) .collect::>(); @@ -86,7 +94,10 @@ impl BackendExecutionContext for OpenvinoExecutionContext { // TODO There must be some good way to discover the layout here; this // should not have to default to NHWC. let desc = TensorDesc::new(Layout::NHWC, &dimensions, precision); - let data = tensor.data.as_slice()?; + let data = tensor + .data + .as_slice()? + .expect("cannot use with shared memories; use `to_vec` instead"); let blob = openvino::Blob::new(&desc, &data)?; // Actually assign the blob to the request. diff --git a/crates/wiggle/src/lib.rs b/crates/wiggle/src/lib.rs index 79b02e4d3ab7..713820056f22 100644 --- a/crates/wiggle/src/lib.rs +++ b/crates/wiggle/src/lib.rs @@ -497,6 +497,17 @@ impl<'a, T> GuestPtr<'a, [T]> { self.pointer.1 } + /// Return the number of bytes necessary to represent the pointed-to value. + pub fn checked_byte_len(&self) -> Result + where + T: GuestTypeTransparent<'a>, + { + match self.pointer.1.checked_mul(T::guest_size()) { + Some(l) => Ok(l), + None => Err(GuestError::PtrOverflow), + } + } + /// Returns an iterator over interior pointers. /// /// Each item is a `Result` indicating whether it overflowed past the end of @@ -522,17 +533,19 @@ impl<'a, T> GuestPtr<'a, [T]> { /// This function will return a `GuestSlice` into host memory if all checks /// succeed (valid utf-8, valid pointers, memory is not borrowed, etc). If /// any checks fail then `GuestError` will be returned. - pub fn as_slice(&self) -> Result, GuestError> + pub fn as_slice(&self) -> Result>, GuestError> where T: GuestTypeTransparent<'a>, { - let len = match self.pointer.1.checked_mul(T::guest_size()) { - Some(l) => l, - None => return Err(GuestError::PtrOverflow), - }; - let ptr = - self.mem - .validate_size_align(self.pointer.0, T::guest_align(), len)? as *mut T; + if self.mem.is_shared_memory() { + return Ok(None); + } + + let len = self.checked_byte_len()?; + let ptr = self + .mem + .validate_size_align(self.pointer.0, T::guest_align(), len)? as *mut T + as *mut T; let borrow = self.mem.shared_borrow(Region { start: self.pointer.0, @@ -545,33 +558,35 @@ impl<'a, T> GuestPtr<'a, [T]> { T::validate(unsafe { ptr.add(offs as usize) })?; } - // SAFETY: iff there are no overlapping mut borrows it is valid to construct a &[T] + // SAFETY: iff there are no overlapping mut borrows it is valid to + // construct a &[T]. let ptr = unsafe { slice::from_raw_parts(ptr, self.pointer.1 as usize) }; - - Ok(GuestSlice { + Ok(Some(GuestSlice { ptr, mem: self.mem, borrow, - }) + })) } - /// Attempts to create a [`GuestSliceMut<'_, T>`] from this pointer, performing - /// bounds checks and type validation. The `GuestSliceMut` is a smart pointer - /// that can be used as a `&[T]` or a `&mut [T]` via the `Deref` and `DerefMut` - /// traits. The region of memory backing the slice will be marked as borrowed - /// by the [`GuestMemory`] until the `GuestSlice` is dropped. + /// Attempts to create a [`GuestSliceMut<'_, T>`] from this pointer, + /// performing bounds checks and type validation. The `GuestSliceMut` is a + /// smart pointer that can be used as a `&[T]` or a `&mut [T]` via the + /// `Deref` and `DerefMut` traits. The region of memory backing the slice + /// will be marked as borrowed by the [`GuestMemory`] until the `GuestSlice` + /// is dropped. /// - /// This function will return a `GuestSliceMut` into host memory if all checks - /// succeed (valid utf-8, valid pointers, memory is not borrowed, etc). If - /// any checks fail then `GuestError` will be returned. - pub fn as_slice_mut(&self) -> Result, GuestError> + /// This function will return a `GuestSliceMut` into host memory if all + /// checks succeed (valid utf-8, valid pointers, memory is not borrowed, + /// etc). If any checks fail then `GuestError` will be returned. + pub fn as_slice_mut(&self) -> Result>, GuestError> where T: GuestTypeTransparent<'a>, { - let len = match self.pointer.1.checked_mul(T::guest_size()) { - Some(l) => l, - None => return Err(GuestError::PtrOverflow), - }; + if self.mem.is_shared_memory() { + return Ok(None); + } + + let len = self.checked_byte_len()?; let ptr = self.mem .validate_size_align(self.pointer.0, T::guest_align(), len)? as *mut T; @@ -582,19 +597,79 @@ impl<'a, T> GuestPtr<'a, [T]> { })?; // Validate all elements in slice. - // SAFETY: ptr has been validated by self.mem.validate_size_align + // SAFETY: `ptr` has been validated by `self.mem.validate_size_align` for offs in 0..self.pointer.1 { T::validate(unsafe { ptr.add(offs as usize) })?; } - // SAFETY: iff there are no overlapping borrows it is valid to construct a &mut [T] + // SAFETY: iff there are no overlapping borrows it is valid to construct + // a `&mut [T]`. let ptr = unsafe { slice::from_raw_parts_mut(ptr, self.pointer.1 as usize) }; - Ok(GuestSliceMut { + Ok(Some(GuestSliceMut { ptr, mem: self.mem, borrow, - }) + })) + } + + /// Similar to `as_slice_mut`, this function will attempt to create a smart + /// pointer to the WebAssembly linear memory. All validation and Wiggle + /// borrow checking is the same, but unlike `as_slice_mut`, the returned + /// `&mut` slice can point to WebAssembly shared memory. Though the Wiggle + /// borrow checker can guarantee no other Wiggle calls will access this + /// slice, it cannot guarantee that another thread is not modifying the + /// `&mut` slice in some other way. Thus, access to that slice is marked + /// `unsafe`. + pub fn as_unsafe_slice_mut(&self) -> Result, GuestError> + where + T: GuestTypeTransparent<'a>, + { + // SAFETY: see `as_slice_mut`. + let len = self.checked_byte_len()?; + let ptr = + self.mem + .validate_size_align(self.pointer.0, T::guest_align(), len)? as *mut T; + + let borrow = self.mem.mut_borrow(Region { + start: self.pointer.0, + len, + })?; + for offs in 0..self.pointer.1 { + T::validate(unsafe { ptr.add(offs as usize) })?; + } + let ptr = unsafe { slice::from_raw_parts_mut(ptr, self.pointer.1 as usize) }; + + // Wrap up a `GuestSliceMut` in its `unsafe` container. + let guest_slice_mut = GuestSliceMut { + ptr, + mem: self.mem, + borrow, + }; + Ok(UnsafeGuestSliceMut(guest_slice_mut)) + } + + /// Copies the data in the guest region into a [`Vec`]. + /// + /// This is useful when one cannot use [`GuestPtr::as_slice`], e.g., when + /// pointing to a region of WebAssembly shared memory. + /// + /// TODO: this copies element by element; should it be a `ptr::copy` + /// instead? + pub fn to_vec(&self) -> Result, GuestError> + where + T: GuestTypeTransparent<'a> + Copy + 'a, + { + let mut vec = Vec::with_capacity(self.pointer.1 as usize); + let len = self.checked_byte_len()?; + let _ = self + .mem + .validate_size_align(self.pointer.0, T::guest_align(), len)?; + for offs in 0..self.pointer.1 { + let elem = self.get(offs).expect("already validated the size"); + vec.push(elem.read()?); + } + Ok(vec) } /// Copies the data pointed to by `slice` into this guest region. @@ -613,8 +688,13 @@ impl<'a, T> GuestPtr<'a, [T]> { where T: GuestTypeTransparent<'a> + Copy + 'a, { - // bounds check ... - let mut self_slice = self.as_slice_mut()?; + // Retrieve the slice of memory to copy to, performing the necessary + // bounds checks... + let mut unsafe_guest_slice = self.as_unsafe_slice_mut()?; + // ... SAFETY: since we copy the same regardless of whether the memory + // is shared or non-shared, we accept that the data in `&mut [T]` may be + // concurrently modified ... + let self_slice = unsafe { unsafe_guest_slice.as_mut() }; // ... length check ... if self_slice.len() != slice.len() { return Err(GuestError::SliceLengthsDiffer); @@ -828,6 +908,19 @@ impl<'a, T> Drop for GuestSliceMut<'a, T> { } } +/// A smart pointer like [`GuestSliceMut`] except to WebAssembly shared memory +/// so it is `unsafe` to hand out slices of `&mut [T]`. +pub struct UnsafeGuestSliceMut<'a, T>(GuestSliceMut<'a, T>); + +impl<'a, T> UnsafeGuestSliceMut<'a, T> { + /// Provide `unsafe` access to the underlying WebAssembly memory. The bounds + /// are guaranteed to be valid, but the data may change due to access by + /// other threads to the shared memory. + pub unsafe fn as_mut(&mut self) -> &mut [T] { + &mut *self.0 + } +} + /// A smart pointer to an shareable `str` in guest memory. /// Usable as a `&'a str` via [`std::ops::Deref`]. pub struct GuestStr<'a> { diff --git a/crates/wiggle/tests/wasi.rs b/crates/wiggle/tests/wasi.rs index 9a0783c1555c..6f0b4880ff3c 100644 --- a/crates/wiggle/tests/wasi.rs +++ b/crates/wiggle/tests/wasi.rs @@ -147,7 +147,10 @@ impl<'a> crate::wasi_snapshot_preview1::WasiSnapshotPreview1 for WasiCtx<'a> { let len: u32 = iov.buf_len; let buf: GuestPtr<[u8]> = base.as_array(len); // GuestSlice will remain borrowed until dropped: - let slice = buf.as_slice().expect("borrow slice from iovec"); + let slice = buf + .as_slice() + .expect("borrow slice from iovec") + .expect("expected non-shared memory"); slices.push(slice); } println!("iovec slices: [");