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

AsIoVecMut is unsound #12

Open
goffrie opened this issue Feb 7, 2020 · 3 comments
Open

AsIoVecMut is unsound #12

goffrie opened this issue Feb 7, 2020 · 3 comments

Comments

@goffrie
Copy link

goffrie commented Feb 7, 2020

Well, not the trait itself, but the bound isn't enough to stop various read methods from violating aliasing rules. Example:

fn main() {
    let data: Vec<u8> = vec![0; 1024];
    crossbeam_utils::thread::scope(|s| {
        let ring = rio::new().expect("create uring");
        let file = std::fs::File::open("file").expect("openat");
        let _c = ring.read_at(&file, &data, 0);
        s.spawn(|_| immut(&data));
    }).unwrap();
}

fn immut(data: &[u8]) {
    loop {
        println!("{:?}", data[0]);
        if data[0] != 0 { break; }
        std::thread::sleep(std::time::Duration::from_millis(1));
    }
}

If you mkfifo fifo and run this, you'll see that once data is written into the fifo, immut will observe a change in data even though it's purported to be behind a shared reference.

@Licenser
Copy link
Contributor

Licenser commented Feb 8, 2020

This sounds very much like #1, I wonder if a solution to this would be for read_* to take ownership of the buffer and return it as part of the completion? That would prevent 'misuse' of the buffer but I might miss problems that would result from that

@spacejam
Copy link
Owner

spacejam commented Feb 8, 2020

@stjepang and I were talking about this this afternoon, and he was also mentioning passing an owned buffer as well. A key part of io_uring is being able to chain multiple operations together that execute sequentially or in parallel on the same buffers though. I'd like to find a nice interface that either relies on an UnsafeCell somewhere, or maybe only allowing these sorts of chained operations on registered buffers that are behind an UnsafeCell and for every other type of buffer only accepting a mutable reference, and not allow concurrent chaining.

@goffrie
Copy link
Author

goffrie commented Mar 13, 2020

Perhaps you could require AsRef<[Cell<u8>]> instead. A &mut [u8] could still be converted beforehand with Cell::from_mut(buf).as_slice_of_cells(). This might be too hard to use though.

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

3 participants