Fix: Aliasing UB Reported by Miri

.github/workflows/ci.yml

@@ -15,7 +15,7 @@ jobs:
     - uses: dtolnay/rust-toolchain@nightly
       with:
         toolchain: ${{ matrix.rust-toolchain }}
-        components: rust-src, clippy, rustfmt
+        components: rust-src, clippy, rustfmt, miri
         targets: ${{ matrix.targets }}
     - name: Check rust version
       run: rustc --version --verbose
@@ -28,6 +28,12 @@ jobs:
     - name: Unit test
       if: ${{ matrix.targets == 'x86_64-unknown-linux-gnu' }}
       run: cargo test --target ${{ matrix.targets }} -- --nocapture
+    - name: Miri
+      if: ${{ matrix.targets == 'x86_64-unknown-linux-gnu' }}
+      # We can use stack borrows model, but need to adjust the following testcase, or skip it:
+      # cargo miri test -- --skip raw_list::tests::test_one_insert_after
+      # Or rather, use the less strick aliasing model tree borrows:
+      run: MIRIFLAGS=-Zmiri-tree-borrows cargo miri test
 
   doc:
     runs-on: ubuntu-latest

src/linked_list.rs

@@ -154,7 +154,7 @@ impl<G: GetLinksWrapped> List<G> {
         let ptr = data.into_pointer();
 
         // SAFETY: We took ownership of the entry, so it is safe to insert it.
-        if !unsafe { self.list.push_back(ptr.as_ref()) } {
+        if !unsafe { self.list.push_back(ptr) } {
             // If insertion failed, rebuild object so that it can be freed.
             // SAFETY: We just called `into_pointer` above.
             unsafe { G::Wrapped::from_pointer(ptr) };
@@ -169,7 +169,7 @@ impl<G: GetLinksWrapped> List<G> {
         let ptr = data.into_pointer();
 
         // SAFETY: We took ownership of the entry, so it is safe to insert it.
-        if !unsafe { self.list.push_front(ptr.as_ref()) } {
+        if !unsafe { self.list.push_front(ptr) } {
             // If insertion failed, rebuild object so that it can be freed.
             unsafe { G::Wrapped::from_pointer(ptr) };
         }

src/raw_list.rs

@@ -150,16 +150,17 @@ impl<G: GetLinks> RawList<G> {
         true
     }
 
-    fn push_back_internal(&mut self, new: &G::EntryType, front: bool) -> bool {
-        let links = G::get_links(new);
+    fn push_back_internal(&mut self, new: NonNull<G::EntryType>, front: bool) -> bool {
+        // SAFETY: We just get the valid EntryType ptr from the caller.
+        let links = unsafe { G::get_links(new.as_ref()) };
         if !links.acquire_for_insertion() {
             // Nothing to do if already inserted.
             return false;
         }
 
         // SAFETY: The links are now owned by the list, so it is safe to get a mutable reference.
         let new_entry = unsafe { &mut *links.entry.get() };
-        let new_ptr = Some(NonNull::from(new));
+        let new_ptr = Some(new);
         match self.back() {
             // SAFETY: `back` is valid as the list cannot change.
             Some(back) => {
@@ -183,7 +184,7 @@ impl<G: GetLinks> RawList<G> {
     /// Rawlist will save the reference as node ptr.
     /// The caller must ensure the validity of the reference while it is on
     /// the linked list.
-    pub unsafe fn push_back(&mut self, new: &G::EntryType) -> bool {
+    pub unsafe fn push_back(&mut self, new: NonNull<G::EntryType>) -> bool {
         self.push_back_internal(new, false)
     }
 
@@ -192,7 +193,7 @@ impl<G: GetLinks> RawList<G> {
     /// Rawlist will save the reference as node ptr.
     /// The caller must ensure the validity of the reference while it is on
     /// the linked list.
-    pub unsafe fn push_front(&mut self, new: &G::EntryType) -> bool {
+    pub unsafe fn push_front(&mut self, new: NonNull<G::EntryType>) -> bool {
         self.push_back_internal(new, true)
     }
 
@@ -467,6 +468,7 @@ impl<G: GetLinks> iter::DoubleEndedIterator for Iterator<'_, G> {
 mod tests {
     extern crate alloc;
     use alloc::{boxed::Box, vec::Vec};
+    use core::ptr::NonNull;
 
     struct Example {
         links: super::Links<Self>,
@@ -530,7 +532,7 @@ mod tests {
                 for j in 0..n {
                     // SAFETY: The entry was allocated above, it's not in any lists yet, is never
                     // moved, and outlives the list.
-                    unsafe { list.push_back(&v[j]) };
+                    unsafe { list.push_back(NonNull::from(&*v[j])) };
                 }
 
                 // Call the test case.
@@ -551,7 +553,7 @@ mod tests {
         for n in 1..=MAX {
             // SAFETY: The entry was allocated above, it's not in any lists yet, is never moved,
             // and outlives the list.
-            unsafe { list.push_back(&v[n - 1]) };
+            unsafe { list.push_back(NonNull::from(&*v[n - 1])) };
             assert_list_contents(&v[..n], &list);
         }
     }
@@ -566,7 +568,7 @@ mod tests {
             // SAFETY: The entry was allocated above, it's not in any lists yet, is never moved,
             // and outlives the list.
             println!("push front: {}", MAX - n);
-            unsafe { list.push_front(&v[MAX - n]) };
+            unsafe { list.push_front(NonNull::from(&*v[MAX - n])) };
             assert_list_contents(&v[MAX - n..MAX], &list);
         }
     }