Skip to content
This repository was archived by the owner on Nov 15, 2023. It is now read-only.

Commit ee3eb8f

Browse files
authored
Prevent unsoundness in environments with broken madvise(MADV_DONTNEED) (#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`
1 parent c28702f commit ee3eb8f

File tree

4 files changed

+187
-14
lines changed

4 files changed

+187
-14
lines changed

Cargo.lock

Lines changed: 80 additions & 9 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

client/executor/wasmtime/Cargo.toml

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,10 @@ sp-runtime-interface = { version = "6.0.0", path = "../../../primitives/runtime-
3232
sp-sandbox = { version = "0.10.0-dev", path = "../../../primitives/sandbox" }
3333
sp-wasm-interface = { version = "6.0.0", features = ["wasmtime"], path = "../../../primitives/wasm-interface" }
3434

35+
[target.'cfg(target_os = "linux")'.dependencies]
36+
rustix = { version = "0.35.6", default-features = false, features = ["std", "mm", "fs", "param"] }
37+
once_cell = "1.12.0"
38+
3539
[dev-dependencies]
3640
wat = "1.0"
3741
sc-runtime-test = { version = "2.0.0", path = "../runtime-test" }

client/executor/wasmtime/src/runtime.rs

Lines changed: 11 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@
2121
use crate::{
2222
host::HostState,
2323
instance_wrapper::{EntryPoint, InstanceWrapper},
24-
util,
24+
util::{self, replace_strategy_if_broken},
2525
};
2626

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

472+
#[derive(Clone)]
471473
pub struct Semantics {
472474
/// The instantiation strategy to use.
473475
pub instantiation_strategy: InstantiationStrategy,
@@ -598,11 +600,13 @@ where
598600
/// [`create_runtime_from_artifact`] to get more details.
599601
unsafe fn do_create_runtime<H>(
600602
code_supply_mode: CodeSupplyMode<'_>,
601-
config: Config,
603+
mut config: Config,
602604
) -> std::result::Result<WasmtimeRuntime, WasmError>
603605
where
604606
H: HostFunctions,
605607
{
608+
replace_strategy_if_broken(&mut config.semantics.instantiation_strategy);
609+
606610
let mut wasmtime_config = common_config(&config.semantics)?;
607611
if let Some(ref cache_path) = config.cache_path {
608612
if let Err(reason) = setup_wasmtime_caching(cache_path, &mut wasmtime_config) {
@@ -719,9 +723,12 @@ pub fn prepare_runtime_artifact(
719723
blob: RuntimeBlob,
720724
semantics: &Semantics,
721725
) -> std::result::Result<Vec<u8>, WasmError> {
722-
let blob = prepare_blob_for_compilation(blob, semantics)?;
726+
let mut semantics = semantics.clone();
727+
replace_strategy_if_broken(&mut semantics.instantiation_strategy);
728+
729+
let blob = prepare_blob_for_compilation(blob, &semantics)?;
723730

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

727734
engine

client/executor/wasmtime/src/util.rs

Lines changed: 92 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@
1616
// You should have received a copy of the GNU General Public License
1717
// along with this program. If not, see <https://www.gnu.org/licenses/>.
1818

19-
use crate::runtime::StoreData;
19+
use crate::{runtime::StoreData, InstantiationStrategy};
2020
use sc_executor_common::{
2121
error::{Error, Result},
2222
util::checked_range,
@@ -98,3 +98,94 @@ pub(crate) fn write_memory_from(
9898
memory[range].copy_from_slice(data);
9999
Ok(())
100100
}
101+
102+
/// Checks whether the `madvise(MADV_DONTNEED)` works as expected.
103+
///
104+
/// In certain environments (e.g. when running under the QEMU user-mode emulator)
105+
/// this syscall is broken.
106+
#[cfg(target_os = "linux")]
107+
fn is_madvise_working() -> std::result::Result<bool, String> {
108+
let page_size = rustix::param::page_size();
109+
110+
unsafe {
111+
// Allocate two memory pages.
112+
let pointer = rustix::mm::mmap_anonymous(
113+
std::ptr::null_mut(),
114+
2 * page_size,
115+
rustix::mm::ProtFlags::READ | rustix::mm::ProtFlags::WRITE,
116+
rustix::mm::MapFlags::PRIVATE,
117+
)
118+
.map_err(|error| format!("mmap failed: {}", error))?;
119+
120+
// Dirty them both.
121+
std::ptr::write_volatile(pointer.cast::<u8>(), b'A');
122+
std::ptr::write_volatile(pointer.cast::<u8>().add(page_size), b'B');
123+
124+
// Clear the first page.
125+
let result_madvise =
126+
rustix::mm::madvise(pointer, page_size, rustix::mm::Advice::LinuxDontNeed)
127+
.map_err(|error| format!("madvise failed: {}", error));
128+
129+
// Fetch the values.
130+
let value_1 = std::ptr::read_volatile(pointer.cast::<u8>());
131+
let value_2 = std::ptr::read_volatile(pointer.cast::<u8>().add(page_size));
132+
133+
let result_munmap = rustix::mm::munmap(pointer, 2 * page_size)
134+
.map_err(|error| format!("munmap failed: {}", error));
135+
136+
result_madvise?;
137+
result_munmap?;
138+
139+
// Verify that the first page was cleared, while the second one was not.
140+
Ok(value_1 == 0 && value_2 == b'B')
141+
}
142+
}
143+
144+
#[cfg(test)]
145+
#[cfg(target_os = "linux")]
146+
#[test]
147+
fn test_is_madvise_working_check_does_not_fail() {
148+
assert!(is_madvise_working().is_ok());
149+
}
150+
151+
/// Checks whether a given instantiation strategy can be safely used, and replaces
152+
/// it with a slower (but sound) alternative if it isn't.
153+
#[cfg(target_os = "linux")]
154+
pub(crate) fn replace_strategy_if_broken(strategy: &mut InstantiationStrategy) {
155+
let replacement_strategy = match *strategy {
156+
// These strategies don't need working `madvise`.
157+
InstantiationStrategy::Pooling | InstantiationStrategy::RecreateInstance => return,
158+
159+
// These strategies require a working `madvise` to be sound.
160+
InstantiationStrategy::PoolingCopyOnWrite => InstantiationStrategy::Pooling,
161+
InstantiationStrategy::RecreateInstanceCopyOnWrite |
162+
InstantiationStrategy::LegacyInstanceReuse => InstantiationStrategy::RecreateInstance,
163+
};
164+
165+
use once_cell::sync::OnceCell;
166+
static IS_OK: OnceCell<bool> = OnceCell::new();
167+
168+
let is_ok = IS_OK.get_or_init(|| {
169+
let is_ok = match is_madvise_working() {
170+
Ok(is_ok) => is_ok,
171+
Err(error) => {
172+
// This should never happen.
173+
log::warn!("Failed to check whether `madvise(MADV_DONTNEED)` works: {}", error);
174+
false
175+
}
176+
};
177+
178+
if !is_ok {
179+
log::warn!("You're running on a system with a broken `madvise(MADV_DONTNEED)` implementation. This will result in lower performance.");
180+
}
181+
182+
is_ok
183+
});
184+
185+
if !is_ok {
186+
*strategy = replacement_strategy;
187+
}
188+
}
189+
190+
#[cfg(not(target_os = "linux"))]
191+
pub(crate) fn replace_strategy_if_broken(_: &mut InstantiationStrategy) {}

0 commit comments

Comments
 (0)