fix check user ptr in syscalls

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<T>(&self, ptr: *const T) -> bool {
+        self.check_array(ptr, 1)
+    }
+    /// Check the pointer is within the writable memory
+    pub fn check_mut_ptr<T>(&self, ptr: *mut T) -> bool {
+        self.check_mut_array(ptr, 1)
+    }
+    /// Check the array is within the readable memory
+    pub fn check_array<S>(&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<S>(&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<String> {
+        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<T: InactivePageTable> MemorySet<T> {
             page_table: T::new_bare(),
         }
     }
-    /*
+    /// Check the pointer is within the readable memory
-    **  @brief  find the memory area from virtual address
+    pub fn check_ptr<S>(&self, ptr: *const S) -> bool {
-    **  @param  addr: VirtAddr       the virtual address to find
+        self.areas.iter().find(|area| area.check_ptr(ptr)).is_some()
-    **  @retval Option<&MemoryArea>  the memory area with the virtual address, if presented
+    }
-    */
+    /// Check the pointer is within the writable memory
-    pub fn find_area(&self, addr: VirtAddr) -> Option<&MemoryArea> {
+    pub fn check_mut_ptr<S>(&self, ptr: *mut S) -> bool {
-        self.areas.iter().find(|area| area.contains(addr))
+        self.areas.iter().find(|area| area.check_mut_ptr(ptr)).is_some()
+    }
+    /// Check the array is within the readable memory
+    pub fn check_array<S>(&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<S>(&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<String> {
+        self.areas.iter()
+            .filter_map(|area| area.check_and_clone_cstr(ptr))
+            .next()
     }
     /*
     **  @brief  add the memory area to the memory set

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 mut proc = process();
-    let path = unsafe { util::from_cstr(path) };
+    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<INode>),
         "stdout:" => (1, crate::fs::STDOUT.clone() as Arc<INode>),
         _ => {
-            let fd = (3..).find(|i| !process().files.contains_key(i)).unwrap();
+            let fd = (3..).find(|i| !proc.files.contains_key(i)).unwrap();
-            let inode = crate::fs::ROOT_INODE.lookup(path)?;
+            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() {