Skip to content

Commit 155ea7f

Browse files
alexcrichtonalexcrichton
authored
Remove unsoundness of widening store borrows (#11481)
* Remove unsoundness of widening store borrows This commit removes preexisting unsoundness in Wasmtime where a `&mut StoreOpaque` borrow was "widened" into encompassing the limiter on the `T` in `StoreInner<T>`, for example, by using the self-pointer located in an instance or the store. This fix is done by threading `&mut StoreOpaque` as a parameter separately from a `StoreResourceLimiter`. This means that various callers now take a new `Option<&mut StoreResourceLimiter<'_>>` parameter in various locations. Closes #11409 * Fix gc-less build
1 parent 6e21cf1 commit 155ea7f

File tree

21 files changed

+249
-141
lines changed

21 files changed

+249
-141
lines changed

‎crates/wasmtime/src/runtime/externals/table.rs‎

Lines changed: 12 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@ use crate::runtime::RootedGcRefImpl;
33
use crate::runtime::vm::{
44
self, GcStore, SendSyncPtr, TableElementType, VMFuncRef, VMGcRef, VMStore,
55
};
6-
use crate::store::{AutoAssertNoGc, StoreInstanceId, StoreOpaque};
6+
use crate::store::{AutoAssertNoGc, StoreInstanceId, StoreOpaque, StoreResourceLimiter};
77
use crate::trampoline::generate_table_export;
88
use crate::{
99
AnyRef, AsContext, AsContextMut, ExnRef, ExternRef, Func, HeapType, Ref, RefType,
@@ -94,7 +94,8 @@ impl Table {
9494
/// # }
9595
/// ```
9696
pub fn new(mut store: impl AsContextMut, ty: TableType, init: Ref) -> Result<Table> {
97-
vm::one_poll(Table::_new(store.as_context_mut().0, ty, init))
97+
let (mut limiter, store) = store.as_context_mut().0.resource_limiter_and_store_opaque();
98+
vm::one_poll(Table::_new(store, limiter.as_mut(), ty, init))
9899
.expect("must use `new_async` when async resource limiters are in use")
99100
}
100101

@@ -112,11 +113,17 @@ impl Table {
112113
ty: TableType,
113114
init: Ref,
114115
) -> Result<Table> {
115-
Table::_new(store.as_context_mut().0, ty, init).await
116+
let (mut limiter, store) = store.as_context_mut().0.resource_limiter_and_store_opaque();
117+
Table::_new(store, limiter.as_mut(), ty, init).await
116118
}
117119

118-
async fn _new(store: &mut StoreOpaque, ty: TableType, init: Ref) -> Result<Table> {
119-
let table = generate_table_export(store, &ty).await?;
120+
async fn _new(
121+
store: &mut StoreOpaque,
122+
limiter: Option<&mut StoreResourceLimiter<'_>>,
123+
ty: TableType,
124+
init: Ref,
125+
) -> Result<Table> {
126+
let table = generate_table_export(store, limiter, &ty).await?;
120127
table._fill(store, 0, init, ty.minimum())?;
121128
Ok(table)
122129
}

‎crates/wasmtime/src/runtime/gc/enabled/arrayref.rs‎

Lines changed: 25 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
//! Working with GC `array` objects.
22
3-
use crate::runtime::vm::VMGcRef;
4-
use crate::store::StoreId;
3+
use crate::runtime::vm::{VMGcRef, VMStore};
4+
use crate::store::{StoreId, StoreResourceLimiter};
55
use crate::vm::{self, VMArrayRef, VMGcHeader};
66
use crate::{AnyRef, FieldType};
77
use crate::{
@@ -297,9 +297,15 @@ impl ArrayRef {
297297
elem: &Val,
298298
len: u32,
299299
) -> Result<Rooted<ArrayRef>> {
300-
let store = store.as_context_mut().0;
300+
let (mut limiter, store) = store.as_context_mut().0.resource_limiter_and_store_opaque();
301301
assert!(!store.async_support());
302-
vm::assert_ready(Self::_new_async(store, allocator, elem, len))
302+
vm::assert_ready(Self::_new_async(
303+
store,
304+
limiter.as_mut(),
305+
allocator,
306+
elem,
307+
len,
308+
))
303309
}
304310

305311
/// Asynchronously allocate a new `array` of the given length, with every
@@ -341,17 +347,19 @@ impl ArrayRef {
341347
elem: &Val,
342348
len: u32,
343349
) -> Result<Rooted<ArrayRef>> {
344-
Self::_new_async(store.as_context_mut().0, allocator, elem, len).await
350+
let (mut limiter, store) = store.as_context_mut().0.resource_limiter_and_store_opaque();
351+
Self::_new_async(store, limiter.as_mut(), allocator, elem, len).await
345352
}
346353

347354
pub(crate) async fn _new_async(
348355
store: &mut StoreOpaque,
356+
limiter: Option<&mut StoreResourceLimiter<'_>>,
349357
allocator: &ArrayRefPre,
350358
elem: &Val,
351359
len: u32,
352360
) -> Result<Rooted<ArrayRef>> {
353361
store
354-
.retry_after_gc_async((), |store, ()| {
362+
.retry_after_gc_async(limiter, (), |store, ()| {
355363
Self::new_from_iter(store, allocator, RepeatN(elem, len))
356364
})
357365
.await
@@ -445,9 +453,14 @@ impl ArrayRef {
445453
allocator: &ArrayRefPre,
446454
elems: &[Val],
447455
) -> Result<Rooted<ArrayRef>> {
448-
let store = store.as_context_mut().0;
456+
let (mut limiter, store) = store.as_context_mut().0.resource_limiter_and_store_opaque();
449457
assert!(!store.async_support());
450-
vm::assert_ready(Self::_new_fixed_async(store, allocator, elems))
458+
vm::assert_ready(Self::_new_fixed_async(
459+
store,
460+
limiter.as_mut(),
461+
allocator,
462+
elems,
463+
))
451464
}
452465

453466
/// Asynchronously allocate a new `array` containing the given elements.
@@ -491,16 +504,18 @@ impl ArrayRef {
491504
allocator: &ArrayRefPre,
492505
elems: &[Val],
493506
) -> Result<Rooted<ArrayRef>> {
494-
Self::_new_fixed_async(store.as_context_mut().0, allocator, elems).await
507+
let (mut limiter, store) = store.as_context_mut().0.resource_limiter_and_store_opaque();
508+
Self::_new_fixed_async(store, limiter.as_mut(), allocator, elems).await
495509
}
496510

497511
pub(crate) async fn _new_fixed_async(
498512
store: &mut StoreOpaque,
513+
limiter: Option<&mut StoreResourceLimiter<'_>>,
499514
allocator: &ArrayRefPre,
500515
elems: &[Val],
501516
) -> Result<Rooted<ArrayRef>> {
502517
store
503-
.retry_after_gc_async((), |store, ()| {
518+
.retry_after_gc_async(limiter, (), |store, ()| {
504519
Self::new_from_iter(store, allocator, elems.iter())
505520
})
506521
.await

‎crates/wasmtime/src/runtime/gc/enabled/exnref.rs‎

Lines changed: 14 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
//! Implementation of `exnref` in Wasmtime.
22
3-
use crate::runtime::vm::VMGcRef;
4-
use crate::store::StoreId;
3+
use crate::runtime::vm::{VMGcRef, VMStore};
4+
use crate::store::{StoreId, StoreResourceLimiter};
55
use crate::vm::{self, VMExnRef, VMGcHeader};
66
use crate::{
77
AsContext, AsContextMut, GcRefImpl, GcRootIndex, HeapType, ManuallyRooted, RefType, Result,
@@ -205,9 +205,15 @@ impl ExnRef {
205205
tag: &Tag,
206206
fields: &[Val],
207207
) -> Result<Rooted<ExnRef>> {
208-
let store = store.as_context_mut().0;
208+
let (mut limiter, store) = store.as_context_mut().0.resource_limiter_and_store_opaque();
209209
assert!(!store.async_support());
210-
vm::assert_ready(Self::_new_async(store, allocator, tag, fields))
210+
vm::assert_ready(Self::_new_async(
211+
store,
212+
limiter.as_mut(),
213+
allocator,
214+
tag,
215+
fields,
216+
))
211217
}
212218

213219
/// Asynchronously allocate a new exception object and get a
@@ -245,18 +251,20 @@ impl ExnRef {
245251
tag: &Tag,
246252
fields: &[Val],
247253
) -> Result<Rooted<ExnRef>> {
248-
Self::_new_async(store.as_context_mut().0, allocator, tag, fields).await
254+
let (mut limiter, store) = store.as_context_mut().0.resource_limiter_and_store_opaque();
255+
Self::_new_async(store, limiter.as_mut(), allocator, tag, fields).await
249256
}
250257

251258
pub(crate) async fn _new_async(
252259
store: &mut StoreOpaque,
260+
limiter: Option<&mut StoreResourceLimiter<'_>>,
253261
allocator: &ExnRefPre,
254262
tag: &Tag,
255263
fields: &[Val],
256264
) -> Result<Rooted<ExnRef>> {
257265
Self::type_check_tag_and_fields(store, allocator, tag, fields)?;
258266
store
259-
.retry_after_gc_async((), |store, ()| {
267+
.retry_after_gc_async(limiter, (), |store, ()| {
260268
Self::new_unchecked(store, allocator, tag, fields)
261269
})
262270
.await

‎crates/wasmtime/src/runtime/gc/enabled/externref.rs‎

Lines changed: 11 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -2,11 +2,11 @@
22
33
use super::{AnyRef, RootedGcRefImpl};
44
use crate::prelude::*;
5-
use crate::runtime::vm::{self, VMGcRef};
5+
use crate::runtime::vm::{self, VMGcRef, VMStore};
66
use crate::{
77
AsContextMut, GcHeapOutOfMemory, GcRefImpl, GcRootIndex, HeapType, ManuallyRooted, RefType,
88
Result, Rooted, StoreContext, StoreContextMut, ValRaw, ValType, WasmTy,
9-
store::{AutoAssertNoGc, StoreOpaque},
9+
store::{AutoAssertNoGc, StoreOpaque, StoreResourceLimiter},
1010
};
1111
use core::any::Any;
1212
use core::mem;
@@ -210,13 +210,13 @@ impl ExternRef {
210210
/// Panics if the `context` is configured for async; use
211211
/// [`ExternRef::new_async`][crate::ExternRef::new_async] to perform
212212
/// asynchronous allocation instead.
213-
pub fn new<T>(mut context: impl AsContextMut, value: T) -> Result<Rooted<ExternRef>>
213+
pub fn new<T>(mut store: impl AsContextMut, value: T) -> Result<Rooted<ExternRef>>
214214
where
215215
T: 'static + Any + Send + Sync,
216216
{
217-
let ctx = context.as_context_mut().0;
218-
assert!(!ctx.async_support());
219-
vm::assert_ready(Self::_new_async(ctx, value))
217+
let (mut limiter, store) = store.as_context_mut().0.resource_limiter_and_store_opaque();
218+
assert!(!store.async_support());
219+
vm::assert_ready(Self::_new_async(store, limiter.as_mut(), value))
220220
}
221221

222222
/// Asynchronously allocates a new `ExternRef` wrapping the given value.
@@ -295,16 +295,17 @@ impl ExternRef {
295295
/// [`ExternRef::new`][crate::ExternRef::new] to perform synchronous
296296
/// allocation instead.
297297
#[cfg(feature = "async")]
298-
pub async fn new_async<T>(mut context: impl AsContextMut, value: T) -> Result<Rooted<ExternRef>>
298+
pub async fn new_async<T>(mut store: impl AsContextMut, value: T) -> Result<Rooted<ExternRef>>
299299
where
300300
T: 'static + Any + Send + Sync,
301301
{
302-
let ctx = context.as_context_mut().0;
303-
Self::_new_async(ctx, value).await
302+
let (mut limiter, store) = store.as_context_mut().0.resource_limiter_and_store_opaque();
303+
Self::_new_async(store, limiter.as_mut(), value).await
304304
}
305305

306306
pub(crate) async fn _new_async<T>(
307307
store: &mut StoreOpaque,
308+
limiter: Option<&mut StoreResourceLimiter<'_>>,
308309
value: T,
309310
) -> Result<Rooted<ExternRef>>
310311
where
@@ -315,7 +316,7 @@ impl ExternRef {
315316
let value: Box<dyn Any + Send + Sync> = Box::new(value);
316317

317318
let gc_ref = store
318-
.retry_after_gc_async(value, |store, value| {
319+
.retry_after_gc_async(limiter, value, |store, value| {
319320
store
320321
.require_gc_store_mut()?
321322
.alloc_externref(value)

‎crates/wasmtime/src/runtime/gc/enabled/structref.rs‎

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -2,13 +2,13 @@
22
33
use crate::runtime::vm::VMGcRef;
44
use crate::store::StoreId;
5-
use crate::vm::{self, VMGcHeader, VMStructRef};
5+
use crate::vm::{self, VMGcHeader, VMStore, VMStructRef};
66
use crate::{AnyRef, FieldType};
77
use crate::{
88
AsContext, AsContextMut, EqRef, GcHeapOutOfMemory, GcRefImpl, GcRootIndex, HeapType,
99
ManuallyRooted, RefType, Rooted, StructType, Val, ValRaw, ValType, WasmTy,
1010
prelude::*,
11-
store::{AutoAssertNoGc, StoreContextMut, StoreOpaque},
11+
store::{AutoAssertNoGc, StoreContextMut, StoreOpaque, StoreResourceLimiter},
1212
};
1313
use core::mem::{self, MaybeUninit};
1414
use wasmtime_environ::{GcLayout, GcStructLayout, VMGcKind, VMSharedTypeIndex};
@@ -231,9 +231,9 @@ impl StructRef {
231231
allocator: &StructRefPre,
232232
fields: &[Val],
233233
) -> Result<Rooted<StructRef>> {
234-
let store = store.as_context_mut().0;
234+
let (mut limiter, store) = store.as_context_mut().0.resource_limiter_and_store_opaque();
235235
assert!(!store.async_support());
236-
vm::assert_ready(Self::_new_async(store, allocator, fields))
236+
vm::assert_ready(Self::_new_async(store, limiter.as_mut(), allocator, fields))
237237
}
238238

239239
/// Asynchronously allocate a new `struct` and get a reference to it.
@@ -264,17 +264,19 @@ impl StructRef {
264264
allocator: &StructRefPre,
265265
fields: &[Val],
266266
) -> Result<Rooted<StructRef>> {
267-
Self::_new_async(store.as_context_mut().0, allocator, fields).await
267+
let (mut limiter, store) = store.as_context_mut().0.resource_limiter_and_store_opaque();
268+
Self::_new_async(store, limiter.as_mut(), allocator, fields).await
268269
}
269270

270271
pub(crate) async fn _new_async(
271272
store: &mut StoreOpaque,
273+
limiter: Option<&mut StoreResourceLimiter<'_>>,
272274
allocator: &StructRefPre,
273275
fields: &[Val],
274276
) -> Result<Rooted<StructRef>> {
275277
Self::type_check_fields(store, allocator, fields)?;
276278
store
277-
.retry_after_gc_async((), |store, ()| {
279+
.retry_after_gc_async(limiter, (), |store, ()| {
278280
Self::new_unchecked(store, allocator, fields)
279281
})
280282
.await

‎crates/wasmtime/src/runtime/instance.rs‎

Lines changed: 14 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -2,9 +2,11 @@ use crate::linker::{Definition, DefinitionType};
22
use crate::prelude::*;
33
use crate::runtime::vm::{
44
self, Imports, ModuleRuntimeInfo, VMFuncRef, VMFunctionImport, VMGlobalImport, VMMemoryImport,
5-
VMTableImport, VMTagImport,
5+
VMStore, VMTableImport, VMTagImport,
6+
};
7+
use crate::store::{
8+
AllocateInstanceKind, InstanceId, StoreInstanceId, StoreOpaque, StoreResourceLimiter,
69
};
7-
use crate::store::{AllocateInstanceKind, InstanceId, StoreInstanceId, StoreOpaque};
810
use crate::types::matching;
911
use crate::{
1012
AsContextMut, Engine, Export, Extern, Func, Global, Memory, Module, ModuleExport, SharedMemory,
@@ -248,9 +250,12 @@ impl Instance {
248250
module: &Module,
249251
imports: Imports<'_>,
250252
) -> Result<Instance> {
251-
// SAFETY: the safety contract of `new_raw` is the same as this
252-
// function.
253-
let (instance, start) = unsafe { Instance::new_raw(store.0, module, imports).await? };
253+
let (instance, start) = {
254+
let (mut limiter, store) = store.0.resource_limiter_and_store_opaque();
255+
// SAFETY: the safety contract of `new_raw` is the same as this
256+
// function.
257+
unsafe { Instance::new_raw(store, limiter.as_mut(), module, imports).await? }
258+
};
254259
if let Some(start) = start {
255260
if store.0.async_support() {
256261
#[cfg(feature = "async")]
@@ -285,6 +290,7 @@ impl Instance {
285290
/// provided as well.
286291
async unsafe fn new_raw(
287292
store: &mut StoreOpaque,
293+
mut limiter: Option<&mut StoreResourceLimiter<'_>>,
288294
module: &Module,
289295
imports: Imports<'_>,
290296
) -> Result<(Instance, Option<FuncIndex>)> {
@@ -295,7 +301,7 @@ impl Instance {
295301

296302
// Allocate the GC heap, if necessary.
297303
if module.env_module().needs_gc_heap {
298-
store.ensure_gc_store().await?;
304+
store.ensure_gc_store(limiter.as_deref_mut()).await?;
299305
}
300306

301307
let compiled_module = module.compiled_module();
@@ -313,6 +319,7 @@ impl Instance {
313319
let id = unsafe {
314320
store
315321
.allocate_instance(
322+
limiter.as_deref_mut(),
316323
AllocateInstanceKind::Module(module_id),
317324
&ModuleRuntimeInfo::Module(module.clone()),
318325
imports,
@@ -349,7 +356,7 @@ impl Instance {
349356
.features()
350357
.contains(WasmFeatures::BULK_MEMORY);
351358

352-
vm::initialize_instance(store, id, compiled_module.module(), bulk_memory).await?;
359+
vm::initialize_instance(store, limiter, id, compiled_module.module(), bulk_memory).await?;
353360

354361
Ok((instance, compiled_module.module().start_func))
355362
}

0 commit comments

Comments
 (0)