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

IoBuf::get_mut_range unsoundly extends arbitrary lifetime to 'static #1442

Open
vikramnitin9 opened this issue Mar 15, 2023 · 2 comments
Open

Comments

@vikramnitin9
Copy link

This method takes &self and returns a 'static bounded reference.

pub(crate) fn get_mut_range(
&self,
at: usize,
len: usize,
) -> &'static mut [u8] {
let buf_ptr = self.buf.get();
unsafe {
assert!((*buf_ptr).1 >= at + len);
std::slice::from_raw_parts_mut(
(*buf_ptr).0.add(self.base + at),
len,
)
}
}

This can cause a use-after-free. The following example fails Miri:

use std::{
    alloc::{alloc, dealloc, Layout},
    sync::Arc,
    cell::UnsafeCell
};

struct AlignedBuf(*mut u8, usize);

impl AlignedBuf {
    fn new(len: usize) -> AlignedBuf {
        let layout = Layout::from_size_align(len, 8192).unwrap();
        let ptr = unsafe { alloc(layout) };

        assert!(!ptr.is_null(), "failed to allocate critical IO buffer");

        AlignedBuf(ptr, len)
    }
}

impl Drop for AlignedBuf {
    fn drop(&mut self) {
        let layout = Layout::from_size_align(self.1, 8192).unwrap();
        unsafe {
            dealloc(self.0, layout);
        }
    }
}

pub(crate) struct IoBuf {
    buf: Arc<UnsafeCell<AlignedBuf>>,
    base: usize,
}

impl IoBuf {
    pub(crate) fn get_mut_range(
        &self,
        at: usize,
        len: usize,
    ) -> &'static mut [u8] {
        let buf_ptr = self.buf.get();

        unsafe {
            assert!((*buf_ptr).1 >= at + len);
            std::slice::from_raw_parts_mut(
                (*buf_ptr).0.add(self.base + at),
                len,
            )
        }
    }

}

fn main() {
    let mut mut_range;
    {
        let iobuf = IoBuf {
            buf: Arc::new(UnsafeCell::new(AlignedBuf::new(16))),
            base: 0
        };

        mut_range = iobuf.get_mut_range(4, 2);
        mut_range[0] = 8;
        mut_range[1] = 9;
    }
    // Use after free
    println!("hello {:?} :)", mut_range);
}

A tangential issue about this same function had been discussed in #1044, but I think that the function as it stands is still unsafe and should be marked as such.

@Wicpar
Copy link

Wicpar commented Nov 12, 2023

i have a plan to rewrite alignedBuf to mostly safe code in #1487
it would completely solve this issue. I'll try to make a PR soon.

@Wicpar
Copy link

Wicpar commented Nov 12, 2023

fixed in #1488

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