From 8f8766afd859543cfcc43248173747315bfca005 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Thu, 9 May 2024 12:52:38 +0200 Subject: [PATCH] offset_from intrinsic: always allow pointers to point to the same address --- .../src/interpret/intrinsics.rs | 33 ++++++++++--------- library/core/src/ptr/const_ptr.rs | 12 +++---- library/core/src/ptr/mut_ptr.rs | 12 +++---- .../pass/zero-sized-accesses-and-offsets.rs | 3 -- tests/ui/consts/offset_from_ub.rs | 33 +++++++++++-------- tests/ui/consts/offset_from_ub.stderr | 20 +++++------ 6 files changed, 59 insertions(+), 54 deletions(-) diff --git a/compiler/rustc_const_eval/src/interpret/intrinsics.rs b/compiler/rustc_const_eval/src/interpret/intrinsics.rs index 67ae71b72b654..d4aedb8b7e361 100644 --- a/compiler/rustc_const_eval/src/interpret/intrinsics.rs +++ b/compiler/rustc_const_eval/src/interpret/intrinsics.rs @@ -17,7 +17,7 @@ use rustc_target::abi::Size; use super::{ memory::MemoryKind, util::ensure_monomorphic_enough, Allocation, CheckInAllocMsg, ConstAllocation, GlobalId, ImmTy, InterpCx, InterpResult, MPlaceTy, Machine, OpTy, Pointer, - PointerArithmetic, Scalar, + PointerArithmetic, Provenance, Scalar, }; use crate::fluent_generated as fluent; @@ -256,25 +256,28 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { // This will always return 0. (a, b) } - (Err(_), _) | (_, Err(_)) => { - // We managed to find a valid allocation for one pointer, but not the other. - // That means they are definitely not pointing to the same allocation. + _ if M::Provenance::OFFSET_IS_ADDR && a.addr() == b.addr() => { + // At least one of the pointers has provenance, but they also point to + // the same address so it doesn't matter; this is fine. `(0, 0)` means + // we pass all the checks below and return 0. + (0, 0) + } + // From here onwards, the pointers are definitely for different addresses + // (or we can't determine their absolute address). + (Ok((a_alloc_id, a_offset, _)), Ok((b_alloc_id, b_offset, _))) + if a_alloc_id == b_alloc_id => + { + // Found allocation for both, and it's the same. + // Use these offsets for distance calculation. + (a_offset.bytes(), b_offset.bytes()) + } + _ => { + // Not into the same allocation -- this is UB. throw_ub_custom!( fluent::const_eval_offset_from_different_allocations, name = intrinsic_name, ); } - (Ok((a_alloc_id, a_offset, _)), Ok((b_alloc_id, b_offset, _))) => { - // Found allocation for both. They must be into the same allocation. - if a_alloc_id != b_alloc_id { - throw_ub_custom!( - fluent::const_eval_offset_from_different_allocations, - name = intrinsic_name, - ); - } - // Use these offsets for distance calculation. - (a_offset.bytes(), b_offset.bytes()) - } }; // Compute distance. diff --git a/library/core/src/ptr/const_ptr.rs b/library/core/src/ptr/const_ptr.rs index 27b0c69d32d0f..ad989860339c3 100644 --- a/library/core/src/ptr/const_ptr.rs +++ b/library/core/src/ptr/const_ptr.rs @@ -679,9 +679,9 @@ impl *const T { /// /// * `self` and `origin` must either /// + /// * point to the same address, or /// * both be *derived from* a pointer to the same [allocated object], and the memory range between - /// the two pointers must be either empty or in bounds of that object. (See below for an example.) - /// * or both be derived from an integer literal/constant, and point to the same address. + /// the two pointers must be in bounds of that object. (See below for an example.) /// /// * The distance between the pointers, in bytes, must be an exact multiple /// of the size of `T`. @@ -744,14 +744,14 @@ impl *const T { /// let ptr1 = Box::into_raw(Box::new(0u8)) as *const u8; /// let ptr2 = Box::into_raw(Box::new(1u8)) as *const u8; /// let diff = (ptr2 as isize).wrapping_sub(ptr1 as isize); - /// // Make ptr2_other an "alias" of ptr2, but derived from ptr1. - /// let ptr2_other = (ptr1 as *const u8).wrapping_offset(diff); + /// // Make ptr2_other an "alias" of ptr2.add(1), but derived from ptr1. + /// let ptr2_other = (ptr1 as *const u8).wrapping_offset(diff).wrapping_offset(1); /// assert_eq!(ptr2 as usize, ptr2_other as usize); /// // Since ptr2_other and ptr2 are derived from pointers to different objects, /// // computing their offset is undefined behavior, even though - /// // they point to the same address! + /// // they point to addresses that are in-bounds of the same object! /// unsafe { - /// let zero = ptr2_other.offset_from(ptr2); // Undefined Behavior + /// let one = ptr2_other.offset_from(ptr2); // Undefined Behavior /// } /// ``` #[stable(feature = "ptr_offset_from", since = "1.47.0")] diff --git a/library/core/src/ptr/mut_ptr.rs b/library/core/src/ptr/mut_ptr.rs index 3d05715f7a46d..fda15b5ce4ab8 100644 --- a/library/core/src/ptr/mut_ptr.rs +++ b/library/core/src/ptr/mut_ptr.rs @@ -907,9 +907,9 @@ impl *mut T { /// /// * `self` and `origin` must either /// + /// * point to the same address, or /// * both be *derived from* a pointer to the same [allocated object], and the memory range between - /// the two pointers must be either empty or in bounds of that object. (See below for an example.) - /// * or both be derived from an integer literal/constant, and point to the same address. + /// the two pointers must be in bounds of that object. (See below for an example.) /// /// * The distance between the pointers, in bytes, must be an exact multiple /// of the size of `T`. @@ -972,14 +972,14 @@ impl *mut T { /// let ptr1 = Box::into_raw(Box::new(0u8)); /// let ptr2 = Box::into_raw(Box::new(1u8)); /// let diff = (ptr2 as isize).wrapping_sub(ptr1 as isize); - /// // Make ptr2_other an "alias" of ptr2, but derived from ptr1. - /// let ptr2_other = (ptr1 as *mut u8).wrapping_offset(diff); + /// // Make ptr2_other an "alias" of ptr2.add(1), but derived from ptr1. + /// let ptr2_other = (ptr1 as *mut u8).wrapping_offset(diff).wrapping_offset(1); /// assert_eq!(ptr2 as usize, ptr2_other as usize); /// // Since ptr2_other and ptr2 are derived from pointers to different objects, /// // computing their offset is undefined behavior, even though - /// // they point to the same address! + /// // they point to addresses that are in-bounds of the same object! /// unsafe { - /// let zero = ptr2_other.offset_from(ptr2); // Undefined Behavior + /// let one = ptr2_other.offset_from(ptr2); // Undefined Behavior /// } /// ``` #[stable(feature = "ptr_offset_from", since = "1.47.0")] diff --git a/src/tools/miri/tests/pass/zero-sized-accesses-and-offsets.rs b/src/tools/miri/tests/pass/zero-sized-accesses-and-offsets.rs index 2d142bef73c4a..a3356b682a6a5 100644 --- a/src/tools/miri/tests/pass/zero-sized-accesses-and-offsets.rs +++ b/src/tools/miri/tests/pass/zero-sized-accesses-and-offsets.rs @@ -39,8 +39,6 @@ fn test_ptr(ptr: *mut ()) { // Distance. let ptr = ptr.cast::(); ptr.offset_from(ptr); - /* - FIXME: this is disabled for now as these cases are not yet allowed. // Distance from other "bad" pointers that have the same address, but different provenance. Some // of this is library UB, but we don't want it to be language UB since that would violate // provenance monotonicity: if we allow computing the distance between two ptrs with no @@ -54,6 +52,5 @@ fn test_ptr(ptr: *mut ()) { // - Distance from use-after-free pointer drop(b); ptr.offset_from(other_ptr.with_addr(ptr.addr())); - */ } } diff --git a/tests/ui/consts/offset_from_ub.rs b/tests/ui/consts/offset_from_ub.rs index 57767e965962b..e9456de994aa4 100644 --- a/tests/ui/consts/offset_from_ub.rs +++ b/tests/ui/consts/offset_from_ub.rs @@ -32,12 +32,6 @@ pub const NOT_MULTIPLE_OF_SIZE: isize = { //~| 1_isize cannot be divided by 2_isize without remainder }; -pub const OFFSET_FROM_NULL: isize = { - let ptr = 0 as *const u8; - // Null isn't special for zero-sized "accesses" (i.e., the range between the two pointers) - unsafe { ptr_offset_from(ptr, ptr) } -}; - pub const DIFFERENT_INT: isize = { // offset_from with two different integers: like DIFFERENT_ALLOC let ptr1 = 8 as *const u8; let ptr2 = 16 as *const u8; @@ -63,14 +57,6 @@ const OUT_OF_BOUNDS_2: isize = { //~| pointer to 10 bytes starting at offset 0 is out-of-bounds }; -const OUT_OF_BOUNDS_SAME: isize = { - let start_ptr = &4 as *const _ as *const u8; - let length = 10; - let end_ptr = (start_ptr).wrapping_add(length); - // Out-of-bounds is fine as long as the range between the pointers is empty. - unsafe { ptr_offset_from(end_ptr, end_ptr) } -}; - pub const DIFFERENT_ALLOC_UNSIGNED: usize = { let uninit = std::mem::MaybeUninit::::uninit(); let base_ptr: *const Struct = &uninit as *const _ as *const Struct; @@ -122,4 +108,23 @@ pub const OFFSET_VERY_FAR2: isize = { //~^ inside }; +// If the pointers are the same, OOB/null/UAF is fine. +pub const OFFSET_FROM_NULL_SAME: isize = { + let ptr = 0 as *const u8; + unsafe { ptr_offset_from(ptr, ptr) } +}; +const OUT_OF_BOUNDS_SAME: isize = { + let start_ptr = &4 as *const _ as *const u8; + let length = 10; + let end_ptr = (start_ptr).wrapping_add(length); + unsafe { ptr_offset_from(end_ptr, end_ptr) } +}; +const UAF_SAME: isize = { + let uaf_ptr = { + let x = 0; + &x as *const i32 + }; + unsafe { ptr_offset_from(uaf_ptr, uaf_ptr) } +}; + fn main() {} diff --git a/tests/ui/consts/offset_from_ub.stderr b/tests/ui/consts/offset_from_ub.stderr index 65f75a6e05875..84085456386ea 100644 --- a/tests/ui/consts/offset_from_ub.stderr +++ b/tests/ui/consts/offset_from_ub.stderr @@ -24,49 +24,49 @@ LL | unsafe { ptr_offset_from(field_ptr, base_ptr as *const u16) } | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ exact_div: 1_isize cannot be divided by 2_isize without remainder error[E0080]: evaluation of constant value failed - --> $DIR/offset_from_ub.rs:44:14 + --> $DIR/offset_from_ub.rs:38:14 | LL | unsafe { ptr_offset_from(ptr2, ptr1) } | ^^^^^^^^^^^^^^^^^^^^^^^^^^^ `ptr_offset_from` called on different pointers without provenance (i.e., without an associated allocation) error[E0080]: evaluation of constant value failed - --> $DIR/offset_from_ub.rs:53:14 + --> $DIR/offset_from_ub.rs:47:14 | LL | unsafe { ptr_offset_from(end_ptr, start_ptr) } | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ out-of-bounds `offset_from`: ALLOC0 has size 4, so pointer to 10 bytes starting at offset 0 is out-of-bounds error[E0080]: evaluation of constant value failed - --> $DIR/offset_from_ub.rs:62:14 + --> $DIR/offset_from_ub.rs:56:14 | LL | unsafe { ptr_offset_from(start_ptr, end_ptr) } | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ out-of-bounds `offset_from`: ALLOC1 has size 4, so pointer to 10 bytes starting at offset 0 is out-of-bounds error[E0080]: evaluation of constant value failed - --> $DIR/offset_from_ub.rs:79:14 + --> $DIR/offset_from_ub.rs:65:14 | LL | unsafe { ptr_offset_from_unsigned(field_ptr, base_ptr) } | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ `ptr_offset_from_unsigned` called on pointers into different allocations error[E0080]: evaluation of constant value failed - --> $DIR/offset_from_ub.rs:86:14 + --> $DIR/offset_from_ub.rs:72:14 | LL | unsafe { ptr_offset_from(ptr2, ptr1) } | ^^^^^^^^^^^^^^^^^^^^^^^^^^^ `ptr_offset_from` called when first pointer is too far ahead of second error[E0080]: evaluation of constant value failed - --> $DIR/offset_from_ub.rs:92:14 + --> $DIR/offset_from_ub.rs:78:14 | LL | unsafe { ptr_offset_from(ptr1, ptr2) } | ^^^^^^^^^^^^^^^^^^^^^^^^^^^ `ptr_offset_from` called when first pointer is too far before second error[E0080]: evaluation of constant value failed - --> $DIR/offset_from_ub.rs:99:14 + --> $DIR/offset_from_ub.rs:85:14 | LL | unsafe { ptr_offset_from_unsigned(p, p.add(2) ) } | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ `ptr_offset_from_unsigned` called when first pointer has smaller offset than second: 0 < 8 error[E0080]: evaluation of constant value failed - --> $DIR/offset_from_ub.rs:106:14 + --> $DIR/offset_from_ub.rs:92:14 | LL | unsafe { ptr_offset_from_unsigned(ptr2, ptr1) } | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ `ptr_offset_from_unsigned` called when first pointer is too far ahead of second @@ -79,7 +79,7 @@ error[E0080]: evaluation of constant value failed note: inside `std::ptr::const_ptr::::offset_from` --> $SRC_DIR/core/src/ptr/const_ptr.rs:LL:COL note: inside `OFFSET_VERY_FAR1` - --> $DIR/offset_from_ub.rs:115:14 + --> $DIR/offset_from_ub.rs:101:14 | LL | unsafe { ptr2.offset_from(ptr1) } | ^^^^^^^^^^^^^^^^^^^^^^ @@ -92,7 +92,7 @@ error[E0080]: evaluation of constant value failed note: inside `std::ptr::const_ptr::::offset_from` --> $SRC_DIR/core/src/ptr/const_ptr.rs:LL:COL note: inside `OFFSET_VERY_FAR2` - --> $DIR/offset_from_ub.rs:121:14 + --> $DIR/offset_from_ub.rs:107:14 | LL | unsafe { ptr1.offset_from(ptr2.wrapping_offset(1)) } | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^