Skip to content

Commit

Permalink
fix(kernel_function): propagate unsafety
Browse files Browse the repository at this point in the history
Signed-off-by: Martin Kröning <[email protected]>
  • Loading branch information
mkroening committed Mar 23, 2024
1 parent 5716d40 commit 5187b10
Show file tree
Hide file tree
Showing 20 changed files with 344 additions and 225 deletions.
2 changes: 1 addition & 1 deletion src/arch/aarch64/kernel/scheduler.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down
2 changes: 1 addition & 1 deletion src/arch/aarch64/kernel/switch.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<R, $($A),*>(f: unsafe extern "C" fn($($A),*) -> R, $($arg: $A),*) -> R {
pub unsafe fn $kernel_function<R, $($A),*>(f: unsafe extern "C" fn($($A),*) -> R, $($arg: $A),*) -> R {
unsafe {
assert!(mem::size_of::<R>() <= mem::size_of::<usize>());

Expand Down
2 changes: 1 addition & 1 deletion src/arch/riscv64/kernel/scheduler.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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() {
Expand Down
2 changes: 1 addition & 1 deletion src/arch/x86_64/kernel/scheduler.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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() {
Expand Down
2 changes: 1 addition & 1 deletion src/arch/x86_64/kernel/switch.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<R, $($A),*>(f: unsafe extern "C" fn($($A),*) -> R, $($arg: $A),*) -> R {
pub unsafe fn $kernel_function<R, $($A),*>(f: unsafe extern "C" fn($($A),*) -> R, $($arg: $A),*) -> R {
unsafe {
assert!(mem::size_of::<R>() <= mem::size_of::<usize>());

Expand Down
10 changes: 9 additions & 1 deletion src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down
80 changes: 72 additions & 8 deletions src/macros.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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()
}
}};
}

Expand Down
12 changes: 6 additions & 6 deletions src/scheduler/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -231,8 +231,8 @@ impl From<NewTask> 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,
Expand Down Expand Up @@ -941,8 +941,8 @@ fn get_scheduler_input(core_id: CoreId) -> &'static InterruptTicketMutex<Schedul
SCHEDULER_INPUTS.lock()[usize::try_from(core_id).unwrap()]
}

pub fn spawn(
func: extern "C" fn(usize),
pub unsafe fn spawn(
func: unsafe extern "C" fn(usize),
arg: usize,
prio: Priority,
stack_size: usize,
Expand All @@ -957,7 +957,7 @@ pub fn spawn(
selector as u32
};

PerCoreScheduler::spawn(func, arg, prio, core_id, stack_size)
unsafe { PerCoreScheduler::spawn(func, arg, prio, core_id, stack_size) }
}

pub fn getpid() -> TaskId {
Expand Down
2 changes: 1 addition & 1 deletion src/scheduler/task.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
20 changes: 10 additions & 10 deletions src/syscalls/condvar.rs
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ impl CondQueue {
}
}

extern "C" fn __sys_destroy_queue(ptr: usize) -> i32 {
unsafe extern "C" fn __sys_destroy_queue(ptr: usize) -> i32 {
unsafe {
let id = ptr::from_exposed_addr_mut::<usize>(ptr);
if id.is_null() {
Expand All @@ -42,10 +42,10 @@ extern "C" fn __sys_destroy_queue(ptr: usize) -> i32 {

#[no_mangle]
pub unsafe extern "C" fn sys_destroy_queue(ptr: usize) -> i32 {
kernel_function!(__sys_destroy_queue(ptr))
unsafe { kernel_function!(__sys_destroy_queue(ptr)) }
}

extern "C" fn __sys_notify(ptr: usize, count: i32) -> i32 {
unsafe extern "C" fn __sys_notify(ptr: usize, count: i32) -> i32 {
unsafe {
let id = ptr::from_exposed_addr::<usize>(ptr);

Expand Down Expand Up @@ -83,10 +83,10 @@ extern "C" fn __sys_notify(ptr: usize, count: i32) -> i32 {

#[no_mangle]
pub unsafe extern "C" fn sys_notify(ptr: usize, count: i32) -> i32 {
kernel_function!(__sys_notify(ptr, count))
unsafe { kernel_function!(__sys_notify(ptr, count)) }
}

extern "C" fn __sys_init_queue(ptr: usize) -> i32 {
unsafe extern "C" fn __sys_init_queue(ptr: usize) -> i32 {
unsafe {
let id = ptr::from_exposed_addr_mut::<usize>(ptr);
if id.is_null() {
Expand All @@ -106,10 +106,10 @@ extern "C" fn __sys_init_queue(ptr: usize) -> i32 {

#[no_mangle]
pub unsafe extern "C" fn sys_init_queue(ptr: usize) -> i32 {
kernel_function!(__sys_init_queue(ptr))
unsafe { kernel_function!(__sys_init_queue(ptr)) }
}

extern "C" fn __sys_add_queue(ptr: usize, timeout_ns: i64) -> i32 {
unsafe extern "C" fn __sys_add_queue(ptr: usize, timeout_ns: i64) -> i32 {
unsafe {
let id = ptr::from_exposed_addr_mut::<usize>(ptr);
if id.is_null() {
Expand Down Expand Up @@ -138,10 +138,10 @@ extern "C" fn __sys_add_queue(ptr: usize, timeout_ns: i64) -> i32 {

#[no_mangle]
pub unsafe extern "C" fn sys_add_queue(ptr: usize, timeout_ns: i64) -> i32 {
kernel_function!(__sys_add_queue(ptr, timeout_ns))
unsafe { kernel_function!(__sys_add_queue(ptr, timeout_ns)) }
}

extern "C" fn __sys_wait(ptr: usize) -> i32 {
unsafe extern "C" fn __sys_wait(ptr: usize) -> i32 {
unsafe {
let id = ptr::from_exposed_addr_mut::<usize>(ptr);
if id.is_null() {
Expand All @@ -164,5 +164,5 @@ extern "C" fn __sys_wait(ptr: usize) -> i32 {

#[no_mangle]
pub unsafe extern "C" fn sys_wait(ptr: usize) -> i32 {
kernel_function!(__sys_wait(ptr))
unsafe { kernel_function!(__sys_wait(ptr)) }
}
9 changes: 3 additions & 6 deletions src/syscalls/entropy.rs
Original file line number Diff line number Diff line change
Expand Up @@ -53,10 +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]
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"))]
Expand All @@ -81,10 +80,9 @@ unsafe extern "C" fn __sys_secure_rand32(value: *mut u32) -> i32 {
/// the underlying hardware. If the required hardware isn't available,
/// the function returns `-1`.
#[cfg(not(feature = "newlib"))]
#[allow(unsafe_op_in_unsafe_fn)]
#[no_mangle]
pub unsafe extern "C" fn sys_secure_rand32(value: *mut u32) -> i32 {
kernel_function!(__sys_secure_rand32(value))
unsafe { kernel_function!(__sys_secure_rand32(value)) }
}

#[cfg(not(feature = "newlib"))]
Expand All @@ -109,10 +107,9 @@ unsafe extern "C" fn __sys_secure_rand64(value: *mut u64) -> i32 {
/// the underlying hardware. If the required hardware isn't available,
/// the function returns -1.
#[cfg(not(feature = "newlib"))]
#[allow(unsafe_op_in_unsafe_fn)]
#[no_mangle]
pub unsafe extern "C" fn sys_secure_rand64(value: *mut u64) -> i32 {
kernel_function!(__sys_secure_rand64(value))
unsafe { kernel_function!(__sys_secure_rand64(value)) }
}

extern "C" fn __sys_rand() -> u32 {
Expand Down
12 changes: 6 additions & 6 deletions src/syscalls/futex.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ use crate::time::timespec;
/// * `address` is null
/// * `timeout` is negative
/// * `flags` contains unknown flags
extern "C" fn __sys_futex_wait(
unsafe extern "C" fn __sys_futex_wait(
address: *mut u32,
expected: u32,
timeout: *const timespec,
Expand Down Expand Up @@ -38,19 +38,19 @@ extern "C" fn __sys_futex_wait(
}

#[no_mangle]
pub extern "C" fn sys_futex_wait(
pub unsafe extern "C" fn sys_futex_wait(
address: *mut u32,
expected: u32,
timeout: *const timespec,
flags: u32,
) -> i32 {
kernel_function!(__sys_futex_wait(address, expected, timeout, flags))
unsafe { kernel_function!(__sys_futex_wait(address, expected, timeout, flags)) }
}

/// Like `synch::futex_wake`, but does extra sanity checks.
///
/// Returns -EINVAL if `address` is null.
extern "C" fn __sys_futex_wake(address: *mut u32, count: i32) -> i32 {
unsafe extern "C" fn __sys_futex_wake(address: *mut u32, count: i32) -> i32 {
if address.is_null() {
return -EINVAL;
}
Expand All @@ -60,6 +60,6 @@ extern "C" fn __sys_futex_wake(address: *mut u32, count: i32) -> i32 {
}

#[no_mangle]
pub extern "C" fn sys_futex_wake(address: *mut u32, count: i32) -> i32 {
kernel_function!(__sys_futex_wake(address, count))
pub unsafe extern "C" fn sys_futex_wake(address: *mut u32, count: i32) -> i32 {
unsafe { kernel_function!(__sys_futex_wake(address, count)) }
}
6 changes: 3 additions & 3 deletions src/syscalls/lwip.rs
Original file line number Diff line number Diff line change
Expand Up @@ -42,13 +42,13 @@ pub extern "C" fn sys_putchar(character: u8) {
kernel_function!(__sys_putchar(character))
}

extern "C" fn __sys_release_putchar_lock() {
unsafe extern "C" fn __sys_release_putchar_lock() {
unsafe {
console::CONSOLE.force_unlock();
}
}

#[no_mangle]
pub extern "C" fn sys_release_putchar_lock() {
kernel_function!(__sys_release_putchar_lock())
pub unsafe extern "C" fn sys_release_putchar_lock() {
unsafe { kernel_function!(__sys_release_putchar_lock()) }
}
Loading

0 comments on commit 5187b10

Please sign in to comment.