diwic / reffers-rs

Rust wrappers around references, boxes and Arcs.
68 stars 3 forks source link

Unsound implementation of `From` leads to dangling pointer #12

Open shinmao opened 1 year ago

shinmao commented 1 year ago

The source of unsoundness

https://github.com/diwic/reffers-rs/blob/5984523d1d77fcb418fd70fde2428ed8febf5b18/src/rmba.rs#L167-L174

To reproduce the bug

use reffers::RMBA;
use std::{iter, sync};

fn make_a_few<'a, T>(t: T, count: usize) -> Vec<RMBA<'a, T>> {
    match count {
        0 => vec![],
        1 => vec![RMBA::new_box(t)],
        _ => iter::repeat(sync::Arc::new(t))
            .take(count).map(|a| RMBA::from(a)).collect()
    }
}

fn main() {
    let a: u8 = 0;
    let v = make_a_few(a, 2usize);
    println!("{:?}", v[0]);     // UB occurs
}

run with miri

error: Undefined Behavior: dereferencing pointer failed: 0x24c78[noalloc] is a dangling pointer (it has no provenance)
   --> /root/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/core/src/ptr/non_null.rs:377:18
    |
377 |         unsafe { &*self.as_ptr().cast_const() }
    |                  ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ dereferencing pointer failed: 0x24c78[noalloc] is a dangling pointer (it has no provenance)
    |
    = help: this indicates a bug in the program: it performed an invalid operation, and caused Undefined Behavior

when we tried to backtrace the issue

// ${TOOLCHAIN}/lib/rustlib/src/rust/library/alloc/src/sync.rs:1255:18: 1255:35
#[inline]
    fn inner(&self) -> &ArcInner<T> {
        // This unsafety is ok because while this arc is alive we're guaranteed
        // that the inner pointer is valid. Furthermore, we know that the
        // `ArcInner` structure itself is `Sync` because the inner data is
        // `Sync` as well, so we're ok loaning out an immutable pointer to these
        // contents.
        unsafe { self.ptr.as_ref() }
    }
// ...
// ${TOOLCLAIN}/lib/rustlib/src/rust/library/core/src/ptr/non_null.rs:376:18: 376:33
    #[stable(feature = "nonnull", since = "1.25.0")]
    #[rustc_const_unstable(feature = "const_ptr_as_ref", issue = "91822")]
    #[must_use]
    #[inline(always)]
    pub const unsafe fn as_ref<'a>(&self) -> &'a T {
        // SAFETY: the caller must guarantee that `self` meets all the
        // requirements for a reference.
        unsafe { &*self.as_ptr() }
    }
diwic commented 1 year ago

Hmm. I can't exactly tell what's wrong here. Could you elaborate on what the actual issue is? Or is this possibly a miri false positive, that it warns for something that is actually okay?