Skip to content

Commit def5f39

Browse files
committed
Make mmap not use expose semantics
1 parent 9828125 commit def5f39

File tree

6 files changed

+12
-91
lines changed

6 files changed

+12
-91
lines changed

src/shims/unix/linux/mem.rs

+2-5
Original file line numberDiff line numberDiff line change
@@ -15,14 +15,14 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
1515
) -> InterpResult<'tcx, Scalar<Provenance>> {
1616
let this = self.eval_context_mut();
1717

18-
let old_address = this.read_target_usize(old_address)?;
18+
let old_address = this.read_pointer(old_address)?;
1919
let old_size = this.read_target_usize(old_size)?;
2020
let new_size = this.read_target_usize(new_size)?;
2121
let flags = this.read_scalar(flags)?.to_i32()?;
2222

2323
// old_address must be a multiple of the page size
2424
#[allow(clippy::arithmetic_side_effects)] // PAGE_SIZE is nonzero
25-
if old_address % this.machine.page_size != 0 || new_size == 0 {
25+
if old_address.addr().bytes() % this.machine.page_size != 0 || new_size == 0 {
2626
this.set_last_error(Scalar::from_i32(this.eval_libc_i32("EINVAL")))?;
2727
return Ok(this.eval_libc("MAP_FAILED"));
2828
}
@@ -41,7 +41,6 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
4141
return Ok(Scalar::from_maybe_pointer(Pointer::null(), this));
4242
}
4343

44-
let old_address = Machine::ptr_from_addr_cast(this, old_address)?;
4544
let align = this.machine.page_align();
4645
let ptr = this.reallocate_ptr(
4746
old_address,
@@ -59,8 +58,6 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
5958
)
6059
.unwrap();
6160
}
62-
// Memory mappings are always exposed
63-
Machine::expose_ptr(this, ptr)?;
6461

6562
Ok(Scalar::from_pointer(ptr, this))
6663
}

src/shims/unix/mem.rs

+5-30
Original file line numberDiff line numberDiff line change
@@ -100,8 +100,6 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
100100
std::iter::repeat(0u8).take(usize::try_from(map_length).unwrap()),
101101
)
102102
.unwrap();
103-
// Memory mappings don't use provenance, and are always exposed.
104-
Machine::expose_ptr(this, ptr)?;
105103

106104
Ok(Scalar::from_pointer(ptr, this))
107105
}
@@ -113,43 +111,20 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
113111
) -> InterpResult<'tcx, Scalar<Provenance>> {
114112
let this = self.eval_context_mut();
115113

116-
let addr = this.read_target_usize(addr)?;
114+
let addr = this.read_pointer(addr)?;
117115
let length = this.read_target_usize(length)?;
118116

119117
// addr must be a multiple of the page size
120118
#[allow(clippy::arithmetic_side_effects)] // PAGE_SIZE is nonzero
121-
if addr % this.machine.page_size != 0 {
119+
if addr.addr().bytes() % this.machine.page_size != 0 {
122120
this.set_last_error(Scalar::from_i32(this.eval_libc_i32("EINVAL")))?;
123121
return Ok(Scalar::from_i32(-1));
124122
}
125123

126-
let length = round_to_next_multiple_of(length, this.machine.page_size);
127-
128-
let ptr = Machine::ptr_from_addr_cast(this, addr)?;
129-
130-
let Ok(ptr) = ptr.into_pointer_or_addr() else {
131-
throw_unsup_format!("Miri only supports munmap on memory allocated directly by mmap");
132-
};
133-
let Some((alloc_id, offset, _prov)) = Machine::ptr_get_alloc(this, ptr) else {
134-
throw_unsup_format!("Miri only supports munmap on memory allocated directly by mmap");
135-
};
136-
137-
// Elsewhere in this function we are careful to check what we can and throw an unsupported
138-
// error instead of Undefined Behavior when use of this function falls outside of the
139-
// narrow scope we support. We deliberately do not check the MemoryKind of this allocation,
140-
// because we want to report UB on attempting to unmap memory that Rust "understands", such
141-
// the stack, heap, or statics.
142-
let (_kind, alloc) = this.memory.alloc_map().get(alloc_id).unwrap();
143-
if offset != Size::ZERO || alloc.len() as u64 != length {
144-
throw_unsup_format!(
145-
"Miri only supports munmap calls that exactly unmap a region previously returned by mmap"
146-
);
147-
}
148-
149-
let len = Size::from_bytes(alloc.len() as u64);
124+
let length = Size::from_bytes(round_to_next_multiple_of(length, this.machine.page_size));
150125
this.deallocate_ptr(
151-
ptr.into(),
152-
Some((len, this.machine.page_align())),
126+
addr,
127+
Some((length, this.machine.page_align())),
153128
MemoryKind::Machine(MiriMemoryKind::Mmap),
154129
)?;
155130

tests/fail-dep/shims/mmap_use_after_munmap.stderr

+1-16
Original file line numberDiff line numberDiff line change
@@ -1,18 +1,3 @@
1-
warning: integer-to-pointer cast
2-
--> $DIR/mmap_use_after_munmap.rs:LL:CC
3-
|
4-
LL | libc::munmap(ptr, 4096);
5-
| ^^^^^^^^^^^^^^^^^^^^^^^ integer-to-pointer cast
6-
|
7-
= help: This program is using integer-to-pointer casts or (equivalently) `ptr::from_exposed_addr`,
8-
= help: which means that Miri might miss pointer bugs in this program.
9-
= help: See https://doc.rust-lang.org/nightly/std/ptr/fn.from_exposed_addr.html for more details on that operation.
10-
= 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.
11-
= help: You can then pass the `-Zmiri-strict-provenance` flag to Miri, to ensure you are not relying on `from_exposed_addr` semantics.
12-
= help: Alternatively, the `-Zmiri-permissive-provenance` flag disables this warning.
13-
= note: BACKTRACE:
14-
= note: inside `main` at $DIR/mmap_use_after_munmap.rs:LL:CC
15-
161
error: Undefined Behavior: memory access failed: ALLOC has been freed, so this pointer is dangling
172
--> $DIR/mmap_use_after_munmap.rs:LL:CC
183
|
@@ -43,5 +28,5 @@ LL | libc::munmap(ptr, 4096);
4328

4429
note: some details are omitted, run with `MIRIFLAGS=-Zmiri-backtrace=full` for a verbose backtrace
4530

46-
error: aborting due to 1 previous error; 1 warning emitted
31+
error: aborting due to 1 previous error
4732

tests/fail-dep/shims/munmap.stderr

+1-21
Original file line numberDiff line numberDiff line change
@@ -1,23 +1,3 @@
1-
warning: integer-to-pointer cast
2-
--> $DIR/munmap.rs:LL:CC
3-
|
4-
LL | / libc::munmap(
5-
LL | |
6-
LL | | // Some high address we surely have not allocated anything at
7-
LL | | ptr::invalid_mut(1 << 30),
8-
LL | | 4096,
9-
LL | | )
10-
| |_________^ integer-to-pointer cast
11-
|
12-
= help: This program is using integer-to-pointer casts or (equivalently) `ptr::from_exposed_addr`,
13-
= help: which means that Miri might miss pointer bugs in this program.
14-
= help: See https://doc.rust-lang.org/nightly/std/ptr/fn.from_exposed_addr.html for more details on that operation.
15-
= 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.
16-
= help: You can then pass the `-Zmiri-strict-provenance` flag to Miri, to ensure you are not relying on `from_exposed_addr` semantics.
17-
= help: Alternatively, the `-Zmiri-permissive-provenance` flag disables this warning.
18-
= note: BACKTRACE:
19-
= note: inside `main` at $DIR/munmap.rs:LL:CC
20-
211
error: unsupported operation: Miri only supports munmap on memory allocated directly by mmap
222
--> $DIR/munmap.rs:LL:CC
233
|
@@ -35,5 +15,5 @@ LL | | )
3515

3616
note: some details are omitted, run with `MIRIFLAGS=-Zmiri-backtrace=full` for a verbose backtrace
3717

38-
error: aborting due to 1 previous error; 1 warning emitted
18+
error: aborting due to 1 previous error
3919

+1-16
Original file line numberDiff line numberDiff line change
@@ -1,18 +1,3 @@
1-
warning: integer-to-pointer cast
2-
--> $DIR/munmap_partial.rs:LL:CC
3-
|
4-
LL | libc::munmap(ptr, 1);
5-
| ^^^^^^^^^^^^^^^^^^^^ integer-to-pointer cast
6-
|
7-
= help: This program is using integer-to-pointer casts or (equivalently) `ptr::from_exposed_addr`,
8-
= help: which means that Miri might miss pointer bugs in this program.
9-
= help: See https://doc.rust-lang.org/nightly/std/ptr/fn.from_exposed_addr.html for more details on that operation.
10-
= 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.
11-
= help: You can then pass the `-Zmiri-strict-provenance` flag to Miri, to ensure you are not relying on `from_exposed_addr` semantics.
12-
= help: Alternatively, the `-Zmiri-permissive-provenance` flag disables this warning.
13-
= note: BACKTRACE:
14-
= note: inside `main` at $DIR/munmap_partial.rs:LL:CC
15-
161
error: unsupported operation: Miri only supports munmap calls that exactly unmap a region previously returned by mmap
172
--> $DIR/munmap_partial.rs:LL:CC
183
|
@@ -25,5 +10,5 @@ LL | libc::munmap(ptr, 1);
2510

2611
note: some details are omitted, run with `MIRIFLAGS=-Zmiri-backtrace=full` for a verbose backtrace
2712

28-
error: aborting due to 1 previous error; 1 warning emitted
13+
error: aborting due to 1 previous error
2914

tests/pass-dep/shims/mmap.rs

+2-3
Original file line numberDiff line numberDiff line change
@@ -28,9 +28,8 @@ fn test_mmap() {
2828
}
2929
assert!(slice.iter().all(|b| *b == 1));
3030

31-
// Ensure that we can munmap with just an integer
32-
let just_an_address = ptr::invalid_mut(ptr.addr());
33-
let res = unsafe { libc::munmap(just_an_address, page_size) };
31+
// Ensure that we can munmap
32+
let res = unsafe { libc::munmap(ptr, page_size) };
3433
assert_eq!(res, 0i32);
3534
}
3635

0 commit comments

Comments
 (0)