Skip to content

Commit

Permalink
Prevent unsoundness in environments with broken `madvise(MADV_DONTNEE…
Browse files Browse the repository at this point in the history
…D)` (#11722)

* Prevend unsoundness in environments with broken `madvise(MADV_DONTNEED)`

* Add the `std` feature to `rustix` dependency

Apparently not having this breaks compilation on non-nightly toolchains.

* Autodetect the page size when checking whether `madvise` works

* Only make sure that the madvice check doesn't return `Err`
  • Loading branch information
koute authored Jun 28, 2022
1 parent c28702f commit ee3eb8f
Show file tree
Hide file tree
Showing 4 changed files with 187 additions and 14 deletions.
89 changes: 80 additions & 9 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 4 additions & 0 deletions client/executor/wasmtime/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,10 @@ sp-runtime-interface = { version = "6.0.0", path = "../../../primitives/runtime-
sp-sandbox = { version = "0.10.0-dev", path = "../../../primitives/sandbox" }
sp-wasm-interface = { version = "6.0.0", features = ["wasmtime"], path = "../../../primitives/wasm-interface" }

[target.'cfg(target_os = "linux")'.dependencies]
rustix = { version = "0.35.6", default-features = false, features = ["std", "mm", "fs", "param"] }
once_cell = "1.12.0"

[dev-dependencies]
wat = "1.0"
sc-runtime-test = { version = "2.0.0", path = "../runtime-test" }
Expand Down
15 changes: 11 additions & 4 deletions client/executor/wasmtime/src/runtime.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@
use crate::{
host::HostState,
instance_wrapper::{EntryPoint, InstanceWrapper},
util,
util::{self, replace_strategy_if_broken},
};

use sc_allocator::FreeingBumpHeapAllocator;
Expand Down Expand Up @@ -411,6 +411,7 @@ fn common_config(semantics: &Semantics) -> std::result::Result<wasmtime::Config,
/// See [here][stack_height] for more details of the instrumentation
///
/// [stack_height]: https://github.com/paritytech/wasm-utils/blob/d9432baf/src/stack_height/mod.rs#L1-L50
#[derive(Clone)]
pub struct DeterministicStackLimit {
/// A number of logical "values" that can be pushed on the wasm stack. A trap will be triggered
/// if exceeded.
Expand Down Expand Up @@ -468,6 +469,7 @@ enum InternalInstantiationStrategy {
Builtin,
}

#[derive(Clone)]
pub struct Semantics {
/// The instantiation strategy to use.
pub instantiation_strategy: InstantiationStrategy,
Expand Down Expand Up @@ -598,11 +600,13 @@ where
/// [`create_runtime_from_artifact`] to get more details.
unsafe fn do_create_runtime<H>(
code_supply_mode: CodeSupplyMode<'_>,
config: Config,
mut config: Config,
) -> std::result::Result<WasmtimeRuntime, WasmError>
where
H: HostFunctions,
{
replace_strategy_if_broken(&mut config.semantics.instantiation_strategy);

let mut wasmtime_config = common_config(&config.semantics)?;
if let Some(ref cache_path) = config.cache_path {
if let Err(reason) = setup_wasmtime_caching(cache_path, &mut wasmtime_config) {
Expand Down Expand Up @@ -719,9 +723,12 @@ pub fn prepare_runtime_artifact(
blob: RuntimeBlob,
semantics: &Semantics,
) -> std::result::Result<Vec<u8>, WasmError> {
let blob = prepare_blob_for_compilation(blob, semantics)?;
let mut semantics = semantics.clone();
replace_strategy_if_broken(&mut semantics.instantiation_strategy);

let blob = prepare_blob_for_compilation(blob, &semantics)?;

let engine = Engine::new(&common_config(semantics)?)
let engine = Engine::new(&common_config(&semantics)?)
.map_err(|e| WasmError::Other(format!("cannot create the engine: {}", e)))?;

engine
Expand Down
93 changes: 92 additions & 1 deletion client/executor/wasmtime/src/util.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@
// You should have received a copy of the GNU General Public License
// along with this program. If not, see <https://www.gnu.org/licenses/>.

use crate::runtime::StoreData;
use crate::{runtime::StoreData, InstantiationStrategy};
use sc_executor_common::{
error::{Error, Result},
util::checked_range,
Expand Down Expand Up @@ -98,3 +98,94 @@ pub(crate) fn write_memory_from(
memory[range].copy_from_slice(data);
Ok(())
}

/// Checks whether the `madvise(MADV_DONTNEED)` works as expected.
///
/// In certain environments (e.g. when running under the QEMU user-mode emulator)
/// this syscall is broken.
#[cfg(target_os = "linux")]
fn is_madvise_working() -> std::result::Result<bool, String> {
let page_size = rustix::param::page_size();

unsafe {
// Allocate two memory pages.
let pointer = rustix::mm::mmap_anonymous(
std::ptr::null_mut(),
2 * page_size,
rustix::mm::ProtFlags::READ | rustix::mm::ProtFlags::WRITE,
rustix::mm::MapFlags::PRIVATE,
)
.map_err(|error| format!("mmap failed: {}", error))?;

// Dirty them both.
std::ptr::write_volatile(pointer.cast::<u8>(), b'A');
std::ptr::write_volatile(pointer.cast::<u8>().add(page_size), b'B');

// Clear the first page.
let result_madvise =
rustix::mm::madvise(pointer, page_size, rustix::mm::Advice::LinuxDontNeed)
.map_err(|error| format!("madvise failed: {}", error));

// Fetch the values.
let value_1 = std::ptr::read_volatile(pointer.cast::<u8>());
let value_2 = std::ptr::read_volatile(pointer.cast::<u8>().add(page_size));

let result_munmap = rustix::mm::munmap(pointer, 2 * page_size)
.map_err(|error| format!("munmap failed: {}", error));

result_madvise?;
result_munmap?;

// Verify that the first page was cleared, while the second one was not.
Ok(value_1 == 0 && value_2 == b'B')
}
}

#[cfg(test)]
#[cfg(target_os = "linux")]
#[test]
fn test_is_madvise_working_check_does_not_fail() {
assert!(is_madvise_working().is_ok());
}

/// Checks whether a given instantiation strategy can be safely used, and replaces
/// it with a slower (but sound) alternative if it isn't.
#[cfg(target_os = "linux")]
pub(crate) fn replace_strategy_if_broken(strategy: &mut InstantiationStrategy) {
let replacement_strategy = match *strategy {
// These strategies don't need working `madvise`.
InstantiationStrategy::Pooling | InstantiationStrategy::RecreateInstance => return,

// These strategies require a working `madvise` to be sound.
InstantiationStrategy::PoolingCopyOnWrite => InstantiationStrategy::Pooling,
InstantiationStrategy::RecreateInstanceCopyOnWrite |
InstantiationStrategy::LegacyInstanceReuse => InstantiationStrategy::RecreateInstance,
};

use once_cell::sync::OnceCell;
static IS_OK: OnceCell<bool> = OnceCell::new();

let is_ok = IS_OK.get_or_init(|| {
let is_ok = match is_madvise_working() {
Ok(is_ok) => is_ok,
Err(error) => {
// This should never happen.
log::warn!("Failed to check whether `madvise(MADV_DONTNEED)` works: {}", error);
false
}
};

if !is_ok {
log::warn!("You're running on a system with a broken `madvise(MADV_DONTNEED)` implementation. This will result in lower performance.");
}

is_ok
});

if !is_ok {
*strategy = replacement_strategy;
}
}

#[cfg(not(target_os = "linux"))]
pub(crate) fn replace_strategy_if_broken(_: &mut InstantiationStrategy) {}

0 comments on commit ee3eb8f

Please sign in to comment.