From adc3a8103b90d8c4442a7f3d3d51ce8a4580d9fd Mon Sep 17 00:00:00 2001 From: Yotam Ofek Date: Mon, 13 Nov 2023 10:35:46 +0000 Subject: [PATCH 1/4] Fix UB in `RawStorageMut::swap_unchecked_linear` --- src/base/storage.rs | 15 +++++++++++++-- 1 file changed, 13 insertions(+), 2 deletions(-) diff --git a/src/base/storage.rs b/src/base/storage.rs index d5f1de61..fc40bb41 100644 --- a/src/base/storage.rs +++ b/src/base/storage.rs @@ -208,8 +208,19 @@ pub unsafe trait RawStorageMut: RawStorage { /// If the indices are out of bounds, the method will cause undefined behavior. #[inline] unsafe fn swap_unchecked_linear(&mut self, i1: usize, i2: usize) { - let a = self.get_address_unchecked_linear_mut(i1); - let b = self.get_address_unchecked_linear_mut(i2); + // we can't just use the pointers returned from `get_address_unchecked_linear_mut` because calling a + // method taking self mutably invalidates any existing (mutable) pointers. since `get_address_unchecked_linear_mut` can + // also be overriden by a custom implementation, we can't just use `wrapping_add` assuming that's what the method does. + // instead, we use `offset_from` to compute the re-calculate the pointers from the base pointer. + // this is safe as long as this trait is implemented safely + // (and it's the caller's responsibility to ensure the indices are in-bounds). + let base = self.ptr_mut(); + let offset1 = self.get_address_unchecked_linear_mut(i1).offset_from(base); + let offset2 = self.get_address_unchecked_linear_mut(i2).offset_from(base); + + let base = self.ptr_mut(); + let a = base.offset(offset1); + let b = base.offset(offset2); ptr::swap(a, b); } From 546d06b541ea82d5fd975946b3b26db9bd97b18c Mon Sep 17 00:00:00 2001 From: Yotam Ofek Date: Tue, 19 Mar 2024 18:26:36 +0200 Subject: [PATCH 2/4] Update src/base/storage.rs Co-authored-by: tpdickso --- src/base/storage.rs | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/src/base/storage.rs b/src/base/storage.rs index fc40bb41..a83e8878 100644 --- a/src/base/storage.rs +++ b/src/base/storage.rs @@ -206,6 +206,14 @@ pub unsafe trait RawStorageMut: RawStorage { /// /// # Safety /// If the indices are out of bounds, the method will cause undefined behavior. + /// + /// # Validity + /// The default implementation of this trait function is only guaranteed to be + /// sound if invocations of `self.ptr_mut()` and `self.get_address_unchecked_linear_mut()` + /// result in stable references. If any of the data pointed to by these trait methods + /// moves as a consequence of invoking either of these methods then this default + /// trait implementation may be invalid or unsound and should be overridden. + #[inline] unsafe fn swap_unchecked_linear(&mut self, i1: usize, i2: usize) { // we can't just use the pointers returned from `get_address_unchecked_linear_mut` because calling a From d884a7e2d06345af565af7a7ee064d0965fb65c2 Mon Sep 17 00:00:00 2001 From: Yotam Ofek Date: Tue, 19 Mar 2024 16:37:11 +0000 Subject: [PATCH 3/4] fmt --- src/base/storage.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/base/storage.rs b/src/base/storage.rs index a83e8878..60718765 100644 --- a/src/base/storage.rs +++ b/src/base/storage.rs @@ -206,14 +206,14 @@ pub unsafe trait RawStorageMut: RawStorage { /// /// # Safety /// If the indices are out of bounds, the method will cause undefined behavior. - /// + /// /// # Validity /// The default implementation of this trait function is only guaranteed to be /// sound if invocations of `self.ptr_mut()` and `self.get_address_unchecked_linear_mut()` /// result in stable references. If any of the data pointed to by these trait methods /// moves as a consequence of invoking either of these methods then this default /// trait implementation may be invalid or unsound and should be overridden. - + #[inline] unsafe fn swap_unchecked_linear(&mut self, i1: usize, i2: usize) { // we can't just use the pointers returned from `get_address_unchecked_linear_mut` because calling a From 095c561b60db21f62ad98976882a68fb1ebb0fa3 Mon Sep 17 00:00:00 2001 From: Yotam Ofek Date: Wed, 20 Mar 2024 19:55:03 +0200 Subject: [PATCH 4/4] Apply suggestions from code review Co-authored-by: tpdickso --- src/base/storage.rs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/base/storage.rs b/src/base/storage.rs index 60718765..b39d0f3d 100644 --- a/src/base/storage.rs +++ b/src/base/storage.rs @@ -213,14 +213,13 @@ pub unsafe trait RawStorageMut: RawStorage { /// result in stable references. If any of the data pointed to by these trait methods /// moves as a consequence of invoking either of these methods then this default /// trait implementation may be invalid or unsound and should be overridden. - #[inline] unsafe fn swap_unchecked_linear(&mut self, i1: usize, i2: usize) { // we can't just use the pointers returned from `get_address_unchecked_linear_mut` because calling a // method taking self mutably invalidates any existing (mutable) pointers. since `get_address_unchecked_linear_mut` can // also be overriden by a custom implementation, we can't just use `wrapping_add` assuming that's what the method does. // instead, we use `offset_from` to compute the re-calculate the pointers from the base pointer. - // this is safe as long as this trait is implemented safely + // this is sound as long as this trait matches the Validity preconditions // (and it's the caller's responsibility to ensure the indices are in-bounds). let base = self.ptr_mut(); let offset1 = self.get_address_unchecked_linear_mut(i1).offset_from(base);