Make mmap not use expose semantics

This commit is contained in:
Ben Kimock 2023-12-10 12:38:26 -05:00
parent 92ab9d65bb
commit 4e2235a480
8 changed files with 29 additions and 138 deletions

View file

@ -15,14 +15,14 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
) -> InterpResult<'tcx, Scalar<Provenance>> {
let this = self.eval_context_mut();
let old_address = this.read_target_usize(old_address)?;
let old_address = this.read_pointer(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 {
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"));
}
@ -41,7 +41,6 @@ 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,
@ -59,8 +58,6 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
)
.unwrap();
}
// Memory mappings are always exposed
Machine::expose_ptr(this, ptr)?;
Ok(Scalar::from_pointer(ptr, this))
}

View file

@ -6,6 +6,13 @@
//! 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.
//!
//! Note that in addition to only supporting malloc-like calls to mmap, we only support free-like
//! calls to munmap, but for a very different reason. In principle, according to the man pages, it
//! is possible to unmap arbitrary regions of address space. But in a high-level language like Rust
//! this amounts to partial deallocation, which LLVM does not support. So any attempt to call our
//! munmap shim which would partily unmap a region of address space previously mapped by mmap will
//! report UB.
use crate::{helpers::round_to_next_multiple_of, *};
use rustc_target::abi::Size;
@ -100,8 +107,6 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
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))
}
@ -113,43 +118,21 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
) -> InterpResult<'tcx, Scalar<Provenance>> {
let this = self.eval_context_mut();
let addr = this.read_target_usize(addr)?;
let addr = this.read_pointer(addr)?;
let length = this.read_target_usize(length)?;
// addr must be a multiple of the page size
// addr must be a multiple of the page size, but apart from that munmap is just implemented
// as a dealloc.
#[allow(clippy::arithmetic_side_effects)] // PAGE_SIZE is nonzero
if addr % this.machine.page_size != 0 {
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 = round_to_next_multiple_of(length, this.machine.page_size);
let ptr = Machine::ptr_from_addr_cast(this, addr)?;
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");
};
// 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!(
"Miri only supports munmap calls that exactly unmap a region previously returned by mmap"
);
}
let len = Size::from_bytes(alloc.len() as u64);
let length = Size::from_bytes(round_to_next_multiple_of(length, this.machine.page_size));
this.deallocate_ptr(
ptr.into(),
Some((len, this.machine.page_align())),
addr,
Some((length, this.machine.page_align())),
MemoryKind::Machine(MiriMemoryKind::Mmap),
)?;

View file

@ -1,18 +1,3 @@
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: memory access failed: ALLOC has been freed, so this pointer is dangling
--> $DIR/mmap_use_after_munmap.rs:LL:CC
|
@ -43,5 +28,5 @@ LL | libc::munmap(ptr, 4096);
note: some details are omitted, run with `MIRIFLAGS=-Zmiri-backtrace=full` for a verbose backtrace
error: aborting due to 1 previous error; 1 warning emitted
error: aborting due to 1 previous error

View file

@ -1,22 +0,0 @@
//@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);
}

View file

@ -1,39 +0,0 @@
warning: integer-to-pointer cast
--> $DIR/munmap.rs:LL:CC
|
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.
= 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 only supports munmap on memory allocated directly by mmap
--> $DIR/munmap.rs:LL:CC
|
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:
= 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 1 previous error; 1 warning emitted

View file

@ -1,6 +1,8 @@
//! 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.
//! The man pages for mmap/munmap suggest that it is possible to partly unmap a previously-mapped
//! region of addres space, but to LLVM that would be partial deallocation, which LLVM does not
//! support. So even though the man pages say this sort of use is possible, we must report UB.
//@ignore-target-windows: No libc on Windows
//@normalize-stderr-test: "size [0-9]+ and alignment" -> "size SIZE and alignment"
fn main() {
unsafe {
@ -13,6 +15,6 @@ fn main() {
0,
);
libc::munmap(ptr, 1);
//~^ ERROR: unsupported operation
//~^ ERROR: Undefined Behavior
}
}

View file

@ -1,29 +1,15 @@
warning: integer-to-pointer cast
error: Undefined Behavior: incorrect layout on deallocation: ALLOC has size SIZE and alignment ALIGN, but gave size SIZE and alignment ALIGN
--> $DIR/munmap_partial.rs:LL:CC
|
LL | libc::munmap(ptr, 1);
| ^^^^^^^^^^^^^^^^^^^^ integer-to-pointer cast
| ^^^^^^^^^^^^^^^^^^^^ incorrect layout on deallocation: ALLOC has size SIZE and alignment ALIGN, but gave size SIZE and alignment ALIGN
|
= 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 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 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
= 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/munmap_partial.rs:LL:CC
note: some details are omitted, run with `MIRIFLAGS=-Zmiri-backtrace=full` for a verbose backtrace
error: aborting due to 1 previous error; 1 warning emitted
error: aborting due to 1 previous error

View file

@ -28,9 +28,8 @@ fn test_mmap() {
}
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) };
// Ensure that we can munmap
let res = unsafe { libc::munmap(ptr, page_size) };
assert_eq!(res, 0i32);
}