Skip to content

Commit fde8fd1

Browse files
committed
Update host-side error handling
- Update initialized_multi_use.rs to use new Result-like error handling - Update mem/mgr.rs to handle host function errors properly - Update sandbox/outb.rs for new error propagation pattern Signed-off-by: Ludvig Liljenberg <[email protected]>
1 parent 37fb131 commit fde8fd1

File tree

3 files changed

+84
-32
lines changed

3 files changed

+84
-32
lines changed

src/hyperlight_host/src/mem/mgr.rs

Lines changed: 15 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -16,11 +16,11 @@ limitations under the License.
1616

1717
use std::cmp::Ordering;
1818

19+
use flatbuffers::FlatBufferBuilder;
1920
use hyperlight_common::flatbuffer_wrappers::function_call::{
2021
FunctionCall, validate_guest_function_call_buffer,
2122
};
22-
use hyperlight_common::flatbuffer_wrappers::function_types::ReturnValue;
23-
use hyperlight_common::flatbuffer_wrappers::guest_error::GuestError;
23+
use hyperlight_common::flatbuffer_wrappers::function_types::FunctionCallResult;
2424
use hyperlight_common::flatbuffer_wrappers::guest_log_data::GuestLogData;
2525
use hyperlight_common::flatbuffer_wrappers::host_function_details::HostFunctionDetails;
2626
use tracing::{Span, instrument};
@@ -470,18 +470,19 @@ impl SandboxMemoryManager<HostSharedMemory> {
470470
)
471471
}
472472

473-
/// Writes a function call result to memory
473+
/// Writes a host function call result to memory
474474
#[instrument(err(Debug), skip_all, parent = Span::current(), level= "Trace")]
475-
pub(crate) fn write_response_from_host_method_call(&mut self, res: &ReturnValue) -> Result<()> {
476-
let function_call_ret_val_buffer = Vec::<u8>::try_from(res).map_err(|_| {
477-
new_error!(
478-
"write_response_from_host_method_call: failed to convert ReturnValue to Vec<u8>"
479-
)
480-
})?;
475+
pub(crate) fn write_response_from_host_function_call(
476+
&mut self,
477+
res: &FunctionCallResult,
478+
) -> Result<()> {
479+
let mut builder = FlatBufferBuilder::new();
480+
let data = res.encode(&mut builder);
481+
481482
self.shared_mem.push_buffer(
482483
self.layout.input_data_buffer_offset,
483484
self.layout.sandbox_memory_config.get_input_data_size(),
484-
function_call_ret_val_buffer.as_slice(),
485+
data,
485486
)
486487
}
487488

@@ -502,10 +503,11 @@ impl SandboxMemoryManager<HostSharedMemory> {
502503
)
503504
}
504505

505-
/// Reads a function call result from memory
506+
/// Reads a function call result from memory.
507+
/// A function call result can be either an error or a successful return value.
506508
#[instrument(err(Debug), skip_all, parent = Span::current(), level= "Trace")]
507-
pub(crate) fn get_guest_function_call_result(&mut self) -> Result<ReturnValue> {
508-
self.shared_mem.try_pop_buffer_into::<ReturnValue>(
509+
pub(crate) fn get_guest_function_call_result(&mut self) -> Result<FunctionCallResult> {
510+
self.shared_mem.try_pop_buffer_into::<FunctionCallResult>(
509511
self.layout.output_data_buffer_offset,
510512
self.layout.sandbox_memory_config.get_output_data_size(),
511513
)
@@ -520,14 +522,6 @@ impl SandboxMemoryManager<HostSharedMemory> {
520522
)
521523
}
522524

523-
/// Get the guest error data
524-
pub(crate) fn get_guest_error(&mut self) -> Result<GuestError> {
525-
self.shared_mem.try_pop_buffer_into::<GuestError>(
526-
self.layout.output_data_buffer_offset,
527-
self.layout.sandbox_memory_config.get_output_data_size(),
528-
)
529-
}
530-
531525
pub(crate) fn clear_io_buffers(&mut self) {
532526
// Clear the output data buffer
533527
loop {

src/hyperlight_host/src/sandbox/initialized_multi_use.rs

Lines changed: 61 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -28,22 +28,24 @@ use hyperlight_common::flatbuffer_wrappers::function_call::{FunctionCall, Functi
2828
use hyperlight_common::flatbuffer_wrappers::function_types::{
2929
ParameterValue, ReturnType, ReturnValue,
3030
};
31+
use hyperlight_common::flatbuffer_wrappers::guest_error::ErrorCode;
3132
use hyperlight_common::flatbuffer_wrappers::util::estimate_flatbuffer_capacity;
3233
use tracing::{Span, instrument};
3334

3435
use super::host_funcs::FunctionRegistry;
3536
use super::snapshot::Snapshot;
3637
use super::{Callable, WrapperGetter};
37-
use crate::HyperlightError::SnapshotSandboxMismatch;
38-
use crate::func::guest_err::check_for_guest_error;
38+
use crate::HyperlightError::{self, SnapshotSandboxMismatch};
3939
use crate::func::{ParameterTuple, SupportedReturnType};
4040
use crate::hypervisor::{Hypervisor, InterruptHandle};
4141
#[cfg(unix)]
4242
use crate::mem::memory_region::MemoryRegionType;
4343
use crate::mem::memory_region::{MemoryRegion, MemoryRegionFlags};
4444
use crate::mem::ptr::RawPtr;
4545
use crate::mem::shared_mem::HostSharedMemory;
46-
use crate::metrics::maybe_time_and_emit_guest_call;
46+
use crate::metrics::{
47+
METRIC_GUEST_ERROR, METRIC_GUEST_ERROR_LABEL_CODE, maybe_time_and_emit_guest_call,
48+
};
4749
use crate::sandbox::mem_mgr::MemMgrWrapper;
4850
use crate::{Result, log_then_return};
4951

@@ -418,11 +420,28 @@ impl MultiUseSandbox {
418420
)?;
419421

420422
self.mem_mgr.check_stack_guard()?;
421-
check_for_guest_error(self.get_mgr_wrapper_mut())?;
422423

423-
self.get_mgr_wrapper_mut()
424+
let guest_result = self
425+
.get_mgr_wrapper_mut()
424426
.as_mut()
425-
.get_guest_function_call_result()
427+
.get_guest_function_call_result()?
428+
.into_inner();
429+
430+
match guest_result {
431+
Ok(val) => Ok(val),
432+
Err(guest_error) => {
433+
metrics::counter!(
434+
METRIC_GUEST_ERROR,
435+
METRIC_GUEST_ERROR_LABEL_CODE => (guest_error.code as u64).to_string()
436+
)
437+
.increment(1);
438+
439+
Err(match guest_error.code {
440+
ErrorCode::StackOverflow => HyperlightError::StackOverflow(),
441+
_ => HyperlightError::GuestError(guest_error.code, guest_error.message),
442+
})
443+
}
444+
}
426445
})();
427446

428447
// In the happy path we do not need to clear io-buffers from the host because:
@@ -512,6 +531,36 @@ mod tests {
512531
use crate::sandbox::SandboxConfiguration;
513532
use crate::{GuestBinary, HyperlightError, MultiUseSandbox, Result, UninitializedSandbox};
514533

534+
/// Make sure input/output buffers are properly reset after guest call (with host call)
535+
#[test]
536+
#[ignore = "added this test before fixing bug"]
537+
fn host_func_error() {
538+
let path = simple_guest_as_string().unwrap();
539+
let mut sandbox = UninitializedSandbox::new(GuestBinary::FilePath(path), None).unwrap();
540+
sandbox
541+
.register("HostError", || -> Result<()> {
542+
Err(HyperlightError::Error("hi".to_string()))
543+
})
544+
.unwrap();
545+
let mut sandbox = sandbox.evolve().unwrap();
546+
547+
// will exhaust io if leaky
548+
for _ in 0..1000 {
549+
let result = sandbox
550+
.call::<i64>(
551+
"CallGivenParamlessHostFuncThatReturnsI64",
552+
"HostError".to_string(),
553+
)
554+
.unwrap_err();
555+
556+
assert!(
557+
matches!(result, HyperlightError::Error(ref msg) if msg == "hi"),
558+
"Expected HyperlightError::Error('hi'), got {:?}",
559+
result
560+
);
561+
}
562+
}
563+
515564
/// Make sure input/output buffers are properly reset after guest call (with host call)
516565
#[test]
517566
fn io_buffer_reset() {
@@ -625,6 +674,9 @@ mod tests {
625674
#[ignore]
626675
#[cfg(target_os = "linux")]
627676
fn test_violate_seccomp_filters() -> Result<()> {
677+
#[cfg(feature = "seccomp")]
678+
use hyperlight_common::flatbuffer_wrappers::guest_error::ErrorCode;
679+
628680
fn make_get_pid_syscall() -> Result<u64> {
629681
let pid = unsafe { libc::syscall(libc::SYS_getpid) };
630682
Ok(pid as u64)
@@ -648,7 +700,9 @@ mod tests {
648700
match res {
649701
Ok(_) => panic!("Expected to fail due to seccomp violation"),
650702
Err(e) => match e {
651-
HyperlightError::DisallowedSyscall => {}
703+
HyperlightError::GuestError(t, msg)
704+
if t == ErrorCode::HostFunctionError
705+
&& msg.contains("Seccomp filter trapped on disallowed syscall") => {}
652706
_ => panic!("Expected DisallowedSyscall error: {}", e),
653707
},
654708
}

src/hyperlight_host/src/sandbox/outb.rs

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -22,8 +22,8 @@ use std::sync::{Arc, Mutex};
2222
use fallible_iterator::FallibleIterator;
2323
#[cfg(feature = "unwind_guest")]
2424
use framehop::Unwinder;
25-
use hyperlight_common::flatbuffer_wrappers::function_types::ParameterValue;
26-
use hyperlight_common::flatbuffer_wrappers::guest_error::ErrorCode;
25+
use hyperlight_common::flatbuffer_wrappers::function_types::{FunctionCallResult, ParameterValue};
26+
use hyperlight_common::flatbuffer_wrappers::guest_error::{ErrorCode, GuestError};
2727
use hyperlight_common::flatbuffer_wrappers::guest_log_data::GuestLogData;
2828
use hyperlight_common::outb::{Exception, OutBAction};
2929
#[cfg(feature = "trace_guest")]
@@ -284,10 +284,14 @@ pub(crate) fn handle_outb(
284284
let res = host_funcs
285285
.try_lock()
286286
.map_err(|e| new_error!("Error locking at {}:{}: {}", file!(), line!(), e))?
287-
.call_host_function(&name, args)?;
287+
.call_host_function(&name, args)
288+
.map_err(|e| GuestError::new(ErrorCode::HostFunctionError, e.to_string()));
289+
290+
let func_result = FunctionCallResult::new(res);
291+
288292
mem_mgr
289293
.as_mut()
290-
.write_response_from_host_method_call(&res)?; // push input buffers
294+
.write_response_from_host_function_call(&func_result)?;
291295

292296
Ok(())
293297
}

0 commit comments

Comments
 (0)