Skip to content

Commit

Permalink
wiggle: copy guest strings from shared memory
Browse files Browse the repository at this point in the history 
Along the same lines as bytecodealliance#5471, this change adds a new smart pointer,
`GuestStrCow`, to copy the string bytes over from Wasm memory to the
host when the string is found in shared memory. This is necessary to
maintain Rust guarantees: with shared memory, the bytes backing a
`GuestStr` could be altered by another thread and this would invalidate
the assumption that we can dereference at any point to `&str`.
`GuestStrCow` is essentially a wrapper around `GuestStr` when the memory
is not shared but copies the memory region into a `String` when the
memory is shared.

This change updates the uses of Wiggle strings in both wasi-common and
wasi-crypto.
  • Loading branch information
@abrown
abrown committed Dec 20, 2022
1 parent 30cc65d commit cef93fe
Show file tree
Hide file tree
Showing 6 changed files with 96 additions and 54 deletions.
25 changes: 12 additions & 13 deletions crates/wasi-common/src/snapshots/preview_1.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -663,7 +663,7 @@ impl wasi_snapshot_preview1::WasiSnapshotPreview1 for WasiCtx {
self.table()
.get_dir(u32::from(dirfd))?
.get_cap(DirCaps::CREATE_DIRECTORY)?
.create_dir(path.as_str()?.expect("cannot use with shared memories; see https://github.com/bytecodealliance/wasmtime/issues/5235 (TODO)").deref())
.create_dir(path.as_cow()?.deref())
.await
}

Expand All @@ -678,7 +678,7 @@ impl wasi_snapshot_preview1::WasiSnapshotPreview1 for WasiCtx {
.get_dir(u32::from(dirfd))?
.get_cap(DirCaps::PATH_FILESTAT_GET)?
.get_path_filestat(
path.as_str()?.expect("cannot use with shared memories; see https://github.com/bytecodealliance/wasmtime/issues/5235 (TODO)").deref(),
path.as_cow()?.deref(),
flags.contains(types::Lookupflags::SYMLINK_FOLLOW),
)
.await?;
Expand All @@ -705,7 +705,7 @@ impl wasi_snapshot_preview1::WasiSnapshotPreview1 for WasiCtx {
.get_dir(u32::from(dirfd))?
.get_cap(DirCaps::PATH_FILESTAT_SET_TIMES)?
.set_times(
path.as_str()?.expect("cannot use with shared memories; see https://github.com/bytecodealliance/wasmtime/issues/5235 (TODO)").deref(),
path.as_cow()?.deref(),
atim,
mtim,
flags.contains(types::Lookupflags::SYMLINK_FOLLOW),
Expand Down Expand Up @@ -736,9 +736,9 @@ impl wasi_snapshot_preview1::WasiSnapshotPreview1 for WasiCtx {

src_dir
.hard_link(
src_path.as_str()?.expect("cannot use with shared memories; see https://github.com/bytecodealliance/wasmtime/issues/5235 (TODO)").deref(),
src_path.as_cow()?.deref(),
target_dir.deref(),
target_path.as_str()?.expect("cannot use with shared memories; see https://github.com/bytecodealliance/wasmtime/issues/5235 (TODO)").deref(),
target_path.as_cow()?.deref(),
)
.await
}
Expand All @@ -764,7 +764,7 @@ impl wasi_snapshot_preview1::WasiSnapshotPreview1 for WasiCtx {

let oflags = OFlags::from(&oflags);
let fdflags = FdFlags::from(fdflags);
let path = path.as_str()?.expect("cannot use with shared memories; see https://github.com/bytecodealliance/wasmtime/issues/5235 (TODO)");
let path = path.as_cow()?;
if oflags.contains(OFlags::DIRECTORY) {
if oflags.contains(OFlags::CREATE)
|| oflags.contains(OFlags::EXCLUSIVE)
Expand Down Expand Up @@ -813,7 +813,7 @@ impl wasi_snapshot_preview1::WasiSnapshotPreview1 for WasiCtx {
.table()
.get_dir(u32::from(dirfd))?
.get_cap(DirCaps::READLINK)?
.read_link(path.as_str()?.expect("cannot use with shared memories; see https://github.com/bytecodealliance/wasmtime/issues/5235 (TODO)").deref())
.read_link(path.as_cow()?.deref())
.await?
.into_os_string()
.into_string()
Expand All @@ -835,7 +835,7 @@ impl wasi_snapshot_preview1::WasiSnapshotPreview1 for WasiCtx {
self.table()
.get_dir(u32::from(dirfd))?
.get_cap(DirCaps::REMOVE_DIRECTORY)?
.remove_dir(path.as_str()?.expect("cannot use with shared memories; see https://github.com/bytecodealliance/wasmtime/issues/5235 (TODO)").deref())
.remove_dir(path.as_cow()?.deref())
.await
}

Expand All @@ -855,9 +855,9 @@ impl wasi_snapshot_preview1::WasiSnapshotPreview1 for WasiCtx {
.get_cap(DirCaps::RENAME_TARGET)?;
src_dir
.rename(
src_path.as_str()?.expect("cannot use with shared memories; see https://github.com/bytecodealliance/wasmtime/issues/5235 (TODO)").deref(),
src_path.as_cow()?.deref(),
dest_dir.deref(),
dest_path.as_str()?.expect("cannot use with shared memories; see https://github.com/bytecodealliance/wasmtime/issues/5235 (TODO)").deref(),
dest_path.as_cow()?.deref(),
)
.await
}
Expand All @@ -871,7 +871,7 @@ impl wasi_snapshot_preview1::WasiSnapshotPreview1 for WasiCtx {
self.table()
.get_dir(u32::from(dirfd))?
.get_cap(DirCaps::SYMLINK)?
.symlink(src_path.as_str()?.expect("cannot use with shared memories; see https://github.com/bytecodealliance/wasmtime/issues/5235 (TODO)").deref(), dest_path.as_str()?.expect("cannot use with shared memories; see https://github.com/bytecodealliance/wasmtime/issues/5235 (TODO)").deref())
.symlink(src_path.as_cow()?.deref(), dest_path.as_cow()?.deref())
.await
}

Expand All @@ -883,8 +883,7 @@ impl wasi_snapshot_preview1::WasiSnapshotPreview1 for WasiCtx {
self.table()
.get_dir(u32::from(dirfd))?
.get_cap(DirCaps::UNLINK_FILE)?
.unlink_file(path.as_str()?
.expect("cannot use with shared memories; see https://github.com/bytecodealliance/wasmtime/issues/5235 (TODO)").deref())
.unlink_file(path.as_cow()?.deref())
.await
}

Expand Down
10 changes: 5 additions & 5 deletions crates/wasi-crypto/src/wiggle_interfaces/asymmetric_common.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ impl super::wasi_ephemeral_crypto_asymmetric_common::WasiEphemeralCryptoAsymmetr
alg_str: &wiggle::GuestPtr<'_, str>,
options_handle: &guest_types::OptOptions,
) -> Result<guest_types::Keypair, guest_types::CryptoErrno> {
let alg_str = &*alg_str.as_str()?.expect("cannot use with shared memories; see https://github.com/bytecodealliance/wasmtime/issues/5235 (TODO)");
let alg_str = &*alg_str.as_cow()?;
let options_handle = match *options_handle {
guest_types::OptOptions::Some(options_handle) => Some(options_handle),
guest_types::OptOptions::None => None,
Expand Down Expand Up @@ -89,7 +89,7 @@ impl super::wasi_ephemeral_crypto_asymmetric_common::WasiEphemeralCryptoAsymmetr
alg_str: &wiggle::GuestPtr<'_, str>,
options_handle: &guest_types::OptOptions,
) -> Result<guest_types::Keypair, guest_types::CryptoErrno> {
let alg_str = &*alg_str.as_str()?.expect("cannot use with shared memories; see https://github.com/bytecodealliance/wasmtime/issues/5235 (TODO)");
let alg_str = &*alg_str.as_cow()?;
let options_handle = match *options_handle {
guest_types::OptOptions::Some(options_handle) => Some(options_handle),
guest_types::OptOptions::None => None,
Expand All @@ -107,7 +107,7 @@ impl super::wasi_ephemeral_crypto_asymmetric_common::WasiEphemeralCryptoAsymmetr
encoded_len: guest_types::Size,
encoding: guest_types::KeypairEncoding,
) -> Result<guest_types::Keypair, guest_types::CryptoErrno> {
let alg_str = &*alg_str.as_str()?.expect("cannot use with shared memories; see https://github.com/bytecodealliance/wasmtime/issues/5235 (TODO)");
let alg_str = &*alg_str.as_cow()?;
let encoded = &*encoded_ptr
.as_array(encoded_len)
.as_slice()?
Expand Down Expand Up @@ -167,7 +167,7 @@ impl super::wasi_ephemeral_crypto_asymmetric_common::WasiEphemeralCryptoAsymmetr
encoded_len: guest_types::Size,
encoding: guest_types::PublickeyEncoding,
) -> Result<guest_types::Publickey, guest_types::CryptoErrno> {
let alg_str = &*alg_str.as_str()?.expect("cannot use with shared memories; see https://github.com/bytecodealliance/wasmtime/issues/5235 (TODO)");
let alg_str = &*alg_str.as_cow()?;
let encoded = &*encoded_ptr
.as_array(encoded_len)
.as_slice()?
Expand Down Expand Up @@ -218,7 +218,7 @@ impl super::wasi_ephemeral_crypto_asymmetric_common::WasiEphemeralCryptoAsymmetr
encoded_len: guest_types::Size,
encoding: guest_types::SecretkeyEncoding,
) -> Result<guest_types::Secretkey, guest_types::CryptoErrno> {
let alg_str = &*alg_str.as_str()?.expect("cannot use with shared memories; see https://github.com/bytecodealliance/wasmtime/issues/5235 (TODO)");
let alg_str = &*alg_str.as_cow()?;
let encoded = &*encoded_ptr
.as_array(encoded_len)
.as_slice()?
Expand Down
6 changes: 3 additions & 3 deletions crates/wasi-crypto/src/wiggle_interfaces/common.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ impl super::wasi_ephemeral_crypto_common::WasiEphemeralCryptoCommon for WasiCryp
value_ptr: &wiggle::GuestPtr<'_, u8>,
value_len: guest_types::Size,
) -> Result<(), guest_types::CryptoErrno> {
let name_str: &str = &*name_str.as_str()?.expect("cannot use with shared memories; see https://github.com/bytecodealliance/wasmtime/issues/5235 (TODO)");
let name_str: &str = &*name_str.as_cow()?;
let value: &[u8] = {
&*value_ptr
.as_array(value_len)
Expand All @@ -44,7 +44,7 @@ impl super::wasi_ephemeral_crypto_common::WasiEphemeralCryptoCommon for WasiCryp
buffer_ptr: &wiggle::GuestPtr<'_, u8>,
buffer_len: guest_types::Size,
) -> Result<(), guest_types::CryptoErrno> {
let name_str: &str = &*name_str.as_str()?.expect("cannot use with shared memories; see https://github.com/bytecodealliance/wasmtime/issues/5235 (TODO)");
let name_str: &str = &*name_str.as_cow()?;
let buffer: &'static mut [u8] = unsafe {
std::mem::transmute(
&mut *buffer_ptr
Expand All @@ -62,7 +62,7 @@ impl super::wasi_ephemeral_crypto_common::WasiEphemeralCryptoCommon for WasiCryp
name_str: &wiggle::GuestPtr<'_, str>,
value: u64,
) -> Result<(), guest_types::CryptoErrno> {
let name_str: &str = &*name_str.as_str()?.expect("cannot use with shared memories; see https://github.com/bytecodealliance/wasmtime/issues/5235 (TODO)");
let name_str: &str = &*name_str.as_cow()?;
Ok((&*self).options_set_u64(options_handle.into(), name_str, value)?)
}

Expand Down
2 changes: 1 addition & 1 deletion crates/wasi-crypto/src/wiggle_interfaces/signatures.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ impl super::wasi_ephemeral_crypto_signatures::WasiEphemeralCryptoSignatures for
encoded_len: guest_types::Size,
encoding: guest_types::SignatureEncoding,
) -> Result<guest_types::Signature, guest_types::CryptoErrno> {
let alg_str = &*alg_str.as_str()?.expect("cannot use with shared memories; see https://github.com/bytecodealliance/wasmtime/issues/5235 (TODO)");
let alg_str = &*alg_str.as_cow()?;
let encoded = &*encoded_ptr
.as_array(encoded_len)
.as_slice()?
Expand Down
14 changes: 7 additions & 7 deletions crates/wasi-crypto/src/wiggle_interfaces/symmetric.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ impl super::wasi_ephemeral_crypto_symmetric::WasiEphemeralCryptoSymmetric for Wa
alg_str: &wiggle::GuestPtr<'_, str>,
options_handle: &guest_types::OptOptions,
) -> Result<guest_types::SymmetricKey, guest_types::CryptoErrno> {
let alg_str = &*alg_str.as_str()?.expect("cannot use with shared memories; see https://github.com/bytecodealliance/wasmtime/issues/5235 (TODO)");
let alg_str = &*alg_str.as_cow()?;
let options_handle = match *options_handle {
guest_types::OptOptions::Some(options_handle) => Some(options_handle),
guest_types::OptOptions::None => None,
Expand Down Expand Up @@ -86,7 +86,7 @@ impl super::wasi_ephemeral_crypto_symmetric::WasiEphemeralCryptoSymmetric for Wa
alg_str: &wiggle::GuestPtr<'_, str>,
options_handle: &guest_types::OptOptions,
) -> Result<guest_types::SymmetricKey, guest_types::CryptoErrno> {
let alg_str = &*alg_str.as_str()?.expect("cannot use with shared memories; see https://github.com/bytecodealliance/wasmtime/issues/5235 (TODO)");
let alg_str = &*alg_str.as_cow()?;
let options_handle = match *options_handle {
guest_types::OptOptions::Some(options_handle) => Some(options_handle),
guest_types::OptOptions::None => None,
Expand All @@ -102,7 +102,7 @@ impl super::wasi_ephemeral_crypto_symmetric::WasiEphemeralCryptoSymmetric for Wa
raw_ptr: &wiggle::GuestPtr<'_, u8>,
raw_len: guest_types::Size,
) -> Result<guest_types::SymmetricKey, guest_types::CryptoErrno> {
let alg_str = &*alg_str.as_str()?.expect("cannot use with shared memories; see https://github.com/bytecodealliance/wasmtime/issues/5235 (TODO)");
let alg_str = &*alg_str.as_cow()?;
let raw = &*raw_ptr
.as_array(raw_len)
.as_slice()?
Expand Down Expand Up @@ -153,7 +153,7 @@ impl super::wasi_ephemeral_crypto_symmetric::WasiEphemeralCryptoSymmetric for Wa
key_handle: &guest_types::OptSymmetricKey,
options_handle: &guest_types::OptOptions,
) -> Result<guest_types::SymmetricState, guest_types::CryptoErrno> {
let alg_str = &*alg_str.as_str()?.expect("cannot use with shared memories; see https://github.com/bytecodealliance/wasmtime/issues/5235 (TODO)");
let alg_str = &*alg_str.as_cow()?;
let key_handle = match *key_handle {
guest_types::OptSymmetricKey::Some(key_handle) => Some(key_handle),
guest_types::OptSymmetricKey::None => None,
Expand All @@ -178,7 +178,7 @@ impl super::wasi_ephemeral_crypto_symmetric::WasiEphemeralCryptoSymmetric for Wa
value_ptr: &wiggle::GuestPtr<'_, u8>,
value_max_len: guest_types::Size,
) -> Result<guest_types::Size, guest_types::CryptoErrno> {
let name_str: &str = &*name_str.as_str()?.expect("cannot use with shared memories; see https://github.com/bytecodealliance/wasmtime/issues/5235 (TODO)");
let name_str: &str = &*name_str.as_cow()?;
let value = &mut *value_ptr
.as_array(value_max_len)
.as_slice_mut()?
Expand All @@ -193,7 +193,7 @@ impl super::wasi_ephemeral_crypto_symmetric::WasiEphemeralCryptoSymmetric for Wa
symmetric_state_handle: guest_types::SymmetricState,
name_str: &wiggle::GuestPtr<'_, str>,
) -> Result<u64, guest_types::CryptoErrno> {
let name_str: &str = &*name_str.as_str()?.expect("cannot use with shared memories; see https://github.com/bytecodealliance/wasmtime/issues/5235 (TODO)");
let name_str: &str = &*name_str.as_cow()?;
Ok((&*self).options_get_u64(symmetric_state_handle.into(), name_str)?)
}

Expand Down Expand Up @@ -244,7 +244,7 @@ impl super::wasi_ephemeral_crypto_symmetric::WasiEphemeralCryptoSymmetric for Wa
symmetric_state_handle: guest_types::SymmetricState,
alg_str: &wiggle::GuestPtr<'_, str>,
) -> Result<guest_types::SymmetricKey, guest_types::CryptoErrno> {
let alg_str = &*alg_str.as_str()?.expect("cannot use with shared memories; see https://github.com/bytecodealliance/wasmtime/issues/5235 (TODO)");
let alg_str = &*alg_str.as_cow()?;
Ok((&*self)
.symmetric_state_squeeze_key(symmetric_state_handle.into(), alg_str)?
.into())
Expand Down
93 changes: 68 additions & 25 deletions crates/wiggle/src/lib.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -703,7 +703,8 @@ impl<'a> GuestPtr<'a, str> {
/// `GuestError` will be returned.
///
/// Additionally, because it is `unsafe` to have a `GuestStr` of shared
/// memory, this function will return `None` in this case.
/// memory, this function will return `None` in this case (see
/// [`GuestPtr<'_, str>::as_cow`]).
pub fn as_str(&self) -> Result<Option<GuestStr<'a>>, GuestError> {
match self.as_bytes().as_unsafe_slice_mut()?.shared_borrow() {
UnsafeBorrowResult::Ok(s) => Ok(Some(s.try_into()?)),
Expand Down Expand Up @@ -731,6 +732,24 @@ impl<'a> GuestPtr<'a, str> {
UnsafeBorrowResult::Err(e) => Err(e),
}
}

/// Attempts to create a [`GuestStrCow<'_>`] from this pointer, performing
/// bounds checks and utf-8 checks. Whereas [`GuestPtr::as_str`] will fail
/// with `None` if attempting to access Wasm shared memory, this call will
/// succeed: if used on shared memory, this function will copy the string
/// into [`GuestStrCow::Copied`]. If the memory is non-shared, this returns
/// a [`GuestStrCow::Borrowed`] (a thin wrapper over [`GuestStr<'_, T>]`).
pub fn as_cow(&self) -> Result<GuestStrCow<'a>, GuestError> {
match self.as_bytes().as_unsafe_slice_mut()?.shared_borrow() {
UnsafeBorrowResult::Ok(s) => Ok(GuestStrCow::Borrowed(GuestStr(s))),
UnsafeBorrowResult::Shared(_) => {
let copied = self.as_bytes().to_vec()?;
let utf8_string = String::from_utf8(copied).map_err(|e| e.utf8_error())?;
Ok(GuestStrCow::Copied(utf8_string))
}
UnsafeBorrowResult::Err(e) => Err(e),
}
}
}

impl<'a> GuestPtr<'a, [u8]> {
Expand All @@ -755,30 +774,6 @@ impl<T: ?Sized + Pointee> fmt::Debug for GuestPtr<'_, T> {
}
}

/// A smart pointer for distinguishing between different kinds of Wasm memory:
/// shared and non-shared.
///
/// As with `GuestSlice`, this is usable as a `&'a [T]` via [`std::ops::Deref`].
/// The major difference is that, for shared memories, the memory will be copied
/// out of Wasm linear memory to avoid the possibility of concurrent mutation by
/// another thread. This extra copy exists solely to maintain the Rust
/// guarantees regarding `&[T]`.
pub enum GuestCow<'a, T> {
Borrowed(GuestSlice<'a, T>),
Copied(Vec<T>),
}

impl<'a, T> std::ops::Deref for GuestCow<'a, T> {
type Target = [T];

fn deref(&self) -> &Self::Target {
match self {
GuestCow::Borrowed(s) => s,
GuestCow::Copied(s) => s,
}
}
}

/// A smart pointer to an shareable slice in guest memory.
///
/// Usable as a `&'a [T]` via [`std::ops::Deref`].
Expand Down Expand Up @@ -848,6 +843,30 @@ impl<'a, T> Drop for GuestSliceMut<'a, T> {
}
}

/// A smart pointer for distinguishing between different kinds of Wasm memory:
/// shared and non-shared.
///
/// As with `GuestSlice`, this is usable as a `&'a [T]` via [`std::ops::Deref`].
/// The major difference is that, for shared memories, the memory will be copied
/// out of Wasm linear memory to avoid the possibility of concurrent mutation by
/// another thread. This extra copy exists solely to maintain the Rust
/// guarantees regarding `&[T]`.
pub enum GuestCow<'a, T> {
Borrowed(GuestSlice<'a, T>),
Copied(Vec<T>),
}

impl<'a, T> std::ops::Deref for GuestCow<'a, T> {
type Target = [T];

fn deref(&self) -> &Self::Target {
match self {
GuestCow::Borrowed(s) => s,
GuestCow::Copied(s) => s,
}
}
}

/// A smart pointer to an `unsafe` slice in guest memory.
///
/// Accessing guest memory (e.g., WebAssembly linear memory) is inherently
Expand Down Expand Up @@ -1059,6 +1078,30 @@ impl<'a> std::ops::DerefMut for GuestStrMut<'a> {
}
}

/// A smart pointer to a `str` for distinguishing between different kinds of
/// Wasm memory: shared and non-shared.
///
/// As with `GuestStr`, this is usable as a `&'a str` via [`std::ops::Deref`].
/// The major difference is that, for shared memories, the string will be copied
/// out of Wasm linear memory to avoid the possibility of concurrent mutation by
/// another thread. This extra copy exists solely to maintain the Rust
/// guarantees regarding `&str`.
pub enum GuestStrCow<'a> {
Borrowed(GuestStr<'a>),
Copied(String),
}

impl<'a> std::ops::Deref for GuestStrCow<'a> {
type Target = str;

fn deref(&self) -> &Self::Target {
match self {
GuestStrCow::Borrowed(s) => s,
GuestStrCow::Copied(s) => s,
}
}
}

mod private {
pub trait Sealed {}
impl<T> Sealed for T {}
Expand Down

0 comments on commit cef93fe

Please sign in to comment.