wasmtime / PR #11510 Refactor resetting memory on `Memory... · git-wasmtime

Stream: git-wasmtime

Topic: wasmtime / PR #11510 Refactor resetting memory on `Memory...

Wasmtime GitHub notifications bot (Aug 22 2025 at 15:39):

alexcrichton opened PR #11510 from alexcrichton:no-clear-on-drop to bytecodealliance:main:

This commit refactors the behavior of dropping a MemoryImageSlot to no longer map anonymous memory into the slot. This behavior was implemented previously because if a MemoryImageSlot is dropped then the state of the slot is unknown and to prevent any sort of data leakage a reset is performed.

This reset operation, however, is fallible in that it calls mmap. Calls to mmap can fail due to ENOMEM, for example, if the process has reached its VMA limit. This means that if a process is in a near-OOM condition then failing to allocate a memory image could panic the process due to the unwrap() in the destructor of MemoryImageSlot. The purpose of this commit is to avoid this unwrap() and instead move the reset behavior to a location where an error can be propagated.

This commit removes the clear-on-drop behavior of MemoryImageSlot slot. This was already disabled everywhere except the pooling allocator. The pooling allocator now maintains an extra bit of state-per-slot where instead of storing Option<MemoryImageSlot> it now stores effectively one other variant of "unknown". On reuse of an "unknown" slot the memory is reset back to an anonymous mapping and this is all done in a context where an error can be propagated.

Two tests are added in this commit to confirm all of this behavior:

The first test is a new test that passes both before and after this commit which performs a failed allocation of a memory slot. A successful allocation is then made to ensure that the previous image is not present and zero memory is present. This test fails before the commit if the clear-on-drop behavior is removed, and it fails with this commit if the clear-on-reusing-unknown behavior is removed. Effectively this test ensures that the clear-on-unknown-state logic is present.

The second test is a new test that panicked before this commit and passes afterwards. This second test exhausts all VMAs in the current process, or at least most of them, and then tries to allocate some instances with an image. Instance allocation will eventually fail and cause the erroneous path to get executed. This previously unwrapped a ENOMEM failure, and now it can be handled gracefully by the embedder.

Wasmtime GitHub notifications bot (Aug 22 2025 at 15:39):

alexcrichton requested fitzgen for a review on PR #11510.

Wasmtime GitHub notifications bot (Aug 22 2025 at 15:39):

alexcrichton requested wasmtime-core-reviewers for a review on PR #11510.

Wasmtime GitHub notifications bot (Aug 22 2025 at 15:40):

alexcrichton commented on PR #11510:

Some follow-up bits on this:

Personally I wouldn't consider this a CVE since panicking in near-OOM conditions is not something we can protect against. For example Vec::with_capacity can panic in OOM conditions at any time.

This was discovered in Spin recently, so if amenable to others I'd probably do another point release of 36.0.2 so we can update to a fixed version more quickly.

Wasmtime GitHub notifications bot (Aug 22 2025 at 16:57):

fitzgen submitted PR review.

Wasmtime GitHub notifications bot (Aug 22 2025 at 16:57):

fitzgen created PR review comment:

I imagine that these tests will want to return early on WASMTIME_TEST_NO_HOG_MEMORY or whatever the env var is for our tests under qemu.

Wasmtime GitHub notifications bot (Aug 22 2025 at 17:03):

alexcrichton submitted PR review.

Wasmtime GitHub notifications bot (Aug 22 2025 at 17:03):

alexcrichton created PR review comment:

Good point! Let's see what CI says...

Wasmtime GitHub notifications bot (Aug 22 2025 at 23:41):

alexcrichton commented on PR #11510:

qemu-arm: GLib: ../../../glib/gmem.c:106: failed to allocate 40 bytes
qemu-arm: QEMU internal SIGTRAP {code=128, addr=(nil)}
error: test failed, to rerun pass `-p wasmtime --test pooling_alloc_near_oom`

Caused by:
  process didn't exit successfully: `/opt/hostedtoolcache/qemu/bin/qemu-arm -L /usr/arm-linux-gnueabihf -E LD_LIBRARY_PATH=/usr/arm-linux-gnueabihf/lib /home/runner/work/wasmtime/wasmtime/target/armv7-unknown-linux-gnueabihf/debug/deps/pooling_alloc_near_oom-e44c0c4e75189ad2` (signal: 5, SIGTRAP: trace/breakpoint trap)

Whelp looks like I'll need to disable this in QEMU yeah

Wasmtime GitHub notifications bot (Aug 23 2025 at 00:48):

alexcrichton updated PR #11510.

Wasmtime GitHub notifications bot (Aug 23 2025 at 00:48):

alexcrichton has enabled auto merge for PR #11510.

Wasmtime GitHub notifications bot (Aug 23 2025 at 14:46):

tschneidereit submitted PR review.

Wasmtime GitHub notifications bot (Aug 23 2025 at 14:46):

tschneidereit created PR review comment:

I might misunderstand how this works, but shouldn't this return (Some(state), true) to actually reuse the slot after resetting it? By my reading, the current code creates a new slot and then resets that one.

Wasmtime GitHub notifications bot (Aug 23 2025 at 14:51):

tschneidereit submitted PR review.

Wasmtime GitHub notifications bot (Aug 23 2025 at 14:51):

tschneidereit created PR review comment:

Oh, never mind: I get it now. The reset operates on the memory as identified by allocation_index, not the slot itself.

Wasmtime GitHub notifications bot (Aug 25 2025 at 14:36):

alexcrichton updated PR #11510.

Wasmtime GitHub notifications bot (Aug 25 2025 at 14:36):

alexcrichton has enabled auto merge for PR #11510.

Wasmtime GitHub notifications bot (Aug 25 2025 at 20:45):

alexcrichton merged PR #11510.

Last updated: Dec 06 2025 at 07:03 UTC