Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

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

Open
JOE1994 opened this issue Dec 26, 2020 · 2 comments

Comments

@JOE1994
Copy link

JOE1994 commented Dec 26, 2020

Hello 🦀 ,

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: abbychau#10

Thank you for checking out this issue 👍

@JOE1994
Copy link
Author

JOE1994 commented Feb 4, 2021

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
Copy link

Shnatsel commented Feb 4, 2021

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 join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants