schets / multiqueue

A fast mpmc queue with broadcast capabilities
MIT License
201 stars 29 forks source link

Queues allow non-Send types to be sent to other threads, allowing data races #31

Open JOE1994 opened 3 years ago

JOE1994 commented 3 years ago

Hello :crab: ,

I recently submitted a bug report to the multiqueue2 crate which is maintained from a fork of this crate. The bug was fixed a few days ago in version 0.1.7 of the mulltiqueue2 crate. The exact same bug exists for the multiqueue crate as well. FYI, I'll leave a link to the bug report that I submitted for the multiqueue2 crate: https://github.com/abbychau/multiqueue2/issues/10

Thank you for checking out this issue :+1:

JOE1994 commented 3 years ago

FYI, here is a working proof of concept that segfaults at runtime. Below program uses multiqueue = "0.3.2"

#![forbid(unsafe_code)]
use std::cell::Cell;
use std::sync::Arc;
use std::thread;
// futures = "0.1.27"
use futures::{Future, Sink, Stream};

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

use multiqueue::mpmc_fut_queue;

fn main() {
    let (tx, rx) = mpmc_fut_queue(16);
    let cell = Arc::new(Cell::new(RefOrInt::Int(0xdeadbeef)));
    let sent = tx.send(Arc::clone(&cell));

    thread::spawn(move || {
        let mut rx = rx.wait();

        // parent thread sent us an object that is not `Send`!
        let smuggled_cell = rx.next().unwrap().unwrap();

        loop {
            smuggled_cell.set(RefOrInt::Int(0xdeadbeef));
            smuggled_cell.set(RefOrInt::Ref(&X))
        }
    });
    sent.wait().unwrap();

    loop {
        if let RefOrInt::Ref(addr) = cell.get() {
            if addr as *const _ as usize != 0xdeadbeef {
                continue;
            }
            // Due to the data race, obtaining Ref(0xdeadbeef) is possible
            println!("Pointer is now: {:p}", addr);
            println!("Dereferencing addr will now segfault: {}", *addr);
        }
    }
}
Shnatsel commented 3 years ago

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.