From cfe36843edce257ac4492c10951c1240916eaf37 Mon Sep 17 00:00:00 2001 From: Ben Kimock Date: Sun, 10 Dec 2023 12:38:26 -0500 Subject: [PATCH] Make mmap not use expose semantics --- src/shims/unix/linux/mem.rs | 7 ++---- src/shims/unix/mem.rs | 10 +++------ .../shims/mmap_use_after_munmap.stderr | 17 +------------- tests/fail-dep/shims/munmap.stderr | 22 +------------------ tests/fail-dep/shims/munmap_partial.stderr | 17 +------------- tests/pass-dep/shims/mmap.rs | 5 ++--- 6 files changed, 10 insertions(+), 68 deletions(-) diff --git a/src/shims/unix/linux/mem.rs b/src/shims/unix/linux/mem.rs index 026fd6ae5e..7bbe5618df 100644 --- a/src/shims/unix/linux/mem.rs +++ b/src/shims/unix/linux/mem.rs @@ -15,14 +15,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_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)) } diff --git a/src/shims/unix/mem.rs b/src/shims/unix/mem.rs index cf36d4b191..358314b34f 100644 --- a/src/shims/unix/mem.rs +++ b/src/shims/unix/mem.rs @@ -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)) } @@ -113,21 +111,19 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> { ) -> InterpResult<'tcx, Scalar> { 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 { diff --git a/tests/fail-dep/shims/mmap_use_after_munmap.stderr b/tests/fail-dep/shims/mmap_use_after_munmap.stderr index 21b4baa500..49f2b84baa 100644 --- a/tests/fail-dep/shims/mmap_use_after_munmap.stderr +++ b/tests/fail-dep/shims/mmap_use_after_munmap.stderr @@ -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 diff --git a/tests/fail-dep/shims/munmap.stderr b/tests/fail-dep/shims/munmap.stderr index f17473677f..83cc625929 100644 --- a/tests/fail-dep/shims/munmap.stderr +++ b/tests/fail-dep/shims/munmap.stderr @@ -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 | @@ -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 diff --git a/tests/fail-dep/shims/munmap_partial.stderr b/tests/fail-dep/shims/munmap_partial.stderr index 14eb9d3205..93eff86756 100644 --- a/tests/fail-dep/shims/munmap_partial.stderr +++ b/tests/fail-dep/shims/munmap_partial.stderr @@ -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 | @@ -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 diff --git a/tests/pass-dep/shims/mmap.rs b/tests/pass-dep/shims/mmap.rs index 03ffc3b389..72802a7f8c 100644 --- a/tests/pass-dep/shims/mmap.rs +++ b/tests/pass-dep/shims/mmap.rs @@ -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); }