From abcf8c44d1ffdedab172ba05b70396dc940fb0cc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Martin=20Kr=C3=B6ning?= Date: Sat, 23 Mar 2024 12:16:54 +0100 Subject: [PATCH] fix(kernel_function): propagate unsafety MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Martin Kröning --- src/arch/aarch64/kernel/scheduler.rs | 2 +- src/arch/aarch64/kernel/switch.rs | 2 +- src/arch/riscv64/kernel/scheduler.rs | 2 +- src/arch/x86_64/kernel/scheduler.rs | 2 +- src/arch/x86_64/kernel/switch.rs | 2 +- src/lib.rs | 10 +++- src/macros.rs | 80 +++++++++++++++++++++++++--- src/scheduler/mod.rs | 12 ++--- src/scheduler/task.rs | 2 +- src/syscalls/entropy.rs | 4 +- src/syscalls/tasks.rs | 21 ++++---- tests/thread.rs | 28 +++++----- 12 files changed, 120 insertions(+), 47 deletions(-) diff --git a/src/arch/aarch64/kernel/scheduler.rs b/src/arch/aarch64/kernel/scheduler.rs index 96e4d1f8aa..eb850c1800 100644 --- a/src/arch/aarch64/kernel/scheduler.rs +++ b/src/arch/aarch64/kernel/scheduler.rs @@ -338,7 +338,7 @@ extern "C" fn task_start(_f: extern "C" fn(usize), _arg: usize) -> ! { } impl TaskFrame for Task { - fn create_stack_frame(&mut self, func: extern "C" fn(usize), arg: usize) { + fn create_stack_frame(&mut self, func: unsafe extern "C" fn(usize), arg: usize) { // Check if TLS is allocated already and if the task uses thread-local storage. if self.tls.is_none() { self.tls = TaskTLS::from_environment(); diff --git a/src/arch/aarch64/kernel/switch.rs b/src/arch/aarch64/kernel/switch.rs index 9c3102e111..66c80573ae 100644 --- a/src/arch/aarch64/kernel/switch.rs +++ b/src/arch/aarch64/kernel/switch.rs @@ -5,7 +5,7 @@ macro_rules! kernel_function_impl { ($kernel_function:ident($($arg:ident: $A:ident),*) { $($operands:tt)* }) => { /// Executes `f` on the kernel stack. #[allow(dead_code)] - pub fn $kernel_function(f: unsafe extern "C" fn($($A),*) -> R, $($arg: $A),*) -> R { + pub unsafe fn $kernel_function(f: unsafe extern "C" fn($($A),*) -> R, $($arg: $A),*) -> R { unsafe { assert!(mem::size_of::() <= mem::size_of::()); diff --git a/src/arch/riscv64/kernel/scheduler.rs b/src/arch/riscv64/kernel/scheduler.rs index 611b9466fd..64bd1a5a94 100644 --- a/src/arch/riscv64/kernel/scheduler.rs +++ b/src/arch/riscv64/kernel/scheduler.rs @@ -379,7 +379,7 @@ extern "C" fn task_entry(func: extern "C" fn(usize), arg: usize) { } impl TaskFrame for Task { - fn create_stack_frame(&mut self, func: extern "C" fn(usize), arg: usize) { + fn create_stack_frame(&mut self, func: unsafe extern "C" fn(usize), arg: usize) { // Check if the task (process or thread) uses Thread-Local-Storage. // check is TLS is already allocated if self.tls.is_none() { diff --git a/src/arch/x86_64/kernel/scheduler.rs b/src/arch/x86_64/kernel/scheduler.rs index f6d39ea40d..80b38e006e 100644 --- a/src/arch/x86_64/kernel/scheduler.rs +++ b/src/arch/x86_64/kernel/scheduler.rs @@ -336,7 +336,7 @@ extern "C" fn task_entry(func: extern "C" fn(usize), arg: usize) -> ! { } impl TaskFrame for Task { - fn create_stack_frame(&mut self, func: extern "C" fn(usize), arg: usize) { + fn create_stack_frame(&mut self, func: unsafe extern "C" fn(usize), arg: usize) { // Check if TLS is allocated already and if the task uses thread-local storage. #[cfg(not(feature = "common-os"))] if self.tls.is_none() { diff --git a/src/arch/x86_64/kernel/switch.rs b/src/arch/x86_64/kernel/switch.rs index 49368aeae2..a970845cf8 100644 --- a/src/arch/x86_64/kernel/switch.rs +++ b/src/arch/x86_64/kernel/switch.rs @@ -220,7 +220,7 @@ macro_rules! kernel_function_impl { ($kernel_function:ident($($arg:ident: $A:ident),*) { $($operands:tt)* }) => { /// Executes `f` on the kernel stack. #[allow(dead_code)] - pub fn $kernel_function(f: unsafe extern "C" fn($($A),*) -> R, $($arg: $A),*) -> R { + pub unsafe fn $kernel_function(f: unsafe extern "C" fn($($A),*) -> R, $($arg: $A),*) -> R { unsafe { assert!(mem::size_of::() <= mem::size_of::()); diff --git a/src/lib.rs b/src/lib.rs index 54aef2c46b..0e8992a1ac 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -239,7 +239,15 @@ fn boot_processor_main() -> ! { } // Start the initd task. - scheduler::PerCoreScheduler::spawn(initd, 0, scheduler::task::NORMAL_PRIO, 0, USER_STACK_SIZE); + unsafe { + scheduler::PerCoreScheduler::spawn( + initd, + 0, + scheduler::task::NORMAL_PRIO, + 0, + USER_STACK_SIZE, + ) + }; // Run the scheduler loop. PerCoreScheduler::run(); diff --git a/src/macros.rs b/src/macros.rs index 221b9475ff..b63426156d 100644 --- a/src/macros.rs +++ b/src/macros.rs @@ -80,45 +80,109 @@ macro_rules! dbg { macro_rules! kernel_function { ($f:ident()) => {{ use $crate::errno::ToErrno; + + // This propagates any unsafety requirements of `f` to the caller. + if false { + $f(); + } + #[allow(unreachable_code)] - $crate::arch::switch::kernel_function0($f).set_errno() + #[allow(unused_unsafe)] + unsafe { + $crate::arch::switch::kernel_function0($f).set_errno() + } }}; ($f:ident($arg1:expr)) => {{ use $crate::errno::ToErrno; + + // This propagates any unsafety requirements of `f` to the caller. + if false { + $f($arg1); + } + #[allow(unreachable_code)] - $crate::arch::switch::kernel_function1($f, $arg1).set_errno() + #[allow(unused_unsafe)] + unsafe { + $crate::arch::switch::kernel_function1($f, $arg1).set_errno() + } }}; ($f:ident($arg1:expr, $arg2:expr)) => {{ use $crate::errno::ToErrno; + + // This propagates any unsafety requirements of `f` to the caller. + if false { + $f($arg1, $arg2); + } + #[allow(unreachable_code)] - $crate::arch::switch::kernel_function2($f, $arg1, $arg2).set_errno() + #[allow(unused_unsafe)] + unsafe { + $crate::arch::switch::kernel_function2($f, $arg1, $arg2).set_errno() + } }}; ($f:ident($arg1:expr, $arg2:expr, $arg3:expr)) => {{ use $crate::errno::ToErrno; + + // This propagates any unsafety requirements of `f` to the caller. + if false { + $f($arg1, $arg2, $arg3); + } + #[allow(unreachable_code)] - $crate::arch::switch::kernel_function3($f, $arg1, $arg2, $arg3).set_errno() + #[allow(unused_unsafe)] + unsafe { + $crate::arch::switch::kernel_function3($f, $arg1, $arg2, $arg3).set_errno() + } }}; ($f:ident($arg1:expr, $arg2:expr, $arg3:expr, $arg4:expr)) => {{ use $crate::errno::ToErrno; + + // This propagates any unsafety requirements of `f` to the caller. + if false { + $f($arg1, $arg2, $arg3, $arg4); + } + #[allow(unreachable_code)] - $crate::arch::switch::kernel_function4($f, $arg1, $arg2, $arg3, $arg4).set_errno() + #[allow(unused_unsafe)] + unsafe { + $crate::arch::switch::kernel_function4($f, $arg1, $arg2, $arg3, $arg4).set_errno() + } }}; ($f:ident($arg1:expr, $arg2:expr, $arg3:expr, $arg4:expr, $arg5:expr)) => {{ use $crate::errno::ToErrno; + + // This propagates any unsafety requirements of `f` to the caller. + if false { + $f($arg1, $arg2, $arg3, $arg4, $arg5); + } + #[allow(unreachable_code)] - $crate::arch::switch::kernel_function5($f, $arg1, $arg2, $arg3, $arg4, $arg5).set_errno() + #[allow(unused_unsafe)] + unsafe { + $crate::arch::switch::kernel_function5($f, $arg1, $arg2, $arg3, $arg4, $arg5) + .set_errno() + } }}; ($f:ident($arg1:expr, $arg2:expr, $arg3:expr, $arg4:expr, $arg5:expr, $arg6:expr)) => {{ use $crate::errno::ToErrno; + + // This propagates any unsafety requirements of `f` to the caller. + if false { + $f($arg1, $arg2, $arg3, $arg4, $arg5, $arg6); + } + #[allow(unreachable_code)] - $crate::arch::switch::kernel_function6($f, $arg1, $arg2, $arg3, $arg4, $arg5, $arg6) - .set_errno() + #[allow(unused_unsafe)] + unsafe { + $crate::arch::switch::kernel_function6($f, $arg1, $arg2, $arg3, $arg4, $arg5, $arg6) + .set_errno() + } }}; } diff --git a/src/scheduler/mod.rs b/src/scheduler/mod.rs index 0847db7f2c..fd2c774701 100644 --- a/src/scheduler/mod.rs +++ b/src/scheduler/mod.rs @@ -203,7 +203,7 @@ impl PerCoreSchedulerExt for &mut PerCoreScheduler { struct NewTask { tid: TaskId, - func: extern "C" fn(usize), + func: unsafe extern "C" fn(usize), arg: usize, prio: Priority, core_id: CoreId, @@ -231,8 +231,8 @@ impl From for Task { impl PerCoreScheduler { /// Spawn a new task. - pub fn spawn( - func: extern "C" fn(usize), + pub unsafe fn spawn( + func: unsafe extern "C" fn(usize), arg: usize, prio: Priority, core_id: CoreId, @@ -941,8 +941,8 @@ fn get_scheduler_input(core_id: CoreId) -> &'static InterruptTicketMutex TaskId { diff --git a/src/scheduler/task.rs b/src/scheduler/task.rs index 6b31a97956..d6ff54061c 100644 --- a/src/scheduler/task.rs +++ b/src/scheduler/task.rs @@ -406,7 +406,7 @@ pub(crate) struct Task { pub(crate) trait TaskFrame { /// Create the initial stack frame for a new task - fn create_stack_frame(&mut self, func: extern "C" fn(usize), arg: usize); + fn create_stack_frame(&mut self, func: unsafe extern "C" fn(usize), arg: usize); } impl Task { diff --git a/src/syscalls/entropy.rs b/src/syscalls/entropy.rs index dc3cbd8726..26cf7bf484 100644 --- a/src/syscalls/entropy.rs +++ b/src/syscalls/entropy.rs @@ -53,11 +53,9 @@ unsafe extern "C" fn __sys_read_entropy(buf: *mut u8, len: usize, flags: u32) -> /// Returns either the number of bytes written to buf (a positive value) or /// * `-EINVAL` if `flags` contains unknown flags. /// * `-ENOSYS` if the system does not support random data generation. -#[allow(unsafe_op_in_unsafe_fn)] #[no_mangle] -#[cfg_attr(target_arch = "riscv64", allow(unsafe_op_in_unsafe_fn))] // FIXME pub unsafe extern "C" fn sys_read_entropy(buf: *mut u8, len: usize, flags: u32) -> isize { - kernel_function!(__sys_read_entropy(buf, len, flags)) + unsafe { kernel_function!(__sys_read_entropy(buf, len, flags)) } } #[cfg(not(feature = "newlib"))] diff --git a/src/syscalls/tasks.rs b/src/syscalls/tasks.rs index 6938bf7463..d02be3038c 100644 --- a/src/syscalls/tasks.rs +++ b/src/syscalls/tasks.rs @@ -236,36 +236,37 @@ pub extern "C" fn sys_signal(handler: SignalHandler) -> i32 { kernel_function!(__sys_signal(handler)) } -extern "C" fn __sys_spawn2( - func: extern "C" fn(usize), +unsafe extern "C" fn __sys_spawn2( + func: unsafe extern "C" fn(usize), arg: usize, prio: u8, stack_size: usize, selector: isize, ) -> Tid { - scheduler::spawn(func, arg, Priority::from(prio), stack_size, selector).into() + unsafe { scheduler::spawn(func, arg, Priority::from(prio), stack_size, selector).into() } } #[no_mangle] -pub extern "C" fn sys_spawn2( - func: extern "C" fn(usize), +pub unsafe extern "C" fn sys_spawn2( + func: unsafe extern "C" fn(usize), arg: usize, prio: u8, stack_size: usize, selector: isize, ) -> Tid { - kernel_function!(__sys_spawn2(func, arg, prio, stack_size, selector)) + unsafe { kernel_function!(__sys_spawn2(func, arg, prio, stack_size, selector)) } } unsafe extern "C" fn __sys_spawn( id: *mut Tid, - func: extern "C" fn(usize), + func: unsafe extern "C" fn(usize), arg: usize, prio: u8, selector: isize, ) -> i32 { - let new_id = - scheduler::spawn(func, arg, Priority::from(prio), USER_STACK_SIZE, selector).into(); + let new_id = unsafe { + scheduler::spawn(func, arg, Priority::from(prio), USER_STACK_SIZE, selector).into() + }; if !id.is_null() { unsafe { @@ -279,7 +280,7 @@ unsafe extern "C" fn __sys_spawn( #[no_mangle] pub unsafe extern "C" fn sys_spawn( id: *mut Tid, - func: extern "C" fn(usize), + func: unsafe extern "C" fn(usize), arg: usize, prio: u8, selector: isize, diff --git a/tests/thread.rs b/tests/thread.rs index 9a624b1e9b..776bba5b97 100644 --- a/tests/thread.rs +++ b/tests/thread.rs @@ -37,7 +37,7 @@ pub fn thread_test() { let threadnum = 5; for i in 0..threadnum { println!("SPAWNING THREAD {}", i); - let id = sys_spawn2(thread_func, i, NORMAL_PRIO, USER_STACK_SIZE, -1); + let id = unsafe { sys_spawn2(thread_func, i, NORMAL_PRIO, USER_STACK_SIZE, -1) }; children.push(id); } println!("SPAWNED THREADS"); @@ -47,13 +47,13 @@ pub fn thread_test() { } } -extern "C" fn waker_func(futex: usize) { +unsafe extern "C" fn waker_func(futex: usize) { let futex = unsafe { &*(futex as *const AtomicU32) }; sys_usleep(100_000); futex.store(1, Relaxed); - let ret = sys_futex_wake(futex as *const AtomicU32 as *mut u32, i32::MAX); + let ret = unsafe { sys_futex_wake(futex as *const AtomicU32 as *mut u32, i32::MAX) }; assert_eq!(ret, 1); } @@ -62,26 +62,28 @@ pub fn test_futex() { let futex = AtomicU32::new(0); let futex_ptr = &futex as *const AtomicU32 as *mut u32; - let ret = sys_futex_wait(futex_ptr, 1, ptr::null(), 0); + let ret = unsafe { sys_futex_wait(futex_ptr, 1, ptr::null(), 0) }; assert_eq!(ret, -EAGAIN); let timeout = timespec { tv_sec: 0, tv_nsec: 100_000_000, }; - let ret = sys_futex_wait(futex_ptr, 0, &timeout, 1); + let ret = unsafe { sys_futex_wait(futex_ptr, 0, &timeout, 1) }; assert_eq!(ret, -ETIMEDOUT); - let waker = sys_spawn2( - waker_func, - futex_ptr as usize, - NORMAL_PRIO, - USER_STACK_SIZE, - -1, - ); + let waker = unsafe { + sys_spawn2( + waker_func, + futex_ptr as usize, + NORMAL_PRIO, + USER_STACK_SIZE, + -1, + ) + }; assert!(waker >= 0); - let ret = sys_futex_wait(futex_ptr, 0, ptr::null(), 0); + let ret = unsafe { sys_futex_wait(futex_ptr, 0, ptr::null(), 0) }; assert_eq!(ret, 0); assert_eq!(futex.load(Relaxed), 1);