rust-vs-zig icon indicating copy to clipboard operation
rust-vs-zig copied to clipboard

Maybe unsound in ObjClosure

Open lwz23 opened this issue 11 months ago • 3 comments

Hello, thank you for your contribution in this projcet, I'm scanning unsoundness problem in rust project and I notice the following code.

pub struct ObjClosure {
    pub obj: Obj,
    pub function: Gc<ObjFunction>,
    pub upvalues: NonNull<*mut ObjUpvalue>,
    pub upvalue_count: u8,
}
impl ObjClosure{
pub fn upvalue_at_slot(&self, slot: usize) -> Option<NonNull<ObjUpvalue>> {
        if self.upvalue_count == 0 {
            return None;
        }

        unsafe {
            let upvalues =
                slice::from_raw_parts(self.upvalues.as_ptr(), self.upvalue_count as usize);
            upvalues.get(slot).and_then(|upval| NonNull::new(*upval))
        }
    }
}

I am concerned that there may be a potential unsound problem here, since upvalues is a pub field and from_raw_parts is called without any checking, I am concerned that this may cause a potential UB (eg.null pointer), So I thought maybe marking upvalue_at_slot as unsafe would be a more appropriate choice?

lwz23 avatar Dec 06 '24 07:12 lwz23

same problem for https://github.com/zackradisic/rust-vs-zig/blob/b3eec6154a3026f137ce7370b5cbe0c49d3b666b/loxide/src/obj.rs#L492

lwz23 avatar Dec 06 '24 07:12 lwz23

maybe also unsound in https://github.com/zackradisic/rust-vs-zig/blob/b3eec6154a3026f137ce7370b5cbe0c49d3b666b/loxide/src/vm.rs#L167 but this is because

pub struct VM {
    pub stack: Stack,

which may result dereference a null pointer

lwz23 avatar Dec 09 '24 16:12 lwz23

same for

pub fn offset(&mut self, offset: isize) {
        self.top = unsafe { self.top.offset(offset) };
    }

23:52:11|RAP|INFO|: 3: Public function using public struct field in unsafe operation: table::Table::free 23:52:11|RAP|INFO|: unsafe operations with struct fields: 23:52:11|RAP|INFO|: (1) Field table::Table.entries used in std::vec::Vec::<T>::from_raw_parts(_7, _8, _10), 23:52:11|RAP|INFO|: (2) Field table::Table.entries used in std::vec::Vec::<T>::from_raw_parts(_7, _8, _10),

pub struct Table { pub len: u32, pub cap: u32, pub entries: *mut Entry, }

pub fn free(table: &mut Table) { if table.entries.is_null() { return; }

    // free the buckets
    let _entries =
        unsafe { Vec::from_raw_parts(table.entries, table.len as usize, table.cap as usize) };

    table.len = 0;
    table.cap = 0;
    table.entries = null_mut();
}

23:52:11|RAP|INFO|: 4: Public function using public struct field in unsafe operation: obj::ObjClosure::upvalue_at_slot 23:52:11|RAP|INFO|: unsafe operations with struct fields: 23:52:11|RAP|INFO|: (1) Field obj::ObjClosure.upvalues used in std::slice::from_raw_parts(_8, _11),

pub struct ObjClosure { pub obj: Obj, pub function: Gc<ObjFunction>, pub upvalues: NonNull<*mut ObjUpvalue>, pub upvalue_count: u8, }

pub fn upvalue_at_slot(&self, slot: usize) -> Option<NonNull<ObjUpvalue>> {
    if self.upvalue_count == 0 {
        return None;
    }

    unsafe {
        let upvalues =
            slice::from_raw_parts(self.upvalues.as_ptr(), self.upvalue_count as usize);
        upvalues.get(slot).and_then(|upval| NonNull::new(*upval))
    }
}

23:52:11|RAP|INFO|: 5: Public function using public struct field in unsafe operation: vm::Stack::offset 23:52:11|RAP|INFO|: unsafe operations with struct fields: 23:52:11|RAP|INFO|: (1) Field vm::Stack.top used in std::ptr::mut_ptr::<impl *mut T>::offset(_4, _5),

pub struct Stack { pub stack: *mut Value, pub top: *mut Value, }

pub fn offset(&mut self, offset: isize) {
    self.top = unsafe { self.top.offset(offset) };
}

23:52:11|RAP|INFO|: 10: Public function using public struct field in unsafe operation: obj::ObjString::as_str 23:52:11|RAP|INFO|: unsafe operations with struct fields: 23:52:11|RAP|INFO|: (1) Field obj::ObjString.chars used in std::slice::from_raw_parts(_3, _6), 23:52:11|RAP|INFO|: (2) Field obj::ObjString.chars used in std::str::from_utf8_unchecked(_9), 23:52:11|RAP|INFO|:

pub struct ObjString { pub obj: Obj, pub len: u32, pub hash: ObjHash, pub chars: NonNull, }

pub fn as_str(&self) -> &str { unsafe { let bytes = slice::from_raw_parts(self.chars.as_ptr(), self.len as usize); std::str::from_utf8_unchecked(bytes) } }

lwz23 avatar Apr 19 '25 16:04 lwz23