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

TreeFocus lacks bounds on its Send and Sync traits #157

Closed
ammaraskar opened this issue Nov 9, 2020 · 0 comments
Closed

TreeFocus lacks bounds on its Send and Sync traits #157

ammaraskar opened this issue Nov 9, 2020 · 0 comments

Comments

@ammaraskar
Copy link
Contributor

It looks like the version of Focus for RRB-tree backed vectors, TreeFocus lacks bounds for Sync and Send:

im-rs/src/vector/focus.rs

Lines 280 to 285 in 22f6907

#[allow(unsafe_code)]
#[cfg(threadsafe)]
unsafe impl<A> Send for TreeFocus<A> {}
#[allow(unsafe_code)]
#[cfg(threadsafe)]
unsafe impl<A> Sync for TreeFocus<A> {}

I realize TreeFocus is an implementation detail and not really meant to be used by end-users of the library but I think this opens up the possibility of causing data-races through safe Rust code like this:

#![forbid(unsafe_code)]

use im::vector;
use im::vector::{Vector, Focus};

use std::{iter, iter::FromIterator, cell::Cell};

use crossbeam_utils::thread;

#[derive(Debug, Clone, Copy)]
enum RefOrInt<'a> {
    Ref(&'a u64),
    Int(u64),
}
static SOME_INT: u64 = 123;

fn main() {
    let cell = Cell::new(RefOrInt::Ref(&SOME_INT));
    // Make the Vector big enough so that it gets promoted to a RRB tree.
    let mut vec: Vector<&Cell<RefOrInt>> = Vector::from_iter(
        iter::repeat(&cell).take(1024*5));

    let focus = vec.focus();
    if let Focus::Full(tree_focus) = focus {
        thread::scope(|s| {
            s.spawn(move |_| {
                let mut sent_focus = tree_focus;

                let smuggled_cell = sent_focus.get(0).unwrap();
                loop {
                    // Repeatedly write Ref(&addr) and Int(0xdeadbeef) into the cell.
                    smuggled_cell.set(RefOrInt::Ref(&SOME_INT));
                    smuggled_cell.set(RefOrInt::Int(0xdeadbeef));
                }
            });

            loop {
                if let RefOrInt::Ref(addr) = cell.get() {
                    // Hope that between the time we pattern match the object as a
                    // `Ref`, it gets written to by the other thread.
                    if addr as *const u64 == &SOME_INT as *const u64 {
                        continue;
                    }
    
                    // Due to the data race, obtaining Ref(0xdeadbeef) is possible
                    println!("Pointer is now: {:p}", addr);
                    println!("Dereferencing addr will now segfault: {}", *addr);
                }
            }
        });
    }
}

which outputs

Pointer is now: 0xdeadbeef

Return Code: -11 (SIGSEGV)

(Issue found by the Rust group at @sslab-gatech)

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