Repository: bytecodealliance/wasmtime
Pull request #7565 (2023-11-21 04:11:37)

clarify custom memory needs to be zero filled
Discussion on [Zulip][1], this bit me for a while as I didn't understand why valid code was getting out of bounds memory accesses when run in different contexts.

[1]: https://bytecodealliance.zulipchat.com/#narrow/stream/217126-wasmtime/topic/.E2.9C.94.20custom.20memory.20APIs.20for.20Wasmtime


github-actions[bot]: #### Subscribe to Label Action

cc @peterhuene

<details>
This issue or pull request has been labeled: "wasmtime:api"

Thus the following users have been cc'd because of the following labels:

* peterhuene: wasmtime:api

To subscribe or unsubscribe from this label, edit the <code>.github/subscribe-to-label.json</code> configuration file.

[Learn more.](https://github.com/bytecodealliance/subscribe-to-label-action)
</details>

Patch:
diff --git a/crates/fiber/src/lib.rs b/crates/fiber/src/lib.rs
index 5b20157cf..4b0e31a87 100644
--- a/crates/fiber/src/lib.rs
+++ b/crates/fiber/src/lib.rs
@@ -65,7 +65,8 @@ impl FiberStack {
 
 /// A creator of RuntimeFiberStacks.
 pub unsafe trait RuntimeFiberStackCreator: Send + Sync {
-    /// Creates a new RuntimeFiberStack with the specified size, guard pages should be included.
+    /// Creates a new RuntimeFiberStack with the specified size, guard pages should be included,
+    /// memory should be zeroed.
     ///
     /// This is useful to plugin previously allocated memory instead of mmap'ing a new stack for
     /// every instance.
diff --git a/crates/wasmtime/src/memory.rs b/crates/wasmtime/src/memory.rs
index 94cb354d4..d79736958 100644
--- a/crates/wasmtime/src/memory.rs
+++ b/crates/wasmtime/src/memory.rs
@@ -670,6 +670,8 @@ pub unsafe trait MemoryCreator: Send + Sync {
     /// are tuned from the various [`Config`](crate::Config) methods about
     /// memory sizes/guards. Additionally these two values are guaranteed to be
     /// multiples of the system page size.
+    ///
+    /// Memory created from this method should be zero filled.
     fn new_memory(
         &self,
         ty: MemoryType,
diff --git a/crates/wasmtime/src/stack.rs b/crates/wasmtime/src/stack.rs
index fde7dd756..d057ad2e9 100644
--- a/crates/wasmtime/src/stack.rs
+++ b/crates/wasmtime/src/stack.rs
@@ -22,7 +22,7 @@ pub unsafe trait StackCreator: Send + Sync {
     ///
     /// Note there should be at least one guard page of protected memory at the bottom
     /// of the stack to catch potential stack overflow scenarios. Additionally, stacks should be
-    /// page aligned.
+    /// page aligned and zero filled.
     fn new_stack(&self, size: usize) -> Result<Box<dyn StackMemory>, Error>;
 }
 


Test patch:
diff --git a/tests/all/memory_creator.rs b/tests/all/memory_creator.rs
index f70d04196..5c1cc72c2 100644
--- a/tests/all/memory_creator.rs
+++ b/tests/all/memory_creator.rs
@@ -28,6 +28,9 @@ mod not_for_windows {
             let mem = mmap_anonymous(null_mut(), size, ProtFlags::empty(), MapFlags::PRIVATE)
                 .expect("mmap failed");
 
+            // NOTE: mmap_anonymous returns zero initialized memory, which is relied upon by this
+            // API.
+
             mprotect(mem, minimum, MprotectFlags::READ | MprotectFlags::WRITE)
                 .expect("mprotect failed");
             *glob_counter.lock().unwrap() += minimum;
diff --git a/tests/all/stack_creator.rs b/tests/all/stack_creator.rs
index 5b46d8942..952723890 100644
--- a/tests/all/stack_creator.rs
+++ b/tests/all/stack_creator.rs
@@ -55,6 +55,8 @@ impl CustomStackCreator {
             let mem = System.alloc(layout);
             let notnull = NonNull::new(mem);
             if let Some(mem) = notnull {
+                // It's required that stack memory is zeroed for wasmtime
+                libc::memset(mem.as_ptr().cast(), 0, layout.size());
                 // Mark guard page as protected
                 rustix::mm::mprotect(
                     mem.as_ptr().cast(),