about summary refs log tree commit diff
diff options
context:
space:
mode:
authorbors <bors@rust-lang.org>2023-08-15 12:18:56 +0000
committerbors <bors@rust-lang.org>2023-08-15 12:18:56 +0000
commit5b9168a32d143067ffe99fb1c6979b8daa6d715e (patch)
tree6745ee760e87a7c30b7d11858d45a32aa28c3450
parent46aa9bc7f905557d847db4f1b0729966e0239173 (diff)
parent6147260f38e86b5a2c1601d3b3747a7bd08051dc (diff)
downloadrust-5b9168a32d143067ffe99fb1c6979b8daa6d715e.tar.gz
rust-5b9168a32d143067ffe99fb1c6979b8daa6d715e.zip
Auto merge of #2972 - RalfJung:c-mem-functions, r=RalfJung
C `mem` function shims: consistently treat "invalid" pointers as UB

Depends on https://github.com/rust-lang/rust/pull/113435.
-rw-r--r--src/tools/miri/src/shims/foreign_items.rs12
-rw-r--r--src/tools/miri/tests/fail/shims/memchr_null.stderr4
-rw-r--r--src/tools/miri/tests/fail/shims/memcmp_null.stderr4
-rw-r--r--src/tools/miri/tests/fail/shims/memcmp_zero.rs13
-rw-r--r--src/tools/miri/tests/fail/shims/memcmp_zero.stderr15
-rw-r--r--src/tools/miri/tests/fail/shims/memrchr_null.stderr4
6 files changed, 46 insertions, 6 deletions
diff --git a/src/tools/miri/src/shims/foreign_items.rs b/src/tools/miri/src/shims/foreign_items.rs
index 99bcce6ab74..7996e72615f 100644
--- a/src/tools/miri/src/shims/foreign_items.rs
+++ b/src/tools/miri/src/shims/foreign_items.rs
@@ -690,6 +690,10 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
                 let right = this.read_pointer(right)?;
                 let n = Size::from_bytes(this.read_target_usize(n)?);
 
+                // C requires that this must always be a valid pointer (C18 §7.1.4).
+                this.ptr_get_alloc_id(left)?;
+                this.ptr_get_alloc_id(right)?;
+
                 let result = {
                     let left_bytes = this.read_bytes_ptr_strip_provenance(left, n)?;
                     let right_bytes = this.read_bytes_ptr_strip_provenance(right, n)?;
@@ -714,6 +718,9 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
                 #[allow(clippy::cast_sign_loss, clippy::cast_possible_truncation)]
                 let val = val as u8;
 
+                // C requires that this must always be a valid pointer (C18 §7.1.4).
+                this.ptr_get_alloc_id(ptr)?;
+
                 if let Some(idx) = this
                     .read_bytes_ptr_strip_provenance(ptr, Size::from_bytes(num))?
                     .iter()
@@ -738,6 +745,9 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
                 #[allow(clippy::cast_sign_loss, clippy::cast_possible_truncation)]
                 let val = val as u8;
 
+                // C requires that this must always be a valid pointer (C18 §7.1.4).
+                this.ptr_get_alloc_id(ptr)?;
+
                 let idx = this
                     .read_bytes_ptr_strip_provenance(ptr, Size::from_bytes(num))?
                     .iter()
@@ -752,6 +762,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
             "strlen" => {
                 let [ptr] = this.check_shim(abi, Abi::C { unwind: false }, link_name, args)?;
                 let ptr = this.read_pointer(ptr)?;
+                // This reads at least 1 byte, so we are already enforcing that this is a valid pointer.
                 let n = this.read_c_str(ptr)?.len();
                 this.write_scalar(
                     Scalar::from_target_usize(u64::try_from(n).unwrap(), this),
@@ -791,6 +802,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
                 // pointer provenance is preserved by this implementation of `strcpy`.
                 // That is probably overly cautious, but there also is no fundamental
                 // reason to have `strcpy` destroy pointer provenance.
+                // This reads at least 1 byte, so we are already enforcing that this is a valid pointer.
                 let n = this.read_c_str(ptr_src)?.len().checked_add(1).unwrap();
                 this.mem_copy(
                     ptr_src,
diff --git a/src/tools/miri/tests/fail/shims/memchr_null.stderr b/src/tools/miri/tests/fail/shims/memchr_null.stderr
index d48606f34ad..54b58f22c6c 100644
--- a/src/tools/miri/tests/fail/shims/memchr_null.stderr
+++ b/src/tools/miri/tests/fail/shims/memchr_null.stderr
@@ -1,8 +1,8 @@
-error: Undefined Behavior: memory access failed: null pointer is a dangling pointer (it has no provenance)
+error: Undefined Behavior: out-of-bounds pointer use: null pointer is a dangling pointer (it has no provenance)
   --> $DIR/memchr_null.rs:LL:CC
    |
 LL |         libc::memchr(ptr::null(), 0, 0);
-   |         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ memory access failed: null pointer is a dangling pointer (it has no provenance)
+   |         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ out-of-bounds pointer use: null pointer is a dangling pointer (it has no provenance)
    |
    = 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
diff --git a/src/tools/miri/tests/fail/shims/memcmp_null.stderr b/src/tools/miri/tests/fail/shims/memcmp_null.stderr
index 7a09c779894..8b2882fc243 100644
--- a/src/tools/miri/tests/fail/shims/memcmp_null.stderr
+++ b/src/tools/miri/tests/fail/shims/memcmp_null.stderr
@@ -1,8 +1,8 @@
-error: Undefined Behavior: memory access failed: null pointer is a dangling pointer (it has no provenance)
+error: Undefined Behavior: out-of-bounds pointer use: null pointer is a dangling pointer (it has no provenance)
   --> $DIR/memcmp_null.rs:LL:CC
    |
 LL |         libc::memcmp(ptr::null(), ptr::null(), 0);
-   |         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ memory access failed: null pointer is a dangling pointer (it has no provenance)
+   |         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ out-of-bounds pointer use: null pointer is a dangling pointer (it has no provenance)
    |
    = 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
diff --git a/src/tools/miri/tests/fail/shims/memcmp_zero.rs b/src/tools/miri/tests/fail/shims/memcmp_zero.rs
new file mode 100644
index 00000000000..f2ddc200563
--- /dev/null
+++ b/src/tools/miri/tests/fail/shims/memcmp_zero.rs
@@ -0,0 +1,13 @@
+//@ignore-target-windows: No libc on Windows
+//@compile-flags: -Zmiri-permissive-provenance
+
+// C says that passing "invalid" pointers is UB for all string functions.
+// It is unclear whether `(int*)42` is "invalid", but there is no actually
+// a `char` living at that address, so arguably it cannot be a valid pointer.
+// Hence this is UB.
+fn main() {
+    let ptr = 42 as *const u8;
+    unsafe {
+        libc::memcmp(ptr.cast(), ptr.cast(), 0); //~ERROR: dangling
+    }
+}
diff --git a/src/tools/miri/tests/fail/shims/memcmp_zero.stderr b/src/tools/miri/tests/fail/shims/memcmp_zero.stderr
new file mode 100644
index 00000000000..e21b9b06008
--- /dev/null
+++ b/src/tools/miri/tests/fail/shims/memcmp_zero.stderr
@@ -0,0 +1,15 @@
+error: Undefined Behavior: out-of-bounds pointer use: 0x2a[noalloc] is a dangling pointer (it has no provenance)
+  --> $DIR/memcmp_zero.rs:LL:CC
+   |
+LL |         libc::memcmp(ptr.cast(), ptr.cast(), 0);
+   |         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ out-of-bounds pointer use: 0x2a[noalloc] is a dangling pointer (it has no provenance)
+   |
+   = 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/memcmp_zero.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/shims/memrchr_null.stderr b/src/tools/miri/tests/fail/shims/memrchr_null.stderr
index b5b7630e7fd..cc11ba89f8f 100644
--- a/src/tools/miri/tests/fail/shims/memrchr_null.stderr
+++ b/src/tools/miri/tests/fail/shims/memrchr_null.stderr
@@ -1,8 +1,8 @@
-error: Undefined Behavior: memory access failed: null pointer is a dangling pointer (it has no provenance)
+error: Undefined Behavior: out-of-bounds pointer use: null pointer is a dangling pointer (it has no provenance)
   --> $DIR/memrchr_null.rs:LL:CC
    |
 LL |         libc::memrchr(ptr::null(), 0, 0);
-   |         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ memory access failed: null pointer is a dangling pointer (it has no provenance)
+   |         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ out-of-bounds pointer use: null pointer is a dangling pointer (it has no provenance)
    |
    = 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