Fix a possible use-after-free introduced in #3231 (#3238)

In #3231 the wasm data sections were moved from the
`wasmtime_environ::Module` structure into the `CompilationArtifacts`.
Each `wasmtime_runtime::Instance` holds raw pointers into the data
section owned by the compilation artifacts under the assumption that the
runtime keeps the artifacts alive while the module is in use. Data is
needed beyond original initialization for `memory.init` instructions as
well as lazy-initialization with the `uffd` feature.

The intention of #3231 was that all `CompiledModule` structures, which
own `CompilationArtifacts` were owned by a store's `ModuleRegistry`, so
this was already taken care of. It turns out, however, that empty
modules which contain no functions are not held within a
`ModuleRegistry` since there was no need prior to retain them. This
commit remedies this mistake by retaining the `CompiledModule`
structure, even if there aren't any functions compiled in.

This should unblock #3235 and fixes the spurious error found there. The
test here, at least on Linux, will deterministically reproduce the error
before this commit since `uffd` was initializing wasm memory with free'd
host memory.
This commit is contained in:
Alex Crichton
2021-08-25 12:14:13 -05:00
committed by GitHub
parent 02ef6a02b8
commit da5c82b786
2 changed files with 73 additions and 10 deletions

View File

@@ -459,3 +459,55 @@ fn instantiation_limit() -> Result<()> {
Ok(())
}
#[test]
fn preserve_data_segments() -> Result<()> {
let mut config = Config::new();
config.allocation_strategy(InstanceAllocationStrategy::Pooling {
strategy: PoolingAllocationStrategy::NextAvailable,
module_limits: ModuleLimits {
memory_pages: 1,
table_elements: 10,
..Default::default()
},
instance_limits: InstanceLimits { count: 2 },
});
let engine = Engine::new(&config)?;
let m = Module::new(
&engine,
r#"
(module
(memory (export "mem") 1 1)
(data (i32.const 0) "foo"))
"#,
)?;
let mut store = Store::new(&engine, ());
let i = Instance::new(&mut store, &m, &[])?;
// Drop the module. This should *not* drop the actual data referenced by the
// module, especially when uffd is enabled. If uffd is enabled we'll lazily
// fault in the memory of the module, which means it better still be alive
// after we drop this.
drop(m);
// Spray some stuff on the heap. If wasm data lived on the heap this should
// paper over things and help us catch use-after-free here if it would
// otherwise happen.
let mut strings = Vec::new();
for _ in 0..1000 {
let mut string = String::new();
for _ in 0..1000 {
string.push('g');
}
strings.push(string);
}
drop(strings);
let mem = i.get_memory(&mut store, "mem").unwrap();
// This will segfault with uffd enabled, and then the uffd will lazily
// initialize the memory. Hopefully it's still `foo`!
assert!(mem.data(&store).starts_with(b"foo"));
Ok(())
}