From 1f65826b41fc58ed9919ab580af20530feb0754f Mon Sep 17 00:00:00 2001 From: Lucy Menon <168595099+syntactically@users.noreply.github.com> Date: Wed, 12 Aug 2026 10:35:38 +0100 Subject: [PATCH] [hyperlight_component_util] Expose the fallibility of calls to the host API Previously, the generated bindings code for the host side of a component interface would panic in a number of situations if something went wrong while trying to call into the guest. This is undesirable when building hosts that should be reliable in the face of malicious guests that manage to crash the guest partition. This commit changes the host-side bindgen to wrap the return types of guest calls in a `Result` wrapper, making it easier to deal with these errors gracefully. Co-authored-by: James Sturtevant Signed-off-by: Lucy Menon <168595099+syntactically@users.noreply.github.com> --- src/hyperlight_common/src/component.rs | 12 +++++ src/hyperlight_component_util/src/host.rs | 36 ++++++++------- src/hyperlight_component_util/src/rtypes.rs | 8 ++-- src/hyperlight_host/tests/wit_test.rs | 50 +++++++++++++-------- src/tests/rust_guests/witguest/guest.wit | 5 +++ src/tests/rust_guests/witguest/src/main.rs | 10 +++++ 6 files changed, 83 insertions(+), 38 deletions(-) diff --git a/src/hyperlight_common/src/component.rs b/src/hyperlight_common/src/component.rs index 524d90f114..27cbc084f0 100644 --- a/src/hyperlight_common/src/component.rs +++ b/src/hyperlight_common/src/component.rs @@ -41,6 +41,8 @@ mod private { /// imports. pub trait Positivity: private::Sealed { type NegativeOfThis: Positivity; + /// How a call to one of the interface's functions returns. + type CallResult; /// How a borrowed resource handle reaches the implementation. type Borrow<'a, T: 'a>; } @@ -58,6 +60,8 @@ impl private::Sealed for Positive {} impl Positivity for Negative { type NegativeOfThis = Positive; + /// A host implementation is called directly, so it cannot fail. + type CallResult = T; /// A handle arrives as an index into the resource table, held borrowed /// for the duration of the call. type Borrow<'a, T: 'a> = BorrowedResourceGuard<'a, T>; @@ -65,6 +69,14 @@ impl Positivity for Negative { impl Positivity for Positive { type NegativeOfThis = Negative; + /// Every call from the host crosses into the VM, where the guest can trap. + #[cfg(feature = "std")] + type CallResult = anyhow::Result; + /// The guest is not permitted to semantically enlarge its + /// functions to include kinds of failures other than the usual + /// trap/VM issue + #[cfg(not(feature = "std"))] + type CallResult = T; /// The host owns the value, so it hands out a plain reference. type Borrow<'a, T: 'a> = &'a T; } diff --git a/src/hyperlight_component_util/src/host.rs b/src/hyperlight_component_util/src/host.rs index 96f6043c9d..d5f40f5941 100644 --- a/src/hyperlight_component_util/src/host.rs +++ b/src/hyperlight_component_util/src/host.rs @@ -59,19 +59,22 @@ fn emit_export_extern_decl<'a, 'b, 'c>( fn #n(&mut self, #(#param_decls),*) -> #result_decl { let mut to_cleanup = Vec::>::new(); let marshalled = { - let mut rts = self.rt.lock().unwrap(); + let mut rts = self.rt + .lock() + .map_err(<::hyperlight_host::error::HyperlightError as From<_>>::from)?; #[allow(clippy::unused_unit)] (#(#marshal,)*) }; let #ret = ::hyperlight_host::sandbox::Callable::call::<::std::vec::Vec::>(&mut self.sb, #hln, marshalled, - ); - let ::std::result::Result::Ok(#ret) = #ret else { panic!("bad return from guest {:?}", #ret) }; + )?; #[allow(clippy::unused_unit)] - let mut rts = self.rt.lock().unwrap(); + let mut rts = self.rt + .lock() + .map_err(<::hyperlight_host::error::HyperlightError as From<_>>::from)?; #[allow(clippy::unused_unit)] - #unmarshal + ::std::result::Result::Ok(#unmarshal) } } } @@ -167,7 +170,8 @@ impl SelfInfo { orig_id, type_id: vec![(format_ident!("I"), imports_trait_bound)], inner_preamble: quote! { - let mut #inner_id = #outer_id.lock().unwrap(); + let mut #inner_id = #outer_id.lock() + .map_err(<::hyperlight_host::error::HyperlightError as From<_>>::from)?; let mut #inner_id = ::std::ops::DerefMut::deref_mut(&mut #inner_id); }, outer_id, @@ -268,7 +272,8 @@ fn emit_import_extern_decl<'a, 'b, 'c>( let #outer_id = #orig_id.clone(); let captured_rts = rts.clone(); sb.register_host_function(#hln, move |#(#pds),*| { - let mut rts = captured_rts.lock().unwrap(); + let mut rts = captured_rts.lock() + .map_err(<::hyperlight_host::error::HyperlightError as From<_>>::from)?; #inner_preamble let #ret = #callname( ::std::borrow::BorrowMut::<#type_inst>::borrow_mut( @@ -277,8 +282,7 @@ fn emit_import_extern_decl<'a, 'b, 'c>( #(#pus),* ); Ok(#marshal_result) - }) - .unwrap(); + })?; } } ExternDesc::Type(_) => { @@ -407,24 +411,24 @@ fn emit_component<'a, 'b, 'c>(s: &'c mut State<'a, 'b>, wn: WitName, ct: &'c Com pub(crate) sb: S, pub(crate) rt: ::std::sync::Arc<::std::sync::Mutex<#rtsid>>, } - pub(crate) fn register_host_functions + ::std::marker::Send + 'static, S: ::hyperlight_host::func::Registerable>(sb: &mut S, i: I) -> ::std::sync::Arc<::std::sync::Mutex<#rtsid>> { + pub(crate) fn register_host_functions + ::std::marker::Send + 'static, S: ::hyperlight_host::func::Registerable>(sb: &mut S, i: I) -> <::hyperlight_common::component::Positive as ::hyperlight_common::component::Positivity>::CallResult<::std::sync::Arc<::std::sync::Mutex<#rtsid>>> { let rts = ::std::sync::Arc::new(::std::sync::Mutex::new(#rtsid::new())); let #import_id = ::std::sync::Arc::new(::std::sync::Mutex::new(i)); #(#imports)* - rts + Ok(rts) } impl + ::std::marker::Send, S: ::hyperlight_host::sandbox::Callable> #ns::#export_trait<::hyperlight_common::component::Positive, I> for #wrapper_name { #(#exports)* } impl #ns::#r#trait<::hyperlight_common::component::Positive> for ::hyperlight_host::sandbox::UninitializedSandbox { type Exports + ::std::marker::Send> = #wrapper_name; - fn instantiate + ::std::marker::Send + 'static>(mut self, i: I) -> Self::Exports { - let rts = register_host_functions(&mut self, i); - let sb = self.evolve().unwrap(); - #wrapper_name { + fn instantiate + ::std::marker::Send + 'static>(mut self, i: I) -> <::hyperlight_common::component::Positive as ::hyperlight_common::component::Positivity>::CallResult> { + let rts = register_host_functions(&mut self, i)?; + let sb = self.evolve()?; + Ok(#wrapper_name { sb, rt: rts, - } + }) } } }); diff --git a/src/hyperlight_component_util/src/rtypes.rs b/src/hyperlight_component_util/src/rtypes.rs index 4ff6ea3513..c216cc3712 100644 --- a/src/hyperlight_component_util/src/rtypes.rs +++ b/src/hyperlight_component_util/src/rtypes.rs @@ -607,10 +607,12 @@ pub fn emit_func_param(s: &mut State, p: &Param) -> TokenStream { /// Precondition: the result type must only be a named result if there /// are no names in it (i.e. a unit type) pub fn emit_func_result(s: &mut State, r: &etypes::Result<'_>) -> TokenStream { - match r { + let inner = match r { Some(vt) => emit_value(s, vt), None => quote! { () }, - } + }; + let p = s.positivity_param.clone().unwrap_or(quote! { P }); + quote! { <#p as ::hyperlight_common::component::Positivity>::CallResult<#inner> } } /// Emit a Rust typeversion of a component function type. This is only @@ -966,7 +968,7 @@ fn emit_component<'a, 'b, 'c>(s: &'c mut State<'a, 'b>, wn: WitName, ct: &'c Com type Exports + ::core::marker::Send>: #export_name; // todo: can/should this 'static bound be avoided? // it is important right now because this is closed over in host functions - fn instantiate + ::core::marker::Send + 'static>(self, imports: I) -> Self::Exports; + fn instantiate + ::core::marker::Send + 'static>(self, imports: I) ->

::CallResult>; } }); } diff --git a/src/hyperlight_host/tests/wit_test.rs b/src/hyperlight_host/tests/wit_test.rs index 9838048429..a3073ade3b 100644 --- a/src/hyperlight_host/tests/wit_test.rs +++ b/src/hyperlight_host/tests/wit_test.rs @@ -289,14 +289,15 @@ fn sb() -> TestSandbox { let path = wit_guest_as_pathbuf(); let guest_path = GuestBinary::FilePath(path); let uninit = UninitializedSandbox::new(guest_path, None).unwrap(); - test::wit::Test::instantiate(uninit, Host {}) + test::wit::Test::instantiate(uninit, Host {}).unwrap() } mod wit_test { - use proptest::prelude::*; - use crate::bindings::test::wit::{Roundtrip, TestExports, TestHostResource, roundtrip}; + use crate::bindings::test::wit::{ + Failable, Roundtrip, TestExports, TestHostResource, roundtrip, + }; use crate::sb; prop_compose! { @@ -348,7 +349,7 @@ mod wit_test { proptest! { #[test] fn $fn(x $($ty)*) { - assert_eq!(x, sb().roundtrip().$fn(x.clone())) + assert_eq!(x, sb().roundtrip().$fn(x.clone()).unwrap()) } } } @@ -395,7 +396,16 @@ mod wit_test { #[test] fn test_roundtrip_no_result() { - sb().roundtrip().roundtrip_no_result(42); + sb().roundtrip().roundtrip_no_result(42).unwrap(); + } + + #[test] + fn test_guest_trap_returns_error() { + let err = sb().failable().will_trap().unwrap_err(); + assert!( + format!("{err:?}").contains("Guest aborted"), + "unexpected error: {err:?}" + ); } use std::sync::atomic::Ordering::Relaxed; @@ -405,7 +415,7 @@ mod wit_test { let guard = crate::SERIALIZE_TEST_RESOURCE_TESTS.lock(); crate::HAS_BEEN_DROPPED.store(false, Relaxed); { - sb().test_host_resource().test_uses_locally(); + sb().test_host_resource().test_uses_locally().unwrap(); } assert!(crate::HAS_BEEN_DROPPED.load(Relaxed)); drop(guard); @@ -417,10 +427,10 @@ mod wit_test { { let mut sb = sb(); let inst = sb.test_host_resource(); - let r = inst.test_makes(); - inst.test_accepts_borrow(&r); - inst.test_accepts_own(r); - inst.test_returns(); + let r = inst.test_makes().unwrap(); + inst.test_accepts_borrow(&r).unwrap(); + inst.test_accepts_own(r).unwrap(); + inst.test_returns().unwrap(); } assert!(crate::HAS_BEEN_DROPPED.load(Relaxed)); drop(guard); @@ -498,18 +508,20 @@ mod bindgen_test_cases { struct ExportHost; impl test::bindgen_test_cases::Executor for ExportHost { - fn execute(&mut self) -> test::bindgen_test_cases::executor::ExecutionResult { - test::bindgen_test_cases::executor::ExecutionResult { + fn execute( + &mut self, + ) -> anyhow::Result { + Ok(test::bindgen_test_cases::executor::ExecutionResult { message: String::from("executed"), - } + }) } } impl test::bindgen_test_cases::Types for ExportHost { - fn get_status(&mut self) -> test::bindgen_test_cases::types::Status { - test::bindgen_test_cases::types::Status { + fn get_status(&mut self) -> anyhow::Result { + Ok(test::bindgen_test_cases::types::Status { message: String::from("ok"), - } + }) } } @@ -519,10 +531,10 @@ mod bindgen_test_cases { test::bindgen_test_cases::types::Status, > for ExportHost { - fn get_status(&mut self) -> test::bindgen_test_cases::types::Status { - test::bindgen_test_cases::types::Status { + fn get_status(&mut self) -> anyhow::Result { + Ok(test::bindgen_test_cases::types::Status { message: String::from("ok"), - } + }) } } diff --git a/src/tests/rust_guests/witguest/guest.wit b/src/tests/rust_guests/witguest/guest.wit index 9ed164b803..3000e01050 100644 --- a/src/tests/rust_guests/witguest/guest.wit +++ b/src/tests/rust_guests/witguest/guest.wit @@ -5,6 +5,7 @@ world test { import host-resource; export roundtrip; export test-host-resource; + export failable; } interface roundtrip { @@ -89,4 +90,8 @@ interface test-host-resource { test-accepts-borrow: func(x: borrow); test-accepts-own: func(x: own); test-returns: func() -> own; +} + +interface failable { + will-trap: func() -> string; } \ No newline at end of file diff --git a/src/tests/rust_guests/witguest/src/main.rs b/src/tests/rust_guests/witguest/src/main.rs index 37ad9ca89c..51da0bdeac 100644 --- a/src/tests/rust_guests/witguest/src/main.rs +++ b/src/tests/rust_guests/witguest/src/main.rs @@ -206,6 +206,12 @@ impl test::wit::TestHostResource>::T> } } +impl test::wit::Failable for Guest { + fn will_trap(&mut self) -> String { + panic!("deliberate guest crash") + } +} + #[allow(refining_impl_trait)] impl test::wit::TestExports for Guest { type Roundtrip = Self; @@ -216,6 +222,10 @@ impl test::wit::TestExports for Guest { fn test_host_resource(&mut self) -> &mut Self { self } + type Failable = Self; + fn failable(&mut self) -> &mut Self { + self + } } static GUEST_STATE: Mutex = Mutex::new(Guest {