Use new check_and_clone_cstr{,array}

crate/memory/src/memory_set/mod.rs

@@ -49,20 +49,6 @@ impl MemoryArea {
             self.check_read_array(ptr, count)
         }
     }
-    /// 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))
-    }
     /// Test whether this area is (page) overlap with area [`start_addr`, `end_addr`]
     pub fn is_overlap_with(&self, start_addr: VirtAddr, end_addr: VirtAddr) -> bool {
         let p0 = Page::of_addr(self.start_addr);
@@ -188,35 +174,6 @@ impl<T: PageTableExt> MemorySet<T> {
         }
         Err(VMError::InvalidPtr)
     }
-    /// Check the null-end C string pointer array
-    /// Used for getting argv & envp
-    pub unsafe fn check_and_clone_cstr_array(
-        &self,
-        mut argv: *const *const u8,
-    ) -> VMResult<Vec<String>> {
-        let mut args = Vec::new();
-        loop {
-            let cstr = *self.check_read_ptr(argv)?;
-            if cstr.is_null() {
-                break;
-            }
-            let arg = self.check_and_clone_cstr(cstr)?;
-            args.push(arg);
-            argv = argv.add(1);
-        }
-        Ok(args)
-    }
-    /// 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) -> VMResult<String> {
-        self.areas
-            .iter()
-            .filter_map(|area| area.check_and_clone_cstr(ptr))
-            .next()
-            .ok_or(VMError::InvalidPtr)
-    }
     /// Find a free area with hint address `addr_hint` and length `len`.
     /// Return the start address of found free area.
     /// Used for mmap.

kernel/src/arch/mipsel/interrupt.rs

@@ -283,6 +283,15 @@ fn page_fault(tf: &mut TrapFrame) {
         }
         Err(()) => {
             if !crate::memory::handle_page_fault(addr) {
+                extern "C" {
+                    fn _copy_user_start();
+                    fn _copy_user_end();
+                }
+                if tf.epc >= _copy_user_start as usize && tf.epc < _copy_user_end as usize {
+                    debug!("fixup for addr {:x?}", addr);
+                    tf.epc = crate::memory::read_user_fixup as usize;
+                    return;
+                }
                 crate::trap::error(tf);
             }
         }

kernel/src/memory.rs

@@ -21,6 +21,7 @@ use alloc::boxed::Box;
 use bitmap_allocator::BitAlloc;
 use buddy_system_allocator::Heap;
 use core::mem;
+use core::mem::size_of;
 use lazy_static::*;
 use log::*;
 pub use rcore_memory::memory_set::{handler::*, MemoryArea, MemoryAttr};
@@ -196,7 +197,7 @@ pub fn copy_from_user_u8(addr: *const u8) -> Option<u8> {
         dst.copy_from_nonoverlapping(src, 1);
         0
     }
-    if !access_ok(addr as usize, 1) {
+    if !access_ok(addr as usize, size_of::<u8>()) {
         return None;
     }
     let mut dst: u8 = 0;
@@ -205,3 +206,22 @@ pub fn copy_from_user_u8(addr: *const u8) -> Option<u8> {
         _ => None,
     }
 }
+
+#[no_mangle]
+pub fn copy_from_user_usize(addr: *const usize) -> Option<usize> {
+    #[naked]
+    #[inline(never)]
+    #[link_section = ".text.copy_user"]
+    unsafe extern "C" fn read_user_usize(dst: *mut usize, src: *const usize) -> usize {
+        dst.copy_from_nonoverlapping(src, 1);
+        0
+    }
+    if !access_ok(addr as usize, size_of::<usize>()) {
+        return None;
+    }
+    let mut dst: usize = 0;
+    match unsafe { read_user_usize((&mut dst) as *mut usize, addr) } {
+        0 => Some(dst),
+        _ => None,
+    }
+}

kernel/src/syscall/fs.rs

@@ -272,7 +272,7 @@ impl Syscall<'_> {
         mode: usize,
     ) -> SysResult {
         let mut proc = self.process();
-        let path = unsafe { self.vm().check_and_clone_cstr(path)? };
+        let path = unsafe { check_and_clone_cstr(path)? };
         let flags = OpenFlags::from_bits_truncate(flags);
         info!(
             "openat: dir_fd: {}, path: {:?}, flags: {:?}, mode: {:#o}",
@@ -336,7 +336,7 @@ impl Syscall<'_> {
     ) -> SysResult {
         // TODO: check permissions based on uid/gid
         let proc = self.process();
-        let path = unsafe { self.vm().check_and_clone_cstr(path)? };
+        let path = unsafe { check_and_clone_cstr(path)? };
         let flags = AtFlags::from_bits_truncate(flags);
         if !proc.pid.is_init() {
             // we trust pid 0 process
@@ -386,7 +386,7 @@ impl Syscall<'_> {
         flags: usize,
     ) -> SysResult {
         let proc = self.process();
-        let path = unsafe { self.vm().check_and_clone_cstr(path)? };
+        let path = unsafe { check_and_clone_cstr(path)? };
         let stat_ref = unsafe { self.vm().check_write_ptr(stat_ptr)? };
         let flags = AtFlags::from_bits_truncate(flags);
         info!(
@@ -417,7 +417,7 @@ impl Syscall<'_> {
         len: usize,
     ) -> SysResult {
         let proc = self.process();
-        let path = unsafe { self.vm().check_and_clone_cstr(path)? };
+        let path = unsafe { check_and_clone_cstr(path)? };
         let slice = unsafe { self.vm().check_write_array(base, len)? };
         info!(
             "readlinkat: dirfd: {}, path: {:?}, base: {:?}, len: {}",
@@ -463,7 +463,7 @@ impl Syscall<'_> {
 
     pub fn sys_truncate(&mut self, path: *const u8, len: usize) -> SysResult {
         let proc = self.process();
-        let path = unsafe { self.vm().check_and_clone_cstr(path)? };
+        let path = unsafe { check_and_clone_cstr(path)? };
         info!("truncate: path: {:?}, len: {}", path, len);
         proc.lookup_inode(&path)?.resize(len)?;
         Ok(0)
@@ -537,7 +537,7 @@ impl Syscall<'_> {
 
     pub fn sys_chdir(&mut self, path: *const u8) -> SysResult {
         let mut proc = self.process();
-        let path = unsafe { self.vm().check_and_clone_cstr(path)? };
+        let path = unsafe { check_and_clone_cstr(path)? };
         if !proc.pid.is_init() {
             // we trust pid 0 process
             info!("chdir: path: {:?}", path);
@@ -590,8 +590,8 @@ impl Syscall<'_> {
         newpath: *const u8,
     ) -> SysResult {
         let proc = self.process();
-        let oldpath = unsafe { self.vm().check_and_clone_cstr(oldpath)? };
-        let newpath = unsafe { self.vm().check_and_clone_cstr(newpath)? };
+        let oldpath = unsafe { check_and_clone_cstr(oldpath)? };
+        let newpath = unsafe { check_and_clone_cstr(newpath)? };
         info!(
             "renameat: olddirfd: {}, oldpath: {:?}, newdirfd: {}, newpath: {:?}",
             olddirfd as isize, oldpath, newdirfd as isize, newpath
@@ -611,7 +611,7 @@ impl Syscall<'_> {
 
     pub fn sys_mkdirat(&mut self, dirfd: usize, path: *const u8, mode: usize) -> SysResult {
         let proc = self.process();
-        let path = unsafe { self.vm().check_and_clone_cstr(path)? };
+        let path = unsafe { check_and_clone_cstr(path)? };
         // TODO: check pathname
         info!(
             "mkdirat: dirfd: {}, path: {:?}, mode: {:#o}",
@@ -629,7 +629,7 @@ impl Syscall<'_> {
 
     pub fn sys_rmdir(&mut self, path: *const u8) -> SysResult {
         let proc = self.process();
-        let path = unsafe { self.vm().check_and_clone_cstr(path)? };
+        let path = unsafe { check_and_clone_cstr(path)? };
         info!("rmdir: path: {:?}", path);
 
         let (dir_path, file_name) = split_path(&path);
@@ -655,8 +655,8 @@ impl Syscall<'_> {
         flags: usize,
     ) -> SysResult {
         let proc = self.process();
-        let oldpath = unsafe { self.vm().check_and_clone_cstr(oldpath)? };
-        let newpath = unsafe { self.vm().check_and_clone_cstr(newpath)? };
+        let oldpath = unsafe { check_and_clone_cstr(oldpath)? };
+        let newpath = unsafe { check_and_clone_cstr(newpath)? };
         let flags = AtFlags::from_bits_truncate(flags);
         info!(
             "linkat: olddirfd: {}, oldpath: {:?}, newdirfd: {}, newpath: {:?}, flags: {:?}",
@@ -676,7 +676,7 @@ impl Syscall<'_> {
 
     pub fn sys_unlinkat(&mut self, dirfd: usize, path: *const u8, flags: usize) -> SysResult {
         let proc = self.process();
-        let path = unsafe { self.vm().check_and_clone_cstr(path)? };
+        let path = unsafe { check_and_clone_cstr(path)? };
         let flags = AtFlags::from_bits_truncate(flags);
         info!(
             "unlinkat: dirfd: {}, path: {:?}, flags: {:?}",

kernel/src/syscall/mod.rs

@@ -10,7 +10,7 @@ use rcore_memory::VMError;
 use crate::arch::cpu;
 use crate::arch::interrupt::TrapFrame;
 use crate::arch::syscall::*;
-use crate::memory::{copy_from_user_u8, MemorySet};
+use crate::memory::{copy_from_user_u8, copy_from_user_usize, MemorySet};
 use crate::process::*;
 use crate::sync::{Condvar, MutexGuard, SpinNoIrq};
 use crate::thread;
@@ -573,3 +573,21 @@ pub fn check_and_clone_cstr(user: *const u8) -> Result<String, SysError> {
     }
     return String::from_utf8(buffer).map_err(|_| SysError::EFAULT);
 }
+
+pub fn check_and_clone_cstr_array(user: *const *const u8) -> Result<Vec<String>, SysError> {
+    let mut buffer = Vec::new();
+    for i in 0.. {
+        let addr = unsafe { user.add(i) };
+        if let Some(str_addr) = copy_from_user_usize(addr as *const usize) {
+            if str_addr > 0 {
+                let string = check_and_clone_cstr(str_addr as *const u8)?;
+                buffer.push(string);
+            } else {
+                break;
+            }
+        } else {
+            return Err(SysError::EFAULT);
+        }
+    }
+    return Ok(buffer);
+}

kernel/src/syscall/proc.rs

@@ -154,8 +154,8 @@ impl Syscall<'_> {
         );
         let mut proc = self.process();
         let path = unsafe { check_and_clone_cstr(path)? };
-        let args = unsafe { self.vm().check_and_clone_cstr_array(argv)? };
-        let envs = unsafe { self.vm().check_and_clone_cstr_array(envp)? };
+        let args = unsafe { check_and_clone_cstr_array(argv)? };
+        let envs = unsafe { check_and_clone_cstr_array(envp)? };
 
         if args.is_empty() {
             error!("exec: args is null");