From b5f3d2796cfde1db943b13de14291d1e261339c8 Mon Sep 17 00:00:00 2001 From: Jorge Prendes Date: Tue, 13 May 2025 16:06:47 +0100 Subject: [PATCH] Remove the host writer from the arguments to UninitializedSandbox::new Signed-off-by: Jorge Prendes --- README.md | 1 - fuzz/fuzz_targets/guest_call.rs | 1 - fuzz/fuzz_targets/host_call.rs | 2 - fuzz/fuzz_targets/host_print.rs | 1 - src/hyperlight_host/benches/benchmarks.rs | 3 +- src/hyperlight_host/examples/func_ctx/main.rs | 3 +- .../examples/guest-debugging/main.rs | 1 - .../examples/hello-world/main.rs | 1 - src/hyperlight_host/examples/logging/main.rs | 11 +- src/hyperlight_host/examples/metrics/main.rs | 11 +- .../examples/tracing-chrome/main.rs | 3 +- .../examples/tracing-otlp/main.rs | 10 +- .../examples/tracing-tracy/main.rs | 3 +- src/hyperlight_host/examples/tracing/main.rs | 11 +- src/hyperlight_host/src/func/call_ctx.rs | 2 +- .../src/func/guest_dispatch.rs | 8 - .../src/func/host_functions.rs | 45 ++++ .../src/hypervisor/hypervisor_handler.rs | 1 - src/hyperlight_host/src/hypervisor/mod.rs | 2 +- src/hyperlight_host/src/metrics/mod.rs | 1 - .../src/sandbox/initialized_multi_use.rs | 9 +- src/hyperlight_host/src/sandbox/mod.rs | 10 +- .../src/sandbox/uninitialized.rs | 204 +++++++++--------- .../src/sandbox/uninitialized_evolve.rs | 2 - src/hyperlight_host/tests/common/mod.rs | 38 ++-- src/hyperlight_host/tests/integration_test.rs | 7 +- .../tests/sandbox_host_tests.rs | 1 - 27 files changed, 193 insertions(+), 199 deletions(-) diff --git a/README.md b/README.md index 80ed29496..c795e01ea 100644 --- a/README.md +++ b/README.md @@ -44,7 +44,6 @@ fn main() -> hyperlight_host::Result<()> { hyperlight_host::GuestBinary::FilePath(hyperlight_testing::simple_guest_as_string().unwrap()), None, // default configuration None, // default run options - None, // default host print function )?; // Registering a host function makes it available to be called by the guest diff --git a/fuzz/fuzz_targets/guest_call.rs b/fuzz/fuzz_targets/guest_call.rs index 8b0bd722e..4cb22fb1e 100644 --- a/fuzz/fuzz_targets/guest_call.rs +++ b/fuzz/fuzz_targets/guest_call.rs @@ -36,7 +36,6 @@ fuzz_target!( GuestBinary::FilePath(simple_guest_for_fuzzing_as_string().expect("Guest Binary Missing")), None, None, - None, ) .unwrap(); diff --git a/fuzz/fuzz_targets/host_call.rs b/fuzz/fuzz_targets/host_call.rs index 83696cb5e..10e9ccf27 100644 --- a/fuzz/fuzz_targets/host_call.rs +++ b/fuzz/fuzz_targets/host_call.rs @@ -20,7 +20,6 @@ use std::sync::{Mutex, OnceLock}; use hyperlight_host::func::{ParameterValue, ReturnType}; use hyperlight_host::sandbox::uninitialized::GuestBinary; -use hyperlight_host::sandbox::SandboxConfiguration; use hyperlight_host::sandbox_state::sandbox::EvolvableSandbox; use hyperlight_host::sandbox_state::transition::Noop; use hyperlight_host::{HyperlightError, MultiUseSandbox, UninitializedSandbox}; @@ -36,7 +35,6 @@ fuzz_target!( GuestBinary::FilePath(simple_guest_for_fuzzing_as_string().expect("Guest Binary Missing")), None, None, - None, ) .unwrap(); diff --git a/fuzz/fuzz_targets/host_print.rs b/fuzz/fuzz_targets/host_print.rs index 576f56eec..33799ac03 100644 --- a/fuzz/fuzz_targets/host_print.rs +++ b/fuzz/fuzz_targets/host_print.rs @@ -22,7 +22,6 @@ fuzz_target!( GuestBinary::FilePath(simple_guest_for_fuzzing_as_string().expect("Guest Binary Missing")), None, None, - None, ) .unwrap(); diff --git a/src/hyperlight_host/benches/benchmarks.rs b/src/hyperlight_host/benches/benchmarks.rs index d680d484f..4477ce04d 100644 --- a/src/hyperlight_host/benches/benchmarks.rs +++ b/src/hyperlight_host/benches/benchmarks.rs @@ -26,7 +26,7 @@ use hyperlight_testing::simple_guest_as_string; fn create_uninit_sandbox() -> UninitializedSandbox { let path = simple_guest_as_string().unwrap(); - UninitializedSandbox::new(GuestBinary::FilePath(path), None, None, None).unwrap() + UninitializedSandbox::new(GuestBinary::FilePath(path), None, None).unwrap() } fn create_multiuse_sandbox() -> MultiUseSandbox { @@ -83,7 +83,6 @@ fn guest_call_benchmark(c: &mut Criterion) { GuestBinary::FilePath(simple_guest_as_string().unwrap()), Some(config), None, - None, ) .unwrap(); let mut sandbox = sandbox.evolve(Noop::default()).unwrap(); diff --git a/src/hyperlight_host/examples/func_ctx/main.rs b/src/hyperlight_host/examples/func_ctx/main.rs index 2967616e4..ee408a328 100644 --- a/src/hyperlight_host/examples/func_ctx/main.rs +++ b/src/hyperlight_host/examples/func_ctx/main.rs @@ -27,8 +27,7 @@ fn main() { // test guest binary let sbox1: MultiUseSandbox = { let path = simple_guest_as_string().unwrap(); - let u_sbox = - UninitializedSandbox::new(GuestBinary::FilePath(path), None, None, None).unwrap(); + let u_sbox = UninitializedSandbox::new(GuestBinary::FilePath(path), None, None).unwrap(); u_sbox.evolve(Noop::default()) } .unwrap(); diff --git a/src/hyperlight_host/examples/guest-debugging/main.rs b/src/hyperlight_host/examples/guest-debugging/main.rs index 842849d5b..514a9a256 100644 --- a/src/hyperlight_host/examples/guest-debugging/main.rs +++ b/src/hyperlight_host/examples/guest-debugging/main.rs @@ -49,7 +49,6 @@ fn main() -> hyperlight_host::Result<()> { ), cfg, // sandbox configuration None, // default run options - None, // default host print function )?; // Register a host functions diff --git a/src/hyperlight_host/examples/hello-world/main.rs b/src/hyperlight_host/examples/hello-world/main.rs index 4bad723d4..6f27a34c5 100644 --- a/src/hyperlight_host/examples/hello-world/main.rs +++ b/src/hyperlight_host/examples/hello-world/main.rs @@ -29,7 +29,6 @@ fn main() -> hyperlight_host::Result<()> { ), None, // default configuration None, // default run options - None, // default host print function )?; // Register a host functions diff --git a/src/hyperlight_host/examples/logging/main.rs b/src/hyperlight_host/examples/logging/main.rs index 6a0a1a74e..b24a5c7c7 100644 --- a/src/hyperlight_host/examples/logging/main.rs +++ b/src/hyperlight_host/examples/logging/main.rs @@ -15,7 +15,6 @@ limitations under the License. */ #![allow(clippy::disallowed_macros)] extern crate hyperlight_host; -use std::sync::{Arc, Mutex}; use hyperlight_common::flatbuffer_wrappers::function_types::{ParameterValue, ReturnType}; use hyperlight_host::sandbox::uninitialized::UninitializedSandbox; @@ -41,15 +40,10 @@ fn main() -> Result<()> { for _ in 0..20 { let path = hyperlight_guest_path.clone(); - let writer_func = Arc::new(Mutex::new(fn_writer)); let res: Result<()> = { // Create a new sandbox. - let usandbox = UninitializedSandbox::new( - GuestBinary::FilePath(path), - None, - None, - Some(&writer_func), - )?; + let mut usandbox = UninitializedSandbox::new(GuestBinary::FilePath(path), None, None)?; + usandbox.register_print(fn_writer)?; // Initialize the sandbox. @@ -91,7 +85,6 @@ fn main() -> Result<()> { GuestBinary::FilePath(hyperlight_guest_path.clone()), None, None, - None, )?; // Initialize the sandbox. diff --git a/src/hyperlight_host/examples/metrics/main.rs b/src/hyperlight_host/examples/metrics/main.rs index 4c88c255d..d443e9ca9 100644 --- a/src/hyperlight_host/examples/metrics/main.rs +++ b/src/hyperlight_host/examples/metrics/main.rs @@ -15,7 +15,6 @@ limitations under the License. */ #![allow(clippy::disallowed_macros)] extern crate hyperlight_host; -use std::sync::{Arc, Mutex}; use std::thread::{spawn, JoinHandle}; use hyperlight_common::flatbuffer_wrappers::function_types::{ParameterValue, ReturnType}; @@ -53,15 +52,10 @@ fn do_hyperlight_stuff() { for _ in 0..20 { let path = hyperlight_guest_path.clone(); - let writer_func = Arc::new(Mutex::new(fn_writer)); let handle = spawn(move || -> Result<()> { // Create a new sandbox. - let usandbox = UninitializedSandbox::new( - GuestBinary::FilePath(path), - None, - None, - Some(&writer_func), - )?; + let mut usandbox = UninitializedSandbox::new(GuestBinary::FilePath(path), None, None)?; + usandbox.register_print(fn_writer)?; // Initialize the sandbox. @@ -103,7 +97,6 @@ fn do_hyperlight_stuff() { GuestBinary::FilePath(hyperlight_guest_path.clone()), None, None, - None, ) .expect("Failed to create UninitializedSandbox"); diff --git a/src/hyperlight_host/examples/tracing-chrome/main.rs b/src/hyperlight_host/examples/tracing-chrome/main.rs index 401043677..c1c797553 100644 --- a/src/hyperlight_host/examples/tracing-chrome/main.rs +++ b/src/hyperlight_host/examples/tracing-chrome/main.rs @@ -33,8 +33,7 @@ fn main() -> Result<()> { simple_guest_as_string().expect("Cannot find the guest binary at the expected location."); // Create a new sandbox. - let usandbox = - UninitializedSandbox::new(GuestBinary::FilePath(simple_guest_path), None, None, None)?; + let usandbox = UninitializedSandbox::new(GuestBinary::FilePath(simple_guest_path), None, None)?; let mut sbox = usandbox .evolve(Noop::::default()) diff --git a/src/hyperlight_host/examples/tracing-otlp/main.rs b/src/hyperlight_host/examples/tracing-otlp/main.rs index bd09faa65..bc89aae1a 100644 --- a/src/hyperlight_host/examples/tracing-otlp/main.rs +++ b/src/hyperlight_host/examples/tracing-otlp/main.rs @@ -116,7 +116,6 @@ fn run_example(wait_input: bool) -> HyperlightResult<()> { for i in 0..10 { let path = hyperlight_guest_path.clone(); let exit = Arc::clone(&should_exit); - let writer_func = Arc::new(Mutex::new(fn_writer)); let handle = spawn(move || -> HyperlightResult<()> { while !*exit.try_lock().unwrap() { // Construct a new span named "hyperlight tracing example thread" with INFO level. @@ -130,12 +129,9 @@ fn run_example(wait_input: bool) -> HyperlightResult<()> { let _entered = span.enter(); // Create a new sandbox. - let usandbox = UninitializedSandbox::new( - GuestBinary::FilePath(path.clone()), - None, - None, - Some(&writer_func), - )?; + let mut usandbox = + UninitializedSandbox::new(GuestBinary::FilePath(path.clone()), None, None)?; + usandbox.register_print(fn_writer)?; // Initialize the sandbox. diff --git a/src/hyperlight_host/examples/tracing-tracy/main.rs b/src/hyperlight_host/examples/tracing-tracy/main.rs index c2d1b2abc..72b929a2d 100644 --- a/src/hyperlight_host/examples/tracing-tracy/main.rs +++ b/src/hyperlight_host/examples/tracing-tracy/main.rs @@ -39,8 +39,7 @@ fn main() -> Result<()> { simple_guest_as_string().expect("Cannot find the guest binary at the expected location."); // Create a new sandbox. - let usandbox = - UninitializedSandbox::new(GuestBinary::FilePath(simple_guest_path), None, None, None)?; + let usandbox = UninitializedSandbox::new(GuestBinary::FilePath(simple_guest_path), None, None)?; let mut sbox = usandbox .evolve(Noop::::default()) diff --git a/src/hyperlight_host/examples/tracing/main.rs b/src/hyperlight_host/examples/tracing/main.rs index 0c572adea..077dec17b 100644 --- a/src/hyperlight_host/examples/tracing/main.rs +++ b/src/hyperlight_host/examples/tracing/main.rs @@ -17,7 +17,6 @@ limitations under the License. use hyperlight_common::flatbuffer_wrappers::function_types::{ParameterValue, ReturnType}; use tracing::{span, Level}; extern crate hyperlight_host; -use std::sync::{Arc, Mutex}; use std::thread::{spawn, JoinHandle}; use hyperlight_host::sandbox::uninitialized::UninitializedSandbox; @@ -60,7 +59,6 @@ fn run_example() -> Result<()> { for i in 0..10 { let path = hyperlight_guest_path.clone(); - let writer_func = Arc::new(Mutex::new(fn_writer)); let handle = spawn(move || -> Result<()> { // Construct a new span named "hyperlight tracing example thread" with INFO level. let id = Uuid::new_v4(); @@ -73,12 +71,8 @@ fn run_example() -> Result<()> { let _entered = span.enter(); // Create a new sandbox. - let usandbox = UninitializedSandbox::new( - GuestBinary::FilePath(path), - None, - None, - Some(&writer_func), - )?; + let mut usandbox = UninitializedSandbox::new(GuestBinary::FilePath(path), None, None)?; + usandbox.register_print(fn_writer)?; // Initialize the sandbox. @@ -119,7 +113,6 @@ fn run_example() -> Result<()> { GuestBinary::FilePath(hyperlight_guest_path.clone()), None, None, - None, )?; // Initialize the sandbox. diff --git a/src/hyperlight_host/src/func/call_ctx.rs b/src/hyperlight_host/src/func/call_ctx.rs index bc48e249c..9d7404410 100644 --- a/src/hyperlight_host/src/func/call_ctx.rs +++ b/src/hyperlight_host/src/func/call_ctx.rs @@ -117,7 +117,7 @@ mod tests { let path = simple_guest_as_string().map_err(|e| { HyperlightError::Error(format!("failed to get simple guest path ({e:?})")) })?; - UninitializedSandbox::new(GuestBinary::FilePath(path), None, None, None) + UninitializedSandbox::new(GuestBinary::FilePath(path), None, None) } /// Test to create a `MultiUseSandbox`, then call several guest functions diff --git a/src/hyperlight_host/src/func/guest_dispatch.rs b/src/hyperlight_host/src/func/guest_dispatch.rs index 9d4cb0c55..8b911c295 100644 --- a/src/hyperlight_host/src/func/guest_dispatch.rs +++ b/src/hyperlight_host/src/func/guest_dispatch.rs @@ -155,7 +155,6 @@ mod tests { GuestBinary::FilePath(simple_guest_as_string().expect("Guest Binary Missing")), None, None, - None, ) .unwrap(); @@ -189,7 +188,6 @@ mod tests { GuestBinary::FilePath(simple_guest_as_string().expect("Guest Binary Missing")), None, None, - None, ) .unwrap(); @@ -221,7 +219,6 @@ mod tests { GuestBinary::FilePath(simple_guest_as_string().expect("Guest Binary Missing")), None, None, - None, ) .unwrap() }; @@ -337,8 +334,6 @@ mod tests { None, // by default, the below represents in-hypervisor mode None, - // just use the built-in host print function - None, ) .unwrap(); test_call_guest_function_by_name(u_sbox); @@ -360,7 +355,6 @@ mod tests { GuestBinary::FilePath(simple_guest_as_string().expect("Guest Binary Missing")), None, None, - None, )?; let sandbox: MultiUseSandbox = usbox.evolve(Noop::default())?; let mut ctx = sandbox.new_call_context(); @@ -409,7 +403,6 @@ mod tests { GuestBinary::FilePath(callback_guest_as_string().expect("Guest Binary Missing")), None, None, - None, ) .unwrap(); @@ -448,7 +441,6 @@ mod tests { GuestBinary::FilePath(simple_guest_as_string().expect("Guest Binary Missing")), None, None, - None, ) .unwrap(); diff --git a/src/hyperlight_host/src/func/host_functions.rs b/src/hyperlight_host/src/func/host_functions.rs index 9bfdd60d5..cbb231111 100644 --- a/src/hyperlight_host/src/func/host_functions.rs +++ b/src/hyperlight_host/src/func/host_functions.rs @@ -90,6 +90,39 @@ macro_rules! impl_host_function { } } + impl HostFunction for &dyn HostFunction + where + $($P: SupportedParameterType + Clone,)* + R: SupportedReturnType, + { + /// Register the host function with the given name in the sandbox. + #[instrument( + err(Debug), skip(self, sandbox), parent = Span::current(), level = "Trace" + )] + fn register( + &self, + sandbox: &mut UninitializedSandbox, + name: &str, + ) -> Result<()> { + (**self).register(sandbox, name) + } + + /// Register the host function with the given name in the sandbox, allowing extra syscalls. + #[cfg(all(feature = "seccomp", target_os = "linux"))] + #[instrument( + err(Debug), skip(self, sandbox, extra_allowed_syscalls), + parent = Span::current(), level = "Trace" + )] + fn register_with_extra_allowed_syscalls( + &self, + sandbox: &mut UninitializedSandbox, + name: &str, + extra_allowed_syscalls: Vec, + ) -> Result<()> { + (**self).register_with_extra_allowed_syscalls(sandbox, name, extra_allowed_syscalls) + } + } + impl IntoHostFunction for F where F: FnMut($($P),*) -> Result + Send + 'static, @@ -126,6 +159,18 @@ macro_rules! impl_host_function { } } + impl IntoHostFunction for &dyn HostFunction + where + R: SupportedReturnType, + $($P: SupportedParameterType + Clone,)* + { + type Output = Self; + + fn into_host_function(self) -> Self::Output { + self + } + } + fn register_host_function( self_: Arc>, sandbox: &mut UninitializedSandbox, diff --git a/src/hyperlight_host/src/hypervisor/hypervisor_handler.rs b/src/hyperlight_host/src/hypervisor/hypervisor_handler.rs index 450668130..5044f0b29 100644 --- a/src/hyperlight_host/src/hypervisor/hypervisor_handler.rs +++ b/src/hyperlight_host/src/hypervisor/hypervisor_handler.rs @@ -976,7 +976,6 @@ mod tests { GuestBinary::FilePath(simple_guest_as_string().expect("Guest Binary Missing")), cfg, None, - None, ) .unwrap(); diff --git a/src/hyperlight_host/src/hypervisor/mod.rs b/src/hyperlight_host/src/hypervisor/mod.rs index 11ded324f..a4617c553 100644 --- a/src/hyperlight_host/src/hypervisor/mod.rs +++ b/src/hyperlight_host/src/hypervisor/mod.rs @@ -364,7 +364,7 @@ pub(crate) mod tests { } let sandbox = - UninitializedSandbox::new(GuestBinary::FilePath(filename.clone()), None, None, None)?; + UninitializedSandbox::new(GuestBinary::FilePath(filename.clone()), None, None)?; let (hshm, gshm) = sandbox.mgr.build(); drop(hshm); diff --git a/src/hyperlight_host/src/metrics/mod.rs b/src/hyperlight_host/src/metrics/mod.rs index 7db1ed4cc..17b6f9668 100644 --- a/src/hyperlight_host/src/metrics/mod.rs +++ b/src/hyperlight_host/src/metrics/mod.rs @@ -111,7 +111,6 @@ mod tests { GuestBinary::FilePath(simple_guest_as_string().unwrap()), None, None, - None, ) .unwrap(); diff --git a/src/hyperlight_host/src/sandbox/initialized_multi_use.rs b/src/hyperlight_host/src/sandbox/initialized_multi_use.rs index d8db71063..f1a67bde1 100644 --- a/src/hyperlight_host/src/sandbox/initialized_multi_use.rs +++ b/src/hyperlight_host/src/sandbox/initialized_multi_use.rs @@ -113,7 +113,6 @@ impl MultiUseSandbox { /// GuestBinary::FilePath("some_guest_binary".to_string()), /// None, /// None, - /// None, /// ).unwrap(); /// let sbox: MultiUseSandbox = u_sbox.evolve(Noop::default()).unwrap(); /// // Next, create a new call context from the single-use sandbox. @@ -279,8 +278,7 @@ mod tests { let sbox1: MultiUseSandbox = { let path = simple_guest_as_string().unwrap(); let u_sbox = - UninitializedSandbox::new(GuestBinary::FilePath(path), Some(cfg), None, None) - .unwrap(); + UninitializedSandbox::new(GuestBinary::FilePath(path), Some(cfg), None).unwrap(); u_sbox.evolve(Noop::default()) } .unwrap(); @@ -299,8 +297,7 @@ mod tests { let sbox2: MultiUseSandbox = { let path = simple_guest_as_string().unwrap(); let u_sbox = - UninitializedSandbox::new(GuestBinary::FilePath(path), Some(cfg), None, None) - .unwrap(); + UninitializedSandbox::new(GuestBinary::FilePath(path), Some(cfg), None).unwrap(); u_sbox.evolve(Noop::default()) } .unwrap(); @@ -326,7 +323,7 @@ mod tests { let sbox1: MultiUseSandbox = { let path = simple_guest_as_string().unwrap(); let u_sbox = - UninitializedSandbox::new(GuestBinary::FilePath(path), None, None, None).unwrap(); + UninitializedSandbox::new(GuestBinary::FilePath(path), None, None).unwrap(); u_sbox.evolve(Noop::default()) } .unwrap(); diff --git a/src/hyperlight_host/src/sandbox/mod.rs b/src/hyperlight_host/src/sandbox/mod.rs index 809d46811..0f532003d 100644 --- a/src/hyperlight_host/src/sandbox/mod.rs +++ b/src/hyperlight_host/src/sandbox/mod.rs @@ -134,13 +134,9 @@ mod tests { for i in 0..10 { let simple_guest_path = simple_guest_as_string().expect("Guest Binary Missing"); - let unintializedsandbox = UninitializedSandbox::new( - GuestBinary::FilePath(simple_guest_path), - None, - None, - None, - ) - .unwrap_or_else(|_| panic!("Failed to create UninitializedSandbox {}", i)); + let unintializedsandbox = + UninitializedSandbox::new(GuestBinary::FilePath(simple_guest_path), None, None) + .unwrap_or_else(|_| panic!("Failed to create UninitializedSandbox {}", i)); unintializedsandbox_queue .push(unintializedsandbox) diff --git a/src/hyperlight_host/src/sandbox/uninitialized.rs b/src/hyperlight_host/src/sandbox/uninitialized.rs index c8ecc9738..a923cf6ae 100644 --- a/src/hyperlight_host/src/sandbox/uninitialized.rs +++ b/src/hyperlight_host/src/sandbox/uninitialized.rs @@ -38,6 +38,21 @@ use crate::sandbox_state::sandbox::EvolvableSandbox; use crate::sandbox_state::transition::Noop; use crate::{log_build_details, log_then_return, new_error, MultiUseSandbox, Result}; +#[cfg(all(target_os = "linux", feature = "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, +]; + /// A preliminary `Sandbox`, not yet ready to execute guest code. /// /// Prior to initializing a full-fledged `Sandbox`, you must create one of @@ -119,14 +134,13 @@ impl UninitializedSandbox { /// The err attribute is used to emit an error should the Result be an error, it uses the std::`fmt::Debug trait` to print the error. #[instrument( err(Debug), - skip(guest_binary, host_print_writer), + skip(guest_binary), parent = Span::current() )] pub fn new( guest_binary: GuestBinary, cfg: Option, sandbox_run_options: Option, - host_print_writer: Option<&dyn HostFunction>, ) -> Result { log_build_details(); @@ -179,52 +193,8 @@ impl UninitializedSandbox { debug_info, }; - // TODO: These only here to accommodate some writer functions. - // We should modify the `UninitializedSandbox` to follow the builder pattern we use in - // hyperlight-wasm to allow the user to specify what syscalls they need specifically. - - #[cfg(all(target_os = "linux", feature = "seccomp"))] - let extra_allowed_syscalls_for_writer_func = vec![ - // 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, - ]; - // If we were passed a writer for host print register it otherwise use the default. - match host_print_writer { - Some(writer_func) => { - #[cfg(any(target_os = "windows", not(feature = "seccomp")))] - writer_func.register(&mut sandbox, "HostPrint")?; - - #[cfg(all(target_os = "linux", feature = "seccomp"))] - writer_func.register_with_extra_allowed_syscalls( - &mut sandbox, - "HostPrint", - extra_allowed_syscalls_for_writer_func, - )?; - } - None => { - let default_writer = Arc::new(Mutex::new(default_writer_func)); - - #[cfg(any(target_os = "windows", not(feature = "seccomp")))] - default_writer.register(&mut sandbox, "HostPrint")?; - - #[cfg(all(target_os = "linux", feature = "seccomp"))] - default_writer.register_with_extra_allowed_syscalls( - &mut sandbox, - "HostPrint", - extra_allowed_syscalls_for_writer_func, - )?; - } - } + sandbox.register_print(default_writer_func)?; crate::debug!("Sandbox created: {:#?}", sandbox); @@ -274,7 +244,9 @@ impl UninitializedSandbox { host_func.into_host_function().register(self, name.as_ref()) } - /// Register the host function with the given name in the sandbox, allowing extra syscalls. + /// Register the host function with the given name in the sandbox. + /// Unlike `register`, this variant takes a list of extra syscalls that will + /// allowed during the execution of the function handler. #[cfg(all(feature = "seccomp", target_os = "linux"))] pub fn register_with_extra_allowed_syscalls( &mut self, @@ -290,6 +262,52 @@ impl UninitializedSandbox { .into_host_function() .register_with_extra_allowed_syscalls(self, name.as_ref(), extra_allowed_syscalls) } + + /// Register a host function named "HostPrint" that will be called by the guest + /// when it wants to print to the console. + /// The "HostPrint" host function is kind of special, as we expect it to have the + /// `FnMut(String) -> i32` signature. + pub fn register_print( + &mut self, + print_func: impl IntoHostFunction, + ) -> Result<()> { + #[cfg(not(all(target_os = "linux", feature = "seccomp")))] + self.register("HostPrint", print_func)?; + + #[cfg(all(target_os = "linux", feature = "seccomp"))] + self.register_with_extra_allowed_syscalls( + "HostPrint", + print_func, + EXTRA_ALLOWED_SYSCALLS_FOR_WRITER_FUNC.iter().copied(), + )?; + + Ok(()) + } + + /// Register a host function named "HostPrint" that will be called by the guest + /// when it wants to print to the console. + /// The "HostPrint" host function is kind of special, as we expect it to have the + /// `FnMut(String) -> i32` signature. + /// Unlike `register_print`, this variant takes a list of extra syscalls that will + /// allowed during the execution of the function handler. + #[cfg(all(feature = "seccomp", target_os = "linux"))] + pub fn register_print_with_extra_allowed_syscalls( + &mut self, + print_func: impl IntoHostFunction, + extra_allowed_syscalls: impl IntoIterator, + ) -> Result<()> { + #[cfg(all(target_os = "linux", feature = "seccomp"))] + self.register_with_extra_allowed_syscalls( + "HostPrint", + print_func, + EXTRA_ALLOWED_SYSCALLS_FOR_WRITER_FUNC + .iter() + .copied() + .chain(extra_allowed_syscalls), + )?; + + Ok(()) + } } // Check to see if the current version of Windows is supported // Hyperlight is only supported on Windows 11 and Windows Server 2022 and later @@ -319,7 +337,8 @@ fn check_windows_version() -> Result<()> { #[cfg(test)] mod tests { use std::path::PathBuf; - use std::sync::{Arc, Mutex}; + use std::sync::mpsc::channel; + use std::sync::Arc; use std::time::Duration; use std::{fs, thread}; @@ -348,7 +367,7 @@ mod tests { let binary_path = simple_guest_as_string().unwrap(); let sandbox = - UninitializedSandbox::new(GuestBinary::FilePath(binary_path.clone()), None, None, None); + UninitializedSandbox::new(GuestBinary::FilePath(binary_path.clone()), None, None); assert!(sandbox.is_ok()); // Guest Binary does not exist at path @@ -359,7 +378,6 @@ mod tests { GuestBinary::FilePath(binary_path_does_not_exist), None, None, - None, ); assert!(uninitialized_sandbox.is_err()); @@ -376,12 +394,11 @@ mod tests { }; let uninitialized_sandbox = - UninitializedSandbox::new(GuestBinary::FilePath(binary_path.clone()), cfg, None, None); + UninitializedSandbox::new(GuestBinary::FilePath(binary_path.clone()), cfg, None); assert!(uninitialized_sandbox.is_ok()); let uninitialized_sandbox = - UninitializedSandbox::new(GuestBinary::FilePath(binary_path), None, None, None) - .unwrap(); + UninitializedSandbox::new(GuestBinary::FilePath(binary_path), None, None).unwrap(); // Get a Sandbox from an uninitialized sandbox without a call back function @@ -394,7 +411,6 @@ mod tests { GuestBinary::Buffer(fs::read(binary_path).unwrap()), None, None, - None, ); assert!(sandbox.is_ok()); @@ -403,7 +419,7 @@ mod tests { let binary_path = simple_guest_as_string().unwrap(); let mut bytes = fs::read(binary_path).unwrap(); let _ = bytes.split_off(100); - let sandbox = UninitializedSandbox::new(GuestBinary::Buffer(bytes), None, None, None); + let sandbox = UninitializedSandbox::new(GuestBinary::Buffer(bytes), None, None); assert!(sandbox.is_err()); } @@ -424,7 +440,6 @@ mod tests { GuestBinary::FilePath(simple_guest_as_string().expect("Guest Binary Missing")), None, None, - None, ) .unwrap() }; @@ -533,28 +548,24 @@ mod tests { // after the Sandbox instance has been dropped // this example is fairly contrived but we should still support such an approach. - let received_msg = Arc::new(Mutex::new(String::new())); - let received_msg_clone = received_msg.clone(); + let (tx, rx) = channel(); let writer = move |msg| { - let mut received_msg = received_msg_clone - .try_lock() - .map_err(|_| new_error!("Error locking")) - .unwrap(); - *received_msg = msg; + let _ = tx.send(msg); Ok(0) }; - let hostfunc = Arc::new(Mutex::new(writer)); - - let sandbox = UninitializedSandbox::new( + let mut sandbox = UninitializedSandbox::new( GuestBinary::FilePath(simple_guest_as_string().expect("Guest Binary Missing")), None, None, - Some(&hostfunc), ) .expect("Failed to create sandbox"); + sandbox + .register_print(writer) + .expect("Failed to register host print function"); + let host_funcs = sandbox .host_funcs .try_lock() @@ -566,14 +577,8 @@ mod tests { drop(sandbox); - assert_eq!( - received_msg - .try_lock() - .map_err(|_| new_error!("Error locking")) - .unwrap() - .as_str(), - "test" - ); + let received_msgs: Vec<_> = rx.into_iter().collect(); + assert_eq!(received_msgs, ["test"]); // There may be cases where a mutable reference to the captured variable is not required to be used outside the closure // e.g. if the function is writing to a file or a socket etc. @@ -640,15 +645,17 @@ mod tests { Ok(0) } - let writer_func = Arc::new(Mutex::new(fn_writer)); - let sandbox = UninitializedSandbox::new( + let mut sandbox = UninitializedSandbox::new( GuestBinary::FilePath(simple_guest_as_string().expect("Guest Binary Missing")), None, None, - Some(&writer_func), ) .expect("Failed to create sandbox"); + sandbox + .register_print(fn_writer) + .expect("Failed to register host print function"); + let host_funcs = sandbox .host_funcs .try_lock() @@ -666,16 +673,17 @@ mod tests { let writer_closure = move |s| test_host_print.write(s); - let writer_method = Arc::new(Mutex::new(writer_closure)); - - let sandbox = UninitializedSandbox::new( + let mut sandbox = UninitializedSandbox::new( GuestBinary::FilePath(simple_guest_as_string().expect("Guest Binary Missing")), None, None, - Some(&writer_method), ) .expect("Failed to create sandbox"); + sandbox + .register_print(writer_closure) + .expect("Failed to register host print function"); + let host_funcs = sandbox .host_funcs .try_lock() @@ -709,13 +717,8 @@ mod tests { let unintializedsandbox = { let err_string = format!("failed to create UninitializedSandbox {i}"); let err_str = err_string.as_str(); - UninitializedSandbox::new( - GuestBinary::FilePath(simple_guest_path), - None, - None, - None, - ) - .expect(err_str) + UninitializedSandbox::new(GuestBinary::FilePath(simple_guest_path), None, None) + .expect(err_str) }; { @@ -837,8 +840,7 @@ mod tests { let mut binary_path = simple_guest_as_string().unwrap(); binary_path.push_str("does_not_exist"); - let sbox = - UninitializedSandbox::new(GuestBinary::FilePath(binary_path), None, None, None); + let sbox = UninitializedSandbox::new(GuestBinary::FilePath(binary_path), None, None); assert!(sbox.is_err()); // Now we should still be in span 1 but span 2 should be created (we created entered and exited span 2 when we called UninitializedSandbox::new) @@ -913,12 +915,8 @@ mod tests { let mut invalid_binary_path = simple_guest_as_string().unwrap(); invalid_binary_path.push_str("does_not_exist"); - let sbox = UninitializedSandbox::new( - GuestBinary::FilePath(invalid_binary_path), - None, - None, - None, - ); + let sbox = + UninitializedSandbox::new(GuestBinary::FilePath(invalid_binary_path), None, None); assert!(sbox.is_err()); // When tracing is creating log records it will create a log @@ -989,7 +987,6 @@ mod tests { GuestBinary::FilePath(valid_binary_path.into_os_string().into_string().unwrap()), None, None, - None, ); assert!(sbox.is_err()); @@ -1024,7 +1021,6 @@ mod tests { GuestBinary::FilePath(simple_guest_as_string().unwrap()), None, None, - None, ); res.unwrap() }; @@ -1039,12 +1035,8 @@ mod tests { #[test] fn test_invalid_path() { let invalid_path = "some/path/that/does/not/exist"; - let sbox = UninitializedSandbox::new( - GuestBinary::FilePath(invalid_path.to_string()), - None, - None, - None, - ); + let sbox = + UninitializedSandbox::new(GuestBinary::FilePath(invalid_path.to_string()), None, None); println!("{:?}", sbox); #[cfg(target_os = "windows")] assert!( diff --git a/src/hyperlight_host/src/sandbox/uninitialized_evolve.rs b/src/hyperlight_host/src/sandbox/uninitialized_evolve.rs index 0aafdc789..2499dd2aa 100644 --- a/src/hyperlight_host/src/sandbox/uninitialized_evolve.rs +++ b/src/hyperlight_host/src/sandbox/uninitialized_evolve.rs @@ -180,7 +180,6 @@ mod tests { GuestBinary::FilePath(guest_bin_path.clone()), None, None, - None, ) .unwrap(); evolve_impl_multi_use(u_sbox).unwrap(); @@ -201,7 +200,6 @@ mod tests { GuestBinary::FilePath(guest_bin_path.clone()), None, Some(SandboxRunOptions::RunInHypervisor), - None, ) .unwrap(); let err = format!("error evolving sandbox with guest binary {guest_bin_path}"); diff --git a/src/hyperlight_host/tests/common/mod.rs b/src/hyperlight_host/tests/common/mod.rs index 8240a1ef1..716035864 100644 --- a/src/hyperlight_host/tests/common/mod.rs +++ b/src/hyperlight_host/tests/common/mod.rs @@ -30,7 +30,6 @@ pub fn new_uninit() -> Result { GuestBinary::FilePath(get_c_or_rust_simpleguest_path()), None, None, - None, ) } @@ -40,7 +39,6 @@ pub fn new_uninit_rust() -> Result { GuestBinary::FilePath(simple_guest_as_string().unwrap()), None, None, - None, ) } @@ -49,13 +47,20 @@ pub fn get_simpleguest_sandboxes( ) -> Vec { let elf_path = get_c_or_rust_simpleguest_path(); - vec![ + let sandboxes = [ // in hypervisor elf - UninitializedSandbox::new(GuestBinary::FilePath(elf_path.clone()), None, None, writer) - .unwrap() - .evolve(Noop::default()) - .unwrap(), - ] + UninitializedSandbox::new(GuestBinary::FilePath(elf_path.clone()), None, None).unwrap(), + ]; + + sandboxes + .into_iter() + .map(|mut sandbox| { + if let Some(writer) = writer { + sandbox.register_print(writer).unwrap(); + } + sandbox.evolve(Noop::default()).unwrap() + }) + .collect() } pub fn get_callbackguest_uninit_sandboxes( @@ -63,11 +68,20 @@ pub fn get_callbackguest_uninit_sandboxes( ) -> Vec { let elf_path = get_c_or_rust_callbackguest_path(); - vec![ + let sandboxes = [ // in hypervisor elf - UninitializedSandbox::new(GuestBinary::FilePath(elf_path.clone()), None, None, writer) - .unwrap(), - ] + UninitializedSandbox::new(GuestBinary::FilePath(elf_path.clone()), None, None).unwrap(), + ]; + + sandboxes + .into_iter() + .map(|mut sandbox| { + if let Some(writer) = writer { + sandbox.register_print(writer).unwrap(); + } + sandbox + }) + .collect() } // returns the the path of simpleguest binary. Picks rust/c version depending on environment variable GUEST (or rust by default if unset) diff --git a/src/hyperlight_host/tests/integration_test.rs b/src/hyperlight_host/tests/integration_test.rs index ad5f9710a..320d0bf10 100644 --- a/src/hyperlight_host/tests/integration_test.rs +++ b/src/hyperlight_host/tests/integration_test.rs @@ -32,7 +32,7 @@ use crate::common::{new_uninit, new_uninit_rust}; fn print_four_args_c_guest() { let path = c_simple_guest_as_string().unwrap(); let guest_path = GuestBinary::FilePath(path); - let uninit = UninitializedSandbox::new(guest_path, None, None, None); + let uninit = UninitializedSandbox::new(guest_path, None, None); let mut sbox1 = uninit.unwrap().evolve(Noop::default()).unwrap(); let res = sbox1.call_guest_function_by_name( @@ -146,7 +146,7 @@ fn guest_abort_with_context2() { fn guest_abort_c_guest() { let path = c_simple_guest_as_string().unwrap(); let guest_path = GuestBinary::FilePath(path); - let uninit = UninitializedSandbox::new(guest_path, None, None, None); + let uninit = UninitializedSandbox::new(guest_path, None, None); let mut sbox1 = uninit.unwrap().evolve(Noop::default()).unwrap(); let res = sbox1 @@ -248,7 +248,6 @@ fn guest_malloc_abort() { GuestBinary::FilePath(simple_guest_as_string().unwrap()), Some(cfg), None, - None, ) .unwrap(); let mut sbox2 = uninit.evolve(Noop::default()).unwrap(); @@ -271,7 +270,7 @@ fn guest_malloc_abort() { fn dynamic_stack_allocate_c_guest() { let path = c_simple_guest_as_string().unwrap(); let guest_path = GuestBinary::FilePath(path); - let uninit = UninitializedSandbox::new(guest_path, None, None, None); + let uninit = UninitializedSandbox::new(guest_path, None, None); let mut sbox1: MultiUseSandbox = uninit.unwrap().evolve(Noop::default()).unwrap(); let res2 = sbox1 diff --git a/src/hyperlight_host/tests/sandbox_host_tests.rs b/src/hyperlight_host/tests/sandbox_host_tests.rs index 801caa771..6407510df 100644 --- a/src/hyperlight_host/tests/sandbox_host_tests.rs +++ b/src/hyperlight_host/tests/sandbox_host_tests.rs @@ -393,7 +393,6 @@ fn max_memory_sandbox() { GuestBinary::FilePath(simple_guest_as_string().unwrap()), Some(cfg), None, - None, ); assert!(matches!(