fixup! Allow the guest to manage its own stack

Remove dead code and fix fuzz harness

Signed-off-by: Lucy Menon <168595099+syntactically@users.noreply.github.com>

Author: syntactically

fuzz/fuzz_targets/guest_trace.rs

@@ -70,7 +70,7 @@ fuzz_target!(
     init: {
         let mut cfg = SandboxConfiguration::default();
         // In local tests, 256 KiB seemed sufficient for deep recursion
-        cfg.set_stack_size(256 * 1024);
+        cfg.set_scratch_size(256 * 1024);
         let path = simple_guest_for_fuzzing_as_string().expect("Guest Binary Missing");
         let u_sbox = UninitializedSandbox::new(
             GuestBinary::FilePath(path),

src/hyperlight_common/src/flatbuffer_wrappers/guest_error.rs

@@ -35,7 +35,6 @@ pub enum ErrorCode {
     GispatchFunctionPointerNotSet = 6,
     OutbError = 7,
     UnknownError = 8,
-    StackOverflow = 9,
     GsCheckFailed = 10,
     TooManyGuestFunctions = 11,
     FailureInDlmalloc = 12,
@@ -59,7 +58,6 @@ impl From<ErrorCode> for FbErrorCode {
             ErrorCode::GispatchFunctionPointerNotSet => Self::GispatchFunctionPointerNotSet,
             ErrorCode::OutbError => Self::OutbError,
             ErrorCode::UnknownError => Self::UnknownError,
-            ErrorCode::StackOverflow => Self::StackOverflow,
             ErrorCode::GsCheckFailed => Self::GsCheckFailed,
             ErrorCode::TooManyGuestFunctions => Self::TooManyGuestFunctions,
             ErrorCode::FailureInDlmalloc => Self::FailureInDlmalloc,
@@ -86,7 +84,6 @@ impl From<FbErrorCode> for ErrorCode {
             }
             FbErrorCode::GispatchFunctionPointerNotSet => Self::GispatchFunctionPointerNotSet,
             FbErrorCode::OutbError => Self::OutbError,
-            FbErrorCode::StackOverflow => Self::StackOverflow,
             FbErrorCode::GsCheckFailed => Self::GsCheckFailed,
             FbErrorCode::TooManyGuestFunctions => Self::TooManyGuestFunctions,
             FbErrorCode::FailureInDlmalloc => Self::FailureInDlmalloc,
@@ -113,7 +110,6 @@ impl From<u64> for ErrorCode {
             6 => Self::GispatchFunctionPointerNotSet,
             7 => Self::OutbError,
             8 => Self::UnknownError,
-            9 => Self::StackOverflow,
             10 => Self::GsCheckFailed,
             11 => Self::TooManyGuestFunctions,
             12 => Self::FailureInDlmalloc,
@@ -138,7 +134,6 @@ impl From<ErrorCode> for u64 {
             ErrorCode::GispatchFunctionPointerNotSet => 6,
             ErrorCode::OutbError => 7,
             ErrorCode::UnknownError => 8,
-            ErrorCode::StackOverflow => 9,
             ErrorCode::GsCheckFailed => 10,
             ErrorCode::TooManyGuestFunctions => 11,
             ErrorCode::FailureInDlmalloc => 12,
@@ -164,7 +159,6 @@ impl From<ErrorCode> for String {
             ErrorCode::GispatchFunctionPointerNotSet => "GispatchFunctionPointerNotSet".to_string(),
             ErrorCode::OutbError => "OutbError".to_string(),
             ErrorCode::UnknownError => "UnknownError".to_string(),
-            ErrorCode::StackOverflow => "StackOverflow".to_string(),
             ErrorCode::GsCheckFailed => "GsCheckFailed".to_string(),
             ErrorCode::TooManyGuestFunctions => "TooManyGuestFunctions".to_string(),
             ErrorCode::FailureInDlmalloc => "FailureInDlmalloc".to_string(),

src/hyperlight_common/src/flatbuffers/hyperlight/generated/error_code_generated.rs

@@ -25,7 +25,7 @@ pub const ENUM_MAX_ERROR_CODE: u64 = 17;
     note = "Use associated constants instead. This will no longer be generated in 2021."
 )]
 #[allow(non_camel_case_types)]
-pub const ENUM_VALUES_ERROR_CODE: [ErrorCode; 17] = [
+pub const ENUM_VALUES_ERROR_CODE: [ErrorCode; 16] = [
     ErrorCode::NoError,
     ErrorCode::UnsupportedParameterType,
     ErrorCode::GuestFunctionNameNotProvided,
@@ -34,7 +34,6 @@ pub const ENUM_VALUES_ERROR_CODE: [ErrorCode; 17] = [
     ErrorCode::GispatchFunctionPointerNotSet,
     ErrorCode::OutbError,
     ErrorCode::UnknownError,
-    ErrorCode::StackOverflow,
     ErrorCode::GsCheckFailed,
     ErrorCode::TooManyGuestFunctions,
     ErrorCode::FailureInDlmalloc,
@@ -58,7 +57,6 @@ impl ErrorCode {
     pub const GispatchFunctionPointerNotSet: Self = Self(6);
     pub const OutbError: Self = Self(7);
     pub const UnknownError: Self = Self(8);
-    pub const StackOverflow: Self = Self(9);
     pub const GsCheckFailed: Self = Self(10);
     pub const TooManyGuestFunctions: Self = Self(11);
     pub const FailureInDlmalloc: Self = Self(12);
@@ -79,7 +77,6 @@ impl ErrorCode {
         Self::GispatchFunctionPointerNotSet,
         Self::OutbError,
         Self::UnknownError,
-        Self::StackOverflow,
         Self::GsCheckFailed,
         Self::TooManyGuestFunctions,
         Self::FailureInDlmalloc,
@@ -102,7 +99,6 @@ impl ErrorCode {
             Self::GispatchFunctionPointerNotSet => Some("GispatchFunctionPointerNotSet"),
             Self::OutbError => Some("OutbError"),
             Self::UnknownError => Some("UnknownError"),
-            Self::StackOverflow => Some("StackOverflow"),
             Self::GsCheckFailed => Some("GsCheckFailed"),
             Self::TooManyGuestFunctions => Some("TooManyGuestFunctions"),
             Self::FailureInDlmalloc => Some("FailureInDlmalloc"),

src/hyperlight_common/src/mem.rs

@@ -28,16 +28,6 @@ pub struct GuestMemoryRegion {
     pub ptr: u64,
 }
 
-/// A memory region in the guest address space that is used for the stack
-#[derive(Debug, Clone, Copy)]
-#[repr(C)]
-pub struct GuestStack {
-    /// The top of the user stack
-    pub min_user_stack_address: u64,
-    /// The user stack pointer
-    pub user_stack_address: u64,
-}
-
 #[derive(Debug, Clone, Copy)]
 #[repr(C)]
 pub struct HyperlightPEB {

src/hyperlight_host/src/error.rs

@@ -422,36 +422,6 @@ mod tests {
     };
     use crate::sandbox::outb::HandleOutbError;
 
-    /// Test that StackOverflow from RunVmError promotes to HyperlightError::StackOverflow
-    #[test]
-    fn test_promote_stack_overflow_from_run_vm() {
-        let err = DispatchGuestCallError::Run(RunVmError::StackOverflow);
-        let (promoted, should_poison) = err.promote();
-
-        assert!(should_poison, "StackOverflow should poison the sandbox");
-        assert!(
-            matches!(promoted, HyperlightError::StackOverflow()),
-            "Expected HyperlightError::StackOverflow, got {:?}",
-            promoted
-        );
-    }
-
-    /// Test that StackOverflow from HandleOutbError promotes to HyperlightError::StackOverflow
-    #[test]
-    fn test_promote_stack_overflow_from_outb() {
-        let err = DispatchGuestCallError::Run(RunVmError::HandleIo(HandleIoError::Outb(
-            HandleOutbError::StackOverflow,
-        )));
-        let (promoted, should_poison) = err.promote();
-
-        assert!(should_poison, "StackOverflow should poison the sandbox");
-        assert!(
-            matches!(promoted, HyperlightError::StackOverflow()),
-            "Expected HyperlightError::StackOverflow, got {:?}",
-            promoted
-        );
-    }
-
     /// Test that ExecutionCancelledByHost promotes to HyperlightError::ExecutionCanceledByHost
     #[test]
     fn test_promote_execution_cancelled_by_host() {
@@ -517,26 +487,6 @@ mod tests {
         }
     }
 
-    /// Test that ConvertRspPointer does not poison the sandbox
-    #[test]
-    fn test_promote_convert_rsp_pointer_no_poison() {
-        let err = DispatchGuestCallError::ConvertRspPointer("test error".to_string());
-        let (promoted, should_poison) = err.promote();
-
-        assert!(
-            !should_poison,
-            "ConvertRspPointer should not poison the sandbox"
-        );
-        assert!(
-            matches!(
-                promoted,
-                HyperlightError::HyperlightVmError(HyperlightVmError::DispatchGuestCall(_))
-            ),
-            "Expected HyperlightError::HyperlightVmError, got {:?}",
-            promoted
-        );
-    }
-
     /// Test that non-promoted Run errors are wrapped in HyperlightVmError
     #[test]
     fn test_promote_other_run_errors_wrapped() {

src/hyperlight_host/src/hypervisor/hyperlight_vm.rs

@@ -116,8 +116,6 @@ pub(crate) struct HyperlightVm {
 /// DispatchGuestCall error
 #[derive(Debug, thiserror::Error)]
 pub enum DispatchGuestCallError {
-    #[error("Failed to convert RSP pointer: {0}")]
-    ConvertRspPointer(String),
     #[error("Failed to run vm: {0}")]
     Run(#[from] RunVmError),
     #[error("Failed to setup registers: {0}")]
@@ -131,9 +129,7 @@ impl DispatchGuestCallError {
             // These errors poison the sandbox because they can leave it in an inconsistent state
             // by returning before the guest can unwind properly
             DispatchGuestCallError::Run(_) => true,
-            DispatchGuestCallError::ConvertRspPointer(_) | DispatchGuestCallError::SetupRegs(_) => {
-                false
-            }
+            DispatchGuestCallError::SetupRegs(_) => false,
         }
     }
 
@@ -145,12 +141,6 @@ impl DispatchGuestCallError {
     pub(crate) fn promote(self) -> (HyperlightError, bool) {
         let should_poison = self.is_poison_error();
         let promoted_error = match self {
-            // These errors poison the sandbox because the guest did not run to completion
-            DispatchGuestCallError::Run(RunVmError::StackOverflow)
-            | DispatchGuestCallError::Run(RunVmError::HandleIo(HandleIoError::Outb(
-                HandleOutbError::StackOverflow,
-            ))) => HyperlightError::StackOverflow(),
-
             DispatchGuestCallError::Run(RunVmError::ExecutionCancelledByHost) => {
                 HyperlightError::ExecutionCanceledByHost()
             }
@@ -188,8 +178,6 @@ pub enum InitializeError {
 /// Errors that can occur during VM execution in the run loop
 #[derive(Debug, thiserror::Error)]
 pub enum RunVmError {
-    #[error("Error checking stack guard: {0}")]
-    CheckStackGuard(Box<HyperlightError>),
     #[cfg(crashdump)]
     #[error("Crashdump generation error: {0}")]
     CrashdumpGeneration(Box<HyperlightError>),
@@ -217,8 +205,6 @@ pub enum RunVmError {
     MmioWriteUnmapped(u64),
     #[error("vCPU run failed: {0}")]
     RunVcpu(#[from] RunVcpuError),
-    #[error("Stack overflow detected")]
-    StackOverflow,
     #[error("Unexpected VM exit: {0}")]
     UnexpectedVmExit(String),
     #[cfg(gdb)]
@@ -278,8 +264,6 @@ pub enum CreateHyperlightVmError {
     #[cfg(gdb)]
     #[error("Failed to add hardware breakpoint: {0}")]
     AddHwBreakpoint(DebugError),
-    #[error("Failed to convert RSP pointer: {0}")]
-    ConvertRspPointer(Box<HyperlightError>),
     #[error("No hypervisor was found")]
     NoHypervisorFound,
     #[cfg(gdb)]

src/hyperlight_host/src/sandbox/initialized_multi_use.rs

@@ -28,7 +28,6 @@ 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};
 
@@ -662,10 +661,10 @@ impl MultiUseSandbox {
                     )
                     .increment(1);
 
-                    Err(match guest_error.code {
-                        ErrorCode::StackOverflow => HyperlightError::StackOverflow(),
-                        _ => HyperlightError::GuestError(guest_error.code, guest_error.message),
-                    })
+                    Err(HyperlightError::GuestError(
+                        guest_error.code,
+                        guest_error.message,
+                    ))
                 }
             }
         })();

src/hyperlight_host/src/sandbox/outb.rs

@@ -35,8 +35,6 @@ use crate::sandbox::trace::MemTraceInfo;
 /// Errors that can occur when handling an outb operation from the guest.
 #[derive(Debug, thiserror::Error)]
 pub enum HandleOutbError {
-    #[error("Stack overflow detected (signaled by guest)")]
-    StackOverflow,
     #[error("Guest aborted: error code {code}, message: {message}")]
     GuestAborted {
         /// The error code from the guest
@@ -145,28 +143,24 @@ fn outb_abort(
     for &b in &bytes[1..=len as usize] {
         if b == ABORT_TERMINATOR {
             let guest_error_code = *buffer.first().unwrap_or(&0);
-            let guest_error = ErrorCode::from(guest_error_code as u64);
-
-            let result = match guest_error {
-                ErrorCode::StackOverflow => Err(HandleOutbError::StackOverflow),
-                _ => {
-                    let message = if let Some(&maybe_exception_code) = buffer.get(1) {
-                        match Exception::try_from(maybe_exception_code) {
-                            Ok(exception) => {
-                                let extra_msg = String::from_utf8_lossy(&buffer[2..]);
-                                format!("Exception: {:?} | {}", exception, extra_msg)
-                            }
-                            Err(_) => String::from_utf8_lossy(&buffer[1..]).into(),
-                        }
-                    } else {
-                        String::new()
-                    };
 
-                    Err(HandleOutbError::GuestAborted {
-                        code: guest_error_code,
-                        message,
-                    })
-                }
+            let result = {
+                let message = if let Some(&maybe_exception_code) = buffer.get(1) {
+                    match Exception::try_from(maybe_exception_code) {
+                        Ok(exception) => {
+                            let extra_msg = String::from_utf8_lossy(&buffer[2..]);
+                            format!("Exception: {:?} | {}", exception, extra_msg)
+                        }
+                        Err(_) => String::from_utf8_lossy(&buffer[1..]).into(),
+                    }
+                } else {
+                    String::new()
+                };
+
+                Err(HandleOutbError::GuestAborted {
+                    code: guest_error_code,
+                    message,
+                })
             };
 
             buffer.clear();

src/hyperlight_host/tests/integration_test.rs

@@ -592,10 +592,6 @@ fn guest_panic_no_alloc() {
         )
         .unwrap_err();
 
-    if let HyperlightError::StackOverflow() = res {
-        panic!("panic on OOM caused stack overflow, this implies allocation in panic handler");
-    }
-
     assert!(matches!(
         res,
         HyperlightError::GuestAborted(code, msg) if code == ErrorCode::UnknownError as u8 && msg.contains("memory allocation of ") && msg.contains("bytes failed")

src/schema/guest_error.fbs

@@ -9,7 +9,6 @@ enum  ErrorCode: ulong {
     GispatchFunctionPointerNotSet = 6,              // Host Call Dispatch Function Pointer is not present.
     OutbError = 7,                                  // Error in OutB Function
     UnknownError = 8,                               // The guest error is unknown.
-    StackOverflow = 9,                              // Guest stack allocations caused stack overflow
     GsCheckFailed  = 10,                            // __security_check_cookie failed
     TooManyGuestFunctions = 11,                     // The guest tried to register too many guest functions
     FailureInDlmalloc = 12,                         // this error is set when dlmalloc calls ABORT (e.g. function defined in  ABORT (dlmalloc_abort() calls setError with this errorcode)