spacejam / rio

pure rust io_uring library, built on libc, thread & async friendly, misuse resistant
936 stars 45 forks source link

AsIoVecMut is unsound #12

Open goffrie opened 4 years ago

goffrie commented 4 years ago

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 commented 4 years ago

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 commented 4 years ago

@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 commented 4 years ago

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.