Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

offset_from: always allow pointers to point to the same address #124921

Merged
merged 1 commit into from
Jul 15, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
33 changes: 18 additions & 15 deletions compiler/rustc_const_eval/src/interpret/intrinsics.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ use super::{
err_inval, err_ub_custom, err_unsup_format, memory::MemoryKind, throw_inval, throw_ub_custom,
throw_ub_format, util::ensure_monomorphic_enough, Allocation, CheckInAllocMsg, ConstAllocation,
GlobalId, ImmTy, InterpCx, InterpResult, MPlaceTy, Machine, OpTy, Pointer, PointerArithmetic,
Scalar,
Provenance, Scalar,
};

use crate::fluent_generated as fluent;
Expand Down Expand Up @@ -259,25 +259,28 @@ impl<'tcx, M: Machine<'tcx>> InterpCx<'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.
Expand Down
12 changes: 6 additions & 6 deletions library/core/src/ptr/const_ptr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -604,9 +604,9 @@ impl<T: ?Sized> *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`.
Expand Down Expand Up @@ -653,14 +653,14 @@ impl<T: ?Sized> *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")]
Expand Down
12 changes: 6 additions & 6 deletions library/core/src/ptr/mut_ptr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -829,9 +829,9 @@ impl<T: ?Sized> *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`.
Expand Down Expand Up @@ -878,14 +878,14 @@ impl<T: ?Sized> *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")]
Expand Down
13 changes: 7 additions & 6 deletions library/core/src/ptr/non_null.rs
Original file line number Diff line number Diff line change
Expand Up @@ -735,9 +735,9 @@ impl<T: ?Sized> NonNull<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`.
Expand Down Expand Up @@ -789,14 +789,15 @@ impl<T: ?Sized> NonNull<T> {
/// let ptr1 = NonNull::new(Box::into_raw(Box::new(0u8))).unwrap();
/// let ptr2 = NonNull::new(Box::into_raw(Box::new(1u8))).unwrap();
/// let diff = (ptr2.addr().get() as isize).wrapping_sub(ptr1.addr().get() as isize);
/// // Make ptr2_other an "alias" of ptr2, but derived from ptr1.
/// let ptr2_other = NonNull::new(ptr1.as_ptr().wrapping_byte_offset(diff)).unwrap();
/// // Make ptr2_other an "alias" of ptr2.add(1), but derived from ptr1.
/// let diff_plus_1 = diff.wrapping_add(1);
/// let ptr2_other = NonNull::new(ptr1.as_ptr().wrapping_byte_offset(diff_plus_1)).unwrap();
/// assert_eq!(ptr2.addr(), ptr2_other.addr());
/// // 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!
///
/// let zero = unsafe { ptr2_other.offset_from(ptr2) }; // Undefined Behavior
/// let one = unsafe { ptr2_other.offset_from(ptr2) }; // Undefined Behavior! ⚠️
/// ```
#[inline]
#[cfg_attr(miri, track_caller)] // even without panics, this helps for Miri backtraces
Expand Down
3 changes: 0 additions & 3 deletions src/tools/miri/tests/pass/zero-sized-accesses-and-offsets.rs
Original file line number Diff line number Diff line change
Expand Up @@ -39,8 +39,6 @@ fn test_ptr(ptr: *mut ()) {
// Distance.
let ptr = ptr.cast::<i32>();
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
Expand All @@ -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()));
*/
}
}
33 changes: 19 additions & 14 deletions tests/ui/consts/offset_from_ub.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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::<Struct>::uninit();
let base_ptr: *const Struct = &uninit as *const _ as *const Struct;
Expand Down Expand Up @@ -130,4 +116,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() {}
22 changes: 11 additions & 11 deletions tests/ui/consts/offset_from_ub.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -24,55 +24,55 @@ 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:100:14
--> $DIR/offset_from_ub.rs:86: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:107:14
--> $DIR/offset_from_ub.rs:93: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:114:14
--> $DIR/offset_from_ub.rs:100:14
|
LL | unsafe { ptr_offset_from_unsigned(ptr2, ptr1) }
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ `ptr_offset_from_unsigned` called when first pointer is too far ahead of second
Expand All @@ -85,7 +85,7 @@ error[E0080]: evaluation of constant value failed
note: inside `std::ptr::const_ptr::<impl *const u8>::offset_from`
--> $SRC_DIR/core/src/ptr/const_ptr.rs:LL:COL
note: inside `OFFSET_VERY_FAR1`
--> $DIR/offset_from_ub.rs:123:14
--> $DIR/offset_from_ub.rs:109:14
|
LL | unsafe { ptr2.offset_from(ptr1) }
| ^^^^^^^^^^^^^^^^^^^^^^
Expand All @@ -98,7 +98,7 @@ error[E0080]: evaluation of constant value failed
note: inside `std::ptr::const_ptr::<impl *const u8>::offset_from`
--> $SRC_DIR/core/src/ptr/const_ptr.rs:LL:COL
note: inside `OFFSET_VERY_FAR2`
--> $DIR/offset_from_ub.rs:129:14
--> $DIR/offset_from_ub.rs:115:14
|
LL | unsafe { ptr1.offset_from(ptr2.wrapping_offset(1)) }
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
Expand Down
Loading