wiggle: copy guest strings from shared memory (#5475)

* wiggle: copy guest strings from shared memory

Along the same lines as #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.

* review: perform UTF-8 check on `GuestStr` construction

crates/wasi-common/src/snapshots/preview_1.rs

@@ -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
     }
 
@@ -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?;
@@ -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),
@@ -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
     }
@@ -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)
@@ -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()
@@ -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
     }
 
@@ -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
     }
@@ -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
     }
 
@@ -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()?
+            .unlink_file(path.as_cow()?.deref())
-            .expect("cannot use with shared memories; see https://github.com/bytecodealliance/wasmtime/issues/5235 (TODO)").deref())
             .await
     }
 

crates/wasi-crypto/src/wiggle_interfaces/asymmetric_common.rs

@@ -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,
@@ -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,
@@ -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()?
@@ -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()?
@@ -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()?

crates/wasi-crypto/src/wiggle_interfaces/common.rs

@@ -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)
@@ -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
@@ -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)?)
     }
 

crates/wasi-crypto/src/wiggle_interfaces/signatures.rs

@@ -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()?

crates/wasi-crypto/src/wiggle_interfaces/symmetric.rs

@@ -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,
@@ -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,
@@ -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()?
@@ -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,
@@ -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()?
@@ -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)?)
     }
 
@@ -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())

crates/wiggle/src/lib.rs

@@ -708,7 +708,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()?)),
@@ -736,6 +737,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(s.try_into()?)),
+            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]> {
@@ -760,30 +779,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`].
@@ -853,6 +848,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
@@ -1067,6 +1086,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 {}