Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Send and Sync traits for Future should be bound by T: Send #1

Closed
ammaraskar opened this issue Dec 8, 2020 · 1 comment
Closed

Send and Sync traits for Future should be bound by T: Send #1

ammaraskar opened this issue Dec 8, 2020 · 1 comment

Comments

@ammaraskar
Copy link

Hi there, we (Rust group @sslab-gatech) are scanning crates on crates.io for potential soundness bugs. We noticed that tiny_future implements Send and Sync for all types:

https://github.com/KizzyCode/tiny_future/blob/50bd80f874c851413a721bbe144eeba4dfb68b4e/src/lib.rs#L165-L166

This should probably be bound by T: Send, otherwise this allows non-Send types such as Rc to be sent across thread boundaries which might invoke undefined behavior.

Here's an example of this in action with an Rc segfaulting safe Rust code:

#![forbid(unsafe_code)]

use tiny_future::Future;
use std::{thread, rc::Rc};

fn main() {
    let rc = Rc::new(());
    let rc_clone = rc.clone();

    let f = Future::with_state(());
    f.set(rc_clone);

    thread::spawn(move || {
        let smuggled_rc = f.get().unwrap();

        println!("Thread: {:p}", smuggled_rc);
        // Race the refcount with the main thread.
        for _ in 0..100_000_000 {
            smuggled_rc.clone();
        }
    });

    println!("Main:   {:p}", rc);
    for _ in 0..100_000_000 {
        rc.clone();
    }
}

Output:

Main:   0x5580ab8c1b50
Thread: 0x5580ab8c1b50

Terminated with signal 4 (SIGILL)
@KizzyCode
Copy link
Owner

Oh damn, you're right; I'll fix this 😅

Thank you very much for catching and reporting this bug; you even made a proof of concept – thats pretty cool!

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

No branches or pull requests

2 participants