-
-
Notifications
You must be signed in to change notification settings - Fork 40
Open
Description
Hello, thank you for your contribution in this project, I an testing our static analysis tool in github's Rust project and I notice the following code:
Line 425 in 94e6038
| fn exec(&self, args_ptr: usize, args_len: usize) { |
fn exec(&self, args_ptr: usize, args_len: usize) {
let page_table = unsafe { sys::process::page_table() };
let mut mapper = unsafe {
OffsetPageTable::new(page_table, VirtAddr::new(phys_mem_offset()))
};
// Copy args to user memory
let args_addr = self.code_addr + (self.stack_addr - self.code_addr) / 2;
sys::mem::alloc_pages(&mut mapper, args_addr, 1).
expect("proc args alloc");
let args: &[&str] = unsafe {
let ptr = ptr_from_addr(args_ptr as u64) as usize;
core::slice::from_raw_parts(ptr as *const &str, args_len)
};
.......................
}
I think there is a unsound problem because this function doesn't varify the args_ptr is valid and pass it to unsafe function form_raw_parts. It will trigger UB. Although it is a private function, I notice a possible way to call this function from a pub function spawn.
1. pub fn spawn -> fn exec
pub fn spawn -> fn exec
pub fn spawn(bin: &[u8], args_ptr: usize, args_len: usize) -> Result<(), ExitCode> {
if let Ok(id) = Self::create(bin) {
let proc = {
let table = PROCESS_TABLE.read();
table[id].clone()
};
proc.exec(args_ptr, args_len);
Ok(())
} else {
Err(ExitCode::ExecError)
}
}
fn exec(&self, args_ptr: usize, args_len: usize) {
let page_table = unsafe { sys::process::page_table() };
let phys_mem_offset = unsafe { sys::mem::PHYS_MEM_OFFSET.unwrap() };
let mut mapper = unsafe {
OffsetPageTable::new(page_table, VirtAddr::new(phys_mem_offset))
};
let args_addr = self.code_addr + (self.stack_addr - self.code_addr) / 2;
sys::mem::alloc_pages(&mut mapper, args_addr, 1).expect("proc args alloc");
let args: &[&str] = unsafe {
let ptr = ptr_from_addr(args_ptr as u64) as usize;
core::slice::from_raw_parts(ptr as *const &str, args_len)
};
..................
}
I think a appricate fix would be valid the pointer before pass it to unsafe function. Alternatively, the exec function could be marked as unsafe and add proper doc to warn the caller.
Reactions are currently unavailable
Metadata
Metadata
Assignees
Labels
No labels