T0b1/wasmtime
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Panic on resetting image slots back to anonymous memory (#3841)

Browse Source 
* Panic on resetting image slots back to anonymous memory

This commit updates `Drop for MemoryImageSlot` to panic instead of
ignoring errors when resetting memory back to a clean slate. On reading
some of this code again for a different change I realized that if an
error happens in `reset_with_anon_memory` it would be possible,
depending on where another error happened, to leak memory from one image
to another.

For example if `clear_and_remain_ready` failed its `madvise` (for
whatever reason) and didn't actually reset any memory, then if `Drop for
MemoryImageSlot` also hit an error trying to remap memory (for whatever
reason), then nothing about memory has changed and when the
`MemoryImageSlot` is recreated it'll think that it's 0-length when
actually it's a bit larger and may leak data.

I don't think this is a serious problem since we don't know any
situation under which the `madvise` would fail and/or the resetting with
anonymous memory, but given that these aren't expected to fail I figure
it's best to be a bit more defensive here and/or loud about failures.

* Update a comment
This commit is contained in:
Alex Crichton
2022-02-23 14:00:06 -06:00
committed by GitHub
parent 5b7df72bce
commit 434e35c490
1 changed files with 7 additions and 6 deletions
Download Patch File Download Diff File Expand all files Collapse all files

13
crates/runtime/src/cow.rs
View File

@@ -566,17 +566,18 @@ impl Drop for MemoryImageSlot {
// over by the next MemoryImageSlot later. // over by the next MemoryImageSlot later.
// //
// Since we're in drop(), we can't sanely return an error if // Since we're in drop(), we can't sanely return an error if
// this mmap fails. Let's ignore the failure if so; the next // this mmap fails. Instead though the result is unwrapped here to
// MemoryImageSlot to be created for this slot will try to overwrite // trigger a panic if something goes wrong. Otherwise if this
// the existing stale mappings, and return a failure properly // reset-the-mapping fails then on reuse it might be possible, depending
// if we still cannot map new memory. // on precisely where errors happened, that stale memory could get
// leaked through.
// //
// The exception to all of this is if the `unmap_on_drop` flag // The exception to all of this is if the `clear_on_drop` flag
// (which is set by default) is false. If so, the owner of // (which is set by default) is false. If so, the owner of
// this MemoryImageSlot has indicated that it will clean up in some // this MemoryImageSlot has indicated that it will clean up in some
// other way. // other way.
if self.clear_on_drop { if self.clear_on_drop {
let _ = self.reset_with_anon_memory(); self.reset_with_anon_memory().unwrap();
} }
} }
} }