Skip to content

Commit 0f28058

Browse files
authored
Handle OOM in Linker::instantiate_pre (#12683)
1 parent c7578bd commit 0f28058

File tree

4 files changed

+79
-27
lines changed

4 files changed

+79
-27
lines changed

crates/fuzzing/tests/oom.rs

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -508,6 +508,51 @@ fn linker_func_wrap() -> Result<()> {
508508
})
509509
}
510510

511+
#[test]
512+
#[cfg(arc_try_new)]
513+
fn linker_instantiate_pre() -> Result<()> {
514+
let module_bytes = {
515+
let mut config = Config::new();
516+
config.concurrency_support(false);
517+
let engine = Engine::new(&config)?;
518+
let module = Module::new(
519+
&engine,
520+
r#"
521+
(module
522+
(import "module" "func" (func (param i32) (result i32)))
523+
524+
(memory (export "memory") 1)
525+
(data (i32.const 0) "a")
526+
527+
(table (export "table") 1 funcref)
528+
(elem (i32.const 0) func 1)
529+
530+
(func (export "func") (param i32) (result i32)
531+
(call 0 (local.get 0))
532+
)
533+
)
534+
"#,
535+
)?;
536+
module.serialize()?
537+
};
538+
539+
let mut config = Config::new();
540+
config.enable_compiler(false);
541+
config.concurrency_support(false);
542+
543+
let engine = Engine::new(&config)?;
544+
545+
let mut linker = Linker::<()>::new(&engine);
546+
linker.func_wrap("module", "func", |x: i32| x * 2)?;
547+
548+
let module = unsafe { Module::deserialize(&engine, &module_bytes)? };
549+
550+
OomTest::new().test(|| {
551+
let _ = linker.instantiate_pre(&module)?;
552+
Ok(())
553+
})
554+
}
555+
511556
#[test]
512557
#[cfg(arc_try_new)]
513558
fn module_deserialize() -> Result<()> {

crates/wasmtime/src/runtime/instance.rs

Lines changed: 17 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@ use core::ptr::NonNull;
1717
use wasmparser::WasmFeatures;
1818
use wasmtime_environ::{
1919
EntityIndex, EntityType, FuncIndex, GlobalIndex, MemoryIndex, PrimaryMap, TableIndex, TagIndex,
20-
TypeTrace,
20+
TypeTrace, collections,
2121
};
2222

2323
/// An instantiated WebAssembly module.
@@ -787,10 +787,10 @@ pub struct InstancePre<T> {
787787
/// provided, passed to `Instance::new_started` after inserting them into a
788788
/// `Store`.
789789
///
790-
/// Note that this is stored as an `Arc<[T]>` to quickly move a strong
791-
/// reference to everything internally into a `Store<T>` without having to
792-
/// clone each individual item.
793-
items: Arc<[Definition]>,
790+
/// Note that this is stored as an `Arc` to quickly move a strong reference
791+
/// to everything internally into a `Store<T>` without having to clone each
792+
/// individual item.
793+
items: Arc<collections::Vec<Definition>>,
794794

795795
/// A count of `Definition::HostFunc` entries in `items` above to
796796
/// preallocate space in a `Store` up front for all entries to be inserted.
@@ -801,8 +801,8 @@ pub struct InstancePre<T> {
801801
/// `VMFuncRef`s so that we don't have to do it at
802802
/// instantiation time.
803803
///
804-
/// This is an `Arc<[T]>` for the same reason as `items`.
805-
func_refs: Arc<[VMFuncRef]>,
804+
/// This is an `Arc` for the same reason as `items`.
805+
func_refs: Arc<collections::Vec<VMFuncRef>>,
806806

807807
/// Whether or not any import in `items` is flagged as needing async.
808808
///
@@ -836,10 +836,13 @@ impl<T: 'static> InstancePre<T> {
836836
/// This method is unsafe as the `T` of the `InstancePre<T>` is not
837837
/// guaranteed to be the same as the `T` within the `Store`, the caller must
838838
/// verify that.
839-
pub(crate) unsafe fn new(module: &Module, items: Vec<Definition>) -> Result<InstancePre<T>> {
839+
pub(crate) unsafe fn new(
840+
module: &Module,
841+
items: collections::Vec<Definition>,
842+
) -> Result<InstancePre<T>> {
840843
typecheck(module, &items, |cx, ty, item| cx.definition(ty, &item.ty()))?;
841844

842-
let mut func_refs = vec![];
845+
let mut func_refs = collections::Vec::with_capacity(items.len())?;
843846
let mut host_funcs = 0;
844847
let mut asyncness = Asyncness::No;
845848
for item in &items {
@@ -853,7 +856,7 @@ impl<T: 'static> InstancePre<T> {
853856
.wasm_to_array_trampoline(f.sig_index())
854857
.map(|f| f.into()),
855858
..*f.func_ref()
856-
});
859+
})?;
857860
}
858861
asyncness = asyncness | f.asyncness();
859862
}
@@ -862,9 +865,9 @@ impl<T: 'static> InstancePre<T> {
862865

863866
Ok(InstancePre {
864867
module: module.clone(),
865-
items: items.into(),
868+
items: try_new::<Arc<_>>(items)?,
866869
host_funcs,
867-
func_refs: func_refs.into(),
870+
func_refs: try_new::<Arc<_>>(func_refs)?,
868871
asyncness,
869872
_marker: core::marker::PhantomData,
870873
})
@@ -959,9 +962,9 @@ impl<T: 'static> InstancePre<T> {
959962
fn pre_instantiate_raw(
960963
store: &mut StoreOpaque,
961964
module: &Module,
962-
items: &Arc<[Definition]>,
965+
items: &Arc<collections::Vec<Definition>>,
963966
host_funcs: usize,
964-
func_refs: &Arc<[VMFuncRef]>,
967+
func_refs: &Arc<collections::Vec<VMFuncRef>>,
965968
asyncness: Asyncness,
966969
) -> Result<OwnedImports> {
967970
// Register this module and use it to fill out any funcref wasm_call holes

crates/wasmtime/src/runtime/linker.rs

Lines changed: 12 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,10 @@ use core::future::Future;
1313
use core::marker;
1414
use core::mem::MaybeUninit;
1515
use log::warn;
16-
use wasmtime_environ::{Atom, StringPool, collections::HashMap};
16+
use wasmtime_environ::{
17+
Atom, StringPool,
18+
collections::{HashMap, Vec},
19+
};
1720

1821
/// Structure used to link wasm modules/instances together.
1922
///
@@ -633,15 +636,15 @@ impl<T> Linker<T> {
633636
T: 'static,
634637
{
635638
let mut store = store.as_context_mut();
636-
let exports = instance
639+
let exports: Vec<_> = instance
637640
.exports(&mut store)
638641
.map(|e| {
639642
Ok((
640643
self.import_key(module_name, Some(e.name()))?,
641644
e.into_extern(),
642645
))
643646
})
644-
.collect::<Result<Vec<_>>>()?;
647+
.try_collect::<_, Error>()?;
645648
for (key, export) in exports {
646649
self.insert(key, Definition::new(store.0, export))?;
647650
}
@@ -988,12 +991,12 @@ impl<T> Linker<T> {
988991
pub fn alias_module(&mut self, module: &str, as_module: &str) -> Result<()> {
989992
let module = self.pool.insert(module)?;
990993
let as_module = self.pool.insert(as_module)?;
991-
let items = self
994+
let items: Vec<_> = self
992995
.map
993996
.iter()
994997
.filter(|(key, _def)| key.module == module)
995-
.map(|(key, def)| (key.name, def.clone()))
996-
.collect::<Vec<_>>();
998+
.map(|(key, def)| Ok((key.name, def.clone())))
999+
.try_collect::<_, Error>()?;
9971000
for (name, item) in items {
9981001
self.insert(
9991002
ImportKey {
@@ -1177,10 +1180,10 @@ impl<T> Linker<T> {
11771180
where
11781181
T: 'static,
11791182
{
1180-
let mut imports = module
1183+
let mut imports: Vec<_> = module
11811184
.imports()
1182-
.map(|import| self._get_by_import(&import))
1183-
.collect::<Result<Vec<_>, _>>()?;
1185+
.map(|import| Ok(self._get_by_import(&import)?))
1186+
.try_collect::<_, Error>()?;
11841187
if let Some(store) = store {
11851188
for import in imports.iter_mut() {
11861189
import.update_size(store);

crates/wasmtime/src/runtime/store/func_refs.rs

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ use crate::runtime::HostFunc;
88
use crate::runtime::vm::{AlwaysMut, SendSyncPtr, VMArrayCallHostFuncContext, VMFuncRef};
99
use alloc::sync::Arc;
1010
use core::ptr::NonNull;
11+
use wasmtime_environ::collections;
1112

1213
/// An arena of `VMFuncRef`s.
1314
///
@@ -39,15 +40,15 @@ enum Storage {
3940
/// inside them, etc.
4041
InstancePreDefinitions {
4142
#[expect(dead_code, reason = "only here to keep the original value alive")]
42-
defs: Arc<[Definition]>,
43+
defs: Arc<collections::Vec<Definition>>,
4344
},
4445

4546
/// Pinned `VMFuncRef`s that had their `wasm_call` field
4647
/// pre-patched when constructing an `InstancePre`, and which we need to
4748
/// keep alive for our owning store's lifetime.
4849
InstancePreFuncRefs {
4950
#[expect(dead_code, reason = "only here to keep the original value alive")]
50-
funcs: Arc<[VMFuncRef]>,
51+
funcs: Arc<collections::Vec<VMFuncRef>>,
5152
},
5253

5354
/// A uniquely-owned host function within a `Store`. This comes about with
@@ -118,7 +119,7 @@ impl FuncRefs {
118119
///
119120
/// This is used to ensure that the store itself persists the entire list of
120121
/// `funcs` for the entire lifetime of the store.
121-
pub fn push_instance_pre_func_refs(&mut self, funcs: Arc<[VMFuncRef]>) {
122+
pub fn push_instance_pre_func_refs(&mut self, funcs: Arc<collections::Vec<VMFuncRef>>) {
122123
self.storage.push(Storage::InstancePreFuncRefs { funcs });
123124
}
124125

@@ -127,7 +128,7 @@ impl FuncRefs {
127128
///
128129
/// This is used to keep linker-defined functions' vmctx values alive, for
129130
/// example.
130-
pub fn push_instance_pre_definitions(&mut self, defs: Arc<[Definition]>) {
131+
pub fn push_instance_pre_definitions(&mut self, defs: Arc<collections::Vec<Definition>>) {
131132
self.storage.push(Storage::InstancePreDefinitions { defs });
132133
}
133134

0 commit comments

Comments
 (0)