Skip to content

Commit

Permalink
Make mmap not use expose semantics
Browse files Browse the repository at this point in the history
  • Loading branch information
saethlin committed Dec 10, 2023
1 parent 9828125 commit cfe3684
Show file tree
Hide file tree
Showing 6 changed files with 10 additions and 68 deletions.
7 changes: 2 additions & 5 deletions src/shims/unix/linux/mem.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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"));
}
Expand All @@ -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,
Expand All @@ -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))
}
Expand Down
10 changes: 3 additions & 7 deletions src/shims/unix/mem.rs
Original file line number Diff line number Diff line change
Expand Up @@ -100,8 +100,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))
}
Expand All @@ -113,21 +111,19 @@ 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
#[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 {
let Ok(ptr) = addr.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 {
Expand Down
17 changes: 1 addition & 16 deletions tests/fail-dep/shims/mmap_use_after_munmap.stderr
Original file line number Diff line number Diff line change
@@ -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
|
Expand Down Expand Up @@ -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

22 changes: 1 addition & 21 deletions tests/fail-dep/shims/munmap.stderr
Original file line number Diff line number Diff line change
@@ -1,23 +1,3 @@
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
|
Expand All @@ -35,5 +15,5 @@ LL | | )

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

17 changes: 1 addition & 16 deletions tests/fail-dep/shims/munmap_partial.stderr
Original file line number Diff line number Diff line change
@@ -1,18 +1,3 @@
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 only supports munmap calls that exactly unmap a region previously returned by mmap
--> $DIR/munmap_partial.rs:LL:CC
|
Expand All @@ -25,5 +10,5 @@ LL | libc::munmap(ptr, 1);

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

5 changes: 2 additions & 3 deletions tests/pass-dep/shims/mmap.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}

Expand Down

0 comments on commit cfe3684

Please sign in to comment.