Skip to content

Commit

Permalink
Remove write-xor-execute feature + dead code (#84)
Browse files Browse the repository at this point in the history
If anyone actually cares, we can always add it later, but for now it's
just added cognitive burden.
  • Loading branch information
mkeeter authored Apr 14, 2024
1 parent 10263c6 commit 74113aa
Show file tree
Hide file tree
Showing 4 changed files with 5 additions and 72 deletions.
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,8 @@
(see [this scale-invariant adversarial model](https://www.mattkeeter.com/blog/2023-04-23-adversarial/)),
so the extra complexity isn't worth it.
- Added Windows support (including JIT)
- Removed `write-xor-execute` feature, which was extra cognitive load that no
one had actually asked for; if you care about it, let me know!

# 0.2.4
The highlight of this release is a refactoring of how shapes are handled in Rhai
Expand Down
5 changes: 0 additions & 5 deletions fidget/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -63,11 +63,6 @@ mesh = ["dep:crossbeam-deque"]
## evaluator type, e.g. `float_slice_tests!(...)`.
eval-tests = []

## On Linux, this feature uses `mprotect` to prevent JIT buffers from being both
## writable and executable at the same time. This is best practice from a
## security perspective, but incurs a 25% slowdown.
write-xor-execute = []

[dev-dependencies]
criterion = { version = "0.5", features = ["html_reports"] }

Expand Down
67 changes: 2 additions & 65 deletions fidget/src/jit/mmap.rs
Original file line number Diff line number Diff line change
Expand Up @@ -190,68 +190,21 @@ impl Mmap {
}
macos::ThreadWriteGuard
}

/// Modifies the region's W^X state to allow writing
///
/// This is only relevant on Linux and is a no-op on MacOS
pub fn make_write(&self) {
// Nothing to do here
}
}

#[cfg(target_os = "linux")]
impl Mmap {
#[cfg(feature = "write-xor-execute")]
pub const MMAP_PROT: i32 = libc::PROT_READ | libc::PROT_WRITE;

#[cfg(not(feature = "write-xor-execute"))]
pub const MMAP_PROT: i32 =
libc::PROT_READ | libc::PROT_WRITE | libc::PROT_EXEC;

pub const MMAP_FLAGS: i32 = libc::MAP_PRIVATE | libc::MAP_ANON;
pub const PAGE_SIZE: usize = 4096;

/// Flushes caches and remaps to an executable binding
/// Flushes caches in preparation for evaluation
///
/// The former is a no-op on systems with coherent D/I-caches (i.e. x86);
/// the latter is a no-op if the `write-xor-execute` feature is not enabled.
/// The former is a no-op on systems with coherent D/I-caches (i.e. x86).
pub fn finalize(&self, size: usize) {
self.flush_cache(size);

// This is deliberately done as a cfg! conditional (instead of #[cfg]),
// so that the code is type-checked even if the feature is disabled.
if cfg!(feature = "write-xor-execute") {
unsafe {
libc::mprotect(
self.ptr,
size,
libc::PROT_READ | libc::PROT_EXEC,
);
}
}
}

/// Modifies the **per-thread** W^X state to allow writing of memory-mapped
/// regions.
///
/// This is only relevant on macOS and is a no-op on Linux.
pub fn thread_mode_write() {
// Nothing to do here
}

/// Modifies the region's W^X state to allow writing
///
/// This is a no-op if the `write-xor-execute` feature is not enabled.
pub fn make_write(&self) {
if cfg!(feature = "write-xor-execute") {
unsafe {
libc::mprotect(
self.ptr,
self.len,
libc::PROT_READ | libc::PROT_WRITE,
);
}
}
}
}

Expand All @@ -262,22 +215,6 @@ impl Mmap {
pub fn finalize(&self, size: usize) {
self.flush_cache(size);
}

/// Modifies the **per-thread** W^X state to allow writing of memory-mapped
/// regions.
///
/// This is only relevant on macOS and is a no-op on Linux.
pub fn thread_mode_write() {
// Nothing to do here
}

/// Modifies the region's W^X state to allow writing
///
/// This is a no-op if the `write-xor-execute` feature is not enabled.
pub fn make_write(&self) {
#[cfg(feature = "write-xor-execute")]
compile_error!("write-xor-execute is not supported on Windows");
}
}

#[cfg(not(target_os = "windows"))]
Expand Down
3 changes: 1 addition & 2 deletions fidget/src/jit/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -635,15 +635,14 @@ fn build_asm_fn_with_storage<A: Assembler>(
mut s: Mmap,
) -> Mmap {
// This guard may be a unit value on some systems
#[allow(clippy::let_unit_value)]
#[cfg(target_os = "macos")]
let _guard = Mmap::thread_mode_write();

let size_estimate = t.len() * A::bytes_per_clause();
if size_estimate > 2 * s.len() {
s = Mmap::new(size_estimate).expect("failed to build mmap")
}

s.make_write();
let mut asm = A::init(s, t.slot_count());

for op in t.iter_asm() {
Expand Down

0 comments on commit 74113aa

Please sign in to comment.