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

LateStatic #1

Closed
ammaraskar opened this issue Nov 10, 2020 · 2 comments · Fixed by #2
Closed

LateStatic #1

ammaraskar opened this issue Nov 10, 2020 · 2 comments · Fixed by #2

Comments

@ammaraskar
Copy link

ammaraskar commented Nov 10, 2020

Currently LateStatic implements Send and Sync unconditionally for all types.

late-static/src/lib.rs

Lines 29 to 30 in 3b72ba5

unsafe impl<T: Send> core::marker::Send for LateStatic<T> {}
unsafe impl<T: Send> core::marker::Sync for LateStatic<T> {}

This should probably only be for Sync types (this is the same limitation that Rust applies to static types) otherwise there is a soundness issue for multithreaded programs . It is possible to wrap a type that isn't safe to use across threads like Cell in a LateStatic and then cause data races like so:

use late_static::LateStatic;

use std::cell::Cell;
use std::thread;

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

static STATIC_CELL: LateStatic<Cell<RefOrInt>> = LateStatic::new();

fn main() {
    unsafe {
        LateStatic::assign(&STATIC_CELL, Cell::new(RefOrInt::Ref(&SOME_INT)));
    }

    thread::spawn(move || {
        loop {
            // Repeatedly write Ref(&addr) and Int(0xdeadbeef) into the cell.
            STATIC_CELL.set(RefOrInt::Ref(&SOME_INT));
            STATIC_CELL.set(RefOrInt::Int(0xdeadbeef));
        }
    });

    loop {
        if let RefOrInt::Ref(addr) = STATIC_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);
        }
    }
}

This outputs:

Pointer is now: 0xdeadbeef

Return Code: -11 (SIGSEGV)

(Issue found by @sslab-gatech's Rust group)

@Richard-W
Copy link
Owner

Thank you for the thorough report!

On first glance I think the line

unsafe impl<T: Send> core::marker::Sync for LateStatic<T> {}

may just be a typo and it should be

unsafe impl<T: Sync> core::marker::Sync for LateStatic<T> {}

I will verify and fix it (hopefully later today).

@ammaraskar
Copy link
Author

Thank you for the quick response, the fix looks good!

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

Successfully merging a pull request may close this issue.

2 participants