Stream: git-wasmtime

Topic: wasmtime / issue #10933 Technically `&mut Instance` is no...

Wasmtime GitHub notifications bot (Jun 05 2025 at 20:17):

alexcrichton opened issue #10933:

Currently in Wasmtime it's safe to acquire &mut wasmtime::runtime::vm::Instance in various locations and this is used pervasively. Technically though this alone is not sound as it allows mem::swap-ing two instances. This would mean that the offsets, used to calculate the size of the allocation used for deallocation, could get mixed up (among other issues). Ideally we would have an abstraction which allows mutable access but disallows usage of mem::swap. In a sense this is similar to StoreContextMut which basically entirely exists to prevent mem::swap-ing two stores. This approach has a lot of ergonomic downsides though and it's not fun to write against.

Another alternative is to assume that this basically doesn't happen (it's an internal type in Wasmtime, and it's pretty unlikely we reach for doing this naturally). That would mean we'd document on some unsafe block somewhere "this requires auditing the entire codebase" which practically wouldn't happen but would in-practice work alright.

Another possible alternative would be something like:

struct InstanceFields { /* ... */ } // what `struct Instance` is today

pub struct Instance {
    fields: InstanceFields,
    _something_forcing_mut_ref_to_be_dst: [u8],
}

We'd keep methods and such on impl Instance { ... } and Instance would itself be a dynamically sized type which would disallow overwriting the entire structure via the type system. Runtime-wise this would have a slight cost as passing it by reference passes two values and one value is always zero (the _something_forcing_mut_ref_to_be_dst would always be zero-length, we wouldn't want it to encompass the VMContext as that would inaccurately model how &Instance actually has mutable provenance to VMContext, but if [u8] covered the whole field then it would also seemingly be only shared provenance). LLVM might be able to figure that out though with enough inlining?

Perosnally I'm tempted to pursue this last solution of a dynamically-sized type. It would still require auditing the entire vm/instance.rs module (as you could still overwrite InstanceFields in there) but that's a much less daunting task than the entire codebase.

Wasmtime GitHub notifications bot (Jun 05 2025 at 20:19):

alexcrichton commented on issue #10933:

I'll also note that after https://github.com/bytecodealliance/wasmtime/pull/10934 this issue is equally applicable to components as well.

Wasmtime GitHub notifications bot (Jun 05 2025 at 20:47):

bjorn3 commented on issue #10933:

So basically Instance needs to be pinned?

Wasmtime GitHub notifications bot (Jun 05 2025 at 20:52):

alexcrichton commented on issue #10933:

Hm yes good point that's a much better articulation! Handing out &mut Instance would not be safe and we would instead have to use Pin<&mut Instance> everywhere. Projection to each field would be unsafe but managable. That's probably a better idea than the DST trick I was thinking of.

Wasmtime GitHub notifications bot (Jun 06 2025 at 22:57):

alexcrichton added the wasm-proposal:component-model label to Issue #10933.

Wasmtime GitHub notifications bot (Jun 14 2025 at 01:52):

fitzgen closed issue #10933:

Currently in Wasmtime it's safe to acquire &mut wasmtime::runtime::vm::Instance in various locations and this is used pervasively. Technically though this alone is not sound as it allows mem::swap-ing two instances. This would mean that the offsets, used to calculate the size of the allocation used for deallocation, could get mixed up (among other issues). Ideally we would have an abstraction which allows mutable access but disallows usage of mem::swap. In a sense this is similar to StoreContextMut which basically entirely exists to prevent mem::swap-ing two stores. This approach has a lot of ergonomic downsides though and it's not fun to write against.

Another alternative is to assume that this basically doesn't happen (it's an internal type in Wasmtime, and it's pretty unlikely we reach for doing this naturally). That would mean we'd document on some unsafe block somewhere "this requires auditing the entire codebase" which practically wouldn't happen but would in-practice work alright.

Another possible alternative would be something like:

struct InstanceFields { /* ... */ } // what `struct Instance` is today

pub struct Instance {
    fields: InstanceFields,
    _something_forcing_mut_ref_to_be_dst: [u8],
}

We'd keep methods and such on impl Instance { ... } and Instance would itself be a dynamically sized type which would disallow overwriting the entire structure via the type system. Runtime-wise this would have a slight cost as passing it by reference passes two values and one value is always zero (the _something_forcing_mut_ref_to_be_dst would always be zero-length, we wouldn't want it to encompass the VMContext as that would inaccurately model how &Instance actually has mutable provenance to VMContext, but if [u8] covered the whole field then it would also seemingly be only shared provenance). LLVM might be able to figure that out though with enough inlining?

Perosnally I'm tempted to pursue this last solution of a dynamically-sized type. It would still require auditing the entire vm/instance.rs module (as you could still overwrite InstanceFields in there) but that's a much less daunting task than the entire codebase.

Last updated: Dec 06 2025 at 06:05 UTC