SyncChannel<T> should have a bound on T: Send

[ammaraskar]

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

signal-rust/src/channel.rs

Lines 58 to 59 in 2d671bd

	unsafe impl<T> Send for SyncChannel<T> {}
	unsafe impl<T> Sync for SyncChannel<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 signal_simple::channel::SyncChannel;

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 channel = SyncChannel::new();
    channel.send(&cell);

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

which outputs:

Pointer is now: 0xdeadbeef

Return Code: -11 (SIGSEGV)

[Shnatsel]

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.