[host/sandbox/{mem_access,uninit_evolve}] brought back mem_access_handler + re-added seccomp test

Previously set mem_access_handler_fn to be NOOP. Brought back original functionality;
albeit a bit questionable.

Signed-off-by: danbugs <[email protected]>

Author: danbugs

src/hyperlight_host/src/sandbox/mem_access.rs

@@ -18,18 +18,33 @@ use std::sync::{Arc, Mutex};
 
 use tracing::{instrument, Span};
 
+use crate::error::HyperlightError::StackOverflow;
 #[cfg(gdb)]
 use crate::hypervisor::handlers::{DbgMemAccessHandlerCaller, DbgMemAccessHandlerWrapper};
 use crate::hypervisor::handlers::{
     MemAccessHandler, MemAccessHandlerFunction, MemAccessHandlerWrapper,
 };
-#[cfg(gdb)]
 use crate::mem::mgr::SandboxMemoryManager;
+use crate::mem::shared_mem::HostSharedMemory;
+use crate::{log_then_return, Result};
+
+#[instrument(err(Debug), skip_all, parent = Span::current(), level= "Trace")]
+pub(super) fn handle_mem_access_impl(
+    wrapper: &SandboxMemoryManager<HostSharedMemory>,
+) -> Result<()> {
+    if !wrapper.check_stack_guard()? {
+        log_then_return!(StackOverflow());
+    }
+
+    Ok(())
+}
 
 #[instrument(skip_all, parent = Span::current(), level= "Trace")]
-pub(crate) fn mem_access_handler_wrapper() -> MemAccessHandlerWrapper {
-    // TODO(danbugs:297): fix
-    let mem_access_func: MemAccessHandlerFunction = Box::new(move || Ok(()));
+pub(crate) fn mem_access_handler_wrapper(
+    wrapper: SandboxMemoryManager<HostSharedMemory>,
+) -> MemAccessHandlerWrapper {
+    let mem_access_func: MemAccessHandlerFunction =
+        Box::new(move || handle_mem_access_impl(&wrapper));
     let mem_access_hdl = MemAccessHandler::from(mem_access_func);
     Arc::new(Mutex::new(mem_access_hdl))
 }

src/hyperlight_host/src/sandbox/sandbox_builder.rs

@@ -888,7 +888,7 @@ mod tests {
     use hyperlight_testing::simple_guest_as_string;
 
     use super::*;
-    use crate::func::HostFunction2;
+    use crate::func::{HostFunction0, HostFunction2};
     use crate::sandbox_state::sandbox::EvolvableSandbox;
     use crate::sandbox_state::transition::Noop;
     use crate::HyperlightError;
@@ -1079,4 +1079,87 @@ mod tests {
 
         Ok(())
     }
+
+    #[test]
+    #[ignore]
+    #[cfg(target_os = "linux")]
+    fn test_sandbox_builder_violate_seccomp_filters() -> Result<()> {
+        fn make_get_pid_syscall() -> Result<u64> {
+            let pid = unsafe { libc::syscall(libc::SYS_getpid) };
+            Ok(pid as u64)
+        }
+
+        // Tests two flows:
+        // 1. Calling a host function with the seccomp feature turned on, but without
+        // allowing the syscall. This should fail.
+        // 2. Calling a host function with the seccomp feature turned off. This should succeed.
+        {
+            // Tests building an uninitialized sandbox w/ the sandbox builder
+            let sandbox_builder =
+                SandboxBuilder::new(GuestBinary::FilePath(simple_guest_as_string()?))?;
+
+            let mut uninitialized_sandbox = sandbox_builder.build()?;
+
+            let make_get_pid_syscall_func = Arc::new(Mutex::new(make_get_pid_syscall));
+            make_get_pid_syscall_func.register(&mut uninitialized_sandbox, "MakeGetpidSyscall")?;
+
+            // Tests evolving to a multi-use sandbox
+            let mut multi_use_sandbox = uninitialized_sandbox.evolve(Noop::default())?;
+
+            let result = multi_use_sandbox.call_guest_function_by_name(
+                "ViolateSeccompFilters",
+                ReturnType::ULong,
+                None,
+            );
+
+            #[cfg(feature = "seccomp")]
+            match result {
+                Ok(_) => panic!("Expected to fail due to seccomp violation"),
+                Err(e) => match e {
+                    HyperlightError::DisallowedSyscall => {}
+                    _ => panic!("Expected DisallowedSyscall error: {}", e),
+                },
+            }
+
+            #[cfg(not(feature = "seccomp"))]
+            match result {
+                Ok(_) => (),
+                Err(e) => panic!("Expected to succeed without seccomp: {}", e),
+            }
+        }
+
+        // Tests calling a host function with the seccomp feature turned on, but allowing
+        // the syscall. This should succeed.
+        #[cfg(feature = "seccomp")]
+        {
+            // Tests building an uninitialized sandbox w/ the sandbox builder
+            let sandbox_builder =
+                SandboxBuilder::new(GuestBinary::FilePath(simple_guest_as_string()?))?;
+
+            let mut uninitialized_sandbox = sandbox_builder.build()?;
+
+            let make_get_pid_syscall_func = Arc::new(Mutex::new(make_get_pid_syscall));
+            make_get_pid_syscall_func.register_with_extra_allowed_syscalls(
+                &mut uninitialized_sandbox,
+                "MakeGetpidSyscall",
+                vec![libc::SYS_getpid],
+            )?;
+
+            // Tests evolving to a multi-use sandbox
+            let mut multi_use_sandbox = uninitialized_sandbox.evolve(Noop::default())?;
+
+            let result = multi_use_sandbox.call_guest_function_by_name(
+                "ViolateSeccompFilters",
+                ReturnType::ULong,
+                None,
+            );
+
+            match result {
+                Ok(_) => {}
+                Err(e) => panic!("Expected to succeed due to seccomp violation: {}", e),
+            }
+        }
+
+        Ok(())
+    }
 }

src/hyperlight_host/src/sandbox/uninitialized_evolve.rs

@@ -99,7 +99,7 @@ fn hv_init(
 ) -> Result<HypervisorHandler> {
     let (hshm, gshm) = shm;
     let outb_hdl = outb_handler_wrapper(hshm.clone(), host_funcs);
-    let mem_access_hdl = mem_access_handler_wrapper();
+    let mem_access_hdl = mem_access_handler_wrapper(hshm.clone());
     #[cfg(gdb)]
     let dbg_mem_access_hdl = dbg_mem_access_handler_wrapper(hshm.clone());
 

src/hyperlight_host/src/seccomp/guest.rs

@@ -23,7 +23,6 @@ use seccompiler::{
 use crate::sandbox::ExtraAllowedSyscall;
 use crate::{and, or, Result};
 
-// TODO(danbugs:297): reconsider
 #[allow(unused)]
 fn syscalls_allowlist() -> Result<Vec<(i64, Vec<SeccompRule>)>> {
     Ok(vec![
@@ -70,8 +69,6 @@ fn syscalls_allowlist() -> Result<Vec<(i64, Vec<SeccompRule>)>> {
 /// `SeccompRules` for operations we definitely perform but are outside the handler thread
 /// (e.g., `KVM_SET_USER_MEMORY_REGION`, `KVM_GET_API_VERSION`, `KVM_CREATE_VM`,
 /// or `KVM_CREATE_VCPU`).
-// TODO(danbugs:297): reconsider
-#[allow(unused)]
 pub(crate) fn get_seccomp_filter_for_host_function_worker_thread(
     extra_allowed_syscalls: Option<Vec<ExtraAllowedSyscall>>,
 ) -> Result<BpfProgram> {