From ca45f8f15279788ea0af9a90e568d244658c16c3 Mon Sep 17 00:00:00 2001 From: WangRunji Date: Fri, 22 Feb 2019 16:30:56 +0800 Subject: [PATCH] fix check user ptr in syscalls --- crate/memory/src/memory_set/mod.rs | 72 ++++++++++++++++++++++++++---- kernel/src/syscall/fs.rs | 33 +++++++++----- 2 files changed, 84 insertions(+), 21 deletions(-) diff --git a/crate/memory/src/memory_set/mod.rs b/crate/memory/src/memory_set/mod.rs index b311aea..eb5b910 100644 --- a/crate/memory/src/memory_set/mod.rs +++ b/crate/memory/src/memory_set/mod.rs @@ -1,10 +1,13 @@ //! memory set, area //! and the inactive page table -use alloc::{vec::Vec, boxed::Box}; +use alloc::{boxed::Box, vec::Vec, string::String}; use core::fmt::{Debug, Error, Formatter}; -use super::*; + use crate::paging::*; + +use super::*; + use self::handler::MemoryHandler; pub mod handler; @@ -42,6 +45,40 @@ impl MemoryArea { pub fn contains(&self, addr: VirtAddr) -> bool { addr >= self.start_addr && addr < self.end_addr } + /// Check the pointer is within the readable memory + pub fn check_ptr(&self, ptr: *const T) -> bool { + self.check_array(ptr, 1) + } + /// Check the pointer is within the writable memory + pub fn check_mut_ptr(&self, ptr: *mut T) -> bool { + self.check_mut_array(ptr, 1) + } + /// Check the array is within the readable memory + pub fn check_array(&self, ptr: *const S, count: usize) -> bool { + // FIXME: check readable + ptr as usize >= self.start_addr && + unsafe { ptr.offset(count as isize) as usize } <= self.end_addr + } + /// Check the array is within the writable memory + pub fn check_mut_array(&self, ptr: *mut S, count: usize) -> bool { + // FIXME: check writable + ptr as usize >= self.start_addr && + unsafe { ptr.offset(count as isize) as usize } <= self.end_addr + } + /// Check the null-end C string is within the readable memory, and is valid. + /// If so, clone it to a String. + /// + /// Unsafe: the page table must be active. + pub unsafe fn check_and_clone_cstr(&self, ptr: *const u8) -> Option { + if ptr as usize >= self.end_addr { + return None; + } + let max_len = self.end_addr - ptr as usize; + (0..max_len) + .find(|&i| ptr.offset(i as isize).read() == 0) + .and_then(|len| core::str::from_utf8(core::slice::from_raw_parts(ptr, len)).ok()) + .map(|s| String::from(s)) + } /* ** @brief test whether the memory area is overlap with another memory area ** @param other: &MemoryArea another memory area to test @@ -158,13 +195,30 @@ impl MemorySet { page_table: T::new_bare(), } } - /* - ** @brief find the memory area from virtual address - ** @param addr: VirtAddr the virtual address to find - ** @retval Option<&MemoryArea> the memory area with the virtual address, if presented - */ - pub fn find_area(&self, addr: VirtAddr) -> Option<&MemoryArea> { - self.areas.iter().find(|area| area.contains(addr)) + /// Check the pointer is within the readable memory + pub fn check_ptr(&self, ptr: *const S) -> bool { + self.areas.iter().find(|area| area.check_ptr(ptr)).is_some() + } + /// Check the pointer is within the writable memory + pub fn check_mut_ptr(&self, ptr: *mut S) -> bool { + self.areas.iter().find(|area| area.check_mut_ptr(ptr)).is_some() + } + /// Check the array is within the readable memory + pub fn check_array(&self, ptr: *const S, count: usize) -> bool { + self.areas.iter().find(|area| area.check_array(ptr, count)).is_some() + } + /// Check the array is within the writable memory + pub fn check_mut_array(&self, ptr: *mut S, count: usize) -> bool { + self.areas.iter().find(|area| area.check_mut_array(ptr, count)).is_some() + } + /// Check the null-end C string is within the readable memory, and is valid. + /// If so, clone it to a String. + /// + /// Unsafe: the page table must be active. + pub unsafe fn check_and_clone_cstr(&self, ptr: *const u8) -> Option { + self.areas.iter() + .filter_map(|area| area.check_and_clone_cstr(ptr)) + .next() } /* ** @brief add the memory area to the memory set diff --git a/kernel/src/syscall/fs.rs b/kernel/src/syscall/fs.rs index bf9450a..6c8f1a1 100644 --- a/kernel/src/syscall/fs.rs +++ b/kernel/src/syscall/fs.rs @@ -3,39 +3,44 @@ use super::*; pub fn sys_read(fd: usize, base: *mut u8, len: usize) -> SysResult { - // TODO: check ptr info!("read: fd: {}, base: {:?}, len: {:#x}", fd, base, len); - let slice = unsafe { slice::from_raw_parts_mut(base, len) }; let mut proc = process(); + if !proc.memory_set.check_mut_array(base, len) { + return Err(SysError::Inval); + } + let slice = unsafe { slice::from_raw_parts_mut(base, len) }; let len = get_file(&mut proc, fd)?.read(slice)?; Ok(len as isize) } pub fn sys_write(fd: usize, base: *const u8, len: usize) -> SysResult { - // TODO: check ptr info!("write: fd: {}, base: {:?}, len: {:#x}", fd, base, len); - let slice = unsafe { slice::from_raw_parts(base, len) }; let mut proc = process(); + if !proc.memory_set.check_array(base, len) { + return Err(SysError::Inval); + } + let slice = unsafe { slice::from_raw_parts(base, len) }; let len = get_file(&mut proc, fd)?.write(slice)?; Ok(len as isize) } pub fn sys_open(path: *const u8, flags: usize) -> SysResult { - // TODO: check ptr - let path = unsafe { util::from_cstr(path) }; + let mut proc = process(); + let path = unsafe { proc.memory_set.check_and_clone_cstr(path) } + .ok_or(SysError::Inval)?; let flags = VfsFlags::from_ucore_flags(flags); info!("open: path: {:?}, flags: {:?}", path, flags); - let (fd, inode) = match path { + let (fd, inode) = match path.as_str() { "stdin:" => (0, crate::fs::STDIN.clone() as Arc), "stdout:" => (1, crate::fs::STDOUT.clone() as Arc), _ => { - let fd = (3..).find(|i| !process().files.contains_key(i)).unwrap(); - let inode = crate::fs::ROOT_INODE.lookup(path)?; + let fd = (3..).find(|i| !proc.files.contains_key(i)).unwrap(); + let inode = crate::fs::ROOT_INODE.lookup(path.as_str())?; (fd, inode) } }; let file = File::new(inode, flags.contains(VfsFlags::READABLE), flags.contains(VfsFlags::WRITABLE)); - process().files.insert(fd, file); + proc.files.insert(fd, file); Ok(fd as isize) } @@ -48,9 +53,11 @@ pub fn sys_close(fd: usize) -> SysResult { } pub fn sys_fstat(fd: usize, stat_ptr: *mut Stat) -> SysResult { - // TODO: check ptr info!("fstat: {}", fd); let mut proc = process(); + if !proc.memory_set.check_mut_ptr(stat_ptr) { + return Err(SysError::Inval); + } let file = get_file(&mut proc, fd)?; let stat = Stat::from(file.info()?); unsafe { stat_ptr.write(stat); } @@ -61,9 +68,11 @@ pub fn sys_fstat(fd: usize, stat_ptr: *mut Stat) -> SysResult { /// dentry.name = entry_name /// dentry.offset += 256 pub fn sys_getdirentry(fd: usize, dentry_ptr: *mut DirEntry) -> SysResult { - // TODO: check ptr info!("getdirentry: {}", fd); let mut proc = process(); + if !proc.memory_set.check_mut_ptr(dentry_ptr) { + return Err(SysError::Inval); + } let file = get_file(&mut proc, fd)?; let dentry = unsafe { &mut *dentry_ptr }; if !dentry.check() {