Skip to content
This repository has been archived by the owner on May 10, 2022. It is now read-only.

Queue<T> should have a Send bound on its Send/Sync traits #1

Open
ammaraskar opened this issue Nov 15, 2020 · 1 comment
Open

Queue<T> should have a Send bound on its Send/Sync traits #1

ammaraskar opened this issue Nov 15, 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 the Queue object implements Send and Sync unconiditionally:

unsafe impl<T> Sync for Queue<T> {}
unsafe impl<T> Send for Queue<T> {}

However, this should probably be bounded by T: Send in both, otherwise it allows sending types that should never be sent across threads such as Rc or references to cells. You can see an example of such a data-race with cells below:

#![forbid(unsafe_code)]

use scottqueue::tlqueue::Queue;

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

// A simple tagged union used to demonstrate problems with data races in Cell.
#[derive(Debug, Clone, Copy)]
enum RefOrInt { Ref(&'static u64), Int(u64) }
static SOME_INT: u64 = 123;

fn main() {
    let cell = Cell::new(RefOrInt::Ref(&SOME_INT));

    let queue = Queue::new();
    queue.push(&cell);

    thread::scope(|s| {    
        s.spawn(|_| {
            let smuggled_cell = queue.pop().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; }

                println!("Pointer is now: {:p}", addr);
                println!("Dereferencing addr will now segfault: {}", *addr);
            }
        }
    });
}

This outputs:

Pointer is now: 0xdeadbeef

Return Code: -11 (SIGSEGV)
@Shnatsel
Copy link

Heads up: this issue has been included in the RustSec advisory database. It will be surfaced by tools such as cargo-audit or cargo-deny from now on.

Once a fix is released to crates.io, please open a pull request to update the advisory with the patched version, or file an issue on the advisory database repository.

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

No branches or pull requests

2 participants