Remove in-process mode from hyperlight-guest and remaining clean up in hyperlight-host (#496)

Signed-off-by: Simon Davies <[email protected]>

Author: simongdavies

README.md

@@ -42,8 +42,7 @@ fn main() -> hyperlight_host::Result<()> {
     // Create an uninitialized sandbox with a guest binary
     let mut uninitialized_sandbox = UninitializedSandbox::new(
         hyperlight_host::GuestBinary::FilePath(hyperlight_testing::simple_guest_as_string().unwrap()),
-        None, // default configuration
-        None, // default run options
+        None // default configuration
     )?;
 
     // Registering a host function makes it available to be called by the guest

fuzz/fuzz_targets/guest_call.rs

@@ -35,7 +35,6 @@ fuzz_target!(
         let u_sbox = UninitializedSandbox::new(
             GuestBinary::FilePath(simple_guest_for_fuzzing_as_string().expect("Guest Binary Missing")),
             None,
-            None,
         )
         .unwrap();
 

fuzz/fuzz_targets/host_call.rs

@@ -33,8 +33,7 @@ fuzz_target!(
     init: {
         let u_sbox = UninitializedSandbox::new(
             GuestBinary::FilePath(simple_guest_for_fuzzing_as_string().expect("Guest Binary Missing")),
-            None,
-            None,
+            None
         )
         .unwrap();
 

fuzz/fuzz_targets/host_print.rs

@@ -21,7 +21,6 @@ fuzz_target!(
         let u_sbox = UninitializedSandbox::new(
             GuestBinary::FilePath(simple_guest_for_fuzzing_as_string().expect("Guest Binary Missing")),
             None,
-            None,
         )
         .unwrap();
 

src/hyperlight_common/src/mem.rs

@@ -22,16 +22,6 @@ pub const PAGE_SIZE_USIZE: usize = 1 << 12;
 
 use core::ffi::{c_char, c_void};
 
-#[repr(u64)]
-#[derive(Debug, Copy, Clone, PartialEq, Eq)]
-pub enum RunMode {
-    None = 0,
-    Hypervisor = 1,
-    InProcessWindows = 2,
-    InProcessLinux = 3,
-    Invalid = 4,
-}
-
 #[repr(C)]
 pub struct InputData {
     pub inputDataSize: u64,
@@ -67,9 +57,6 @@ pub struct HyperlightPEB {
     pub security_cookie_seed: u64,
     pub guest_function_dispatch_ptr: u64,
     pub pCode: *mut c_char,
-    pub pOutb: *mut c_void,
-    pub pOutbContext: *mut c_void,
-    pub runMode: RunMode,
     pub inputdata: InputData,
     pub outputdata: OutputData,
     pub guestheapData: GuestHeapData,

src/hyperlight_guest/src/chkstk.rs

@@ -15,12 +15,9 @@ limitations under the License.
 */
 
 use core::arch::global_asm;
-use core::mem::size_of;
 
-use hyperlight_common::mem::RunMode;
-
-use crate::guest_error::{set_invalid_runmode_error, set_stack_allocate_error};
-use crate::{MIN_STACK_ADDRESS, RUNNING_MODE};
+use crate::guest_error::set_stack_allocate_error;
+use crate::MIN_STACK_ADDRESS;
 
 extern "win64" {
     fn __chkstk();
@@ -35,21 +32,6 @@ global_asm!(
         push r10
         push r11
 
-        /* Load run_mode into r10 */
-        mov r10, qword ptr [rip+{run_mode}]
-
-        cmp r10, 0
-        je handle_none
-        cmp r10, 1
-        je handle_hypervisor
-        cmp r10, 2
-        je handle_inproc_windows
-        cmp r10, 3
-        je handle_inproc_linux
-        /* run_mode > 3 (invalid), so treat like handle_none */
-        jmp handle_invalid
-
-    handle_hypervisor:
         /* Load the minimum stack address from the PEB */
         mov r11, [rip+{min_stack_addr}]  
 
@@ -72,49 +54,11 @@ global_asm!(
         call {set_error}
         hlt
     
-    handle_inproc_windows:
-        /* Get the current stack pointer */
-        lea r10, [rsp + 0x18]
-
-        /* Calculate what the new stack pointer will be */
-        sub r10, rax
-        cmovb r10, r11
-        mov r11, qword ptr gs:[0x0000000000000010]
-        cmp r10, r11
-        jae cs_ret
-        and r10w, 0x0F000
-    csip_stackprobe:
-        lea r11, [r11 + 0x0FFFFFFFFFFFFF000]
-        mov byte ptr [r11], 0
-        cmp r10, r11
-        jne csip_stackprobe
     cs_ret:
         /* Restore RAX, R11 */
         pop r11
         pop r10
-        ret
-    handle_inproc_linux:
-        /* no-op */
-        jmp cs_ret
-    handle_none:
-        /* no-op. This can entrypoint has a large stack allocation
-            before RunMode variable is set */
-        jmp cs_ret
-    handle_invalid:
-        call {invalid_runmode}",
-    run_mode = sym RUNNING_MODE,
+        ret",
     min_stack_addr = sym MIN_STACK_ADDRESS,
     set_error = sym set_stack_allocate_error,
-    invalid_runmode = sym set_invalid_runmode_error
 );
-
-// Assumptions made in implementation above. If these are no longer true, compilation will fail
-// and the developer will need to update the assembly code.
-const _: () = {
-    assert!(size_of::<RunMode>() == size_of::<u64>());
-    assert!(RunMode::None as u64 == 0);
-    assert!(RunMode::Hypervisor as u64 == 1);
-    assert!(RunMode::InProcessWindows as u64 == 2);
-    assert!(RunMode::InProcessLinux as u64 == 3);
-    assert!(RunMode::Invalid as u64 == 4);
-};

src/hyperlight_guest/src/entrypoint.rs

@@ -15,9 +15,9 @@ limitations under the License.
 */
 
 use core::arch::asm;
-use core::ffi::{c_char, c_void, CStr};
+use core::ffi::{c_char, CStr};
 
-use hyperlight_common::mem::{HyperlightPEB, RunMode};
+use hyperlight_common::mem::HyperlightPEB;
 use hyperlight_common::outb::OutBAction;
 use log::LevelFilter;
 use spin::Once;
@@ -27,18 +27,11 @@ use crate::guest_function_call::dispatch_function;
 use crate::guest_logger::init_logger;
 use crate::host_function_call::outb;
 use crate::idtr::load_idt;
-use crate::{
-    __security_cookie, HEAP_ALLOCATOR, MIN_STACK_ADDRESS, OS_PAGE_SIZE, OUTB_PTR,
-    OUTB_PTR_WITH_CONTEXT, P_PEB, RUNNING_MODE,
-};
+use crate::{__security_cookie, HEAP_ALLOCATOR, MIN_STACK_ADDRESS, OS_PAGE_SIZE, P_PEB};
 
 #[inline(never)]
 pub fn halt() {
-    unsafe {
-        if RUNNING_MODE == RunMode::Hypervisor {
-            asm!("hlt", options(nostack))
-        }
-    }
+    unsafe { asm!("hlt", options(nostack)) }
 }
 
 #[no_mangle]
@@ -105,45 +98,14 @@ pub extern "win64" fn entrypoint(peb_address: u64, seed: u64, ops: u64, max_log_
                 .expect("Invalid log level");
             init_logger(max_log_level);
 
-            match (*peb_ptr).runMode {
-                RunMode::Hypervisor => {
-                    RUNNING_MODE = RunMode::Hypervisor;
-                    // This static is to make it easier to implement the __chkstk function in assembly.
-                    // It also means that should we change the layout of the struct in the future, we
-                    // don't have to change the assembly code.
-                    MIN_STACK_ADDRESS = (*peb_ptr).gueststackData.minUserStackAddress;
-
-                    // Setup GDT and IDT
-                    load_gdt();
-                    load_idt();
-                }
-                RunMode::InProcessLinux | RunMode::InProcessWindows => {
-                    RUNNING_MODE = (*peb_ptr).runMode;
-
-                    OUTB_PTR = {
-                        let outb_ptr: extern "win64" fn(u16, *const u8, u64) =
-                            core::mem::transmute((*peb_ptr).pOutb);
-                        Some(outb_ptr)
-                    };
-
-                    if (*peb_ptr).pOutbContext.is_null() {
-                        panic!("OutbContext is null");
-                    }
-
-                    OUTB_PTR_WITH_CONTEXT = {
-                        let outb_ptr_with_context: extern "win64" fn(
-                            *mut c_void,
-                            u16,
-                            *const u8,
-                            u64,
-                        ) = core::mem::transmute((*peb_ptr).pOutb);
-                        Some(outb_ptr_with_context)
-                    };
-                }
-                _ => {
-                    panic!("Invalid runmode in PEB");
-                }
-            }
+            // This static is to make it easier to implement the __chkstk function in assembly.
+            // It also means that should we change the layout of the struct in the future, we
+            // don't have to change the assembly code.
+            MIN_STACK_ADDRESS = (*peb_ptr).gueststackData.minUserStackAddress;
+
+            // Setup GDT and IDT
+            load_gdt();
+            load_idt();
 
             let heap_start = (*peb_ptr).guestheapData.guestHeapBuffer as usize;
             let heap_size = (*peb_ptr).guestheapData.guestHeapSize as usize;

src/hyperlight_guest/src/guest_error.rs

@@ -52,11 +52,6 @@ pub(crate) extern "win64" fn set_stack_allocate_error() {
     outb(OutBAction::Abort as u16, &[ErrorCode::StackOverflow as u8]);
 }
 
-#[no_mangle]
-pub(crate) extern "win64" fn set_invalid_runmode_error() {
-    panic!("Invalid run mode in __chkstk");
-}
-
 /// Exposes a C API to allow the guest to set an error
 ///
 /// # Safety

src/hyperlight_guest/src/host_function_call.rs

@@ -25,13 +25,11 @@ use hyperlight_common::flatbuffer_wrappers::function_types::{
 };
 use hyperlight_common::flatbuffer_wrappers::guest_error::ErrorCode;
 use hyperlight_common::flatbuffer_wrappers::util::get_flatbuffer_result;
-use hyperlight_common::mem::RunMode;
 use hyperlight_common::outb::OutBAction;
 
 use crate::error::{HyperlightGuestError, Result};
 use crate::shared_input_data::try_pop_shared_input_data_into;
 use crate::shared_output_data::push_shared_output_data;
-use crate::{OUTB_PTR, OUTB_PTR_WITH_CONTEXT, P_PEB, RUNNING_MODE};
 
 /// Get a return value from a host function call.
 /// This usually requires a host function to be called first using `call_host_function`.
@@ -77,39 +75,16 @@ pub fn call_host_function(
 
 pub fn outb(port: u16, data: &[u8]) {
     unsafe {
-        match RUNNING_MODE {
-            RunMode::Hypervisor => {
-                let mut i = 0;
-                while i < data.len() {
-                    let remaining = data.len() - i;
-                    let chunk_len = remaining.min(3);
-                    let mut chunk = [0u8; 4];
-                    chunk[0] = chunk_len as u8;
-                    chunk[1..1 + chunk_len].copy_from_slice(&data[i..i + chunk_len]);
-                    let val = u32::from_le_bytes(chunk);
-                    out32(port, val);
-                    i += chunk_len;
-                }
-            }
-            RunMode::InProcessLinux | RunMode::InProcessWindows => {
-                if let Some(outb_func) = OUTB_PTR_WITH_CONTEXT {
-                    if let Some(peb_ptr) = P_PEB {
-                        outb_func(
-                            (*peb_ptr).pOutbContext,
-                            port,
-                            data.as_ptr(),
-                            data.len() as u64,
-                        );
-                    }
-                } else if let Some(outb_func) = OUTB_PTR {
-                    outb_func(port, data.as_ptr(), data.len() as u64);
-                } else {
-                    panic!("Tried to call outb without hypervisor and without outb function ptrs");
-                }
-            }
-            _ => {
-                panic!("Tried to call outb in invalid runmode");
-            }
+        let mut i = 0;
+        while i < data.len() {
+            let remaining = data.len() - i;
+            let chunk_len = remaining.min(3);
+            let mut chunk = [0u8; 4];
+            chunk[0] = chunk_len as u8;
+            chunk[1..1 + chunk_len].copy_from_slice(&data[i..i + chunk_len]);
+            let val = u32::from_le_bytes(chunk);
+            out32(port, val);
+            i += chunk_len;
         }
     }
 }

src/hyperlight_guest/src/lib.rs

@@ -21,7 +21,7 @@ use alloc::string::ToString;
 use buddy_system_allocator::LockedHeap;
 use guest_function_register::GuestFunctionRegister;
 use hyperlight_common::flatbuffer_wrappers::guest_error::ErrorCode;
-use hyperlight_common::mem::{HyperlightPEB, RunMode};
+use hyperlight_common::mem::HyperlightPEB;
 
 use crate::entrypoint::abort_with_code_and_message;
 extern crate alloc;
@@ -59,6 +59,7 @@ pub mod logging;
 pub(crate) extern "C" fn __CxxFrameHandler3() {}
 ///cbindgen:ignore
 #[no_mangle]
+#[clippy::allow(clippy::non_upper_case_globals)]
 pub(crate) static _fltused: i32 = 0;
 
 // It looks like rust-analyzer doesn't correctly manage no_std crates,
@@ -83,17 +84,13 @@ pub(crate) static HEAP_ALLOCATOR: LockedHeap<32> = LockedHeap::<32>::empty();
 
 ///cbindgen:ignore
 #[no_mangle]
+#[clippy::allow(clippy::non_upper_case_globals)]
 pub(crate) static mut __security_cookie: u64 = 0;
 
 pub(crate) static mut P_PEB: Option<*mut HyperlightPEB> = None;
 pub static mut MIN_STACK_ADDRESS: u64 = 0;
 
 pub static mut OS_PAGE_SIZE: u32 = 0;
-pub(crate) static mut OUTB_PTR: Option<extern "win64" fn(u16, *const u8, u64)> = None;
-pub(crate) static mut OUTB_PTR_WITH_CONTEXT: Option<
-    extern "win64" fn(*mut core::ffi::c_void, u16, *const u8, u64),
-> = None;
-pub static mut RUNNING_MODE: RunMode = RunMode::None;
 
 pub(crate) static mut REGISTERED_GUEST_FUNCTIONS: GuestFunctionRegister =
     GuestFunctionRegister::new();