Skip to content

Commit

Permalink
Merge pull request from GHSA-wh6w-3828-g9qf
Browse files Browse the repository at this point in the history
This is a minimal fix for the release branch to fix the issue of having
a memory slot get reused between a module with an image and one without.
  • Loading branch information
alexcrichton committed Nov 10, 2022
1 parent 96ae44a commit 2614f2e
Show file tree
Hide file tree
Showing 2 changed files with 65 additions and 3 deletions.
7 changes: 4 additions & 3 deletions crates/runtime/src/instance/allocator/pooling.rs
Original file line number Diff line number Diff line change
Expand Up @@ -393,13 +393,13 @@ impl InstancePool {
)
};

let mut slot = self
.memories
.take_memory_image_slot(instance_index, defined_index);
if let Some(image) = runtime_info
.memory_image(defined_index)
.map_err(|err| InstantiationError::Resource(err.into()))?
{
let mut slot = self
.memories
.take_memory_image_slot(instance_index, defined_index);
let initial_size = plan.memory.minimum * WASM_PAGE_SIZE as u64;

// If instantiation fails, we can propagate the error
Expand All @@ -425,6 +425,7 @@ impl InstancePool {
.map_err(InstantiationError::Resource)?,
);
} else {
drop(slot);
memories.push(
Memory::new_static(plan, memory, Some(commit_memory_pages), None, unsafe {
&mut *store.unwrap()
Expand Down
61 changes: 61 additions & 0 deletions tests/all/pooling_allocator.rs
Original file line number Diff line number Diff line change
Expand Up @@ -615,6 +615,67 @@ fn drop_externref_global_during_module_init() -> Result<()> {
Ok(())
}

#[test]
fn switch_image_and_non_image() -> Result<()> {
let mut c = Config::new();
c.allocation_strategy(InstanceAllocationStrategy::Pooling {
instance_limits: InstanceLimits {
count: 1,
..Default::default()
},
strategy: Default::default(),
});
let engine = Engine::new(&c)?;
let module1 = Module::new(
&engine,
r#"
(module
(memory 1)
(func (export "load") (param i32) (result i32)
local.get 0
i32.load
)
)
"#,
)?;
let module2 = Module::new(
&engine,
r#"
(module
(memory (export "memory") 1)
(data (i32.const 0) "1234")
)
"#,
)?;

let assert_zero = || -> Result<()> {
let mut store = Store::new(&engine, ());
let instance = Instance::new(&mut store, &module1, &[])?;
let func = instance.get_typed_func::<i32, i32, _>(&mut store, "load")?;
assert_eq!(func.call(&mut store, 0)?, 0);
Ok(())
};

// Initialize with a heap image and make sure the next instance, without an
// image, is zeroed
Instance::new(&mut Store::new(&engine, ()), &module2, &[])?;
assert_zero()?;

// ... transition back to heap image and do this again
Instance::new(&mut Store::new(&engine, ()), &module2, &[])?;
assert_zero()?;

// And go back to an image and make sure it's read/write on the host.
let mut store = Store::new(&engine, ());
let instance = Instance::new(&mut store, &module2, &[])?;
let memory = instance.get_memory(&mut store, "memory").unwrap();
let mem = memory.data_mut(&mut store);
assert!(mem.starts_with(b"1234"));
mem[..6].copy_from_slice(b"567890");

Ok(())
}

#[test]
#[cfg(target_pointer_width = "64")]
fn instance_too_large() -> Result<()> {
Expand Down

0 comments on commit 2614f2e

Please sign in to comment.