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

ReadTicket and WriteTicket should only be sendable when T is Send #7

Closed
ammaraskar opened this issue Nov 17, 2020 · 0 comments · Fixed by #8
Closed

ReadTicket and WriteTicket should only be sendable when T is Send #7

ammaraskar opened this issue Nov 17, 2020 · 0 comments · Fixed by #8
Labels

Comments

@ammaraskar
Copy link

Hi there, we (Rust group @sslab-gatech) are scanning crates on crates.io for potential soundness bugs. We noticed that ReadTicket

unsafe impl<T> Send for ReadTicket<T> {}

and WriteTicket

unsafe impl<T> Send for WriteTicket<T> {}

implement Send for all types T. However, this should probably be bounded by T: Send, otherwise it allows smuggling non-Send types across thread boundaries. Here's an example of a data race with Rcs that segfaults safe Rust code:

#![forbid(unsafe_code)]

use ticketed_lock::TicketedLock;

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

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

    let read_ticket = lock.read();
    thread::spawn(move || {
        let smuggled_rc = read_ticket.wait().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();
    }
}

This outputs:

Main:   0x55998cf48a50
Thread: 0x55998cf48a50

Return Code: -4 (SIGILL)
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants