From ee5daf32b9d2f8068c4b8af7bb2601012d8cdd2f Mon Sep 17 00:00:00 2001 From: Mark Poliakov Date: Thu, 4 Nov 2021 14:14:31 +0200 Subject: [PATCH] refactor: improve system call ABI --- init/src/main.rs | 4 +- kernel/src/syscall/arg.rs | 113 +++++++++++++++++++++ kernel/src/{syscall.rs => syscall/mod.rs} | 114 ++-------------------- libusr/src/io.rs | 23 ++--- libusr/src/lib.rs | 2 +- libusr/src/os.rs | 2 +- syscall/Cargo.toml | 3 - syscall/src/abi.rs | 1 - syscall/src/calls.rs | 66 +++++++++---- syscall/src/lib.rs | 3 - syscall/src/stat.rs | 2 +- 11 files changed, 179 insertions(+), 154 deletions(-) create mode 100644 kernel/src/syscall/arg.rs rename kernel/src/{syscall.rs => syscall/mod.rs} (51%) diff --git a/init/src/main.rs b/init/src/main.rs index c90312c..fbf3456 100644 --- a/init/src/main.rs +++ b/init/src/main.rs @@ -15,9 +15,7 @@ fn main() -> i32 { loop { print!("> "); - let count = unsafe { - libusr::sys::sys_read(0, buf.as_mut_ptr(), buf.len()) - }; + let count = unsafe { libusr::sys::sys_read(0, &mut buf) }; if count < 0 { trace!("Read from stdio failed"); break; diff --git a/kernel/src/syscall/arg.rs b/kernel/src/syscall/arg.rs new file mode 100644 index 0000000..2183536 --- /dev/null +++ b/kernel/src/syscall/arg.rs @@ -0,0 +1,113 @@ +//! System call argument ABI helpers + +use crate::mem; +use core::mem::size_of; +use error::Errno; + +fn translate(virt: usize) -> Option { + let mut res: usize; + unsafe { + asm!("at s1e1r, {}; mrs {}, par_el1", in(reg) virt, out(reg) res); + } + if res & 1 == 0 { + Some(res & !(0xFFF | (0xFF << 56))) + } else { + None + } +} + +/// Unwraps a slim structure pointer +pub fn validate_user_ptr_struct<'a, T>(base: usize) -> Result<&'a mut T, Errno> { + let bytes = validate_user_ptr(base, size_of::())?; + Ok(unsafe { &mut *(bytes.as_mut_ptr() as *mut T) }) +} + +/// Unwraps an user buffer reference +pub fn validate_user_ptr<'a>(base: usize, len: usize) -> Result<&'a mut [u8], Errno> { + if base > mem::KERNEL_OFFSET || base + len > mem::KERNEL_OFFSET { + warnln!( + "User region refers to kernel memory: base={:#x}, len={:#x}", + base, + len + ); + return Err(Errno::InvalidArgument); + } + + for i in (base / mem::PAGE_SIZE)..((base + len + mem::PAGE_SIZE - 1) / mem::PAGE_SIZE) { + if translate(i * mem::PAGE_SIZE).is_none() { + warnln!( + "User region refers to unmapped memory: base={:#x}, len={:#x} (page {:#x})", + base, + len, + i * mem::PAGE_SIZE + ); + return Err(Errno::InvalidArgument); + } + } + + Ok(unsafe { core::slice::from_raw_parts_mut(base as *mut u8, len) }) +} + +/// Unwraps a nullable user buffer reference +pub fn validate_user_ptr_null<'a>(base: usize, len: usize) -> Result, Errno> { + if base == 0 { + Ok(None) + } else { + validate_user_ptr(base, len).map(Some) + } +} + +/// Unwraps user string argument +pub fn validate_user_str<'a>(base: usize, len: usize) -> Result<&'a str, Errno> { + let bytes = validate_user_ptr(base, len)?; + core::str::from_utf8(bytes).map_err(|_| { + warnln!("User string contains invalid UTF-8 characters: base={:#x}, len={:#x}", base, len); + Errno::InvalidArgument + }) +} +// if base > mem::KERNEL_OFFSET { +// warnln!("User string refers to kernel memory: base={:#x}", base); +// return Err(Errno::InvalidArgument); +// } +// +// let base_ptr = base as *const u8; +// let mut len = 0; +// let mut page_valid = false; +// loop { +// if len == limit { +// warnln!("User string exceeded limit: base={:#x}", base); +// return Err(Errno::InvalidArgument); +// } +// +// if (base + len) % mem::PAGE_SIZE == 0 { +// page_valid = false; +// } +// +// if !page_valid && translate((base + len) & !0xFFF).is_none() { +// warnln!( +// "User string refers to unmapped memory: base={:#x}, off={:#x}", +// base, +// len +// ); +// return Err(Errno::InvalidArgument); +// } +// +// page_valid = true; +// +// let byte = unsafe { *base_ptr.add(len) }; +// if byte == 0 { +// break; +// } +// +// len += 1; +// } +// +// let slice = unsafe { core::slice::from_raw_parts(base_ptr, len) }; +// core::str::from_utf8(slice).map_err(|_| { +// warnln!( +// "User string contains invalid UTF-8 characters: base={:#x}", +// base +// ); +// Errno::InvalidArgument +// }) +// } diff --git a/kernel/src/syscall.rs b/kernel/src/syscall/mod.rs similarity index 51% rename from kernel/src/syscall.rs rename to kernel/src/syscall/mod.rs index bf8c131..d890406 100644 --- a/kernel/src/syscall.rs +++ b/kernel/src/syscall/mod.rs @@ -1,113 +1,17 @@ //! System call implementation +use crate::arch::platform::exception::ExceptionFrame; use crate::debug::Level; -use crate::mem; -use crate::proc::{wait, Process, Pid}; +use crate::proc::{wait, Pid, Process}; use core::mem::size_of; use core::time::Duration; use error::Errno; use libcommon::{Read, Write}; use syscall::{abi, stat::AT_FDCWD}; use vfs::{FileMode, OpenFlags, Stat}; -use crate::arch::platform::exception::ExceptionFrame; -fn translate(virt: usize) -> Option { - let mut res: usize; - unsafe { - asm!("at s1e1r, {}; mrs {}, par_el1", in(reg) virt, out(reg) res); - } - if res & 1 == 0 { - Some(res & !(0xFFF | (0xFF << 56))) - } else { - None - } -} - -fn validate_user_ptr_struct<'a, T>(base: usize) -> Result<&'a mut T, Errno> { - let bytes = validate_user_ptr(base, size_of::())?; - Ok(unsafe { &mut *(bytes.as_mut_ptr() as *mut T) }) -} - -fn validate_user_ptr<'a>(base: usize, len: usize) -> Result<&'a mut [u8], Errno> { - if base > mem::KERNEL_OFFSET || base + len > mem::KERNEL_OFFSET { - warnln!( - "User region refers to kernel memory: base={:#x}, len={:#x}", - base, - len - ); - return Err(Errno::InvalidArgument); - } - - for i in (base / mem::PAGE_SIZE)..((base + len + mem::PAGE_SIZE - 1) / mem::PAGE_SIZE) { - if translate(i * mem::PAGE_SIZE).is_none() { - warnln!( - "User region refers to unmapped memory: base={:#x}, len={:#x} (page {:#x})", - base, - len, - i * mem::PAGE_SIZE - ); - return Err(Errno::InvalidArgument); - } - } - - Ok(unsafe { core::slice::from_raw_parts_mut(base as *mut u8, len) }) -} - -fn validate_user_ptr_null<'a>(base: usize, len: usize) -> Result, Errno> { - if base == 0 { - Ok(None) - } else { - validate_user_ptr(base, len).map(Some) - } -} - -fn validate_user_str<'a>(base: usize, limit: usize) -> Result<&'a str, Errno> { - if base > mem::KERNEL_OFFSET { - warnln!("User string refers to kernel memory: base={:#x}", base); - return Err(Errno::InvalidArgument); - } - - let base_ptr = base as *const u8; - let mut len = 0; - let mut page_valid = false; - loop { - if len == limit { - warnln!("User string exceeded limit: base={:#x}", base); - return Err(Errno::InvalidArgument); - } - - if (base + len) % mem::PAGE_SIZE == 0 { - page_valid = false; - } - - if !page_valid && translate((base + len) & !0xFFF).is_none() { - warnln!( - "User string refers to unmapped memory: base={:#x}, off={:#x}", - base, - len - ); - return Err(Errno::InvalidArgument); - } - - page_valid = true; - - let byte = unsafe { *base_ptr.add(len) }; - if byte == 0 { - break; - } - - len += 1; - } - - let slice = unsafe { core::slice::from_raw_parts(base_ptr, len) }; - core::str::from_utf8(slice).map_err(|_| { - warnln!( - "User string contains invalid UTF-8 characters: base={:#x}", - base - ); - Errno::InvalidArgument - }) -} +mod arg; +use arg::*; /// Creates a "fork" process from current one using its register frame. /// See [Process::fork()]. @@ -132,9 +36,9 @@ pub fn syscall(num: usize, args: &[usize]) -> Result { // I/O system calls abi::SYS_OPENAT => { let at_fd = args[0]; - let path = validate_user_str(args[1], 256)?; - let mode = FileMode::from_bits(args[2] as u32).ok_or(Errno::InvalidArgument)?; - let opts = OpenFlags::from_bits(args[3] as u32).ok_or(Errno::InvalidArgument)?; + let path = validate_user_str(args[1], args[2])?; + let mode = FileMode::from_bits(args[3] as u32).ok_or(Errno::InvalidArgument)?; + let opts = OpenFlags::from_bits(args[4] as u32).ok_or(Errno::InvalidArgument)?; let at = if at_fd as i32 == AT_FDCWD { None @@ -166,8 +70,8 @@ pub fn syscall(num: usize, args: &[usize]) -> Result { let proc = Process::current(); let mut io = proc.io.lock(); let fd = args[0]; - let filename = validate_user_str(args[1], 256)?; - let buf = validate_user_ptr_struct::(args[2])?; + let filename = validate_user_str(args[1], args[2])?; + let buf = validate_user_ptr_struct::(args[3])?; // TODO "self" flag let at = if fd as i32 != AT_FDCWD { diff --git a/libusr/src/io.rs b/libusr/src/io.rs index 836cd40..1efc96d 100644 --- a/libusr/src/io.rs +++ b/libusr/src/io.rs @@ -1,25 +1,18 @@ use crate::sys::{self, Stat}; use syscall::stat::AT_FDCWD; -use core::mem::MaybeUninit; use core::fmt; const STDOUT_FILENO: i32 = 0; pub fn stat(pathname: &str) -> Result { - let mut buf = MaybeUninit::::uninit(); - let mut path = [0u8; 256]; - - let bytes = pathname.as_bytes(); - path[..pathname.len()].copy_from_slice(&bytes); - path[pathname.len()] = 0; - - unsafe { - let res = sys::sys_fstatat(AT_FDCWD, path.as_ptr(), buf.as_mut_ptr(), 0); - if res != 0 { - todo!(); - } - Ok(buf.assume_init()) + let mut buf = Stat::default(); + let res = unsafe { + sys::sys_fstatat(AT_FDCWD, pathname, &mut buf, 0) + }; + if res != 0 { + todo!(); } + Ok(buf) } // print!/println! group @@ -58,6 +51,6 @@ pub fn _print(args: fmt::Arguments) { }; writer.write_fmt(args).ok(); unsafe { - sys::sys_write(STDOUT_FILENO, &BUFFER as *const _, writer.pos); + sys::sys_write(STDOUT_FILENO, &BUFFER[..writer.pos]); } } diff --git a/libusr/src/lib.rs b/libusr/src/lib.rs index 014814d..0b3f6d7 100644 --- a/libusr/src/lib.rs +++ b/libusr/src/lib.rs @@ -9,7 +9,7 @@ pub mod io; pub mod sys { pub use syscall::calls::*; - pub use syscall::stat::{AT_FDCWD, Stat, OpenFlags}; + pub use syscall::stat::{AT_FDCWD, Stat, OpenFlags, FileMode}; } #[link_section = ".text._start"] diff --git a/libusr/src/os.rs b/libusr/src/os.rs index ac8e5f6..a502bdb 100644 --- a/libusr/src/os.rs +++ b/libusr/src/os.rs @@ -30,6 +30,6 @@ pub fn _trace(args: fmt::Arguments) { }; writer.write_fmt(args).ok(); unsafe { - sys::sys_ex_debug_trace(&BUFFER as *const _, writer.pos); + sys::sys_ex_debug_trace(&BUFFER[..writer.pos]); } } diff --git a/syscall/Cargo.toml b/syscall/Cargo.toml index 16a7f53..878f881 100644 --- a/syscall/Cargo.toml +++ b/syscall/Cargo.toml @@ -10,6 +10,3 @@ bitflags = "^1.3.0" [features] user = [] - -# TODO implement this feature -linux_compat = [] diff --git a/syscall/src/abi.rs b/syscall/src/abi.rs index cc08598..1186086 100644 --- a/syscall/src/abi.rs +++ b/syscall/src/abi.rs @@ -2,7 +2,6 @@ pub const SYS_EX_DEBUG_TRACE: usize = 128; pub const SYS_EX_NANOSLEEP: usize = 129; -// From aarch64 linux pub const SYS_EXIT: usize = 1; pub const SYS_READ: usize = 2; pub const SYS_WRITE: usize = 3; diff --git a/syscall/src/calls.rs b/syscall/src/calls.rs index 16dc49d..c5d5d75 100644 --- a/syscall/src/calls.rs +++ b/syscall/src/calls.rs @@ -1,5 +1,7 @@ use crate::abi; -use crate::stat::Stat; +use crate::stat::{Stat, OpenFlags, FileMode}; + +// TODO document the syscall ABI macro_rules! syscall { ($num:expr) => {{ @@ -35,58 +37,80 @@ macro_rules! syscall { in("x3") $a3, in("x8") $num, options(nostack)); res }}; + ($num:expr, $a0:expr, $a1:expr, $a2:expr, $a3:expr, $a4:expr) => {{ + let mut res: usize = $a0; + asm!("svc #0", + inout("x0") res, in("x1") $a1, in("x2") $a2, + in("x3") $a3, in("x4") $a4, in("x8") $num, options(nostack)); + res + }}; } +/// Integer/size argument +macro_rules! argn { + ($a:expr) => ($a as usize) +} +/// Pointer/base argument +macro_rules! argp { + ($a:expr) => ($a as *mut core::ffi::c_void as usize) +} +// /// Immutable pointer/base argument +// macro_rules! argpi { +// ($a:expr) => ($a as *const core::ffi::c_void as usize) +// } + #[inline(always)] pub unsafe fn sys_exit(status: i32) -> ! { - syscall!(abi::SYS_EXIT, status as usize); + syscall!(abi::SYS_EXIT, argn!(status)); loop {} } #[inline(always)] pub unsafe fn sys_close(fd: i32) -> i32 { - syscall!(abi::SYS_CLOSE, fd as usize) as i32 + syscall!(abi::SYS_CLOSE, argn!(fd)) as i32 } #[inline(always)] -pub unsafe fn sys_ex_nanosleep(ns: u64, rem: *mut [u64; 2]) -> i32 { - syscall!(abi::SYS_EX_NANOSLEEP, ns as usize, rem as usize) as i32 +pub unsafe fn sys_ex_nanosleep(ns: u64, rem: &mut [u64; 2]) -> i32 { + syscall!(abi::SYS_EX_NANOSLEEP, argn!(ns), argp!(rem.as_mut_ptr())) as i32 } #[inline(always)] -pub unsafe fn sys_ex_debug_trace(msg: *const u8, len: usize) -> usize { - syscall!(abi::SYS_EX_DEBUG_TRACE, msg as usize, len) +pub unsafe fn sys_ex_debug_trace(msg: &[u8]) -> usize { + syscall!(abi::SYS_EX_DEBUG_TRACE, argp!(msg.as_ptr()), argn!(msg.len())) } #[inline(always)] -pub unsafe fn sys_openat(at: i32, pathname: *const u8, mode: u32, flags: u32) -> i32 { +pub unsafe fn sys_openat(at: i32, pathname: &str, mode: FileMode, flags: OpenFlags) -> i32 { syscall!( abi::SYS_OPENAT, - at as usize, - pathname as usize, - mode as usize, - flags as usize + argn!(at), + argp!(pathname.as_ptr()), + argn!(pathname.len()), + argn!(mode.bits()), + argn!(flags.bits()) ) as i32 } #[inline(always)] -pub unsafe fn sys_read(fd: i32, data: *mut u8, len: usize) -> isize { - syscall!(abi::SYS_READ, fd as usize, data as usize, len) as isize +pub unsafe fn sys_read(fd: i32, data: &mut [u8]) -> isize { + syscall!(abi::SYS_READ, argn!(fd), argp!(data.as_mut_ptr()), argn!(data.len())) as isize } #[inline(always)] -pub unsafe fn sys_write(fd: i32, data: *const u8, len: usize) -> isize { - syscall!(abi::SYS_WRITE, fd as usize, data as usize, len) as isize +pub unsafe fn sys_write(fd: i32, data: &[u8]) -> isize { + syscall!(abi::SYS_WRITE, argn!(fd), argp!(data.as_ptr()), argn!(data.len())) as isize } #[inline(always)] -pub unsafe fn sys_fstatat(at: i32, pathname: *const u8, statbuf: *mut Stat, flags: i32) -> i32 { +pub unsafe fn sys_fstatat(at: i32, pathname: &str, statbuf: &mut Stat, flags: i32) -> i32 { syscall!( abi::SYS_FSTATAT, - at as usize, - pathname as usize, - statbuf as usize, - flags as usize + argn!(at), + argp!(pathname.as_ptr()), + argn!(pathname.len()), + argp!(statbuf as *mut Stat), + argn!(flags) ) as i32 } diff --git a/syscall/src/lib.rs b/syscall/src/lib.rs index b515e0f..9228f8f 100644 --- a/syscall/src/lib.rs +++ b/syscall/src/lib.rs @@ -1,9 +1,6 @@ #![feature(asm)] #![no_std] -#[cfg(feature = "linux_compat")] -compile_error!("Not yet implemented"); - #[macro_use] extern crate bitflags; diff --git a/syscall/src/stat.rs b/syscall/src/stat.rs index 70b0d01..ff5f5cc 100644 --- a/syscall/src/stat.rs +++ b/syscall/src/stat.rs @@ -25,7 +25,7 @@ bitflags! { } } -#[derive(Clone, Copy, Debug)] +#[derive(Clone, Copy, Debug, Default)] #[repr(C)] pub struct Stat { pub mode: u32,