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

Unsound implementation of From leads to dangling pointer #12

Open
shinmao opened this issue Aug 22, 2023 · 1 comment
Open

Unsound implementation of From leads to dangling pointer #12

shinmao opened this issue Aug 22, 2023 · 1 comment

Comments

@shinmao
Copy link

shinmao commented Aug 22, 2023

The source of unsoundness

reffers-rs/src/rmba.rs

Lines 167 to 174 in 5984523

impl<'a, T: 'a + ?Sized> From<Arc<T>> for RMBA<'a, T> {
fn from(t: Arc<T>) -> RMBA<'a, T> {
let z = RMBA::make(&*t, 2);
// debug_assert_eq!(unsafe { Self::arc(z.unpack().0) }, &t as *const Arc<T>);
mem::forget(t);
z
}
}

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
Copy link
Owner

diwic commented Aug 23, 2023

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?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants