Update host-side error handling

- Update initialized_multi_use.rs to use new Result-like error handling
- Update mem/mgr.rs to handle host function errors properly
- Update sandbox/outb.rs for new error propagation pattern

Signed-off-by: Ludvig Liljenberg <[email protected]>

Author: ludfjig

src/hyperlight_host/src/mem/mgr.rs

@@ -16,11 +16,11 @@ limitations under the License.
 
 use std::cmp::Ordering;
 
+use flatbuffers::FlatBufferBuilder;
 use hyperlight_common::flatbuffer_wrappers::function_call::{
     FunctionCall, validate_guest_function_call_buffer,
 };
-use hyperlight_common::flatbuffer_wrappers::function_types::ReturnValue;
-use hyperlight_common::flatbuffer_wrappers::guest_error::GuestError;
+use hyperlight_common::flatbuffer_wrappers::function_types::FunctionCallResult;
 use hyperlight_common::flatbuffer_wrappers::guest_log_data::GuestLogData;
 use hyperlight_common::flatbuffer_wrappers::host_function_details::HostFunctionDetails;
 use tracing::{Span, instrument};
@@ -501,18 +501,19 @@ impl SandboxMemoryManager<HostSharedMemory> {
         )
     }
 
-    /// Writes a function call result to memory
+    /// Writes a host function call result to memory
     #[instrument(err(Debug), skip_all, parent = Span::current(), level= "Trace")]
-    pub(crate) fn write_response_from_host_method_call(&mut self, res: &ReturnValue) -> Result<()> {
-        let function_call_ret_val_buffer = Vec::<u8>::try_from(res).map_err(|_| {
-            new_error!(
-                "write_response_from_host_method_call: failed to convert ReturnValue to Vec<u8>"
-            )
-        })?;
+    pub(crate) fn write_response_from_host_function_call(
+        &mut self,
+        res: &FunctionCallResult,
+    ) -> Result<()> {
+        let mut builder = FlatBufferBuilder::new();
+        let data = res.encode(&mut builder);
+
         self.shared_mem.push_buffer(
             self.layout.input_data_buffer_offset,
             self.layout.sandbox_memory_config.get_input_data_size(),
-            function_call_ret_val_buffer.as_slice(),
+            data,
         )
     }
 
@@ -533,10 +534,11 @@ impl SandboxMemoryManager<HostSharedMemory> {
         )
     }
 
-    /// Reads a function call result from memory
+    /// Reads a function call result from memory.
+    /// A function call result can be either an error or a successful return value.
     #[instrument(err(Debug), skip_all, parent = Span::current(), level= "Trace")]
-    pub(crate) fn get_guest_function_call_result(&mut self) -> Result<ReturnValue> {
-        self.shared_mem.try_pop_buffer_into::<ReturnValue>(
+    pub(crate) fn get_guest_function_call_result(&mut self) -> Result<FunctionCallResult> {
+        self.shared_mem.try_pop_buffer_into::<FunctionCallResult>(
             self.layout.output_data_buffer_offset,
             self.layout.sandbox_memory_config.get_output_data_size(),
         )
@@ -551,14 +553,6 @@ impl SandboxMemoryManager<HostSharedMemory> {
         )
     }
 
-    /// Get the guest error data
-    pub(crate) fn get_guest_error(&mut self) -> Result<GuestError> {
-        self.shared_mem.try_pop_buffer_into::<GuestError>(
-            self.layout.output_data_buffer_offset,
-            self.layout.sandbox_memory_config.get_output_data_size(),
-        )
-    }
-
     pub(crate) fn clear_io_buffers(&mut self) {
         // Clear the output data buffer
         loop {

src/hyperlight_host/src/sandbox/initialized_multi_use.rs

@@ -28,14 +28,14 @@ use hyperlight_common::flatbuffer_wrappers::function_call::{FunctionCall, Functi
 use hyperlight_common::flatbuffer_wrappers::function_types::{
     ParameterValue, ReturnType, ReturnValue,
 };
+use hyperlight_common::flatbuffer_wrappers::guest_error::ErrorCode;
 use hyperlight_common::flatbuffer_wrappers::util::estimate_flatbuffer_capacity;
 use tracing::{Span, instrument};
 
 use super::Callable;
 use super::host_funcs::FunctionRegistry;
 use super::snapshot::Snapshot;
-use crate::HyperlightError::SnapshotSandboxMismatch;
-use crate::func::guest_err::check_for_guest_error;
+use crate::HyperlightError::{self, SnapshotSandboxMismatch};
 use crate::func::{ParameterTuple, SupportedReturnType};
 use crate::hypervisor::{Hypervisor, InterruptHandle};
 #[cfg(unix)]
@@ -44,7 +44,9 @@ use crate::mem::memory_region::{MemoryRegion, MemoryRegionFlags};
 use crate::mem::mgr::SandboxMemoryManager;
 use crate::mem::ptr::RawPtr;
 use crate::mem::shared_mem::HostSharedMemory;
-use crate::metrics::maybe_time_and_emit_guest_call;
+use crate::metrics::{
+    METRIC_GUEST_ERROR, METRIC_GUEST_ERROR_LABEL_CODE, maybe_time_and_emit_guest_call,
+};
 use crate::{Result, log_then_return};
 
 /// Global counter for assigning unique IDs to sandboxes
@@ -410,9 +412,24 @@ impl MultiUseSandbox {
             )?;
 
             self.mem_mgr.check_stack_guard()?;
-            check_for_guest_error(&mut self.mem_mgr)?;
 
-            self.mem_mgr.get_guest_function_call_result()
+            let guest_result = self.mem_mgr.get_guest_function_call_result()?.into_inner();
+
+            match guest_result {
+                Ok(val) => Ok(val),
+                Err(guest_error) => {
+                    metrics::counter!(
+                        METRIC_GUEST_ERROR,
+                        METRIC_GUEST_ERROR_LABEL_CODE => (guest_error.code as u64).to_string()
+                    )
+                    .increment(1);
+
+                    Err(match guest_error.code {
+                        ErrorCode::StackOverflow => HyperlightError::StackOverflow(),
+                        _ => HyperlightError::GuestError(guest_error.code, guest_error.message),
+                    })
+                }
+            }
         })();
 
         // In the happy path we do not need to clear io-buffers from the host because:
@@ -493,6 +510,36 @@ mod tests {
     use crate::sandbox::SandboxConfiguration;
     use crate::{GuestBinary, HyperlightError, MultiUseSandbox, Result, UninitializedSandbox};
 
+    /// Make sure input/output buffers are properly reset after guest call (with host call)
+    #[test]
+    #[ignore = "added this test before fixing bug"]
+    fn host_func_error() {
+        let path = simple_guest_as_string().unwrap();
+        let mut sandbox = UninitializedSandbox::new(GuestBinary::FilePath(path), None).unwrap();
+        sandbox
+            .register("HostError", || -> Result<()> {
+                Err(HyperlightError::Error("hi".to_string()))
+            })
+            .unwrap();
+        let mut sandbox = sandbox.evolve().unwrap();
+
+        // will exhaust io if leaky
+        for _ in 0..1000 {
+            let result = sandbox
+                .call::<i64>(
+                    "CallGivenParamlessHostFuncThatReturnsI64",
+                    "HostError".to_string(),
+                )
+                .unwrap_err();
+
+            assert!(
+                matches!(result, HyperlightError::Error(ref msg) if msg == "hi"),
+                "Expected HyperlightError::Error('hi'), got {:?}",
+                result
+            );
+        }
+    }
+
     /// Make sure input/output buffers are properly reset after guest call (with host call)
     #[test]
     fn io_buffer_reset() {
@@ -606,6 +653,9 @@ mod tests {
     #[ignore]
     #[cfg(target_os = "linux")]
     fn test_violate_seccomp_filters() -> Result<()> {
+        #[cfg(feature = "seccomp")]
+        use hyperlight_common::flatbuffer_wrappers::guest_error::ErrorCode;
+
         fn make_get_pid_syscall() -> Result<u64> {
             let pid = unsafe { libc::syscall(libc::SYS_getpid) };
             Ok(pid as u64)
@@ -629,7 +679,9 @@ mod tests {
             match res {
                 Ok(_) => panic!("Expected to fail due to seccomp violation"),
                 Err(e) => match e {
-                    HyperlightError::DisallowedSyscall => {}
+                    HyperlightError::GuestError(t, msg)
+                        if t == ErrorCode::HostFunctionError
+                            && msg.contains("Seccomp filter trapped on disallowed syscall") => {}
                     _ => panic!("Expected DisallowedSyscall error: {}", e),
                 },
             }

src/hyperlight_host/src/sandbox/outb.rs

@@ -22,8 +22,8 @@ use std::sync::{Arc, Mutex};
 use fallible_iterator::FallibleIterator;
 #[cfg(feature = "unwind_guest")]
 use framehop::Unwinder;
-use hyperlight_common::flatbuffer_wrappers::function_types::ParameterValue;
-use hyperlight_common::flatbuffer_wrappers::guest_error::ErrorCode;
+use hyperlight_common::flatbuffer_wrappers::function_types::{FunctionCallResult, ParameterValue};
+use hyperlight_common::flatbuffer_wrappers::guest_error::{ErrorCode, GuestError};
 use hyperlight_common::flatbuffer_wrappers::guest_log_data::GuestLogData;
 use hyperlight_common::outb::{Exception, OutBAction};
 #[cfg(feature = "trace_guest")]
@@ -283,8 +283,12 @@ pub(crate) fn handle_outb(
             let res = host_funcs
                 .try_lock()
                 .map_err(|e| new_error!("Error locking at {}:{}: {}", file!(), line!(), e))?
-                .call_host_function(&name, args)?;
-            mem_mgr.write_response_from_host_method_call(&res)?; // push input buffers
+                .call_host_function(&name, args)
+                .map_err(|e| GuestError::new(ErrorCode::HostFunctionError, e.to_string()));
+
+            let func_result = FunctionCallResult::new(res);
+
+            mem_mgr.write_response_from_host_function_call(&func_result)?;
 
             Ok(())
         }