Remove seccomp feature from hyperlight-host

Signed-off-by: Doru Blânzeanu <dblnz@pm.me>

Author: dblnz

Cargo.lock

@@ -75,7 +75,6 @@ windows-version = "0.1"
 lazy_static = "1.4.0"
 
 [target.'cfg(unix)'.dependencies]
-seccompiler = { version = "0.5.0", optional = true }
 kvm-bindings = { version = "0.14", features = ["fam-wrappers"], optional = true }
 kvm-ioctls = { version = "0.24", optional = true }
 mshv-bindings2 = { package="mshv-bindings", version = "=0.2.1", optional = true }
@@ -126,8 +125,7 @@ cfg_aliases = "0.2.1"
 built = { version = "0.8.0", optional = true, features = ["chrono", "git2"] }
 
 [features]
-default = ["kvm", "mshv3", "seccomp", "build-metadata", "init-paging"]
-seccomp = ["dep:seccompiler"]
+default = ["kvm", "mshv3", "build-metadata", "init-paging"]
 function_call_metrics = []
 executable_heap = []
 # This feature enables printing of debug information to stdout in debug builds

src/hyperlight_host/Cargo.toml

@@ -101,7 +101,6 @@ fn main() -> Result<()> {
         // the other features they want.
         mshv2: { all(feature = "mshv2", target_os = "linux") },
         mshv3: { all(feature = "mshv3", not(feature="mshv2"), target_os = "linux") },
-        seccomp: { all(feature = "seccomp", target_os = "linux", not(target_env = "musl")) },
     }
 
     #[cfg(feature = "build-metadata")]

src/hyperlight_host/build.rs

@@ -69,11 +69,6 @@ pub enum HyperlightError {
     #[error("Error converting CString {0:?}")]
     CStringConversionError(#[from] std::ffi::NulError),
 
-    /// A disallowed syscall was caught
-    #[error("Seccomp filter trapped on disallowed syscall (check STDERR for offending syscall)")]
-    #[cfg(seccomp)]
-    DisallowedSyscall,
-
     /// A generic error with a message
     #[error("{0}")]
     Error(String),
@@ -216,16 +211,6 @@ pub enum HyperlightError {
     #[error("Stack overflow detected")]
     StackOverflow(),
 
-    /// a backend error occurred with seccomp filters
-    #[error("Backend Error with Seccomp Filter {0:?}")]
-    #[cfg(seccomp)]
-    SeccompFilterBackendError(#[from] seccompiler::BackendError),
-
-    /// an error occurred with seccomp filters
-    #[error("Error with Seccomp Filter {0:?}")]
-    #[cfg(seccomp)]
-    SeccompFilterError(#[from] seccompiler::Error),
-
     /// Tried to restore snapshot to a sandbox that is not the same as the one the snapshot was taken from
     #[error("Snapshot was taken from a different sandbox")]
     SnapshotSandboxMismatch,

src/hyperlight_host/src/error.rs

@@ -20,8 +20,8 @@ use hyperlight_common::flatbuffer_wrappers::function_types::{ParameterValue, Ret
 
 use super::utils::for_each_tuple;
 use super::{ParameterTuple, ResultType, SupportedReturnType};
+use crate::sandbox::UninitializedSandbox;
 use crate::sandbox::host_funcs::FunctionEntry;
-use crate::sandbox::{ExtraAllowedSyscall, UninitializedSandbox};
 use crate::{Result, new_error};
 
 /// A sandbox on which (primitive) host functions can be registered
@@ -33,15 +33,6 @@ pub trait Registerable {
         name: &str,
         hf: impl Into<HostFunction<Output, Args>>,
     ) -> Result<()>;
-    /// Register a primitive host function whose worker thread has
-    /// extra permissive seccomp filters installed
-    #[cfg(seccomp)]
-    fn register_host_function_with_syscalls<Args: ParameterTuple, Output: SupportedReturnType>(
-        &mut self,
-        name: &str,
-        hf: impl Into<HostFunction<Output, Args>>,
-        eas: Vec<ExtraAllowedSyscall>,
-    ) -> Result<()>;
 }
 impl Registerable for UninitializedSandbox {
     fn register_host_function<Args: ParameterTuple, Output: SupportedReturnType>(
@@ -56,28 +47,6 @@ impl Registerable for UninitializedSandbox {
 
         let entry = FunctionEntry {
             function: hf.into().into(),
-            extra_allowed_syscalls: None,
-            parameter_types: Args::TYPE,
-            return_type: Output::TYPE,
-        };
-
-        (*hfs).register_host_function(name.to_string(), entry, &mut self.mgr)
-    }
-    #[cfg(seccomp)]
-    fn register_host_function_with_syscalls<Args: ParameterTuple, Output: SupportedReturnType>(
-        &mut self,
-        name: &str,
-        hf: impl Into<HostFunction<Output, Args>>,
-        eas: Vec<ExtraAllowedSyscall>,
-    ) -> Result<()> {
-        let mut hfs = self
-            .host_funcs
-            .try_lock()
-            .map_err(|e| new_error!("Error locking at {}:{}: {}", file!(), line!(), e))?;
-
-        let entry = FunctionEntry {
-            function: hf.into().into(),
-            extra_allowed_syscalls: Some(eas),
             parameter_types: Args::TYPE,
             return_type: Output::TYPE,
         };
@@ -195,13 +164,11 @@ pub(crate) fn register_host_function<Args: ParameterTuple, Output: SupportedRetu
     func: impl Into<HostFunction<Output, Args>>,
     sandbox: &mut UninitializedSandbox,
     name: &str,
-    extra_allowed_syscalls: Option<Vec<ExtraAllowedSyscall>>,
 ) -> Result<()> {
     let func = func.into().into();
 
     let entry = FunctionEntry {
         function: func,
-        extra_allowed_syscalls: extra_allowed_syscalls.clone(),
         parameter_types: Args::TYPE,
         return_type: Output::TYPE,
     };

src/hyperlight_host/src/func/host_functions.rs

@@ -76,8 +76,6 @@ pub mod metrics;
 /// outside this file. Types from this module needed for public consumption are
 /// re-exported below.
 pub mod sandbox;
-#[cfg(seccomp)]
-pub(crate) mod seccomp;
 /// Signal handling for Linux
 #[cfg(target_os = "linux")]
 pub(crate) mod signal_handlers;

src/hyperlight_host/src/lib.rs

@@ -133,11 +133,7 @@ mod tests {
             if #[cfg(feature = "function_call_metrics")] {
                 use metrics::Label;
 
-                let expected_num_metrics = if cfg!(all(seccomp)) {
-                    3 // if seccomp enabled, the host call duration metric is emitted on a separate thread which this local recorder doesn't capture
-                } else {
-                    4
-                };
+                let expected_num_metrics = 4;
 
                 // Verify that the histogram metrics are recorded correctly
                 assert_eq!(snapshot.len(), expected_num_metrics);
@@ -185,25 +181,6 @@ mod tests {
                     ),
                     "Histogram metric does not match expected value"
                 );
-
-                if !cfg!(all(seccomp)) {
-                    // 4. Host call duration
-                    let histogram_key = CompositeKey::new(
-                        metrics_util::MetricKind::Histogram,
-                        Key::from_parts(
-                            METRIC_HOST_FUNC_DURATION,
-                            vec![Label::new("function_name", "HostPrint")],
-                        ),
-                    );
-                    let histogram_value = &snapshot.get(&histogram_key).unwrap().2;
-                    assert!(
-                        matches!(
-                            histogram_value,
-                            metrics_util::debugging::DebugValue::Histogram(histogram) if histogram.len() == 1
-                        ),
-                        "Histogram metric does not match expected value"
-                    );
-                }
             } else {
                 // Verify that the counter metrics are recorded correctly
                 assert_eq!(snapshot.len(), 1);

src/hyperlight_host/src/metrics/mod.rs

@@ -25,7 +25,6 @@ use hyperlight_common::flatbuffer_wrappers::host_function_details::HostFunctionD
 use termcolor::{Color, ColorChoice, ColorSpec, StandardStream, WriteColor};
 use tracing::{Span, instrument};
 
-use super::ExtraAllowedSyscall;
 use crate::HyperlightError::HostFunctionNotFound;
 use crate::func::host_functions::TypeErasedHostFunction;
 use crate::mem::mgr::SandboxMemoryManager;
@@ -58,7 +57,6 @@ impl From<&mut FunctionRegistry> for HostFunctionDetails {
 
 pub struct FunctionEntry {
     pub function: TypeErasedHostFunction,
-    pub extra_allowed_syscalls: Option<Vec<ExtraAllowedSyscall>>,
     pub parameter_types: &'static [ParameterType],
     pub return_type: ReturnType,
 }
@@ -119,18 +117,15 @@ impl FunctionRegistry {
     fn call_host_func_impl(&self, name: &str, args: Vec<ParameterValue>) -> Result<ReturnValue> {
         let FunctionEntry {
             function,
-            extra_allowed_syscalls,
             parameter_types: _,
             return_type: _,
         } = self
             .functions_map
             .get(name)
             .ok_or_else(|| HostFunctionNotFound(name.to_string()))?;
 
-        // Create a new thread when seccomp is enabled on Linux
-        maybe_with_seccomp(name, extra_allowed_syscalls.as_deref(), || {
-            crate::metrics::maybe_time_and_emit_host_call(name, || function.call(args))
-        })
+        // Make the host function call
+        crate::metrics::maybe_time_and_emit_host_call(name, || function.call(args))
     }
 }
 
@@ -153,58 +148,3 @@ pub(super) fn default_writer_func(s: String) -> Result<i32> {
         }
     }
 }
-
-#[cfg(seccomp)]
-fn maybe_with_seccomp<T: Send>(
-    name: &str,
-    syscalls: Option<&[ExtraAllowedSyscall]>,
-    f: impl FnOnce() -> Result<T> + Send,
-) -> Result<T> {
-    use std::thread;
-
-    use crate::seccomp::guest::get_seccomp_filter_for_host_function_worker_thread;
-
-    // Use a scoped thread so that we can pass around references without having to clone them.
-    thread::scope(|s| {
-        thread::Builder::new()
-            .name(format!("Host Function Worker Thread for: {name:?}"))
-            .spawn_scoped(s, move || {
-                let seccomp_filter = get_seccomp_filter_for_host_function_worker_thread(syscalls)?;
-                seccomp_filter
-                    .iter()
-                    .try_for_each(|filter| seccompiler::apply_filter(filter))?;
-
-                // We have a `catch_unwind` here because, if a disallowed syscall is issued,
-                // we handle it by panicking. This is to avoid returning execution to the
-                // offending host function—for two reasons: (1) if a host function is issuing
-                // disallowed syscalls, it could be unsafe to return to, and (2) returning
-                // execution after trapping the disallowed syscall can lead to UB (e.g., try
-                // running a host function that attempts to sleep without `SYS_clock_nanosleep`,
-                // you'll block the syscall but panic in the aftermath).
-                match std::panic::catch_unwind(std::panic::AssertUnwindSafe(f)) {
-                    Ok(val) => val,
-                    Err(err) => {
-                        if let Some(crate::HyperlightError::DisallowedSyscall) =
-                            err.downcast_ref::<crate::HyperlightError>()
-                        {
-                            return Err(crate::HyperlightError::DisallowedSyscall);
-                        }
-
-                        crate::log_then_return!("Host function {} panicked", name);
-                    }
-                }
-            })?
-            .join()
-            .map_err(|_| new_error!("Error joining thread executing host function"))?
-    })
-}
-
-#[cfg(not(seccomp))]
-fn maybe_with_seccomp<T: Send>(
-    _name: &str,
-    _syscalls: Option<&[ExtraAllowedSyscall]>,
-    f: impl FnOnce() -> Result<T> + Send,
-) -> Result<T> {
-    // No seccomp, just call the function
-    f()
-}

src/hyperlight_host/src/sandbox/host_funcs.rs

@@ -35,21 +35,6 @@ use crate::mem::shared_mem::ExclusiveSharedMemory;
 use crate::sandbox::SandboxConfiguration;
 use crate::{MultiUseSandbox, Result, new_error};
 
-#[cfg(seccomp)]
-const EXTRA_ALLOWED_SYSCALLS_FOR_WRITER_FUNC: &[super::ExtraAllowedSyscall] = &[
-    // Fuzzing fails without `mmap` being an allowed syscall on our seccomp filter.
-    // All fuzzing does is call `PrintOutput` (which calls `HostPrint` ). Thing is, `println!`
-    // is designed to be thread-safe in Rust and the std lib ensures this by using
-    // buffered I/O, which I think relies on `mmap`. This gets surfaced in fuzzing with an
-    // OOM error, which I think is happening because `println!` is not being able to allocate
-    // more memory for its buffers for the fuzzer's huge inputs.
-    libc::SYS_mmap,
-    libc::SYS_brk,
-    libc::SYS_mprotect,
-    #[cfg(mshv)]
-    libc::SYS_close,
-];
-
 #[cfg(any(crashdump, gdb))]
 #[derive(Clone, Debug, Default)]
 pub(crate) struct SandboxRuntimeConfig {
@@ -304,25 +289,7 @@ impl UninitializedSandbox {
         name: impl AsRef<str>,
         host_func: impl Into<HostFunction<Output, Args>>,
     ) -> Result<()> {
-        register_host_function(host_func, self, name.as_ref(), None)
-    }
-
-    /// Registers a host function with additional allowed syscalls during execution.
-    ///
-    /// Unlike [`register`](Self::register), this variant allows specifying extra syscalls
-    /// that will be permitted when the function handler runs.
-    #[cfg(seccomp)]
-    pub fn register_with_extra_allowed_syscalls<
-        Args: ParameterTuple,
-        Output: SupportedReturnType,
-    >(
-        &mut self,
-        name: impl AsRef<str>,
-        host_func: impl Into<HostFunction<Output, Args>>,
-        extra_allowed_syscalls: impl IntoIterator<Item = crate::sandbox::ExtraAllowedSyscall>,
-    ) -> Result<()> {
-        let extra_allowed_syscalls: Vec<_> = extra_allowed_syscalls.into_iter().collect();
-        register_host_function(host_func, self, name.as_ref(), Some(extra_allowed_syscalls))
+        register_host_function(host_func, self, name.as_ref())
     }
 
     /// Registers the special "HostPrint" function for guest printing.
@@ -334,40 +301,7 @@ impl UninitializedSandbox {
         &mut self,
         print_func: impl Into<HostFunction<i32, (String,)>>,
     ) -> Result<()> {
-        #[cfg(not(seccomp))]
-        self.register("HostPrint", print_func)?;
-
-        #[cfg(seccomp)]
-        self.register_with_extra_allowed_syscalls(
-            "HostPrint",
-            print_func,
-            EXTRA_ALLOWED_SYSCALLS_FOR_WRITER_FUNC.iter().copied(),
-        )?;
-
-        Ok(())
-    }
-
-    /// Registers the "HostPrint" function with additional allowed syscalls.
-    ///
-    /// Like [`register_print`](Self::register_print), but allows specifying extra syscalls
-    /// that will be permitted during function execution.
-    #[cfg(seccomp)]
-    pub fn register_print_with_extra_allowed_syscalls(
-        &mut self,
-        print_func: impl Into<HostFunction<i32, (String,)>>,
-        extra_allowed_syscalls: impl IntoIterator<Item = crate::sandbox::ExtraAllowedSyscall>,
-    ) -> Result<()> {
-        #[cfg(seccomp)]
-        self.register_with_extra_allowed_syscalls(
-            "HostPrint",
-            print_func,
-            EXTRA_ALLOWED_SYSCALLS_FOR_WRITER_FUNC
-                .iter()
-                .copied()
-                .chain(extra_allowed_syscalls),
-        )?;
-
-        Ok(())
+        self.register("HostPrint", print_func)
     }
 }
 // Check to see if the current version of Windows is supported