From a00405649b5d11160768ae1f630a261fcc16315c Mon Sep 17 00:00:00 2001 From: Ben Kimock Date: Sat, 13 Aug 2022 18:37:59 -0400 Subject: [PATCH 1/3] mmap/munmap/mremamp shims --- src/tools/miri/src/concurrency/data_race.rs | 3 +- src/tools/miri/src/machine.rs | 14 +- .../miri/src/shims/unix/foreign_items.rs | 17 ++ .../src/shims/unix/macos/foreign_items.rs | 10 - src/tools/miri/src/shims/unix/mem.rs | 236 ++++++++++++++++++ src/tools/miri/src/shims/unix/mod.rs | 1 + .../miri/tests/fail/mmap_invalid_dealloc.rs | 18 ++ .../tests/fail/mmap_invalid_dealloc.stderr | 15 ++ .../miri/tests/fail/mmap_use_after_munmap.rs | 19 ++ .../tests/fail/mmap_use_after_munmap.stderr | 30 +++ src/tools/miri/tests/fail/munmap.stderr | 29 +++ src/tools/miri/tests/fail/munmap_partial.rs | 18 ++ .../miri/tests/fail/munmap_partial.stderr | 29 +++ src/tools/miri/tests/pass-dep/shims/mmap.rs | 83 ++++++ 14 files changed, 510 insertions(+), 12 deletions(-) create mode 100644 src/tools/miri/src/shims/unix/mem.rs create mode 100644 src/tools/miri/tests/fail/mmap_invalid_dealloc.rs create mode 100644 src/tools/miri/tests/fail/mmap_invalid_dealloc.stderr create mode 100644 src/tools/miri/tests/fail/mmap_use_after_munmap.rs create mode 100644 src/tools/miri/tests/fail/mmap_use_after_munmap.stderr create mode 100644 src/tools/miri/tests/fail/munmap.stderr create mode 100644 src/tools/miri/tests/fail/munmap_partial.rs create mode 100644 src/tools/miri/tests/fail/munmap_partial.stderr create mode 100644 src/tools/miri/tests/pass-dep/shims/mmap.rs diff --git a/src/tools/miri/src/concurrency/data_race.rs b/src/tools/miri/src/concurrency/data_race.rs index f6252c43f9fe..84ef27f73659 100644 --- a/src/tools/miri/src/concurrency/data_race.rs +++ b/src/tools/miri/src/concurrency/data_race.rs @@ -714,7 +714,8 @@ impl VClockAlloc { MiriMemoryKind::Rust | MiriMemoryKind::Miri | MiriMemoryKind::C - | MiriMemoryKind::WinHeap, + | MiriMemoryKind::WinHeap + | MiriMemoryKind::Mmap, ) | MemoryKind::Stack => { let (alloc_index, clocks) = global.current_thread_state(thread_mgr); diff --git a/src/tools/miri/src/machine.rs b/src/tools/miri/src/machine.rs index 0ba7dad56491..f88ad040e051 100644 --- a/src/tools/miri/src/machine.rs +++ b/src/tools/miri/src/machine.rs @@ -112,6 +112,8 @@ pub enum MiriMemoryKind { /// Memory for thread-local statics. /// This memory may leak. Tls, + /// Memory mapped directly by the program + Mmap, } impl From for MemoryKind { @@ -127,7 +129,7 @@ impl MayLeak for MiriMemoryKind { use self::MiriMemoryKind::*; match self { Rust | Miri | C | WinHeap | Runtime => false, - Machine | Global | ExternStatic | Tls => true, + Machine | Global | ExternStatic | Tls | Mmap => true, } } } @@ -145,6 +147,7 @@ impl fmt::Display for MiriMemoryKind { Global => write!(f, "global (static or const)"), ExternStatic => write!(f, "extern static"), Tls => write!(f, "thread-local static"), + Mmap => write!(f, "mmap"), } } } @@ -726,6 +729,15 @@ impl<'mir, 'tcx> MiriMachine<'mir, 'tcx> { // will panic when given the file. drop(self.profiler.take()); } + + pub(crate) fn round_up_to_multiple_of_page_size(&self, length: u64) -> Option { + #[allow(clippy::arithmetic_side_effects)] // page size is nonzero + (length.checked_add(self.page_size - 1)? / self.page_size).checked_mul(self.page_size) + } + + pub(crate) fn page_align(&self) -> Align { + Align::from_bytes(self.page_size).unwrap() + } } impl VisitTags for MiriMachine<'_, '_> { diff --git a/src/tools/miri/src/shims/unix/foreign_items.rs b/src/tools/miri/src/shims/unix/foreign_items.rs index c371e85c312e..d0f65306378a 100644 --- a/src/tools/miri/src/shims/unix/foreign_items.rs +++ b/src/tools/miri/src/shims/unix/foreign_items.rs @@ -10,6 +10,7 @@ use rustc_target::spec::abi::Abi; use crate::*; use shims::foreign_items::EmulateByNameResult; use shims::unix::fs::EvalContextExt as _; +use shims::unix::mem::EvalContextExt as _; use shims::unix::sync::EvalContextExt as _; use shims::unix::thread::EvalContextExt as _; @@ -213,6 +214,22 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> { } } + "mmap" => { + let [addr, length, prot, flags, fd, offset] = this.check_shim(abi, Abi::C {unwind: false}, link_name, args)?; + let ptr = this.mmap(addr, length, prot, flags, fd, offset)?; + this.write_scalar(ptr, dest)?; + } + "mremap" => { + let [old_address, old_size, new_size, flags] = this.check_shim(abi, Abi::C {unwind: false}, link_name, args)?; + let ptr = this.mremap(old_address, old_size, new_size, flags)?; + this.write_scalar(ptr, dest)?; + } + "munmap" => { + let [addr, length] = this.check_shim(abi, Abi::C {unwind: false}, link_name, args)?; + let result = this.munmap(addr, length)?; + this.write_scalar(result, dest)?; + } + // Dynamic symbol loading "dlsym" => { let [handle, symbol] = this.check_shim(abi, Abi::C { unwind: false }, link_name, args)?; diff --git a/src/tools/miri/src/shims/unix/macos/foreign_items.rs b/src/tools/miri/src/shims/unix/macos/foreign_items.rs index 1271788a97ef..85b950da4fe6 100644 --- a/src/tools/miri/src/shims/unix/macos/foreign_items.rs +++ b/src/tools/miri/src/shims/unix/macos/foreign_items.rs @@ -197,16 +197,6 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> { this.write_scalar(res, dest)?; } - // Incomplete shims that we "stub out" just to get pre-main initialization code to work. - // These shims are enabled only when the caller is in the standard library. - "mmap" if this.frame_in_std() => { - // This is a horrible hack, but since the guard page mechanism calls mmap and expects a particular return value, we just give it that value. - let [addr, _, _, _, _, _] = - this.check_shim(abi, Abi::C { unwind: false }, link_name, args)?; - let addr = this.read_scalar(addr)?; - this.write_scalar(addr, dest)?; - } - _ => return Ok(EmulateByNameResult::NotSupported), }; diff --git a/src/tools/miri/src/shims/unix/mem.rs b/src/tools/miri/src/shims/unix/mem.rs new file mode 100644 index 000000000000..1d01bc82a88b --- /dev/null +++ b/src/tools/miri/src/shims/unix/mem.rs @@ -0,0 +1,236 @@ +//! This is an incomplete implementation of mmap/mremap/munmap which is restricted in order to be +//! implementable on top of the existing memory system. The point of these function as-written is +//! to allow memory allocators written entirely in Rust to be executed by Miri. This implementation +//! does not support other uses of mmap such as file mappings. +//! +//! mmap/mremap/munmap behave a lot like alloc/realloc/dealloc, and for simple use they are exactly +//! equivalent. That is the only part we support: no MAP_FIXED or MAP_SHARED or anything +//! else that goes beyond a basic allocation API. + +use crate::*; +use rustc_target::abi::{Align, Size}; + +impl<'mir, 'tcx: 'mir> EvalContextExt<'mir, 'tcx> for crate::MiriInterpCx<'mir, 'tcx> {} +pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> { + fn mmap( + &mut self, + addr: &OpTy<'tcx, Provenance>, + length: &OpTy<'tcx, Provenance>, + prot: &OpTy<'tcx, Provenance>, + flags: &OpTy<'tcx, Provenance>, + fd: &OpTy<'tcx, Provenance>, + offset: &OpTy<'tcx, Provenance>, + ) -> InterpResult<'tcx, Scalar> { + let this = self.eval_context_mut(); + + // We do not support MAP_FIXED, so the addr argument is always ignored + let addr = this.read_pointer(addr)?; + let length = this.read_target_usize(length)?; + let prot = this.read_scalar(prot)?.to_i32()?; + let flags = this.read_scalar(flags)?.to_i32()?; + let fd = this.read_scalar(fd)?.to_i32()?; + let offset = this.read_scalar(offset)?.to_target_usize(this)?; + + let map_private = this.eval_libc_i32("MAP_PRIVATE"); + let map_anonymous = this.eval_libc_i32("MAP_ANONYMOUS"); + let map_shared = this.eval_libc_i32("MAP_SHARED"); + let map_fixed = this.eval_libc_i32("MAP_FIXED"); + + // This is a horrible hack, but on macos the guard page mechanism uses mmap + // in a way we do not support. We just give it the return value it expects. + if this.frame_in_std() && this.tcx.sess.target.os == "macos" && (flags & map_fixed) != 0 { + return Ok(Scalar::from_maybe_pointer(addr, this)); + } + + let prot_read = this.eval_libc_i32("PROT_READ"); + let prot_write = this.eval_libc_i32("PROT_WRITE"); + + // First, we do some basic argument validation as required by mmap + if (flags & (map_private | map_shared)).count_ones() != 1 { + this.set_last_error(Scalar::from_i32(this.eval_libc_i32("EINVAL")))?; + return Ok(Scalar::from_maybe_pointer(Pointer::null(), this)); + } + if length == 0 { + this.set_last_error(Scalar::from_i32(this.eval_libc_i32("EINVAL")))?; + return Ok(Scalar::from_maybe_pointer(Pointer::null(), this)); + } + + // If a user tries to map a file, we want to loudly inform them that this is not going + // to work. It is possible that POSIX gives us enough leeway to return an error, but the + // outcome for the user (I need to add cfg(miri)) is the same, just more frustrating. + if fd != -1 { + throw_unsup_format!("Miri does not support file-backed memory mappings"); + } + + // POSIX says: + // [ENOTSUP] + // * MAP_FIXED or MAP_PRIVATE was specified in the flags argument and the implementation + // does not support this functionality. + // * The implementation does not support the combination of accesses requested in the + // prot argument. + // + // Miri doesn't support MAP_FIXED or any any protections other than PROT_READ|PROT_WRITE. + if flags & map_fixed != 0 || prot != prot_read | prot_write { + this.set_last_error(Scalar::from_i32(this.eval_libc_i32("ENOTSUP")))?; + return Ok(Scalar::from_maybe_pointer(Pointer::null(), this)); + } + + // Miri does not support shared mappings, or any of the other extensions that for example + // Linux has added to the flags arguments. + if flags != map_private | map_anonymous { + throw_unsup_format!( + "Miri only supports calls to mmap which set the flags argument to MAP_PRIVATE|MAP_ANONYMOUS" + ); + } + + // This is only used for file mappings, which we don't support anyway. + if offset != 0 { + throw_unsup_format!("Miri does not support non-zero offsets to mmap"); + } + + let align = Align::from_bytes(this.machine.page_size).unwrap(); + let map_length = this.machine.round_up_to_multiple_of_page_size(length).unwrap_or(u64::MAX); + + let ptr = + this.allocate_ptr(Size::from_bytes(map_length), align, MiriMemoryKind::Mmap.into())?; + // We just allocated this, the access is definitely in-bounds and fits into our address space. + // mmap guarantees new mappings are zero-init. + this.write_bytes_ptr( + ptr.into(), + std::iter::repeat(0u8).take(usize::try_from(map_length).unwrap()), + ) + .unwrap(); + // Memory mappings don't use provenance, and are always exposed. + Machine::expose_ptr(this, ptr)?; + + Ok(Scalar::from_pointer(ptr, this)) + } + + fn mremap( + &mut self, + old_address: &OpTy<'tcx, Provenance>, + old_size: &OpTy<'tcx, Provenance>, + new_size: &OpTy<'tcx, Provenance>, + flags: &OpTy<'tcx, Provenance>, + ) -> InterpResult<'tcx, Scalar> { + let this = self.eval_context_mut(); + + let old_address = this.read_pointer(old_address)?; + let old_size = this.read_scalar(old_size)?.to_target_usize(this)?; + let new_size = this.read_scalar(new_size)?.to_target_usize(this)?; + let flags = this.read_scalar(flags)?.to_i32()?; + + // old_address must be a multiple of the page size + #[allow(clippy::arithmetic_side_effects)] // PAGE_SIZE is nonzero + if old_address.addr().bytes() % this.machine.page_size != 0 || new_size == 0 { + this.set_last_error(Scalar::from_i32(this.eval_libc_i32("EINVAL")))?; + return Ok(this.eval_libc("MAP_FAILED")); + } + + if flags & this.eval_libc_i32("MREMAP_FIXED") != 0 { + throw_unsup_format!("Miri does not support mremap wth MREMAP_FIXED"); + } + + if flags & this.eval_libc_i32("MREMAP_DONTUNMAP") != 0 { + throw_unsup_format!("Miri does not support mremap wth MREMAP_DONTUNMAP"); + } + + if flags & this.eval_libc_i32("MREMAP_MAYMOVE") == 0 { + // We only support MREMAP_MAYMOVE, so not passing the flag is just a failure + this.set_last_error(Scalar::from_i32(this.eval_libc_i32("EINVAL")))?; + return Ok(Scalar::from_maybe_pointer(Pointer::null(), this)); + } + + let align = this.machine.page_align(); + let ptr = this.reallocate_ptr( + old_address, + Some((Size::from_bytes(old_size), align)), + Size::from_bytes(new_size), + align, + MiriMemoryKind::Mmap.into(), + )?; + if let Some(increase) = new_size.checked_sub(old_size) { + // We just allocated this, the access is definitely in-bounds and fits into our address space. + // mmap guarantees new mappings are zero-init. + this.write_bytes_ptr( + ptr.offset(Size::from_bytes(old_size), this).unwrap().into(), + std::iter::repeat(0u8).take(usize::try_from(increase).unwrap()), + ) + .unwrap(); + } + // Memory mappings are always exposed + Machine::expose_ptr(this, ptr)?; + + Ok(Scalar::from_pointer(ptr, this)) + } + + fn munmap( + &mut self, + addr: &OpTy<'tcx, Provenance>, + length: &OpTy<'tcx, Provenance>, + ) -> InterpResult<'tcx, Scalar> { + let this = self.eval_context_mut(); + + let addr = this.read_pointer(addr)?; + let length = this.read_scalar(length)?.to_target_usize(this)?; + + // addr must be a multiple of the page size + #[allow(clippy::arithmetic_side_effects)] // PAGE_SIZE is nonzero + if addr.addr().bytes() % this.machine.page_size != 0 { + this.set_last_error(Scalar::from_i32(this.eval_libc_i32("EINVAL")))?; + return Ok(Scalar::from_i32(-1)); + } + + let length = this.machine.round_up_to_multiple_of_page_size(length).unwrap_or(u64::MAX); + + let mut addr = addr.addr().bytes(); + let mut bytes_unmapped = 0; + while bytes_unmapped < length { + // munmap specifies: + // It is not an error if the indicated range does not contain any mapped pages. + // So we make sure that if our address is not that of an exposed allocation, we just + // step forward to the next page. + let ptr = Machine::ptr_from_addr_cast(this, addr)?; + let Ok(ptr) = ptr.into_pointer_or_addr() else { + bytes_unmapped = bytes_unmapped.checked_add(this.machine.page_size).unwrap(); + addr = addr.wrapping_add(this.machine.page_size); + continue; + }; + // FIXME: This should fail if the pointer is to an unexposed allocation. But it + // doesn't. + let Some((alloc_id, offset, _prov)) = Machine::ptr_get_alloc(this, ptr) else { + bytes_unmapped = bytes_unmapped.checked_add(this.machine.page_size).unwrap(); + addr = addr.wrapping_add(this.machine.page_size); + continue; + }; + + if offset != Size::ZERO { + throw_unsup_format!("Miri does not support partial munmap"); + } + let (_kind, alloc) = this.memory.alloc_map().get(alloc_id).unwrap(); + let this_alloc_len = alloc.len() as u64; + bytes_unmapped = bytes_unmapped.checked_add(this_alloc_len).unwrap(); + if bytes_unmapped > length { + throw_unsup_format!("Miri does not support partial munmap"); + } + + this.deallocate_ptr( + Pointer::new(Some(Provenance::Wildcard), Size::from_bytes(addr)), + Some((Size::from_bytes(this_alloc_len), this.machine.page_align())), + MemoryKind::Machine(MiriMemoryKind::Mmap), + )?; + addr = addr.wrapping_add(this_alloc_len); + } + + Ok(Scalar::from_i32(0)) + } +} + +trait RangeExt { + fn overlaps(&self, other: &Self) -> bool; +} +impl RangeExt for std::ops::Range { + fn overlaps(&self, other: &Self) -> bool { + self.start.max(other.start) <= self.end.min(other.end) + } +} diff --git a/src/tools/miri/src/shims/unix/mod.rs b/src/tools/miri/src/shims/unix/mod.rs index 6fefb054f3c0..a8ebd369abaa 100644 --- a/src/tools/miri/src/shims/unix/mod.rs +++ b/src/tools/miri/src/shims/unix/mod.rs @@ -2,6 +2,7 @@ pub mod dlsym; pub mod foreign_items; mod fs; +mod mem; mod sync; mod thread; diff --git a/src/tools/miri/tests/fail/mmap_invalid_dealloc.rs b/src/tools/miri/tests/fail/mmap_invalid_dealloc.rs new file mode 100644 index 000000000000..70f7a6a7cef1 --- /dev/null +++ b/src/tools/miri/tests/fail/mmap_invalid_dealloc.rs @@ -0,0 +1,18 @@ +//@compile-flags: -Zmiri-disable-isolation +//@ignore-target-windows: No libc on Windows + +#![feature(rustc_private)] + +fn main() { + unsafe { + let ptr = libc::mmap( + std::ptr::null_mut(), + 4096, + libc::PROT_READ | libc::PROT_WRITE, + libc::MAP_PRIVATE | libc::MAP_ANONYMOUS, + -1, + 0, + ); + libc::free(ptr); //~ ERROR: which is mmap memory, using C heap deallocation operation + } +} diff --git a/src/tools/miri/tests/fail/mmap_invalid_dealloc.stderr b/src/tools/miri/tests/fail/mmap_invalid_dealloc.stderr new file mode 100644 index 000000000000..54e0cd5275d5 --- /dev/null +++ b/src/tools/miri/tests/fail/mmap_invalid_dealloc.stderr @@ -0,0 +1,15 @@ +error: Undefined Behavior: deallocating ALLOC, which is mmap memory, using C heap deallocation operation + --> $DIR/mmap_invalid_dealloc.rs:LL:CC + | +LL | libc::free(ptr); + | ^^^^^^^^^^^^^^^ deallocating ALLOC, which is mmap memory, using C heap deallocation operation + | + = help: this indicates a bug in the program: it performed an invalid operation, and caused Undefined Behavior + = help: see https://doc.rust-lang.org/nightly/reference/behavior-considered-undefined.html for further information + = note: BACKTRACE: + = note: inside `main` at $DIR/mmap_invalid_dealloc.rs:LL:CC + +note: some details are omitted, run with `MIRIFLAGS=-Zmiri-backtrace=full` for a verbose backtrace + +error: aborting due to previous error + diff --git a/src/tools/miri/tests/fail/mmap_use_after_munmap.rs b/src/tools/miri/tests/fail/mmap_use_after_munmap.rs new file mode 100644 index 000000000000..1e00bc6b64f9 --- /dev/null +++ b/src/tools/miri/tests/fail/mmap_use_after_munmap.rs @@ -0,0 +1,19 @@ +//@compile-flags: -Zmiri-disable-isolation +//@ignore-target-windows: No libc on Windows + +#![feature(rustc_private)] + +fn main() { + unsafe { + let ptr = libc::mmap( + std::ptr::null_mut(), + 4096, + libc::PROT_READ | libc::PROT_WRITE, + libc::MAP_PRIVATE | libc::MAP_ANONYMOUS, + -1, + 0, + ); + libc::munmap(ptr, 4096); + let _x = *(ptr as *mut u8); //~ ERROR: was dereferenced after this allocation got freed + } +} diff --git a/src/tools/miri/tests/fail/mmap_use_after_munmap.stderr b/src/tools/miri/tests/fail/mmap_use_after_munmap.stderr new file mode 100644 index 000000000000..f90701d400ca --- /dev/null +++ b/src/tools/miri/tests/fail/mmap_use_after_munmap.stderr @@ -0,0 +1,30 @@ +warning: integer-to-pointer cast + --> $DIR/mmap_use_after_munmap.rs:LL:CC + | +LL | libc::munmap(ptr, 4096); + | ^^^^^^^^^^^^^^^^^^^^^^^ integer-to-pointer cast + | + = help: This program is using integer-to-pointer casts or (equivalently) `ptr::from_exposed_addr`, + = help: which means that Miri might miss pointer bugs in this program. + = help: See https://doc.rust-lang.org/nightly/std/ptr/fn.from_exposed_addr.html for more details on that operation. + = help: To ensure that Miri does not miss bugs in your program, use Strict Provenance APIs (https://doc.rust-lang.org/nightly/std/ptr/index.html#strict-provenance, https://crates.io/crates/sptr) instead. + = help: You can then pass the `-Zmiri-strict-provenance` flag to Miri, to ensure you are not relying on `from_exposed_addr` semantics. + = help: Alternatively, the `-Zmiri-permissive-provenance` flag disables this warning. + = note: BACKTRACE: + = note: inside `main` at $DIR/mmap_use_after_munmap.rs:LL:CC + +error: Undefined Behavior: pointer to ALLOC was dereferenced after this allocation got freed + --> $DIR/mmap_use_after_munmap.rs:LL:CC + | +LL | let _x = *(ptr as *mut u8); + | ^^^^^^^^^^^^^^^^^ pointer to ALLOC was dereferenced after this allocation got freed + | + = help: this indicates a bug in the program: it performed an invalid operation, and caused Undefined Behavior + = help: see https://doc.rust-lang.org/nightly/reference/behavior-considered-undefined.html for further information + = note: BACKTRACE: + = note: inside `main` at $DIR/mmap_use_after_munmap.rs:LL:CC + +note: some details are omitted, run with `MIRIFLAGS=-Zmiri-backtrace=full` for a verbose backtrace + +error: aborting due to previous error; 1 warning emitted + diff --git a/src/tools/miri/tests/fail/munmap.stderr b/src/tools/miri/tests/fail/munmap.stderr new file mode 100644 index 000000000000..a8bcbd098f34 --- /dev/null +++ b/src/tools/miri/tests/fail/munmap.stderr @@ -0,0 +1,29 @@ +warning: integer-to-pointer cast + --> $DIR/munmap.rs:LL:CC + | +LL | libc::munmap(ptr, 1); + | ^^^^^^^^^^^^^^^^^^^^ integer-to-pointer cast + | + = help: This program is using integer-to-pointer casts or (equivalently) `ptr::from_exposed_addr`, + = help: which means that Miri might miss pointer bugs in this program. + = help: See https://doc.rust-lang.org/nightly/std/ptr/fn.from_exposed_addr.html for more details on that operation. + = help: To ensure that Miri does not miss bugs in your program, use Strict Provenance APIs (https://doc.rust-lang.org/nightly/std/ptr/index.html#strict-provenance, https://crates.io/crates/sptr) instead. + = help: You can then pass the `-Zmiri-strict-provenance` flag to Miri, to ensure you are not relying on `from_exposed_addr` semantics. + = help: Alternatively, the `-Zmiri-permissive-provenance` flag disables this warning. + = note: BACKTRACE: + = note: inside `main` at $DIR/munmap.rs:LL:CC + +error: unsupported operation: Miri does not support partial munmap + --> $DIR/munmap.rs:LL:CC + | +LL | libc::munmap(ptr, 1); + | ^^^^^^^^^^^^^^^^^^^^ Miri does not support partial munmap + | + = help: this is likely not a bug in the program; it indicates that the program performed an operation that the interpreter does not support + = note: BACKTRACE: + = note: inside `main` at $DIR/munmap.rs:LL:CC + +note: some details are omitted, run with `MIRIFLAGS=-Zmiri-backtrace=full` for a verbose backtrace + +error: aborting due to previous error; 1 warning emitted + diff --git a/src/tools/miri/tests/fail/munmap_partial.rs b/src/tools/miri/tests/fail/munmap_partial.rs new file mode 100644 index 000000000000..41387ae0b9db --- /dev/null +++ b/src/tools/miri/tests/fail/munmap_partial.rs @@ -0,0 +1,18 @@ +//! Our mmap/munmap support is a thin wrapper over Interpcx::allocate_ptr. Since the underlying +//! layer has much more UB than munmap does, we need to be sure we throw an unsupported error here. +//@ignore-target-windows: No libc on Windows + +fn main() { + unsafe { + let ptr = libc::mmap( + std::ptr::null_mut(), + page_size::get() * 2, + libc::PROT_READ | libc::PROT_WRITE, + libc::MAP_PRIVATE | libc::MAP_ANONYMOUS, + -1, + 0, + ); + libc::munmap(ptr, 1); + //~^ ERROR: unsupported operation: Miri does not support partial munmap + } +} diff --git a/src/tools/miri/tests/fail/munmap_partial.stderr b/src/tools/miri/tests/fail/munmap_partial.stderr new file mode 100644 index 000000000000..a8bd999e6609 --- /dev/null +++ b/src/tools/miri/tests/fail/munmap_partial.stderr @@ -0,0 +1,29 @@ +warning: integer-to-pointer cast + --> $DIR/munmap_partial.rs:LL:CC + | +LL | libc::munmap(ptr, 1); + | ^^^^^^^^^^^^^^^^^^^^ integer-to-pointer cast + | + = help: This program is using integer-to-pointer casts or (equivalently) `ptr::from_exposed_addr`, + = help: which means that Miri might miss pointer bugs in this program. + = help: See https://doc.rust-lang.org/nightly/std/ptr/fn.from_exposed_addr.html for more details on that operation. + = help: To ensure that Miri does not miss bugs in your program, use Strict Provenance APIs (https://doc.rust-lang.org/nightly/std/ptr/index.html#strict-provenance, https://crates.io/crates/sptr) instead. + = help: You can then pass the `-Zmiri-strict-provenance` flag to Miri, to ensure you are not relying on `from_exposed_addr` semantics. + = help: Alternatively, the `-Zmiri-permissive-provenance` flag disables this warning. + = note: BACKTRACE: + = note: inside `main` at $DIR/munmap_partial.rs:LL:CC + +error: unsupported operation: Miri does not support partial munmap + --> $DIR/munmap_partial.rs:LL:CC + | +LL | libc::munmap(ptr, 1); + | ^^^^^^^^^^^^^^^^^^^^ Miri does not support partial munmap + | + = help: this is likely not a bug in the program; it indicates that the program performed an operation that the interpreter does not support + = note: BACKTRACE: + = note: inside `main` at $DIR/munmap_partial.rs:LL:CC + +note: some details are omitted, run with `MIRIFLAGS=-Zmiri-backtrace=full` for a verbose backtrace + +error: aborting due to previous error; 1 warning emitted + diff --git a/src/tools/miri/tests/pass-dep/shims/mmap.rs b/src/tools/miri/tests/pass-dep/shims/mmap.rs new file mode 100644 index 000000000000..4e5060e85ae2 --- /dev/null +++ b/src/tools/miri/tests/pass-dep/shims/mmap.rs @@ -0,0 +1,83 @@ +//@ignore-target-windows: No libc on Windows +//@compile-flags: -Zmiri-disable-isolation -Zmiri-permissive-provenance +#![feature(strict_provenance)] + +use std::{ptr, slice}; + +fn test_mmap() { + let page_size = page_size::get(); + let ptr = unsafe { + libc::mmap( + ptr::null_mut(), + page_size, + libc::PROT_READ | libc::PROT_WRITE, + libc::MAP_PRIVATE | libc::MAP_ANONYMOUS, + -1, + 0, + ) + }; + assert!(!ptr.is_null()); + + // Ensure that freshly mapped allocations are zeroed + let slice = unsafe { slice::from_raw_parts_mut(ptr as *mut u8, page_size) }; + assert!(slice.iter().all(|b| *b == 0)); + + // Do some writes, make sure they worked + for b in slice.iter_mut() { + *b = 1; + } + assert!(slice.iter().all(|b| *b == 1)); + + // Ensure that we can munmap with just an integer + let just_an_address = ptr::invalid_mut(ptr.addr()); + let res = unsafe { libc::munmap(just_an_address, page_size) }; + assert_eq!(res, 0i32); +} + +#[cfg(target_os = "linux")] +fn test_mremap() { + let page_size = page_size::get(); + let ptr = unsafe { + libc::mmap( + ptr::null_mut(), + page_size, + libc::PROT_READ | libc::PROT_WRITE, + libc::MAP_PRIVATE | libc::MAP_ANONYMOUS, + -1, + 0, + ) + }; + let slice = unsafe { slice::from_raw_parts_mut(ptr as *mut u8, page_size) }; + for b in slice.iter_mut() { + *b = 1; + } + + let ptr = unsafe { libc::mremap(ptr, page_size, page_size * 2, libc::MREMAP_MAYMOVE) }; + assert!(!ptr.is_null()); + + let slice = unsafe { slice::from_raw_parts_mut(ptr as *mut u8, page_size * 2) }; + assert!(&slice[..page_size].iter().all(|b| *b == 1)); + assert!(&slice[page_size..].iter().all(|b| *b == 0)); + + let res = unsafe { libc::munmap(ptr, page_size * 2) }; + assert_eq!(res, 0i32); +} + +fn test_munmap() { + // Linux specifies that it is not an error if the specified range does not contain any pages. + let res = unsafe { + libc::munmap( + // Some high address we surely have no allocated anything at + ptr::invalid_mut(1 << 30), + 4096, + ) + }; + assert_eq!(res, 0); +} + +fn main() { + test_mmap(); + test_munmap(); + #[cfg(target_os = "linux")] + test_mremap(); +} From 69dc7353a15a8a18f4e99d494cffb38ebb36a695 Mon Sep 17 00:00:00 2001 From: Ben Kimock Date: Fri, 16 Jun 2023 19:10:54 -0400 Subject: [PATCH 2/3] Make munmap throw unsup errors instead of trying to work --- src/tools/miri/src/shims/unix/mem.rs | 67 +++++++------------ src/tools/miri/tests/fail/munmap.rs | 22 ++++++ src/tools/miri/tests/fail/munmap.stderr | 20 ++++-- src/tools/miri/tests/fail/munmap_partial.rs | 2 +- .../miri/tests/fail/munmap_partial.stderr | 4 +- src/tools/miri/tests/pass-dep/shims/mmap.rs | 13 ---- 6 files changed, 66 insertions(+), 62 deletions(-) create mode 100644 src/tools/miri/tests/fail/munmap.rs diff --git a/src/tools/miri/src/shims/unix/mem.rs b/src/tools/miri/src/shims/unix/mem.rs index 1d01bc82a88b..f133dca609c2 100644 --- a/src/tools/miri/src/shims/unix/mem.rs +++ b/src/tools/miri/src/shims/unix/mem.rs @@ -8,7 +8,7 @@ //! else that goes beyond a basic allocation API. use crate::*; -use rustc_target::abi::{Align, Size}; +use rustc_target::abi::Size; impl<'mir, 'tcx: 'mir> EvalContextExt<'mir, 'tcx> for crate::MiriInterpCx<'mir, 'tcx> {} pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> { @@ -88,7 +88,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> { throw_unsup_format!("Miri does not support non-zero offsets to mmap"); } - let align = Align::from_bytes(this.machine.page_size).unwrap(); + let align = this.machine.page_align(); let map_length = this.machine.round_up_to_multiple_of_page_size(length).unwrap_or(u64::MAX); let ptr = @@ -115,14 +115,14 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> { ) -> InterpResult<'tcx, Scalar> { let this = self.eval_context_mut(); - let old_address = this.read_pointer(old_address)?; + let old_address = this.read_scalar(old_address)?.to_target_usize(this)?; let old_size = this.read_scalar(old_size)?.to_target_usize(this)?; let new_size = this.read_scalar(new_size)?.to_target_usize(this)?; let flags = this.read_scalar(flags)?.to_i32()?; // old_address must be a multiple of the page size #[allow(clippy::arithmetic_side_effects)] // PAGE_SIZE is nonzero - if old_address.addr().bytes() % this.machine.page_size != 0 || new_size == 0 { + if old_address % this.machine.page_size != 0 || new_size == 0 { this.set_last_error(Scalar::from_i32(this.eval_libc_i32("EINVAL")))?; return Ok(this.eval_libc("MAP_FAILED")); } @@ -141,6 +141,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> { return Ok(Scalar::from_maybe_pointer(Pointer::null(), this)); } + let old_address = Machine::ptr_from_addr_cast(this, old_address)?; let align = this.machine.page_align(); let ptr = this.reallocate_ptr( old_address, @@ -171,57 +172,41 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> { ) -> InterpResult<'tcx, Scalar> { let this = self.eval_context_mut(); - let addr = this.read_pointer(addr)?; + let addr = this.read_scalar(addr)?.to_target_usize(this)?; let length = this.read_scalar(length)?.to_target_usize(this)?; // addr must be a multiple of the page size #[allow(clippy::arithmetic_side_effects)] // PAGE_SIZE is nonzero - if addr.addr().bytes() % this.machine.page_size != 0 { + if addr % this.machine.page_size != 0 { this.set_last_error(Scalar::from_i32(this.eval_libc_i32("EINVAL")))?; return Ok(Scalar::from_i32(-1)); } let length = this.machine.round_up_to_multiple_of_page_size(length).unwrap_or(u64::MAX); - let mut addr = addr.addr().bytes(); - let mut bytes_unmapped = 0; - while bytes_unmapped < length { - // munmap specifies: - // It is not an error if the indicated range does not contain any mapped pages. - // So we make sure that if our address is not that of an exposed allocation, we just - // step forward to the next page. - let ptr = Machine::ptr_from_addr_cast(this, addr)?; - let Ok(ptr) = ptr.into_pointer_or_addr() else { - bytes_unmapped = bytes_unmapped.checked_add(this.machine.page_size).unwrap(); - addr = addr.wrapping_add(this.machine.page_size); - continue; - }; - // FIXME: This should fail if the pointer is to an unexposed allocation. But it - // doesn't. - let Some((alloc_id, offset, _prov)) = Machine::ptr_get_alloc(this, ptr) else { - bytes_unmapped = bytes_unmapped.checked_add(this.machine.page_size).unwrap(); - addr = addr.wrapping_add(this.machine.page_size); - continue; - }; + let ptr = Machine::ptr_from_addr_cast(this, addr)?; - if offset != Size::ZERO { - throw_unsup_format!("Miri does not support partial munmap"); - } - let (_kind, alloc) = this.memory.alloc_map().get(alloc_id).unwrap(); - let this_alloc_len = alloc.len() as u64; - bytes_unmapped = bytes_unmapped.checked_add(this_alloc_len).unwrap(); - if bytes_unmapped > length { - throw_unsup_format!("Miri does not support partial munmap"); - } + let Ok(ptr) = ptr.into_pointer_or_addr() else { + throw_unsup_format!("Miri only supports munmap on memory allocated directly by mmap"); + }; + let Some((alloc_id, offset, _prov)) = Machine::ptr_get_alloc(this, ptr) else { + throw_unsup_format!("Miri only supports munmap on memory allocated directly by mmap"); + }; - this.deallocate_ptr( - Pointer::new(Some(Provenance::Wildcard), Size::from_bytes(addr)), - Some((Size::from_bytes(this_alloc_len), this.machine.page_align())), - MemoryKind::Machine(MiriMemoryKind::Mmap), - )?; - addr = addr.wrapping_add(this_alloc_len); + let (_kind, alloc) = this.memory.alloc_map().get(alloc_id).unwrap(); + if offset != Size::ZERO || alloc.len() as u64 != length { + throw_unsup_format!( + "Miri only supports munmap calls that exactly unmap a region previously returned by mmap" + ); } + let len = Size::from_bytes(alloc.len() as u64); + this.deallocate_ptr( + Pointer::new(Some(Provenance::Wildcard), Size::from_bytes(addr)), + Some((len, this.machine.page_align())), + MemoryKind::Machine(MiriMemoryKind::Mmap), + )?; + Ok(Scalar::from_i32(0)) } } diff --git a/src/tools/miri/tests/fail/munmap.rs b/src/tools/miri/tests/fail/munmap.rs new file mode 100644 index 000000000000..453437a06cfc --- /dev/null +++ b/src/tools/miri/tests/fail/munmap.rs @@ -0,0 +1,22 @@ +//@compile-flags: -Zmiri-disable-isolation +//@ignore-target-windows: No libc on Windows + +#![feature(rustc_private)] +#![feature(strict_provenance)] + +use std::ptr; + +fn main() { + // Linux specifies that it is not an error if the specified range does not contain any pages. + // But we simply do not support such calls. This test checks that we report this as + // unsupported, not Undefined Behavior. + let res = unsafe { + libc::munmap( + //~^ ERROR: unsupported operation + // Some high address we surely have not allocated anything at + ptr::invalid_mut(1 << 30), + 4096, + ) + }; + assert_eq!(res, 0); +} diff --git a/src/tools/miri/tests/fail/munmap.stderr b/src/tools/miri/tests/fail/munmap.stderr index a8bcbd098f34..cb47769c063f 100644 --- a/src/tools/miri/tests/fail/munmap.stderr +++ b/src/tools/miri/tests/fail/munmap.stderr @@ -1,8 +1,13 @@ warning: integer-to-pointer cast --> $DIR/munmap.rs:LL:CC | -LL | libc::munmap(ptr, 1); - | ^^^^^^^^^^^^^^^^^^^^ integer-to-pointer cast +LL | / libc::munmap( +LL | | +LL | | // Some high address we surely have not allocated anything at +LL | | ptr::invalid_mut(1 << 30), +LL | | 4096, +LL | | ) + | |_________^ integer-to-pointer cast | = help: This program is using integer-to-pointer casts or (equivalently) `ptr::from_exposed_addr`, = help: which means that Miri might miss pointer bugs in this program. @@ -13,11 +18,16 @@ LL | libc::munmap(ptr, 1); = note: BACKTRACE: = note: inside `main` at $DIR/munmap.rs:LL:CC -error: unsupported operation: Miri does not support partial munmap +error: unsupported operation: Miri only supports munmap on memory allocated directly by mmap --> $DIR/munmap.rs:LL:CC | -LL | libc::munmap(ptr, 1); - | ^^^^^^^^^^^^^^^^^^^^ Miri does not support partial munmap +LL | / libc::munmap( +LL | | +LL | | // Some high address we surely have not allocated anything at +LL | | ptr::invalid_mut(1 << 30), +LL | | 4096, +LL | | ) + | |_________^ Miri only supports munmap on memory allocated directly by mmap | = help: this is likely not a bug in the program; it indicates that the program performed an operation that the interpreter does not support = note: BACKTRACE: diff --git a/src/tools/miri/tests/fail/munmap_partial.rs b/src/tools/miri/tests/fail/munmap_partial.rs index 41387ae0b9db..938850ee2862 100644 --- a/src/tools/miri/tests/fail/munmap_partial.rs +++ b/src/tools/miri/tests/fail/munmap_partial.rs @@ -13,6 +13,6 @@ fn main() { 0, ); libc::munmap(ptr, 1); - //~^ ERROR: unsupported operation: Miri does not support partial munmap + //~^ ERROR: unsupported operation } } diff --git a/src/tools/miri/tests/fail/munmap_partial.stderr b/src/tools/miri/tests/fail/munmap_partial.stderr index a8bd999e6609..9a084c504374 100644 --- a/src/tools/miri/tests/fail/munmap_partial.stderr +++ b/src/tools/miri/tests/fail/munmap_partial.stderr @@ -13,11 +13,11 @@ LL | libc::munmap(ptr, 1); = note: BACKTRACE: = note: inside `main` at $DIR/munmap_partial.rs:LL:CC -error: unsupported operation: Miri does not support partial munmap +error: unsupported operation: Miri only supports munmap calls that exactly unmap a region previously returned by mmap --> $DIR/munmap_partial.rs:LL:CC | LL | libc::munmap(ptr, 1); - | ^^^^^^^^^^^^^^^^^^^^ Miri does not support partial munmap + | ^^^^^^^^^^^^^^^^^^^^ Miri only supports munmap calls that exactly unmap a region previously returned by mmap | = help: this is likely not a bug in the program; it indicates that the program performed an operation that the interpreter does not support = note: BACKTRACE: diff --git a/src/tools/miri/tests/pass-dep/shims/mmap.rs b/src/tools/miri/tests/pass-dep/shims/mmap.rs index 4e5060e85ae2..03ffc3b38986 100644 --- a/src/tools/miri/tests/pass-dep/shims/mmap.rs +++ b/src/tools/miri/tests/pass-dep/shims/mmap.rs @@ -63,21 +63,8 @@ fn test_mremap() { assert_eq!(res, 0i32); } -fn test_munmap() { - // Linux specifies that it is not an error if the specified range does not contain any pages. - let res = unsafe { - libc::munmap( - // Some high address we surely have no allocated anything at - ptr::invalid_mut(1 << 30), - 4096, - ) - }; - assert_eq!(res, 0); -} - fn main() { test_mmap(); - test_munmap(); #[cfg(target_os = "linux")] test_mremap(); } From 8fc8f13ebd512ea9990d5178dcfbec48641fa842 Mon Sep 17 00:00:00 2001 From: Ben Kimock Date: Sat, 17 Jun 2023 11:55:30 -0400 Subject: [PATCH 3/3] Improve organization --- .../miri/src/shims/unix/foreign_items.rs | 5 - .../src/shims/unix/linux/foreign_items.rs | 7 ++ src/tools/miri/src/shims/unix/linux/mem.rs | 67 +++++++++++++ src/tools/miri/src/shims/unix/linux/mod.rs | 1 + src/tools/miri/src/shims/unix/mem.rs | 93 +++---------------- .../fail/{ => shims}/mmap_invalid_dealloc.rs | 0 .../{ => shims}/mmap_invalid_dealloc.stderr | 0 .../fail/{ => shims}/mmap_use_after_munmap.rs | 0 .../{ => shims}/mmap_use_after_munmap.stderr | 0 .../miri/tests/fail/{ => shims}/munmap.rs | 0 .../miri/tests/fail/{ => shims}/munmap.stderr | 0 .../tests/fail/{ => shims}/munmap_partial.rs | 0 .../fail/{ => shims}/munmap_partial.stderr | 0 13 files changed, 90 insertions(+), 83 deletions(-) create mode 100644 src/tools/miri/src/shims/unix/linux/mem.rs rename src/tools/miri/tests/fail/{ => shims}/mmap_invalid_dealloc.rs (100%) rename src/tools/miri/tests/fail/{ => shims}/mmap_invalid_dealloc.stderr (100%) rename src/tools/miri/tests/fail/{ => shims}/mmap_use_after_munmap.rs (100%) rename src/tools/miri/tests/fail/{ => shims}/mmap_use_after_munmap.stderr (100%) rename src/tools/miri/tests/fail/{ => shims}/munmap.rs (100%) rename src/tools/miri/tests/fail/{ => shims}/munmap.stderr (100%) rename src/tools/miri/tests/fail/{ => shims}/munmap_partial.rs (100%) rename src/tools/miri/tests/fail/{ => shims}/munmap_partial.stderr (100%) diff --git a/src/tools/miri/src/shims/unix/foreign_items.rs b/src/tools/miri/src/shims/unix/foreign_items.rs index d0f65306378a..bd66a67950c2 100644 --- a/src/tools/miri/src/shims/unix/foreign_items.rs +++ b/src/tools/miri/src/shims/unix/foreign_items.rs @@ -219,11 +219,6 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> { let ptr = this.mmap(addr, length, prot, flags, fd, offset)?; this.write_scalar(ptr, dest)?; } - "mremap" => { - let [old_address, old_size, new_size, flags] = this.check_shim(abi, Abi::C {unwind: false}, link_name, args)?; - let ptr = this.mremap(old_address, old_size, new_size, flags)?; - this.write_scalar(ptr, dest)?; - } "munmap" => { let [addr, length] = this.check_shim(abi, Abi::C {unwind: false}, link_name, args)?; let result = this.munmap(addr, length)?; diff --git a/src/tools/miri/src/shims/unix/linux/foreign_items.rs b/src/tools/miri/src/shims/unix/linux/foreign_items.rs index 4cb7ee8efca7..a32ef54bd9a1 100644 --- a/src/tools/miri/src/shims/unix/linux/foreign_items.rs +++ b/src/tools/miri/src/shims/unix/linux/foreign_items.rs @@ -7,6 +7,7 @@ use crate::*; use shims::foreign_items::EmulateByNameResult; use shims::unix::fs::EvalContextExt as _; use shims::unix::linux::fd::EvalContextExt as _; +use shims::unix::linux::mem::EvalContextExt as _; use shims::unix::linux::sync::futex; use shims::unix::sync::EvalContextExt as _; use shims::unix::thread::EvalContextExt as _; @@ -68,6 +69,12 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> { let result = this.eventfd(val, flag)?; this.write_scalar(result, dest)?; } + "mremap" => { + let [old_address, old_size, new_size, flags] = + this.check_shim(abi, Abi::C { unwind: false }, link_name, args)?; + let ptr = this.mremap(old_address, old_size, new_size, flags)?; + this.write_scalar(ptr, dest)?; + } "socketpair" => { let [domain, type_, protocol, sv] = this.check_shim(abi, Abi::C { unwind: false }, link_name, args)?; diff --git a/src/tools/miri/src/shims/unix/linux/mem.rs b/src/tools/miri/src/shims/unix/linux/mem.rs new file mode 100644 index 000000000000..026fd6ae5e7c --- /dev/null +++ b/src/tools/miri/src/shims/unix/linux/mem.rs @@ -0,0 +1,67 @@ +//! This follows the pattern in src/shims/unix/mem.rs: We only support uses of mremap that would +//! correspond to valid uses of realloc. + +use crate::*; +use rustc_target::abi::Size; + +impl<'mir, 'tcx: 'mir> EvalContextExt<'mir, 'tcx> for crate::MiriInterpCx<'mir, 'tcx> {} +pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> { + fn mremap( + &mut self, + old_address: &OpTy<'tcx, Provenance>, + old_size: &OpTy<'tcx, Provenance>, + new_size: &OpTy<'tcx, Provenance>, + flags: &OpTy<'tcx, Provenance>, + ) -> InterpResult<'tcx, Scalar> { + let this = self.eval_context_mut(); + + let old_address = this.read_target_usize(old_address)?; + let old_size = this.read_target_usize(old_size)?; + let new_size = this.read_target_usize(new_size)?; + let flags = this.read_scalar(flags)?.to_i32()?; + + // old_address must be a multiple of the page size + #[allow(clippy::arithmetic_side_effects)] // PAGE_SIZE is nonzero + if old_address % this.machine.page_size != 0 || new_size == 0 { + this.set_last_error(Scalar::from_i32(this.eval_libc_i32("EINVAL")))?; + return Ok(this.eval_libc("MAP_FAILED")); + } + + if flags & this.eval_libc_i32("MREMAP_FIXED") != 0 { + throw_unsup_format!("Miri does not support mremap wth MREMAP_FIXED"); + } + + if flags & this.eval_libc_i32("MREMAP_DONTUNMAP") != 0 { + throw_unsup_format!("Miri does not support mremap wth MREMAP_DONTUNMAP"); + } + + if flags & this.eval_libc_i32("MREMAP_MAYMOVE") == 0 { + // We only support MREMAP_MAYMOVE, so not passing the flag is just a failure + this.set_last_error(Scalar::from_i32(this.eval_libc_i32("EINVAL")))?; + return Ok(Scalar::from_maybe_pointer(Pointer::null(), this)); + } + + let old_address = Machine::ptr_from_addr_cast(this, old_address)?; + let align = this.machine.page_align(); + let ptr = this.reallocate_ptr( + old_address, + Some((Size::from_bytes(old_size), align)), + Size::from_bytes(new_size), + align, + MiriMemoryKind::Mmap.into(), + )?; + if let Some(increase) = new_size.checked_sub(old_size) { + // We just allocated this, the access is definitely in-bounds and fits into our address space. + // mmap guarantees new mappings are zero-init. + this.write_bytes_ptr( + ptr.offset(Size::from_bytes(old_size), this).unwrap().into(), + std::iter::repeat(0u8).take(usize::try_from(increase).unwrap()), + ) + .unwrap(); + } + // Memory mappings are always exposed + Machine::expose_ptr(this, ptr)?; + + Ok(Scalar::from_pointer(ptr, this)) + } +} diff --git a/src/tools/miri/src/shims/unix/linux/mod.rs b/src/tools/miri/src/shims/unix/linux/mod.rs index 437764c3824e..856ec226de8f 100644 --- a/src/tools/miri/src/shims/unix/linux/mod.rs +++ b/src/tools/miri/src/shims/unix/linux/mod.rs @@ -1,4 +1,5 @@ pub mod dlsym; pub mod fd; pub mod foreign_items; +pub mod mem; pub mod sync; diff --git a/src/tools/miri/src/shims/unix/mem.rs b/src/tools/miri/src/shims/unix/mem.rs index f133dca609c2..a33d784d1665 100644 --- a/src/tools/miri/src/shims/unix/mem.rs +++ b/src/tools/miri/src/shims/unix/mem.rs @@ -1,9 +1,9 @@ -//! This is an incomplete implementation of mmap/mremap/munmap which is restricted in order to be +//! This is an incomplete implementation of mmap/munmap which is restricted in order to be //! implementable on top of the existing memory system. The point of these function as-written is //! to allow memory allocators written entirely in Rust to be executed by Miri. This implementation //! does not support other uses of mmap such as file mappings. //! -//! mmap/mremap/munmap behave a lot like alloc/realloc/dealloc, and for simple use they are exactly +//! mmap/munmap behave a lot like alloc/dealloc, and for simple use they are exactly //! equivalent. That is the only part we support: no MAP_FIXED or MAP_SHARED or anything //! else that goes beyond a basic allocation API. @@ -23,23 +23,23 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> { ) -> InterpResult<'tcx, Scalar> { let this = self.eval_context_mut(); - // We do not support MAP_FIXED, so the addr argument is always ignored - let addr = this.read_pointer(addr)?; + // We do not support MAP_FIXED, so the addr argument is always ignored (except for the MacOS hack) + let addr = this.read_target_usize(addr)?; let length = this.read_target_usize(length)?; let prot = this.read_scalar(prot)?.to_i32()?; let flags = this.read_scalar(flags)?.to_i32()?; let fd = this.read_scalar(fd)?.to_i32()?; - let offset = this.read_scalar(offset)?.to_target_usize(this)?; + let offset = this.read_target_usize(offset)?; let map_private = this.eval_libc_i32("MAP_PRIVATE"); let map_anonymous = this.eval_libc_i32("MAP_ANONYMOUS"); let map_shared = this.eval_libc_i32("MAP_SHARED"); let map_fixed = this.eval_libc_i32("MAP_FIXED"); - // This is a horrible hack, but on macos the guard page mechanism uses mmap + // This is a horrible hack, but on MacOS the guard page mechanism uses mmap // in a way we do not support. We just give it the return value it expects. if this.frame_in_std() && this.tcx.sess.target.os == "macos" && (flags & map_fixed) != 0 { - return Ok(Scalar::from_maybe_pointer(addr, this)); + return Ok(Scalar::from_maybe_pointer(Pointer::from_addr_invalid(addr), this)); } let prot_read = this.eval_libc_i32("PROT_READ"); @@ -106,65 +106,6 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> { Ok(Scalar::from_pointer(ptr, this)) } - fn mremap( - &mut self, - old_address: &OpTy<'tcx, Provenance>, - old_size: &OpTy<'tcx, Provenance>, - new_size: &OpTy<'tcx, Provenance>, - flags: &OpTy<'tcx, Provenance>, - ) -> InterpResult<'tcx, Scalar> { - let this = self.eval_context_mut(); - - let old_address = this.read_scalar(old_address)?.to_target_usize(this)?; - let old_size = this.read_scalar(old_size)?.to_target_usize(this)?; - let new_size = this.read_scalar(new_size)?.to_target_usize(this)?; - let flags = this.read_scalar(flags)?.to_i32()?; - - // old_address must be a multiple of the page size - #[allow(clippy::arithmetic_side_effects)] // PAGE_SIZE is nonzero - if old_address % this.machine.page_size != 0 || new_size == 0 { - this.set_last_error(Scalar::from_i32(this.eval_libc_i32("EINVAL")))?; - return Ok(this.eval_libc("MAP_FAILED")); - } - - if flags & this.eval_libc_i32("MREMAP_FIXED") != 0 { - throw_unsup_format!("Miri does not support mremap wth MREMAP_FIXED"); - } - - if flags & this.eval_libc_i32("MREMAP_DONTUNMAP") != 0 { - throw_unsup_format!("Miri does not support mremap wth MREMAP_DONTUNMAP"); - } - - if flags & this.eval_libc_i32("MREMAP_MAYMOVE") == 0 { - // We only support MREMAP_MAYMOVE, so not passing the flag is just a failure - this.set_last_error(Scalar::from_i32(this.eval_libc_i32("EINVAL")))?; - return Ok(Scalar::from_maybe_pointer(Pointer::null(), this)); - } - - let old_address = Machine::ptr_from_addr_cast(this, old_address)?; - let align = this.machine.page_align(); - let ptr = this.reallocate_ptr( - old_address, - Some((Size::from_bytes(old_size), align)), - Size::from_bytes(new_size), - align, - MiriMemoryKind::Mmap.into(), - )?; - if let Some(increase) = new_size.checked_sub(old_size) { - // We just allocated this, the access is definitely in-bounds and fits into our address space. - // mmap guarantees new mappings are zero-init. - this.write_bytes_ptr( - ptr.offset(Size::from_bytes(old_size), this).unwrap().into(), - std::iter::repeat(0u8).take(usize::try_from(increase).unwrap()), - ) - .unwrap(); - } - // Memory mappings are always exposed - Machine::expose_ptr(this, ptr)?; - - Ok(Scalar::from_pointer(ptr, this)) - } - fn munmap( &mut self, addr: &OpTy<'tcx, Provenance>, @@ -172,8 +113,8 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> { ) -> InterpResult<'tcx, Scalar> { let this = self.eval_context_mut(); - let addr = this.read_scalar(addr)?.to_target_usize(this)?; - let length = this.read_scalar(length)?.to_target_usize(this)?; + let addr = this.read_target_usize(addr)?; + let length = this.read_target_usize(length)?; // addr must be a multiple of the page size #[allow(clippy::arithmetic_side_effects)] // PAGE_SIZE is nonzero @@ -193,6 +134,11 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> { throw_unsup_format!("Miri only supports munmap on memory allocated directly by mmap"); }; + // Elsewhere in this function we are careful to check what we can and throw an unsupported + // error instead of Undefined Behavior when use of this function falls outside of the + // narrow scope we support. We deliberately do not check the MemoryKind of this allocation, + // because we want to report UB on attempting to unmap memory that Rust "understands", such + // the stack, heap, or statics. let (_kind, alloc) = this.memory.alloc_map().get(alloc_id).unwrap(); if offset != Size::ZERO || alloc.len() as u64 != length { throw_unsup_format!( @@ -202,7 +148,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> { let len = Size::from_bytes(alloc.len() as u64); this.deallocate_ptr( - Pointer::new(Some(Provenance::Wildcard), Size::from_bytes(addr)), + ptr.into(), Some((len, this.machine.page_align())), MemoryKind::Machine(MiriMemoryKind::Mmap), )?; @@ -210,12 +156,3 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> { Ok(Scalar::from_i32(0)) } } - -trait RangeExt { - fn overlaps(&self, other: &Self) -> bool; -} -impl RangeExt for std::ops::Range { - fn overlaps(&self, other: &Self) -> bool { - self.start.max(other.start) <= self.end.min(other.end) - } -} diff --git a/src/tools/miri/tests/fail/mmap_invalid_dealloc.rs b/src/tools/miri/tests/fail/shims/mmap_invalid_dealloc.rs similarity index 100% rename from src/tools/miri/tests/fail/mmap_invalid_dealloc.rs rename to src/tools/miri/tests/fail/shims/mmap_invalid_dealloc.rs diff --git a/src/tools/miri/tests/fail/mmap_invalid_dealloc.stderr b/src/tools/miri/tests/fail/shims/mmap_invalid_dealloc.stderr similarity index 100% rename from src/tools/miri/tests/fail/mmap_invalid_dealloc.stderr rename to src/tools/miri/tests/fail/shims/mmap_invalid_dealloc.stderr diff --git a/src/tools/miri/tests/fail/mmap_use_after_munmap.rs b/src/tools/miri/tests/fail/shims/mmap_use_after_munmap.rs similarity index 100% rename from src/tools/miri/tests/fail/mmap_use_after_munmap.rs rename to src/tools/miri/tests/fail/shims/mmap_use_after_munmap.rs diff --git a/src/tools/miri/tests/fail/mmap_use_after_munmap.stderr b/src/tools/miri/tests/fail/shims/mmap_use_after_munmap.stderr similarity index 100% rename from src/tools/miri/tests/fail/mmap_use_after_munmap.stderr rename to src/tools/miri/tests/fail/shims/mmap_use_after_munmap.stderr diff --git a/src/tools/miri/tests/fail/munmap.rs b/src/tools/miri/tests/fail/shims/munmap.rs similarity index 100% rename from src/tools/miri/tests/fail/munmap.rs rename to src/tools/miri/tests/fail/shims/munmap.rs diff --git a/src/tools/miri/tests/fail/munmap.stderr b/src/tools/miri/tests/fail/shims/munmap.stderr similarity index 100% rename from src/tools/miri/tests/fail/munmap.stderr rename to src/tools/miri/tests/fail/shims/munmap.stderr diff --git a/src/tools/miri/tests/fail/munmap_partial.rs b/src/tools/miri/tests/fail/shims/munmap_partial.rs similarity index 100% rename from src/tools/miri/tests/fail/munmap_partial.rs rename to src/tools/miri/tests/fail/shims/munmap_partial.rs diff --git a/src/tools/miri/tests/fail/munmap_partial.stderr b/src/tools/miri/tests/fail/shims/munmap_partial.stderr similarity index 100% rename from src/tools/miri/tests/fail/munmap_partial.stderr rename to src/tools/miri/tests/fail/shims/munmap_partial.stderr