diff options
| author | bors <bors@rust-lang.org> | 2023-08-29 07:53:56 +0000 |
|---|---|---|
| committer | bors <bors@rust-lang.org> | 2023-08-29 07:53:56 +0000 |
| commit | f6faef447536204549eb47f12884d3e229fe87d3 (patch) | |
| tree | 7b3f2907e6a4134b4b34037485ce71844b529196 | |
| parent | cedbe5c715c1fa9359683c5f108bed2054ac258b (diff) | |
| parent | e7a1e4271d3c65fb707518d89890195faab29efe (diff) | |
| download | rust-f6faef447536204549eb47f12884d3e229fe87d3.tar.gz rust-f6faef447536204549eb47f12884d3e229fe87d3.zip | |
Auto merge of #114795 - RalfJung:cell-swap, r=dtolnay
make Cell::swap panic if the Cells partially overlap
The following function ought to be sound:
```rust
fn as_cell_of_array<T, const N: usize>(c: &[Cell<T>; N]) -> &Cell<[T; N]> {
unsafe { transmute(c) }
}
```
However, due to `Cell::swap`, it currently is not -- safe code can [cause a use-after-free](https://play.rust-lang.org/?version=nightly&mode=debug&edition=2018&gist=c9415799722d985ff7d2c2c997b724ca). This PR fixes that.
Fixes https://github.com/rust-lang/rust/issues/80778
| -rw-r--r-- | library/core/src/cell.rs | 17 | ||||
| -rw-r--r-- | library/core/src/intrinsics.rs | 2 |
2 files changed, 16 insertions, 3 deletions
diff --git a/library/core/src/cell.rs b/library/core/src/cell.rs index 4bbe61ca3e7..28950a43d2d 100644 --- a/library/core/src/cell.rs +++ b/library/core/src/cell.rs @@ -237,6 +237,7 @@ use crate::cmp::Ordering; use crate::fmt::{self, Debug, Display}; +use crate::intrinsics::is_nonoverlapping; use crate::marker::{PhantomData, Unsize}; use crate::mem; use crate::ops::{CoerceUnsized, Deref, DerefMut, DispatchFromDyn}; @@ -415,6 +416,12 @@ impl<T> Cell<T> { /// Swaps the values of two `Cell`s. /// Difference with `std::mem::swap` is that this function doesn't require `&mut` reference. /// + /// # Panics + /// + /// This function will panic if `self` and `other` are different `Cell`s that partially overlap. + /// (Using just standard library methods, it is impossible to create such partially overlapping `Cell`s. + /// However, unsafe code is allowed to e.g. create two `&Cell<[i32; 2]>` that partially overlap.) + /// /// # Examples /// /// ``` @@ -430,14 +437,20 @@ impl<T> Cell<T> { #[stable(feature = "move_cell", since = "1.17.0")] pub fn swap(&self, other: &Self) { if ptr::eq(self, other) { + // Swapping wouldn't change anything. return; } + if !is_nonoverlapping(self, other, 1) { + // See <https://github.com/rust-lang/rust/issues/80778> for why we need to stop here. + panic!("`Cell::swap` on overlapping non-identical `Cell`s"); + } // SAFETY: This can be risky if called from separate threads, but `Cell` // is `!Sync` so this won't happen. This also won't invalidate any // pointers since `Cell` makes sure nothing else will be pointing into - // either of these `Cell`s. + // either of these `Cell`s. We also excluded shenanigans like partially overlapping `Cell`s, + // so `swap` will just properly copy two full values of type `T` back and forth. unsafe { - ptr::swap(self.value.get(), other.value.get()); + mem::swap(&mut *self.value.get(), &mut *other.value.get()); } } diff --git a/library/core/src/intrinsics.rs b/library/core/src/intrinsics.rs index 605870b8a7b..eafbaba9ef3 100644 --- a/library/core/src/intrinsics.rs +++ b/library/core/src/intrinsics.rs @@ -2567,7 +2567,7 @@ pub(crate) fn is_nonoverlapping<T>(src: *const T, dst: *const T, count: usize) - let size = mem::size_of::<T>() .checked_mul(count) .expect("is_nonoverlapping: `size_of::<T>() * count` overflows a usize"); - let diff = if src_usize > dst_usize { src_usize - dst_usize } else { dst_usize - src_usize }; + let diff = src_usize.abs_diff(dst_usize); // If the absolute distance between the ptrs is at least as big as the size of the buffer, // they do not overlap. diff >= size |
