diff --git a/kernel/crate_metadata/src/lib.rs b/kernel/crate_metadata/src/lib.rs index 3da3d1fd23..9cc825b050 100644 --- a/kernel/crate_metadata/src/lib.rs +++ b/kernel/crate_metadata/src/lib.rs @@ -790,6 +790,64 @@ impl LoadedSection { Err("this source section has a different length than the destination section") } } + + /// Reinterprets this section's underlying `MappedPages` memory region as an executable function. + /// + /// The generic `F` parameter is the function type signature itself, e.g., `fn(String) -> u8`. + /// + /// Returns a reference to the function that is formed from the underlying memory region, + /// with a lifetime dependent upon the lifetime of this section. + /// + /// # Locking + /// Obtains the lock on this section's `MappedPages` object. + /// + /// # Note + /// Ideally, we would use debug information to know the size of the entire function + /// and test whether that fits within the bounds of the memory region, rather than just checking + /// the size of `F`, the function pointer/signature. + /// Without debug information, checking the size is restricted to in-bounds memory safety + /// rather than actual functional correctness. + /// + /// # Examples + /// Here's how you might call this function: + /// ``` + /// type MyPrintFuncSignature = fn(&str) -> Result<(), &'static str>; + /// let section = mod_mgmt::get_symbol_starting_with("my_crate::print::").upgrade().unwrap(); + /// let print_func: &MyPrintFuncSignature = section.as_func().unwrap(); + /// print_func("hello there"); + /// ``` + /// + pub fn as_func(&self) -> Result<&F, &'static str> { + if false { + debug!("Requested LoadedSection {:#X?} as function {:?}", self, core::any::type_name::()); + } + + let mp = self.mapped_pages.lock(); + // Check flags to make sure these pages are executable (otherwise a page fault would occur when this func is called) + if self.typ != SectionType::Text || !mp.flags().is_executable() { + error!("Requested LoadedSection as function {:?}, but was not an executable text section! (flags: {:?})", + core::any::type_name::(), mp.flags() + ); + return Err("as_func(): section was not an executable text section"); + } + + // Check that the bounds of this entire section fit within its MappedPages + let end = self.mapped_pages_offset + self.size(); + if end > mp.size_in_bytes() { + error!("Requested LoadedSection as function {:?}, but section's end offset ({:X?}) was beyond its MappedPages ({:X?})", + core::any::type_name::(), end, mp.size_in_bytes() + ); + return Err("requested type and offset would not fit within the MappedPages bounds"); + } + + // SAFE: above, we check the section type, executability, and size bounds of its underlying MappedPages + // and tie the lifetime of the returned function reference to this section's lifetime. + Ok(unsafe { + core::mem::transmute( + &(mp.start_address().value() + self.mapped_pages_offset) + ) + }) + } } impl fmt::Debug for LoadedSection { diff --git a/kernel/crate_swap/src/lib.rs b/kernel/crate_swap/src/lib.rs index e531fac430..e7338724b3 100644 --- a/kernel/crate_swap/src/lib.rs +++ b/kernel/crate_swap/src/lib.rs @@ -513,12 +513,7 @@ pub fn swap_crates( // as a backup, search fuzzily to accommodate state transfer function symbol names without full hashes .or_else(|| namespace_of_new_crates.get_symbol_starting_with(&symbol).upgrade()) .ok_or("couldn't find specified state transfer function in the new CrateNamespace")?; - - let mut space: usize = 0; - let st_fn = { - let mapped_pages = state_transfer_fn_sec.mapped_pages.lock(); - mapped_pages.as_func::(state_transfer_fn_sec.mapped_pages_offset, &mut space)? - }; + let st_fn = state_transfer_fn_sec.as_func::()?; #[cfg(not(loscd_eval))] debug!("swap_crates(): invoking the state transfer function {:?} with old_ns: {:?}, new_ns: {:?}", symbol, this_namespace.name(), namespace_of_new_crates.name()); st_fn(this_namespace, &namespace_of_new_crates)?; diff --git a/kernel/memory/src/paging/mapper.rs b/kernel/memory/src/paging/mapper.rs index 7d7aef46ce..12e0e95481 100644 --- a/kernel/memory/src/paging/mapper.rs +++ b/kernel/memory/src/paging/mapper.rs @@ -748,83 +748,6 @@ impl MappedPages { Ok(slc) } - - - /// Reinterprets this `MappedPages`'s underlying memory region as an executable function with any signature. - /// - /// # Arguments - /// * `offset`: the offset (in number of bytes) into the memory region at which the function starts. - /// * `space`: a hack to satisfy the borrow checker's lifetime requirements. - /// - /// Returns a reference to the function that is formed from the underlying memory region, - /// with a lifetime dependent upon the lifetime of the given `space` object. - /// - /// TODO FIXME: this isn't really safe as it stands now. - /// Ideally, we need to have an integrated function that checks with the mod_mgmt crate - /// to see if the size of the function can fit (not just the size of the function POINTER, which will basically always fit) - /// within the bounds of this `MappedPages` object; - /// this integrated function would be based on the given string name of the function, like "task::this::foo", - /// and would invoke this as_func() function directly. - /// - /// We have to accept space for the function pointer to exist, because it cannot live in this function's stack. - /// It has to live in stack of the function that invokes the actual returned function reference, - /// otherwise there would be a lifetime issue and a guaranteed page fault. - /// So, the `space` arg is a hack to ensure lifetimes; - /// we don't care about the actual value of `space`, as the value will be overwritten, - /// and it doesn't matter both before and after the call to this `as_func()`. - /// - /// The generic `F` parameter is the function type signature itself, e.g., `fn(String) -> u8`. - /// - /// # Examples - /// Here's how you might call this function: - /// ``` - /// type PrintFuncSignature = fn(&str) -> Result<(), &'static str>; - /// let mut space = 0; // this must persist throughout the print_func being called - /// let print_func: &PrintFuncSignature = mapped_pages.as_func(func_offset, &mut space).unwrap(); - /// print_func("hi"); - /// ``` - /// Because Rust has lexical lifetimes, the `space` variable must have a lifetime at least as long as the `print_func` variable, - /// meaning that `space` must still be in scope in order for `print_func` to be invoked. - /// - #[doc(hidden)] - pub fn as_func<'a, F>(&self, offset: usize, space: &'a mut usize) -> Result<&'a F, &'static str> { - let size = mem::size_of::(); - if true { - #[cfg(not(downtime_eval))] - debug!("MappedPages::as_func(): requested {} with size {} at offset {}, MappedPages size {}!", - core::any::type_name::(), - size, offset, self.size_in_bytes() - ); - } - - // check flags to make sure these pages are executable (otherwise a page fault would occur when this func is called) - if !self.flags.is_executable() { - error!("MappedPages::as_func(): requested {}, but MappedPages weren't executable (flags: {:?})", - core::any::type_name::(), - self.flags - ); - return Err("as_func(): MappedPages were not executable"); - } - - // check that size of the type F fits within the size of the mapping - let end = offset + size; - if end > self.size_in_bytes() { - error!("MappedPages::as_func(): requested type {} with size {} at offset {}, which is too large for MappedPages of size {}!", - core::any::type_name::(), - size, offset, self.size_in_bytes() - ); - return Err("requested type and offset would not fit within the MappedPages bounds"); - } - - *space = self.pages.start_address().value() + offset; - - // SAFE: we guarantee the size and lifetime are within that of this MappedPages object - let t: &'a F = unsafe { - mem::transmute(space) - }; - - Ok(t) - } } impl Drop for MappedPages { diff --git a/kernel/nano_core/src/lib.rs b/kernel/nano_core/src/lib.rs index 786b7dd889..aaefbedd64 100644 --- a/kernel/nano_core/src/lib.rs +++ b/kernel/nano_core/src/lib.rs @@ -191,8 +191,7 @@ pub extern "C" fn nano_core_start( } // if in loadable mode, parse the crates we always need: the core library (Rust no_std lib), the panic handlers, and the captain - #[cfg(loadable)] - { + #[cfg(loadable)] { use mod_mgmt::CrateNamespace; println_raw!("nano_core_start(): loading the \"captain\" crate..."); let (captain_file, _ns) = try_exit!(CrateNamespace::get_crate_object_file_starting_with(default_namespace, "captain-").ok_or("couldn't find the singular \"captain\" crate object file")); @@ -206,14 +205,12 @@ pub extern "C" fn nano_core_start( // at this point, we load and jump directly to the Captain, which will take it from here. // That's it, the nano_core is done! That's really all it does! println_raw!("nano_core_start(): invoking the captain..."); - #[cfg(not(loadable))] - { + #[cfg(not(loadable))] { try_exit!( captain::init(kernel_mmi_ref, identity_mapped_pages, stack, ap_realmode_begin, ap_realmode_end) ); } - #[cfg(loadable)] - { + #[cfg(loadable)] { use alloc::vec::Vec; use memory::{MmiRef, MappedPages}; @@ -225,10 +222,7 @@ pub extern "C" fn nano_core_start( info!("The nano_core (in loadable mode) is invoking the captain init function: {:?}", section.name); type CaptainInitFunc = fn(MmiRef, Vec, stack::Stack, VirtualAddress, VirtualAddress) -> Result<(), &'static str>; - let mut space = 0; - let func: &CaptainInitFunc = { - try_exit!(section.mapped_pages.lock().as_func(section.mapped_pages_offset, &mut space)) - }; + let func: &CaptainInitFunc = try_exit!(section.as_func()); try_exit!( func(kernel_mmi_ref, identity_mapped_pages, stack, ap_realmode_begin, ap_realmode_end) diff --git a/kernel/panic_entry/src/lib.rs b/kernel/panic_entry/src/lib.rs index 1310f37877..64f0c193c3 100644 --- a/kernel/panic_entry/src/lib.rs +++ b/kernel/panic_entry/src/lib.rs @@ -32,12 +32,10 @@ fn panic_entry_point(info: &PanicInfo) -> ! { let kernel_mmi_ref = memory::get_kernel_mmi_ref(); let res = if kernel_mmi_ref.is_some() { // proceed with calling the panic_wrapper, but don't shutdown with try_exit() if errors occur here - #[cfg(not(loadable))] - { + #[cfg(not(loadable))] { panic_wrapper::panic_wrapper(info) } - #[cfg(loadable)] - { + #[cfg(loadable)] { // An internal function for calling the panic_wrapper, but returning errors along the way. // We must make sure to not hold any locks when invoking the panic_wrapper function. fn invoke_panic_wrapper(info: &PanicInfo) -> Result<(), &'static str> { @@ -48,10 +46,7 @@ fn panic_entry_point(info: &PanicInfo) -> ! { .and_then(|namespace| namespace.get_symbol_starting_with(PANIC_WRAPPER_SYMBOL).upgrade()) .ok_or("Couldn't get single symbol matching \"panic_wrapper::panic_wrapper::\"")? }; - let mut space = 0; - let func: &PanicWrapperFunc = { - section.mapped_pages.lock().as_func(section.mapped_pages_offset, &mut space)? - }; + let func: &PanicWrapperFunc = section.as_func()?; func(info) } @@ -99,12 +94,10 @@ extern "C" fn rust_eh_personality() -> ! { /// but does so dynamically in loadable mode. #[no_mangle] extern "C" fn _Unwind_Resume(arg: usize) -> ! { - #[cfg(not(loadable))] - { + #[cfg(not(loadable))] { unwind::unwind_resume(arg) } - #[cfg(loadable)] - { + #[cfg(loadable)] { // An internal function for calling the real unwind_resume function, but returning errors along the way. // We must make sure to not hold any locks when invoking the function. fn invoke_unwind_resume(arg: usize) -> Result<(), &'static str> { @@ -115,11 +108,7 @@ extern "C" fn _Unwind_Resume(arg: usize) -> ! { .and_then(|namespace| namespace.get_symbol_starting_with(UNWIND_RESUME_SYMBOL).upgrade()) .ok_or("Couldn't get single symbol matching \"unwind::unwind_resume::\"")? }; - let mut space = 0; - let func: &UnwindResumeFunc = { - section.mapped_pages.lock().as_func(section.mapped_pages_offset, &mut space)? - }; - + let func: &UnwindResumeFunc = section.as_func()?; func(arg) } match invoke_unwind_resume(arg) { diff --git a/kernel/simd_personality/src/lib.rs b/kernel/simd_personality/src/lib.rs index 81b47214f2..16fa7f28d0 100644 --- a/kernel/simd_personality/src/lib.rs +++ b/kernel/simd_personality/src/lib.rs @@ -159,10 +159,8 @@ fn internal_setup_simd_personality(simd_ext: SimdExt) -> Result<(), &'static str let section_ref1 = simd_app_namespace.get_symbol_starting_with("simd_test::test1::") .upgrade() .ok_or("no single symbol matching \"simd_test::test1\"")?; - let mut space1 = 0; - let (mapped_pages1, mapped_pages_offset1) = (section_ref1.mapped_pages.clone(), section_ref1.mapped_pages_offset); - let func1: &SimdTestFunc = mapped_pages1.lock().as_func(mapped_pages_offset1, &mut space1)?; - let _task1 = spawn::new_task_builder(func1, ()) + let func1: &SimdTestFunc = section_ref1.as_func()?; + let _task1 = spawn::new_task_builder(*func1, ()) .name(format!("simd_test_1-{}", simd_app_namespace.name())) .pin_on_core(this_core) .simd(simd_ext) @@ -173,10 +171,8 @@ fn internal_setup_simd_personality(simd_ext: SimdExt) -> Result<(), &'static str let section_ref2 = simd_app_namespace.get_symbol_starting_with("simd_test::test2::") .upgrade() .ok_or("no single symbol matching \"simd_test::test2\"")?; - let mut space2 = 0; - let (mapped_pages2, mapped_pages_offset2) = (section_ref2.mapped_pages.clone(), section_ref2.mapped_pages_offset); - let func: &SimdTestFunc = mapped_pages2.lock().as_func(mapped_pages_offset2, &mut space2)?; - let _task2 = spawn::new_task_builder(func, ()) + let func2: &SimdTestFunc = section_ref2.as_func()?; + let _task2 = spawn::new_task_builder(*func2, ()) .name(format!("simd_test_2-{}", simd_app_namespace.name())) .pin_on_core(this_core) .simd(simd_ext) @@ -187,10 +183,8 @@ fn internal_setup_simd_personality(simd_ext: SimdExt) -> Result<(), &'static str let section_ref3 = simd_app_namespace.get_symbol_starting_with("simd_test::test_short::") .upgrade() .ok_or("no single symbol matching \"simd_test::test_short\"")?; - let mut space3 = 0; - let (mapped_pages3, mapped_pages_offset3) = (section_ref3.mapped_pages.clone(), section_ref3.mapped_pages_offset); - let func: &SimdTestFunc = mapped_pages3.lock().as_func(mapped_pages_offset3, &mut space3)?; - let _task3 = spawn::new_task_builder(func, ()) + let func3: &SimdTestFunc = section_ref3.as_func()?; + let _task3 = spawn::new_task_builder(*func3, ()) .name(format!("simd_test_short-{}", simd_app_namespace.name())) .pin_on_core(this_core) .simd(simd_ext) diff --git a/kernel/spawn/src/lib.rs b/kernel/spawn/src/lib.rs index d4cc79c0dd..8cc504d980 100755 --- a/kernel/spawn/src/lib.rs +++ b/kernel/spawn/src/lib.rs @@ -163,12 +163,7 @@ pub fn new_application_task_builder( }; let main_func_sec = main_func_sec_opt.ok_or("spawn::new_application_task_builder(): couldn't find \"main\" function, expected function name like \"::main::\"\ --> Is this an app-level library or kernel crate? (Note: you cannot spawn a library crate with no main function)")?; - - let mut space: usize = 0; // must live as long as main_func, see MappedPages::as_func() - let main_func = { - let mapped_pages = main_func_sec.mapped_pages.lock(); - mapped_pages.as_func::(main_func_sec.mapped_pages_offset, &mut space)? - }; + let main_func = main_func_sec.as_func::()?; // Create the underlying task builder. // Give it a default name based on the app crate's name, but that can be changed later.