Skip to content
This repository has been archived by the owner on Mar 24, 2022. It is now read-only.
Permalink
Browse files Browse the repository at this point in the history
Merge pull request from GHSA-hf79-8hjp-rrvq
* Use manual drop

* Add some comments to `ManuallyDrop` usage

* rustfmt

Co-authored-by: Aaron Turon <aturon@fastly.com>
Co-authored-by: Alex Crichton <alex@alexcrichton.com>
  • Loading branch information
3 people committed Nov 29, 2021
1 parent 8fb1fec commit 7c7757c
Showing 1 changed file with 26 additions and 13 deletions.
39 changes: 26 additions & 13 deletions lucet-runtime/lucet-runtime-internals/src/instance.rs
Expand Up @@ -14,7 +14,6 @@ use crate::error::Error;
#[cfg(feature = "concurrent_testpoints")]
use crate::lock_testpoints::LockTestpoints;
use crate::module::{self, FunctionHandle, Global, GlobalValue, Module, TrapCode};
use crate::region::RegionInternal;
use crate::sysdeps::HOST_PAGE_SIZE_EXPECTED;
use crate::val::{UntypedRetVal, Val};
use crate::vmctx::Vmctx;
Expand All @@ -27,6 +26,7 @@ use std::cell::{BorrowError, BorrowMutError, Ref, RefCell, RefMut, UnsafeCell};
use std::convert::TryFrom;
use std::marker::PhantomData;
use std::mem;
use std::mem::ManuallyDrop;
use std::ops::{Deref, DerefMut};
use std::ptr::{self, NonNull};
use std::sync::Arc;
Expand Down Expand Up @@ -147,20 +147,29 @@ impl Drop for InstanceHandle {
unsafe {
let inst = self.inst.as_mut();

// Grab a handle to the region to ensure it outlives `inst`.
// The `inst.alloc` field manages the memory of the instance
// itself. Note, though, that this field is in a `ManuallyDrop`
// so it won't get dropped automatically in `drop_in_place`.
// This is the point where we take over that precise drop.
//
// This ensures that the region won't be dropped by `inst` being
// dropped, which could result in `inst` being unmapped by the
// Region *during* drop of the Instance's fields.
let region: Arc<dyn RegionInternal> = inst.alloc().region.clone();
// By using `take` here we're basically calling `ptr::read`
// which "duplicates" the `alloc` since the `alloc` local
// variable here is the exact same as `inst.alloc`. All we do
// with `inst`, though, is call `drop_in_place`, which
// invalidates every other field in `inst`.
let alloc: Alloc = ManuallyDrop::take(&mut inst.alloc);

// drop the actual instance
std::ptr::drop_in_place(inst);

// and now we can drop what may be the last Arc<Region>. If it is
// it can safely do what it needs with memory; we're not running
// destructors on it anymore.
mem::drop(region);
// Now that we're 100% done with the instance, destructors and
// all, we can release the memory of the instance back to the
// original allocator from whence it came (be it mmap or uffd
// based). This will run the "official" destructor for `Alloc`
// which internally does the release. Note that after this
// operation the `inst` pointer is invalid and can no longer be
// used.
drop(alloc);
}
}
}
Expand Down Expand Up @@ -233,8 +242,12 @@ pub struct Instance {
/// Conditionally-present helpers to force permutations of possible races in testing.
pub lock_testpoints: Arc<LockTestpoints>,

/// The memory allocated for this instance
alloc: Alloc,
/// The memory allocated for this instance.
///
/// Note that this is in a `ManuallyDrop` because this manages the memory of
/// this `Instance` itself. To have precise control over this memory we
/// handle this in `Drop for InstanceHandle`.
alloc: ManuallyDrop<Alloc>,

/// Handler run for signals that do not arise from a known WebAssembly trap, or that involve
/// memory outside of the current instance.
Expand Down Expand Up @@ -1055,7 +1068,7 @@ impl Instance {
kill_state,
#[cfg(feature = "concurrent_testpoints")]
lock_testpoints,
alloc,
alloc: ManuallyDrop::new(alloc),
fatal_handler: default_fatal_handler,
c_fatal_handler: None,
signal_handler: Box::new(signal_handler_none) as Box<SignalHandler>,
Expand Down

0 comments on commit 7c7757c

Please sign in to comment.