Skip to content

Commit 3535acb

Browse files
authored
Merge pull request from GHSA-wh6w-3828-g9qf
* Unconditionally use `MemoryImageSlot` This commit removes the internal branching within the pooling instance allocator to sometimes use a `MemoryImageSlot` and sometimes now. Instead this is now unconditionally used in all situations on all platforms. This fixes an issue where the state of a slot could get corrupted if modules being instantiated switched from having images to not having an image or vice versa. The bulk of this commit is the removal of the `memory-init-cow` compile-time feature in addition to adding Windows support to the `cow.rs` file. * Fix compile on Unix * Add a stricter assertion for static memory bounds Double-check that when a memory is allocated the configuration required is satisfied by the pooling allocator.
1 parent 47fa1ad commit 3535acb

File tree

16 files changed

+247
-333
lines changed

16 files changed

+247
-333
lines changed

Cargo.toml

-2
Original file line numberDiff line numberDiff line change
@@ -181,13 +181,11 @@ default = [
181181
"vtune",
182182
"wasi-nn",
183183
"pooling-allocator",
184-
"memory-init-cow",
185184
]
186185
jitdump = ["wasmtime/jitdump"]
187186
vtune = ["wasmtime/vtune"]
188187
wasi-crypto = ["dep:wasmtime-wasi-crypto"]
189188
wasi-nn = ["dep:wasmtime-wasi-nn"]
190-
memory-init-cow = ["wasmtime/memory-init-cow", "wasmtime-cli-flags/memory-init-cow"]
191189
pooling-allocator = ["wasmtime/pooling-allocator", "wasmtime-cli-flags/pooling-allocator"]
192190
all-arch = ["wasmtime/all-arch"]
193191
posix-signals-on-macos = ["wasmtime/posix-signals-on-macos"]

crates/c-api/Cargo.toml

+1-2
Original file line numberDiff line numberDiff line change
@@ -33,9 +33,8 @@ cap-std = { workspace = true, optional = true }
3333
wasi-common = { workspace = true, optional = true }
3434

3535
[features]
36-
default = ['jitdump', 'wat', 'wasi', 'cache', 'parallel-compilation', 'memory-init-cow']
36+
default = ['jitdump', 'wat', 'wasi', 'cache', 'parallel-compilation']
3737
jitdump = ["wasmtime/jitdump"]
3838
cache = ["wasmtime/cache"]
3939
parallel-compilation = ['wasmtime/parallel-compilation']
4040
wasi = ['wasi-cap-std-sync', 'wasmtime-wasi', 'cap-std', 'wasi-common']
41-
memory-init-cow = ["wasmtime/memory-init-cow"]

crates/cli-flags/Cargo.toml

-1
Original file line numberDiff line numberDiff line change
@@ -25,5 +25,4 @@ default = [
2525
"wasmtime/parallel-compilation",
2626
]
2727
pooling-allocator = []
28-
memory-init-cow = []
2928
component-model = []

crates/cli-flags/src/lib.rs

-2
Original file line numberDiff line numberDiff line change
@@ -218,7 +218,6 @@ pub struct CommonOptions {
218218

219219
/// Disable the default of attempting to initialize linear memory via a
220220
/// copy-on-write mapping
221-
#[cfg(feature = "memory-init-cow")]
222221
#[clap(long)]
223222
pub disable_memory_init_cow: bool,
224223

@@ -324,7 +323,6 @@ impl CommonOptions {
324323

325324
config.epoch_interruption(self.epoch_interruption);
326325
config.generate_address_map(!self.disable_address_map);
327-
#[cfg(feature = "memory-init-cow")]
328326
config.memory_init_cow(!self.disable_memory_init_cow);
329327

330328
#[cfg(feature = "pooling-allocator")]

crates/runtime/Cargo.toml

+1-3
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@ thiserror = "1.0.4"
2323
cfg-if = "1.0"
2424
rand = "0.8.3"
2525
anyhow = { workspace = true }
26-
memfd = { version = "0.6.1", optional = true }
26+
memfd = "0.6.1"
2727
paste = "1.0.3"
2828
encoding_rs = { version = "0.8.31", optional = true }
2929

@@ -52,8 +52,6 @@ cc = "1.0"
5252
maintenance = { status = "actively-developed" }
5353

5454
[features]
55-
memory-init-cow = ['memfd']
56-
5755
async = ["wasmtime-fiber"]
5856

5957
# Enables support for the pooling instance allocator

crates/runtime/build.rs

-9
Original file line numberDiff line numberDiff line change
@@ -14,13 +14,4 @@ fn main() {
1414
println!("cargo:rerun-if-changed=src/helpers.c");
1515
build.file("src/helpers.c");
1616
build.compile("wasmtime-helpers");
17-
18-
// Check to see if we are on Unix and the `memory-init-cow` feature is
19-
// active. If so, enable the `memory_init_cow` rustc cfg so
20-
// `#[cfg(memory_init_cow)]` will work.
21-
let family = env::var("CARGO_CFG_TARGET_FAMILY").unwrap();
22-
let memory_init_cow = env::var("CARGO_FEATURE_MEMORY_INIT_COW").is_ok();
23-
if &family == "unix" && memory_init_cow {
24-
println!("cargo:rustc-cfg=memory_init_cow");
25-
}
2617
}

crates/runtime/src/cow.rs

+117-48
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,12 @@
11
//! Copy-on-write initialization support: creation of backing images for
22
//! modules, and logic to support mapping these backing images into memory.
33
4+
#![cfg_attr(not(unix), allow(unused_imports, unused_variables))]
5+
46
use crate::InstantiationError;
57
use crate::MmapVec;
68
use anyhow::Result;
79
use libc::c_void;
8-
use rustix::fd::AsRawFd;
910
use std::fs::File;
1011
use std::sync::Arc;
1112
use std::{convert::TryFrom, ops::Range};
@@ -60,24 +61,34 @@ pub struct MemoryImage {
6061

6162
#[derive(Debug)]
6263
enum FdSource {
64+
#[cfg(unix)]
6365
Mmap(Arc<File>),
6466
#[cfg(target_os = "linux")]
6567
Memfd(memfd::Memfd),
6668
}
6769

6870
impl FdSource {
71+
#[cfg(unix)]
6972
fn as_file(&self) -> &File {
7073
match self {
71-
FdSource::Mmap(file) => file,
74+
FdSource::Mmap(ref file) => file,
7275
#[cfg(target_os = "linux")]
73-
FdSource::Memfd(memfd) => memfd.as_file(),
76+
FdSource::Memfd(ref memfd) => memfd.as_file(),
7477
}
7578
}
7679
}
7780

7881
impl PartialEq for FdSource {
7982
fn eq(&self, other: &FdSource) -> bool {
80-
self.as_file().as_raw_fd() == other.as_file().as_raw_fd()
83+
cfg_if::cfg_if! {
84+
if #[cfg(unix)] {
85+
use rustix::fd::AsRawFd;
86+
self.as_file().as_raw_fd() == other.as_file().as_raw_fd()
87+
} else {
88+
drop(other);
89+
match *self {}
90+
}
91+
}
8192
}
8293
}
8394

@@ -111,6 +122,7 @@ impl MemoryImage {
111122
// files, but for now this is still a Linux-specific region of Wasmtime.
112123
// Some work will be needed to get this file compiling for macOS and
113124
// Windows.
125+
#[cfg(not(windows))]
114126
if let Some(mmap) = mmap {
115127
let start = mmap.as_ptr() as usize;
116128
let end = start + mmap.len();
@@ -185,6 +197,42 @@ impl MemoryImage {
185197
}
186198
}
187199
}
200+
201+
unsafe fn map_at(&self, base: usize) -> Result<()> {
202+
cfg_if::cfg_if! {
203+
if #[cfg(unix)] {
204+
let ptr = rustix::mm::mmap(
205+
(base + self.linear_memory_offset) as *mut c_void,
206+
self.len,
207+
rustix::mm::ProtFlags::READ | rustix::mm::ProtFlags::WRITE,
208+
rustix::mm::MapFlags::PRIVATE | rustix::mm::MapFlags::FIXED,
209+
self.fd.as_file(),
210+
self.fd_offset,
211+
)?;
212+
assert_eq!(ptr as usize, base + self.linear_memory_offset);
213+
Ok(())
214+
} else {
215+
match self.fd {}
216+
}
217+
}
218+
}
219+
220+
unsafe fn remap_as_zeros_at(&self, base: usize) -> Result<()> {
221+
cfg_if::cfg_if! {
222+
if #[cfg(unix)] {
223+
let ptr = rustix::mm::mmap_anonymous(
224+
(base + self.linear_memory_offset) as *mut c_void,
225+
self.len,
226+
rustix::mm::ProtFlags::READ | rustix::mm::ProtFlags::WRITE,
227+
rustix::mm::MapFlags::PRIVATE | rustix::mm::MapFlags::FIXED,
228+
)?;
229+
assert_eq!(ptr as usize, base + self.linear_memory_offset);
230+
Ok(())
231+
} else {
232+
match self.fd {}
233+
}
234+
}
235+
}
188236
}
189237

190238
#[cfg(target_os = "linux")]
@@ -374,6 +422,17 @@ impl MemoryImageSlot {
374422
}
375423
}
376424

425+
pub(crate) fn dummy() -> MemoryImageSlot {
426+
MemoryImageSlot {
427+
base: 0,
428+
static_size: 0,
429+
image: None,
430+
accessible: 0,
431+
dirty: false,
432+
clear_on_drop: false,
433+
}
434+
}
435+
377436
/// Inform the MemoryImageSlot that it should *not* clear the underlying
378437
/// address space when dropped. This should be used only when the
379438
/// caller will clear or reuse the address space in some other
@@ -396,10 +455,7 @@ impl MemoryImageSlot {
396455
}
397456

398457
// Otherwise use `mprotect` to make the new pages read/write.
399-
self.set_protection(
400-
self.accessible..size_bytes,
401-
rustix::mm::MprotectFlags::READ | rustix::mm::MprotectFlags::WRITE,
402-
)?;
458+
self.set_protection(self.accessible..size_bytes, true)?;
403459
self.accessible = size_bytes;
404460

405461
Ok(())
@@ -444,14 +500,9 @@ impl MemoryImageSlot {
444500
if self.image.as_ref() != maybe_image {
445501
if let Some(image) = &self.image {
446502
unsafe {
447-
let ptr = rustix::mm::mmap_anonymous(
448-
(self.base + image.linear_memory_offset) as *mut c_void,
449-
image.len,
450-
rustix::mm::ProtFlags::READ | rustix::mm::ProtFlags::WRITE,
451-
rustix::mm::MapFlags::PRIVATE | rustix::mm::MapFlags::FIXED,
452-
)
453-
.map_err(|e| InstantiationError::Resource(e.into()))?;
454-
assert_eq!(ptr as usize, self.base + image.linear_memory_offset);
503+
image
504+
.remap_as_zeros_at(self.base)
505+
.map_err(|e| InstantiationError::Resource(e.into()))?;
455506
}
456507
self.image = None;
457508
}
@@ -461,11 +512,8 @@ impl MemoryImageSlot {
461512
// appropriate. First up is to grow the read/write portion of memory if
462513
// it's not large enough to accommodate `initial_size_bytes`.
463514
if self.accessible < initial_size_bytes {
464-
self.set_protection(
465-
self.accessible..initial_size_bytes,
466-
rustix::mm::MprotectFlags::READ | rustix::mm::MprotectFlags::WRITE,
467-
)
468-
.map_err(|e| InstantiationError::Resource(e.into()))?;
515+
self.set_protection(self.accessible..initial_size_bytes, true)
516+
.map_err(|e| InstantiationError::Resource(e.into()))?;
469517
self.accessible = initial_size_bytes;
470518
}
471519

@@ -480,11 +528,8 @@ impl MemoryImageSlot {
480528
if initial_size_bytes < self.accessible {
481529
match style {
482530
MemoryStyle::Static { .. } => {
483-
self.set_protection(
484-
initial_size_bytes..self.accessible,
485-
rustix::mm::MprotectFlags::empty(),
486-
)
487-
.map_err(|e| InstantiationError::Resource(e.into()))?;
531+
self.set_protection(initial_size_bytes..self.accessible, false)
532+
.map_err(|e| InstantiationError::Resource(e.into()))?;
488533
self.accessible = initial_size_bytes;
489534
}
490535
MemoryStyle::Dynamic { .. } => {}
@@ -503,16 +548,9 @@ impl MemoryImageSlot {
503548
);
504549
if image.len > 0 {
505550
unsafe {
506-
let ptr = rustix::mm::mmap(
507-
(self.base + image.linear_memory_offset) as *mut c_void,
508-
image.len,
509-
rustix::mm::ProtFlags::READ | rustix::mm::ProtFlags::WRITE,
510-
rustix::mm::MapFlags::PRIVATE | rustix::mm::MapFlags::FIXED,
511-
image.fd.as_file(),
512-
image.fd_offset,
513-
)
514-
.map_err(|e| InstantiationError::Resource(e.into()))?;
515-
assert_eq!(ptr as usize, self.base + image.linear_memory_offset);
551+
image
552+
.map_at(self.base)
553+
.map_err(|e| InstantiationError::Resource(e.into()))?;
516554
}
517555
}
518556
}
@@ -675,13 +713,35 @@ impl MemoryImageSlot {
675713
}
676714
}
677715

678-
fn set_protection(&self, range: Range<usize>, flags: rustix::mm::MprotectFlags) -> Result<()> {
716+
fn set_protection(&self, range: Range<usize>, readwrite: bool) -> Result<()> {
679717
assert!(range.start <= range.end);
680718
assert!(range.end <= self.static_size);
681-
let mprotect_start = self.base.checked_add(range.start).unwrap();
682-
if range.len() > 0 {
683-
unsafe {
684-
rustix::mm::mprotect(mprotect_start as *mut _, range.len(), flags)?;
719+
let start = self.base.checked_add(range.start).unwrap();
720+
if range.len() == 0 {
721+
return Ok(());
722+
}
723+
724+
unsafe {
725+
cfg_if::cfg_if! {
726+
if #[cfg(unix)] {
727+
let flags = if readwrite {
728+
rustix::mm::MprotectFlags::READ | rustix::mm::MprotectFlags::WRITE
729+
} else {
730+
rustix::mm::MprotectFlags::empty()
731+
};
732+
rustix::mm::mprotect(start as *mut _, range.len(), flags)?;
733+
} else {
734+
use windows_sys::Win32::System::Memory::*;
735+
736+
let failure = if readwrite {
737+
VirtualAlloc(start as _, range.len(), MEM_COMMIT, PAGE_READWRITE).is_null()
738+
} else {
739+
VirtualFree(start as _, range.len(), MEM_DECOMMIT) == 0
740+
};
741+
if failure {
742+
return Err(std::io::Error::last_os_error().into());
743+
}
744+
}
685745
}
686746
}
687747

@@ -701,13 +761,22 @@ impl MemoryImageSlot {
701761
/// inaccessible. Used both during instantiate and during drop.
702762
fn reset_with_anon_memory(&mut self) -> Result<()> {
703763
unsafe {
704-
let ptr = rustix::mm::mmap_anonymous(
705-
self.base as *mut c_void,
706-
self.static_size,
707-
rustix::mm::ProtFlags::empty(),
708-
rustix::mm::MapFlags::PRIVATE | rustix::mm::MapFlags::FIXED,
709-
)?;
710-
assert_eq!(ptr as usize, self.base);
764+
cfg_if::cfg_if! {
765+
if #[cfg(unix)] {
766+
let ptr = rustix::mm::mmap_anonymous(
767+
self.base as *mut c_void,
768+
self.static_size,
769+
rustix::mm::ProtFlags::empty(),
770+
rustix::mm::MapFlags::PRIVATE | rustix::mm::MapFlags::FIXED,
771+
)?;
772+
assert_eq!(ptr as usize, self.base);
773+
} else {
774+
use windows_sys::Win32::System::Memory::*;
775+
if VirtualFree(self.base as _, self.static_size, MEM_DECOMMIT) == 0 {
776+
return Err(std::io::Error::last_os_error().into());
777+
}
778+
}
779+
}
711780
}
712781

713782
self.image = None;

0 commit comments

Comments
 (0)