diff --git a/src/hyperlight_common/src/clippy.toml b/src/hyperlight_common/src/clippy.toml new file mode 100644 index 000000000..cda217749 --- /dev/null +++ b/src/hyperlight_common/src/clippy.toml @@ -0,0 +1,7 @@ +disallowed-macros = [ + { path = "std::assert", reason = "no asserts in release builds" }, + { path = "std::assert_eq", reason = "no asserts in release builds" }, + { path = "std::assert_ne", reason = "no asserts in release builds" }, + { path = "std::assert_true", reason = "no asserts in release builds" }, + { path = "std::assert_false", reason = "no asserts in release builds" }, +] diff --git a/src/hyperlight_common/src/lib.rs b/src/hyperlight_common/src/lib.rs index e2e75cc9e..f05baa9f9 100644 --- a/src/hyperlight_common/src/lib.rs +++ b/src/hyperlight_common/src/lib.rs @@ -14,6 +14,9 @@ See the License for the specific language governing permissions and limitations under the License. */ +#![cfg_attr(not(any(test, debug_assertions)), warn(clippy::panic))] +#![cfg_attr(not(any(test, debug_assertions)), warn(clippy::expect_used))] +#![cfg_attr(not(any(test, debug_assertions)), warn(clippy::unwrap_used))] // We use Arbitrary during fuzzing, which requires std #![cfg_attr(not(feature = "fuzzing"), no_std)] @@ -26,6 +29,7 @@ pub mod flatbuffer_wrappers; dead_code, unused_imports, clippy::all, + clippy::unwrap_used, unsafe_op_in_unsafe_fn, non_camel_case_types )] diff --git a/src/hyperlight_host/src/clippy.toml b/src/hyperlight_host/src/clippy.toml new file mode 100644 index 000000000..cda217749 --- /dev/null +++ b/src/hyperlight_host/src/clippy.toml @@ -0,0 +1,7 @@ +disallowed-macros = [ + { path = "std::assert", reason = "no asserts in release builds" }, + { path = "std::assert_eq", reason = "no asserts in release builds" }, + { path = "std::assert_ne", reason = "no asserts in release builds" }, + { path = "std::assert_true", reason = "no asserts in release builds" }, + { path = "std::assert_false", reason = "no asserts in release builds" }, +] diff --git a/src/hyperlight_host/src/hypervisor/hypervisor_handler.rs b/src/hyperlight_host/src/hypervisor/hypervisor_handler.rs index a66ebc608..f602b840e 100644 --- a/src/hyperlight_host/src/hypervisor/hypervisor_handler.rs +++ b/src/hyperlight_host/src/hypervisor/hypervisor_handler.rs @@ -246,7 +246,11 @@ impl HypervisorHandler { #[cfg(target_os = "windows")] let in_process = sandbox_memory_manager.is_in_process(); - *self.execution_variables.shm.try_lock().unwrap() = Some(sandbox_memory_manager); + *self + .execution_variables + .shm + .try_lock() + .map_err(|e| new_error!("Failed to lock shm: {}", e))? = Some(sandbox_memory_manager); // Other than running initialization and code execution, the handler thread also handles // cancellation. When we need to cancel the execution there are 2 possible cases @@ -299,13 +303,13 @@ impl HypervisorHandler { HypervisorHandlerAction::Initialise => { { hv = Some(set_up_hypervisor_partition( - execution_variables.shm.try_lock().unwrap().deref_mut().as_mut().unwrap(), + execution_variables.shm.try_lock().map_err(|e| new_error!("Failed to lock shm: {}", e))?.deref_mut().as_mut().ok_or_else(|| new_error!("shm not set"))?, configuration.outb_handler.clone(), #[cfg(gdb)] &debug_info, )?); } - let hv = hv.as_mut().unwrap(); + let hv = hv.as_mut().ok_or_else(|| new_error!("Hypervisor not set"))?; #[cfg(target_os = "windows")] if !in_process { @@ -386,7 +390,7 @@ impl HypervisorHandler { } } HypervisorHandlerAction::DispatchCallFromHost(function_name) => { - let hv = hv.as_mut().unwrap(); + let hv = hv.as_mut().ok_or_else(|| new_error!("Hypervisor not initialized"))?; // Lock to indicate an action is being performed in the hypervisor execution_variables.running.store(true, Ordering::SeqCst); @@ -647,6 +651,8 @@ impl HypervisorHandler { // If the thread has finished, we try to join it and return the error if it has one let res = handle.join(); if res.as_ref().is_ok_and(|inner_res| inner_res.is_err()) { + #[allow(clippy::unwrap_used)] + // We know that the thread has finished and that the inner result is an error, so we can safely unwrap the result and the contained err return Err(res.unwrap().unwrap_err()); } Err(HyperlightError::HypervisorHandlerMessageReceiveTimedout()) @@ -757,7 +763,7 @@ impl HypervisorHandler { if thread_id == u64::MAX { log_then_return!("Failed to get thread id to signal thread"); } - let mut count: i32 = 0; + let mut count: u128 = 0; // We need to send the signal multiple times in case the thread was between checking if it // should be cancelled and entering the run loop @@ -771,7 +777,7 @@ impl HypervisorHandler { while !self.execution_variables.run_cancelled.load() { count += 1; - if count > number_of_iterations.try_into().unwrap() { + if count > number_of_iterations { break; } @@ -797,7 +803,8 @@ impl HypervisorHandler { // partition handle only set when running in-hypervisor (not in-process) unsafe { WHvCancelRunVirtualProcessor( - self.execution_variables.get_partition_handle()?.unwrap(), // safe unwrap + #[allow(clippy::unwrap_used)] + self.execution_variables.get_partition_handle()?.unwrap(), // safe unwrap as we checked is some 0, 0, ) diff --git a/src/hyperlight_host/src/hypervisor/surrogate_process_manager.rs b/src/hyperlight_host/src/hypervisor/surrogate_process_manager.rs index d0358087c..4f3291034 100644 --- a/src/hyperlight_host/src/hypervisor/surrogate_process_manager.rs +++ b/src/hyperlight_host/src/hypervisor/surrogate_process_manager.rs @@ -23,7 +23,7 @@ use std::path::{Path, PathBuf}; use crossbeam_channel::{unbounded, Receiver, Sender}; use hyperlight_common::mem::PAGE_SIZE_USIZE; use rust_embed::RustEmbed; -use tracing::{info, instrument, Span}; +use tracing::{error, info, instrument, Span}; use windows::core::{s, PCSTR}; use windows::Win32::Foundation::HANDLE; use windows::Win32::Security::SECURITY_ATTRIBUTES; @@ -251,27 +251,42 @@ impl Drop for SurrogateProcessManager { #[instrument(skip_all, parent = Span::current(), level= "Trace")] fn drop(&mut self) { let handle: HANDLE = self.job_handle.into(); - unsafe { + if unsafe { // Terminating the job object will terminate all the surrogate // processes. TerminateJobObject(handle, 0) } - .expect("surrogate job objects were not all terminated error:"); + .is_err() + { + error!("surrogate job objects were not all terminated"); + } } } lazy_static::lazy_static! { // see the large comment inside `SurrogateProcessManager` describing // our reasoning behind using `lazy_static`. - static ref SURROGATE_PROCESSES_MANAGER: SurrogateProcessManager = - SurrogateProcessManager::new().unwrap(); + static ref SURROGATE_PROCESSES_MANAGER: std::result::Result = + match SurrogateProcessManager::new() { + Ok(manager) => Ok(manager), + Err(e) => { + error!("Failed to create SurrogateProcessManager: {:?}", e); + Err("Failed to create SurrogateProcessManager") + } + }; } /// Gets the singleton SurrogateProcessManager. This should be called when a new HyperV on Windows Driver is created. #[instrument(err(Debug), skip_all, parent = Span::current(), level= "Trace")] pub(crate) fn get_surrogate_process_manager() -> Result<&'static SurrogateProcessManager> { - Ok(&SURROGATE_PROCESSES_MANAGER) + match &*SURROGATE_PROCESSES_MANAGER { + Ok(manager) => Ok(manager), + Err(e) => { + error!("Failed to get SurrogateProcessManager: {:?}", e); + Err(new_error!("Failed to get SurrogateProcessManager {}", e)) + } + } } // Creates a job object that will terminate all the surrogate processes when the struct instance is dropped. diff --git a/src/hyperlight_host/src/hypervisor/windows_hypervisor_platform.rs b/src/hyperlight_host/src/hypervisor/windows_hypervisor_platform.rs index 18eda5f23..810c6da77 100644 --- a/src/hyperlight_host/src/hypervisor/windows_hypervisor_platform.rs +++ b/src/hyperlight_host/src/hypervisor/windows_hypervisor_platform.rs @@ -104,24 +104,28 @@ impl VMPartition { }; regions.iter().try_for_each(|region| unsafe { + let flags = region + .flags + .iter() + .map(|flag| match flag { + MemoryRegionFlags::NONE => Ok(WHvMapGpaRangeFlagNone), + MemoryRegionFlags::READ => Ok(WHvMapGpaRangeFlagRead), + MemoryRegionFlags::WRITE => Ok(WHvMapGpaRangeFlagWrite), + MemoryRegionFlags::EXECUTE => Ok(WHvMapGpaRangeFlagExecute), + MemoryRegionFlags::STACK_GUARD => Ok(WHvMapGpaRangeFlagNone), + _ => Err(new_error!("Invalid Memory Region Flag")), + }) + .collect::>>()? + .iter() + .fold(WHvMapGpaRangeFlagNone, |acc, flag| acc | *flag); // collect using bitwise OR + let res = whvmapgparange2_func( self.0, process_handle, region.host_region.start as *const c_void, region.guest_region.start as u64, (region.guest_region.end - region.guest_region.start) as u64, - region - .flags - .iter() - .filter_map(|flag| match flag { - MemoryRegionFlags::NONE => Some(WHvMapGpaRangeFlagNone), - MemoryRegionFlags::READ => Some(WHvMapGpaRangeFlagRead), - MemoryRegionFlags::WRITE => Some(WHvMapGpaRangeFlagWrite), - MemoryRegionFlags::EXECUTE => Some(WHvMapGpaRangeFlagExecute), - MemoryRegionFlags::STACK_GUARD => None, - _ => panic!("Invalid flag"), - }) - .fold(WHvMapGpaRangeFlagNone, |acc, flag| acc | flag), // collect using bitwise OR, + flags, ); if res.is_err() { return Err(new_error!("Call to WHvMapGpaRange2 failed")); @@ -161,6 +165,8 @@ pub unsafe fn try_load_whv_map_gpa_range2() -> Result { return Err(new_error!("{}", e)); } + #[allow(clippy::unwrap_used)] + // We know this will succeed because we just checked for an error above let library = library.unwrap(); let address = unsafe { GetProcAddress(library, s!("WHvMapGpaRange2")) }; diff --git a/src/hyperlight_host/src/lib.rs b/src/hyperlight_host/src/lib.rs index b04929e31..1ae0b1801 100644 --- a/src/hyperlight_host/src/lib.rs +++ b/src/hyperlight_host/src/lib.rs @@ -17,6 +17,11 @@ limitations under the License. //! This crate contains an SDK that is used to execute specially- // compiled binaries within a very lightweight hypervisor environment. +#![cfg_attr(not(any(test, debug_assertions)), warn(clippy::panic))] +#![cfg_attr(not(any(test, debug_assertions)), warn(clippy::expect_used))] +#![cfg_attr(not(any(test, debug_assertions)), warn(clippy::unwrap_used))] +#![cfg_attr(any(test, debug_assertions), allow(clippy::disallowed_macros))] + use std::sync::Once; use log::info; diff --git a/src/hyperlight_host/src/mem/elf.rs b/src/hyperlight_host/src/mem/elf.rs index ec1918323..1ac96a2c0 100644 --- a/src/hyperlight_host/src/mem/elf.rs +++ b/src/hyperlight_host/src/mem/elf.rs @@ -52,20 +52,22 @@ impl ElfInfo { self.entry } pub(crate) fn get_base_va(&self) -> u64 { + #[allow(clippy::unwrap_used)] // guaranteed not to panic because of the check in new() let min_phdr = self .phdrs .iter() .find(|phdr| phdr.p_type == PT_LOAD) - .unwrap(); // guaranteed not to panic because of the check in new() + .unwrap(); min_phdr.p_vaddr } pub(crate) fn get_va_size(&self) -> usize { + #[allow(clippy::unwrap_used)] // guaranteed not to panic because of the check in new() let max_phdr = self .phdrs .iter() .rev() .find(|phdr| phdr.p_type == PT_LOAD) - .unwrap(); // guaranteed not to panic because of the check in new() + .unwrap(); (max_phdr.p_vaddr + max_phdr.p_memsz - self.get_base_va()) as usize } pub(crate) fn load_at(&self, load_addr: usize, target: &mut [u8]) -> Result<()> { diff --git a/src/hyperlight_host/src/mem/loaded_lib.rs b/src/hyperlight_host/src/mem/loaded_lib.rs index f579d2e13..9dcf4d939 100644 --- a/src/hyperlight_host/src/mem/loaded_lib.rs +++ b/src/hyperlight_host/src/mem/loaded_lib.rs @@ -54,7 +54,7 @@ impl LoadedLib { // arc reference is dropped, but before the destructor is executed. This however // is ok, as it means that the old library is not going to be used anymore and // we can use it instead. - let mut lock = LOADED_LIB.lock().unwrap(); + let mut lock = LOADED_LIB.lock()?; if lock.upgrade().is_some() { // An owning copy of the loaded library still exists somewhere, // we can't load a new library yet diff --git a/src/hyperlight_host/src/mem/memory_region.rs b/src/hyperlight_host/src/mem/memory_region.rs index 6956d9635..ced79f214 100644 --- a/src/hyperlight_host/src/mem/memory_region.rs +++ b/src/hyperlight_host/src/mem/memory_region.rs @@ -202,6 +202,8 @@ impl MemoryRegionVecBuilder { return guest_end - self.guest_base_phys_addr; } + #[allow(clippy::unwrap_used)] + // we know this is safe because we check if the regions are empty above let last_region = self.regions.last().unwrap(); let new_region = MemoryRegion { guest_region: last_region.guest_region.end..last_region.guest_region.end + size, diff --git a/src/hyperlight_host/src/mem/mgr.rs b/src/hyperlight_host/src/mem/mgr.rs index ba00d94a8..2c4f38e83 100644 --- a/src/hyperlight_host/src/mem/mgr.rs +++ b/src/hyperlight_host/src/mem/mgr.rs @@ -300,6 +300,7 @@ where if last.is_none() { log_then_return!(NoMemorySnapshot); } + #[allow(clippy::unwrap_used)] // We know that last is not None because we checked it above let snapshot = last.unwrap(); snapshot.restore_from_snapshot(&mut self.shared_mem) } diff --git a/src/hyperlight_host/src/mem/pe/base_relocations.rs b/src/hyperlight_host/src/mem/pe/base_relocations.rs index 6dec8fb34..bea511557 100644 --- a/src/hyperlight_host/src/mem/pe/base_relocations.rs +++ b/src/hyperlight_host/src/mem/pe/base_relocations.rs @@ -91,6 +91,7 @@ impl<'a> Iterator for BaseRelocations<'a> { let word = u16::from_le_bytes(block); // Shift the word over so that we are only reading a number from the first (most significant) 4 bits + #[allow(clippy::unwrap_used)] // this is safe as we are only reading 4 bits from a u16 let typ = u8::try_from(word >> 12).unwrap(); // Set the first 4 bits to 0 so that we only use the lower 12 bits for the location of the RVA in the PE file @@ -132,9 +133,10 @@ pub(super) fn get_base_relocations( // Process each block of relocations until we have processed the expected amount of relocation data while size_processed < table_size { // Read the header for the block, which is the same format as a DataDirectory so I'm reusing its parse function - let block_header = - goblin::pe::data_directories::DataDirectory::parse(payload, &mut next_block_offset) - .expect("oops"); + let block_header = goblin::pe::data_directories::DataDirectory::parse( + payload, + &mut next_block_offset, + )?; // All relocation blocks in a page contain offsets against this address let page_virtual_address = block_header.virtual_address; diff --git a/src/hyperlight_host/src/mem/pe/pe_info.rs b/src/hyperlight_host/src/mem/pe/pe_info.rs index ae8ea3ed9..5bd3cf7d1 100644 --- a/src/hyperlight_host/src/mem/pe/pe_info.rs +++ b/src/hyperlight_host/src/mem/pe/pe_info.rs @@ -67,10 +67,10 @@ impl PEInfo { log_then_return!("unsupported PE file, not an executable image") } - let optional_header = pe - .header - .optional_header - .expect("unsupported PE file, missing optional header entry"); + let optional_header = match pe.header.optional_header { + Some(optional_header) => optional_header, + None => log_then_return!("unsupported PE file, no optional header found"), + }; // check that the PE file was built with the option /DYNAMICBASE @@ -117,10 +117,10 @@ impl PEInfo { // we are going to take care of the data section if name == ".data" { // Make sure we fail if we enter this block more than once - assert_eq!( - data_section_additional_bytes, 0, - "Hyperlight currently only supports one .data section" - ); + + if data_section_additional_bytes > 0 { + log_then_return!("Hyperlight currently only supports one .data section") + } data_section_raw_pointer = section.pointer_to_raw_data; data_section_additional_bytes = virtual_size - raw_size; @@ -251,8 +251,7 @@ impl PEInfo { } cur.set_position(patch.offset as u64); - cur.write_all(&patch.relocated_virtual_address.to_le_bytes()) - .expect("failed to write patch to pe file contents"); + cur.write_all(&patch.relocated_virtual_address.to_le_bytes())?; applied += 1; } @@ -279,8 +278,7 @@ impl PEInfo { } let relocations = - base_relocations::get_base_relocations(&self.payload, &self.reloc_section) - .expect("error parsing base relocations"); + base_relocations::get_base_relocations(&self.payload, &self.reloc_section)?; let mut patches = Vec::with_capacity(relocations.len()); for reloc in relocations { diff --git a/src/hyperlight_host/src/mem/shared_mem.rs b/src/hyperlight_host/src/mem/shared_mem.rs index d39b04204..fe6ba8dd9 100644 --- a/src/hyperlight_host/src/mem/shared_mem.rs +++ b/src/hyperlight_host/src/mem/shared_mem.rs @@ -328,10 +328,13 @@ impl ExclusiveSharedMemory { .checked_add(2 * PAGE_SIZE_USIZE) // guard page around the memory .ok_or_else(|| new_error!("Memory required for sandbox exceeded usize::MAX"))?; - assert!( - total_size % PAGE_SIZE_USIZE == 0, - "shared memory must be a multiple of 4096" - ); + if total_size % PAGE_SIZE_USIZE != 0 { + return Err(new_error!( + "shared memory must be a multiple of {}", + PAGE_SIZE_USIZE + )); + } + // usize and isize are guaranteed to be the same size, and // isize::MAX should be positive, so this cast should be safe. if total_size > isize::MAX as usize { @@ -885,7 +888,7 @@ impl HostSharedMemory { buffer_size: usize, data: &[u8], ) -> Result<()> { - let stack_pointer_rel = self.read::(buffer_start_offset).unwrap() as usize; + let stack_pointer_rel = self.read::(buffer_start_offset)? as usize; let buffer_size_u64: u64 = buffer_size.try_into()?; if stack_pointer_rel > buffer_size || stack_pointer_rel < 8 { @@ -953,7 +956,7 @@ impl HostSharedMemory { // go back 8 bytes to get offset to element on top of stack let last_element_offset_rel: usize = - self.read::(last_element_offset_abs - 8).unwrap() as usize; + self.read::(last_element_offset_abs - 8)? as usize; // make it absolute let last_element_offset_abs = last_element_offset_rel + buffer_start_offset; diff --git a/src/hyperlight_host/src/metrics/int_gauge_vec.rs b/src/hyperlight_host/src/metrics/int_gauge_vec.rs index de96f2b25..d9e757429 100644 --- a/src/hyperlight_host/src/metrics/int_gauge_vec.rs +++ b/src/hyperlight_host/src/metrics/int_gauge_vec.rs @@ -16,7 +16,7 @@ limitations under the License. use prometheus::core::{AtomicI64, GenericGaugeVec}; use prometheus::register_int_gauge_vec_with_registry; -use tracing::{instrument, Span}; +use tracing::{error, instrument, Span}; use super::{ get_metric_opts, get_metrics_registry, GetHyperlightMetric, HyperlightMetric, @@ -44,50 +44,53 @@ impl IntGaugeVec { /// Increments a gauge by 1 #[instrument(skip_all, parent = Span::current(), level= "Trace")] pub fn inc(&self, label_vals: &[&str]) { - self.gauge - .get_metric_with_label_values(label_vals) - .unwrap() - .inc(); + match self.gauge.get_metric_with_label_values(label_vals) { + Ok(gauge) => gauge.inc(), + Err(e) => error!("Failed to get metric with label values: {}", e), + } } /// Decrements a gauge by 1 #[instrument(skip_all, parent = Span::current(), level= "Trace")] pub fn dec(&self, label_vals: &[&str]) { - self.gauge - .get_metric_with_label_values(label_vals) - .unwrap() - .dec(); + match self.gauge.get_metric_with_label_values(label_vals) { + Ok(gauge) => gauge.dec(), + Err(e) => error!("Failed to get metric with label values: {}", e), + } } /// Gets the value of a gauge #[instrument(skip_all, parent = Span::current(), level= "Trace")] pub fn get(&self, label_vals: &[&str]) -> i64 { - self.gauge - .get_metric_with_label_values(label_vals) - .unwrap() - .get() + match self.gauge.get_metric_with_label_values(label_vals) { + Ok(gauge) => gauge.get(), + Err(e) => { + error!("Failed to get metric with label values: {}", e); + 0 + } + } } /// Resets a gauge #[instrument(skip_all, parent = Span::current(), level= "Trace")] pub fn set(&self, label_vals: &[&str], val: i64) { - self.gauge - .get_metric_with_label_values(label_vals) - .unwrap() - .set(val); + match self.gauge.get_metric_with_label_values(label_vals) { + Ok(gauge) => gauge.set(val), + Err(e) => error!("Failed to get metric with label values: {}", e), + } } /// Adds a value to a gauge #[instrument(skip_all, parent = Span::current(), level= "Trace")] pub fn add(&self, label_vals: &[&str], val: i64) { - self.gauge - .get_metric_with_label_values(label_vals) - .unwrap() - .add(val); + match self.gauge.get_metric_with_label_values(label_vals) { + Ok(gauge) => gauge.add(val), + Err(e) => error!("Failed to get metric with label values: {}", e), + } } /// Subtracts a value from a gauge #[instrument(skip_all, parent = Span::current(), level= "Trace")] pub fn sub(&self, label_vals: &[&str], val: i64) { - self.gauge - .get_metric_with_label_values(label_vals) - .unwrap() - .sub(val); + match self.gauge.get_metric_with_label_values(label_vals) { + Ok(gauge) => gauge.sub(val), + Err(e) => error!("Failed to get metric with label values: {}", e), + } } } diff --git a/src/hyperlight_host/src/metrics/mod.rs b/src/hyperlight_host/src/metrics/mod.rs index 7ff3e6faf..1502128b5 100644 --- a/src/hyperlight_host/src/metrics/mod.rs +++ b/src/hyperlight_host/src/metrics/mod.rs @@ -259,6 +259,7 @@ fn get_histogram_opts(name: &str, help: &str, buckets: Vec) -> HistogramOpt opts.buckets(buckets) } +#[allow(clippy::disallowed_macros)] /// Provides functionality to help with testing Hyperlight Metrics pub mod tests { use std::collections::HashSet; diff --git a/src/hyperlight_host/src/sandbox/outb.rs b/src/hyperlight_host/src/sandbox/outb.rs index dd359ebb2..fa1c6e09c 100644 --- a/src/hyperlight_host/src/sandbox/outb.rs +++ b/src/hyperlight_host/src/sandbox/outb.rs @@ -137,7 +137,7 @@ fn handle_outb_impl( } OutBAction::Abort => { let guest_error = ErrorCode::from(byte); - let panic_context = mem_mgr.as_mut().read_guest_panic_context_data().unwrap(); + let panic_context = mem_mgr.as_mut().read_guest_panic_context_data()?; // trim off trailing \0 bytes if they exist let index_opt = panic_context.iter().position(|&x| x == 0x00); let trimmed = match index_opt { diff --git a/src/hyperlight_host/src/sandbox/uninitialized_evolve.rs b/src/hyperlight_host/src/sandbox/uninitialized_evolve.rs index f9e99d7aa..e962f0515 100644 --- a/src/hyperlight_host/src/sandbox/uninitialized_evolve.rs +++ b/src/hyperlight_host/src/sandbox/uninitialized_evolve.rs @@ -76,7 +76,9 @@ where { let dispatch_function_addr = hshm.as_ref().get_pointer_to_dispatch_function()?; - assert_ne!(dispatch_function_addr, 0); + if dispatch_function_addr == 0 { + return Err(new_error!("Dispatch function address is null")); + } hv_handler.set_dispatch_function_addr(RawPtr::from(dispatch_function_addr))?; } diff --git a/src/hyperlight_host/src/sandbox_state/sandbox.rs b/src/hyperlight_host/src/sandbox_state/sandbox.rs index 52f49eb5c..b6c172226 100644 --- a/src/hyperlight_host/src/sandbox_state/sandbox.rs +++ b/src/hyperlight_host/src/sandbox_state/sandbox.rs @@ -15,12 +15,11 @@ limitations under the License. */ use std::fmt::Debug; -use std::panic; use tracing::{instrument, Span}; use super::transition::TransitionMetadata; -use crate::Result; +use crate::{new_error, Result}; /// The minimal functionality of a Hyperlight sandbox. Most of the types /// and operations within this crate require `Sandbox` implementations. @@ -48,7 +47,9 @@ pub trait Sandbox: Sized + Debug { // The default implementation is provided so that types that implement Sandbox (e.g. JSSandbox) but do not need to implement this trait do not need to provide an implementation #[instrument(skip_all, parent = Span::current(), level= "Trace")] fn check_stack_guard(&self) -> Result { - panic!("check_stack_guard not implemented for this type"); + Err(new_error!( + "check_stack_guard not implemented for this type" + )) } } diff --git a/src/hyperlight_host/src/seccomp/guest.rs b/src/hyperlight_host/src/seccomp/guest.rs index 97aadccfa..2cc10d655 100644 --- a/src/hyperlight_host/src/seccomp/guest.rs +++ b/src/hyperlight_host/src/seccomp/guest.rs @@ -89,7 +89,7 @@ pub(crate) fn get_seccomp_filter_for_host_function_worker_thread( allowed_syscalls.into_iter().collect(), SeccompAction::Trap, // non-match syscall will kill the offending thread SeccompAction::Allow, // match syscall will be allowed - std::env::consts::ARCH.try_into().unwrap(), + std::env::consts::ARCH.try_into()?, ) .and_then(|filter| filter.try_into())?) }