Repository: bytecodealliance/wasmtime (rust)

Issue:
Safe Memory Access
#### Feature

Currently the only way to access instance memory is with `data_unchecked[_mut]` or `data_ptr`.

It would be very convenient to have methods that can atomically read or write a range of bytes, with internal locking and safety guarantees provided by the implementation.

Something like:

```rust
fn data_read(&self, range: Range<_>, buffer: &mut [u8]) -> Result<(), MemoryAccessError>;
fn data_write(&self, range: Range<_>, source: &[u8]) -> Result<(), MemoryAccessError>;
```

I'd assume this doesn't exist to prevent the need for internal locking or other implementation constraints and may even be unfeasible, but it would be a very helpful feature.

#### Benefit

Prevent users from needing `unsafe` and make wasmtime easier to use.

#### Implementation

I'm not familiar with the codebase or the complications this would involve, but I'd be happy to contribute if I get some pointers.



Pull request:
Add safe Memory::read and Memory::write methods
Introduce `Memory::read` and `Memory::write` methods for accessing memory contents safely.

Also adds a test and mentions the  the new methods in the `Memory` docs.

Fixes #2484.

Currently most fallible methods use `anyhow::Error`. 
I have instead added a new `MemoryError`, since recovering from such errors is possible and it's more expressive.
(future proofed with `#[non_exhaustive]`).

Could to change it to a `anyhow::Error` if that's the preferred strategy (or wrap the MemoryError with anyhow).

I also already added a `MemoryError::Locked` variant, which is not required yet, but apparently will be once threading support lands. Happy to remove it or slap on a `#[doc(hidden)]` to prevent confusion.


Patch:
diff --git a/crates/wasmtime/src/externals.rs b/crates/wasmtime/src/externals.rs
index a3cede9cb..890471b1a 100644
--- a/crates/wasmtime/src/externals.rs
+++ b/crates/wasmtime/src/externals.rs
@@ -605,6 +605,22 @@ impl Table {
     }
 }
 
+/// Error for out of bounds [`Memory`] access.
+#[derive(Debug)]
+#[non_exhaustive]
+pub struct MemoryAccessError {
+    // Keep struct internals private for future extensibility.
+    _private: (),
+}
+
+impl std::fmt::Display for MemoryAccessError {
+    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
+        write!(f, "out of bounds memory access")
+    }
+}
+
+impl std::error::Error for MemoryAccessError {}
+
 /// A WebAssembly linear memory.
 ///
 /// WebAssembly memories represent a contiguous array of bytes that have a size
@@ -661,9 +677,24 @@ impl Table {
 /// Let's run through a few safe examples first of how you can use a `Memory`.
 ///
 /// ```rust
-/// use wasmtime::Memory;
+/// use wasmtime::{Memory, MemoryAccessError};
+///
+/// // Memory can be read and written safely with the `Memory::read` and
+/// // `Memory::write` methods.
+/// // An error is returned if the copy did not succeed.
+/// fn safe_examples(mem: &Memory) -> Result<(), MemoryAccessError> {
+///     let offset = 5;
+///     mem.write(offset, b"hello")?;
+///     let mut buffer = [0u8; 5];
+///     mem.read(offset, &mut buffer)?;
+///     assert_eq!(b"hello", &buffer);
+///     Ok(())
+/// }
+///
+/// // You can also get direct, unsafe access to the memory, but must manually
+/// // ensure that safety invariants are upheld.
 ///
-/// fn safe_examples(mem: &Memory) {
+/// fn correct_unsafe_examples(mem: &Memory) {
 ///     // Just like wasm, it's safe to read memory almost at any time. The
 ///     // gotcha here is that we need to be sure to load from the correct base
 ///     // pointer and perform the bounds check correctly. So long as this is
@@ -871,6 +902,39 @@ impl Memory {
         MemoryType::from_wasmtime_memory(&self.wasmtime_export.memory.memory)
     }
 
+    /// Safely reads memory contents at the given offset into a buffer.
+    ///
+    /// The entire buffer will be filled.
+    ///
+    /// If offset + buffer length exceed the current memory capacity,
+    /// a [`MemoryAccessError`] is returned.
+    pub fn read(&self, offset: usize, buffer: &mut [u8]) -> Result<(), MemoryAccessError> {
+        unsafe {
+            let slice = self
+                .data_unchecked()
+                .get(offset..)
+                .and_then(|s| s.get(..buffer.len()))
+                .ok_or(MemoryAccessError { _private: () })?;
+            buffer.copy_from_slice(slice);
+            Ok(())
+        }
+    }
+
+    /// Safely writes contents of a buffer to this memory at the given offset.
+    ///
+    /// If the offset + buffer length exceed current memory capacity, a
+    /// [`MemoryAccessError`] is returned.
+    pub fn write(&self, offset: usize, buffer: &[u8]) -> Result<(), MemoryAccessError> {
+        unsafe {
+            self.data_unchecked_mut()
+                .get_mut(offset..)
+                .and_then(|s| s.get_mut(..buffer.len()))
+                .ok_or(MemoryAccessError { _private: () })?
+                .copy_from_slice(buffer);
+            Ok(())
+        }
+    }
+
     /// Returns this memory as a slice view that can be read natively in Rust.
     ///
     /// # Safety


Test patch:
diff --git a/tests/all/externals.rs b/tests/all/externals.rs
index 365d5b261..6c13aabc1 100644
--- a/tests/all/externals.rs
+++ b/tests/all/externals.rs
@@ -340,3 +340,40 @@ fn grow_externref_tables_via_api() -> anyhow::Result<()> {
 
     Ok(())
 }
+
+#[test]
+fn read_write_memory_via_api() {
+    let cfg = Config::new();
+    let store = Store::new(&Engine::new(&cfg));
+    let ty = MemoryType::new(Limits::new(1, None));
+    let mem = Memory::new(&store, ty);
+    mem.grow(1).unwrap();
+
+    let value = b"hello wasm";
+    mem.write(mem.data_size() - value.len(), value).unwrap();
+
+    let mut buffer = [0u8; 10];
+    mem.read(mem.data_size() - buffer.len(), &mut buffer)
+        .unwrap();
+    assert_eq!(value, &buffer);
+
+    // Error conditions.
+
+    // Out of bounds write.
+
+    let res = mem.write(mem.data_size() - value.len() + 1, value);
+    assert!(res.is_err());
+
+    // Out of bounds read.
+
+    let res = mem.read(mem.data_size() - buffer.len() + 1, &mut buffer);
+    assert!(res.is_err());
+
+    // Read offset overflow.
+    let res = mem.read(usize::MAX, &mut buffer);
+    assert!(res.is_err());
+
+    // Write offset overflow.
+    let res = mem.write(usize::MAX, &mut buffer);
+    assert!(res.is_err());
+}