Skip to content

Commit c1d830f

Browse files
committed
crashdump: allow crashdump toggle at sandbox level
- this allows a user to configure the crash dump feature at sandbox level - create a SandboxRuntimeConfig struct to contain all the configuration a sandbox would need at runtime to avoid passing the information as multiple functions arguments - add unit tests to verify crashdump behavior Signed-off-by: Doru Blânzeanu <[email protected]>
1 parent de88423 commit c1d830f

File tree

11 files changed

+513
-251
lines changed

11 files changed

+513
-251
lines changed

.github/workflows/dep_rust.yml

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -135,6 +135,12 @@ jobs:
135135
RUST_LOG: debug
136136
run: just test-rust-gdb-debugging ${{ matrix.config }} ${{ matrix.hypervisor == 'mshv3' && 'mshv3' || ''}}
137137

138+
- name: Run Rust Crashdump tests
139+
env:
140+
CARGO_TERM_COLOR: always
141+
RUST_LOG: debug
142+
run: just test-rust-crashdump ${{ matrix.config }} ${{ matrix.hypervisor == 'mshv3' && 'mshv3' || ''}}
143+
138144
### Benchmarks ###
139145
- name: Install github-cli (Linux mariner)
140146
if: runner.os == 'Linux' && matrix.hypervisor == 'mshv'

Justfile

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -72,6 +72,9 @@ test-like-ci config=default-target hypervisor="kvm":
7272
@# without any driver (should fail to compile)
7373
just test-compilation-fail {{config}}
7474

75+
@# test the crashdump feature
76+
just test-rust-crashdump {{config}}
77+
7578
# runs all tests
7679
test target=default-target features="": (test-unit target features) (test-isolated target features) (test-integration "rust" target features) (test-integration "c" target features) (test-seccomp target features)
7780

@@ -114,6 +117,10 @@ test-rust-gdb-debugging target=default-target features="":
114117
cargo test --profile={{ if target == "debug" { "dev" } else { target } }} --example guest-debugging {{ if features =="" {'--features gdb'} else { "--features gdb," + features } }}
115118
cargo test --profile={{ if target == "debug" { "dev" } else { target } }} {{ if features =="" {'--features gdb'} else { "--features gdb," + features } }} -- test_gdb
116119

120+
# rust test for crashdump
121+
test-rust-crashdump target=default-target features="":
122+
cargo test --profile={{ if target == "debug" { "dev" } else { target } }} {{ if features =="" {'--features crashdump'} else { "--features crashdump," + features } }} -- test_crashdump
123+
117124

118125
################
119126
### LINTING ####

src/hyperlight_host/src/hypervisor/crashdump.rs

Lines changed: 198 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@ limitations under the License.
1515
*/
1616

1717
use std::cmp::min;
18+
use std::io::Write;
1819

1920
use chrono;
2021
use elfcore::{
@@ -248,46 +249,80 @@ impl ReadProcessMemory for GuestMemReader {
248249
}
249250
}
250251

251-
/// Create core dump file from the hypervisor information
252+
/// Create core dump file from the hypervisor information if the sandbox is configured
253+
/// to allow core dumps.
252254
///
253255
/// This function generates an ELF core dump file capturing the hypervisor's state,
254-
/// which can be used for debugging when crashes occur. The file is created in the
255-
/// system's temporary directory with extension '.elf' and the path is printed to stdout and logs.
256+
/// which can be used for debugging when crashes occur.
257+
/// The location of the core dump file is determined by the `HYPERLIGHT_CORE_DUMP_DIR`
258+
/// environment variable. If not set, it defaults to the system's temporary directory.
256259
///
257260
/// # Arguments
258261
/// * `hv`: Reference to the hypervisor implementation
259262
///
260263
/// # Returns
261264
/// * `Result<()>`: Success or error
262-
pub(crate) fn crashdump_to_tempfile(hv: &dyn Hypervisor) -> Result<()> {
265+
pub(crate) fn generate_crashdump(hv: &dyn Hypervisor) -> Result<()> {
263266
log::info!("Creating core dump file...");
264267

265268
// Get crash context from hypervisor
266269
let ctx = hv
267270
.crashdump_context()
268271
.map_err(|e| new_error!("Failed to get crashdump context: {:?}", e))?;
269272

270-
// Set up data sources for the core dump
271-
let guest_view = GuestView::new(&ctx);
272-
let memory_reader = GuestMemReader::new(&ctx);
273+
// Get env variable for core dump directory
274+
let core_dump_dir = std::env::var("HYPERLIGHT_CORE_DUMP_DIR").ok();
273275

274-
// Create and write core dump
275-
let core_builder = CoreDumpBuilder::from_source(guest_view, memory_reader);
276+
// Compute file path on the filesystem
277+
let file_path = core_dump_file_path(core_dump_dir);
276278

279+
let create_dump_file = || {
280+
// Create the file
281+
Ok(Box::new(
282+
std::fs::File::create(&file_path)
283+
.map_err(|e| new_error!("Failed to create core dump file: {:?}", e))?,
284+
) as Box<dyn Write>)
285+
};
286+
287+
checked_core_dump(ctx, create_dump_file).map(|_| {
288+
println!("Core dump created successfully: {}", file_path);
289+
log::error!("Core dump file: {}", file_path);
290+
})
291+
}
292+
293+
/// Computes the file path for the core dump file.
294+
///
295+
/// The file path is generated based on the current timestamp and an
296+
/// output directory.
297+
/// If the directory does not exist, it falls back to the system's temp directory.
298+
/// If the variable is not set, it defaults to the system's temporary directory.
299+
/// The filename is formatted as `hl_core_<timestamp>.elf`.
300+
///
301+
/// Arguments:
302+
/// * `dump_dir`: The environment variable value to check for the output directory.
303+
///
304+
/// Returns:
305+
/// * `String`: The file path for the core dump file.
306+
fn core_dump_file_path(dump_dir: Option<String>) -> String {
277307
// Generate timestamp string for the filename using chrono
278308
let timestamp = chrono::Local::now()
279309
.format("%Y%m%d_T%H%M%S%.3f")
280310
.to_string();
281311

282312
// Determine the output directory based on environment variable
283-
let output_dir = if let Ok(dump_dir) = std::env::var("HYPERLIGHT_CORE_DUMP_DIR") {
284-
// Create the directory if it doesn't exist
285-
let path = std::path::Path::new(&dump_dir);
286-
if !path.exists() {
287-
std::fs::create_dir_all(path)
288-
.map_err(|e| new_error!("Failed to create core dump directory: {:?}", e))?;
313+
let output_dir = if let Some(dump_dir) = dump_dir {
314+
// Check if the directory exists
315+
// If it doesn't exist, fall back to the system temp directory
316+
// This is to ensure that the core dump can be created even if the directory is not set
317+
if std::path::Path::new(&dump_dir).exists() {
318+
std::path::PathBuf::from(dump_dir)
319+
} else {
320+
log::warn!(
321+
"Directory \"{}\" does not exist, falling back to temp directory",
322+
dump_dir
323+
);
324+
std::env::temp_dir()
289325
}
290-
std::path::PathBuf::from(dump_dir)
291326
} else {
292327
// Fall back to the system temp directory
293328
std::env::temp_dir()
@@ -297,19 +332,155 @@ pub(crate) fn crashdump_to_tempfile(hv: &dyn Hypervisor) -> Result<()> {
297332
let filename = format!("hl_core_{}.elf", timestamp);
298333
let file_path = output_dir.join(filename);
299334

300-
// Create the file
301-
let file = std::fs::File::create(&file_path)
302-
.map_err(|e| new_error!("Failed to create core dump file: {:?}", e))?;
335+
file_path.to_string_lossy().to_string()
336+
}
303337

304-
// Write the core dump directly to the file
305-
core_builder
306-
.write(&file)
307-
.map_err(|e| new_error!("Failed to write core dump: {:?}", e))?;
338+
/// Create core dump from Hypervisor context if the sandbox is configured to allow core dumps.
339+
///
340+
/// Arguments:
341+
/// * `ctx`: Optional crash dump context from the hypervisor. This contains the information
342+
/// needed to create the core dump. If `None`, no core dump will be created.
343+
/// * `get_writer`: Closure that returns a writer to the output destination.
344+
///
345+
/// Returns:
346+
/// * `Result<usize>`: The number of bytes written to the core dump file.
347+
fn checked_core_dump(
348+
ctx: Option<CrashDumpContext>,
349+
get_writer: impl FnOnce() -> Result<Box<dyn Write>>,
350+
) -> Result<usize> {
351+
let mut nbytes = 0;
352+
// If the HV returned a context it means we can create a core dump
353+
// This is the case when the sandbox has been configured at runtime to allow core dumps
354+
if let Some(ctx) = ctx {
355+
// Set up data sources for the core dump
356+
let guest_view = GuestView::new(&ctx);
357+
let memory_reader = GuestMemReader::new(&ctx);
358+
359+
// Create and write core dump
360+
let core_builder = CoreDumpBuilder::from_source(guest_view, memory_reader);
361+
362+
let writer = get_writer()?;
363+
// Write the core dump directly to the file
364+
nbytes = core_builder
365+
.write(writer)
366+
.map_err(|e| new_error!("Failed to write core dump: {:?}", e))?;
367+
}
308368

309-
let path_string = file_path.to_string_lossy().to_string();
369+
Ok(nbytes)
370+
}
371+
372+
/// Test module for the crash dump functionality
373+
#[cfg(test)]
374+
mod test {
375+
use super::*;
376+
377+
/// Test the core_dump_file_path function when the environment variable is set to an existing
378+
/// directory
379+
#[test]
380+
fn test_crashdump_file_path_valid() {
381+
// Get CWD
382+
let valid_dir = std::env::current_dir()
383+
.unwrap()
384+
.to_string_lossy()
385+
.to_string();
386+
387+
// Call the function
388+
let path = core_dump_file_path(Some(valid_dir.clone()));
389+
390+
// Check if the path is correct
391+
assert!(path.contains(&valid_dir));
392+
}
310393

311-
println!("Core dump created successfully: {}", path_string);
312-
log::error!("Core dump file: {}", path_string);
394+
/// Test the core_dump_file_path function when the environment variable is set to an invalid
395+
/// directory
396+
#[test]
397+
fn test_crashdump_file_path_invalid() {
398+
// Call the function
399+
let path = core_dump_file_path(Some("/tmp/not_existing_dir".to_string()));
313400

314-
Ok(())
401+
// Get the temp directory
402+
let temp_dir = std::env::temp_dir().to_string_lossy().to_string();
403+
404+
// Check if the path is correct
405+
assert!(path.contains(&temp_dir));
406+
}
407+
408+
/// Test the core_dump_file_path function when the environment is not set
409+
/// Check against the default temp directory by using the env::temp_dir() function
410+
#[test]
411+
fn test_crashdump_file_path_default() {
412+
// Call the function
413+
let path = core_dump_file_path(None);
414+
415+
let temp_dir = std::env::temp_dir().to_string_lossy().to_string();
416+
417+
// Check if the path is correct
418+
assert!(path.starts_with(&temp_dir));
419+
}
420+
421+
/// Test core is not created when the context is None
422+
#[test]
423+
fn test_crashdump_not_created_when_context_is_none() {
424+
// Call the function with None context
425+
let result = checked_core_dump(None, || Ok(Box::new(std::io::empty())));
426+
427+
// Check if the result is ok and the number of bytes is 0
428+
assert!(result.is_ok());
429+
assert_eq!(result.unwrap(), 0);
430+
}
431+
432+
/// Test the core dump creation with no regions fails
433+
#[test]
434+
fn test_crashdump_write_fails_when_no_regions() {
435+
// Create a dummy context
436+
let ctx = CrashDumpContext::new(
437+
&[],
438+
[0; 27],
439+
vec![],
440+
0,
441+
Some("dummy_binary".to_string()),
442+
Some("dummy_filename".to_string()),
443+
);
444+
445+
let get_writer = || Ok(Box::new(std::io::empty()) as Box<dyn Write>);
446+
447+
// Call the function
448+
let result = checked_core_dump(Some(ctx), get_writer);
449+
450+
// Check if the result is an error
451+
// This should fail because there are no regions
452+
assert!(result.is_err());
453+
}
454+
455+
/// Check core dump with a dummy region to local vec
456+
/// This test checks if the core dump is created successfully
457+
#[test]
458+
fn test_crashdump_dummy_core_dump() {
459+
let dummy_vec = vec![0; 0x1000];
460+
let regions = vec![MemoryRegion {
461+
guest_region: 0x1000..0x2000,
462+
host_region: dummy_vec.as_ptr() as usize..dummy_vec.as_ptr() as usize + dummy_vec.len(),
463+
flags: MemoryRegionFlags::READ | MemoryRegionFlags::WRITE,
464+
region_type: crate::mem::memory_region::MemoryRegionType::Code,
465+
}];
466+
// Create a dummy context
467+
let ctx = CrashDumpContext::new(
468+
&regions,
469+
[0; 27],
470+
vec![],
471+
0x1000,
472+
Some("dummy_binary".to_string()),
473+
Some("dummy_filename".to_string()),
474+
);
475+
476+
let get_writer = || Ok(Box::new(std::io::empty()) as Box<dyn Write>);
477+
478+
// Call the function
479+
let result = checked_core_dump(Some(ctx), get_writer);
480+
481+
// Check if the result is ok and the number of bytes is 0
482+
assert!(result.is_ok());
483+
// Check the number of bytes written is more than 0x1000 (the size of the region)
484+
assert_eq!(result.unwrap(), 0x2000);
485+
}
315486
}

0 commit comments

Comments
 (0)