From 6dcd43e4517d1b48a4363cc6ded95f8c70117154 Mon Sep 17 00:00:00 2001 From: Steven Malis Date: Tue, 17 Dec 2024 13:28:16 -0500 Subject: [PATCH 1/3] uh_init: Handle safety around set_var --- openhcl/underhill_core/src/worker.rs | 6 ------ openhcl/underhill_init/src/lib.rs | 22 +++++++++++++++++++--- 2 files changed, 19 insertions(+), 9 deletions(-) diff --git a/openhcl/underhill_core/src/worker.rs b/openhcl/underhill_core/src/worker.rs index 1ef0a468ec..af1421dd11 100644 --- a/openhcl/underhill_core/src/worker.rs +++ b/openhcl/underhill_core/src/worker.rs @@ -1165,12 +1165,6 @@ async fn new_underhill_vm( control_send, } = params; - if let Ok(kernel_boot_time) = std::env::var("KERNEL_BOOT_TIME") { - if let Ok(kernel_boot_time_ns) = kernel_boot_time.parse::() { - tracing::info!(kernel_boot_time_ns, "kernel boot time"); - } - } - // Read the initial configuration from the IGVM parameters. let (runtime_params, measured_vtl2_info) = crate::loader::vtl2_config::read_vtl2_params().context("failed to read load parameters")?; diff --git a/openhcl/underhill_init/src/lib.rs b/openhcl/underhill_init/src/lib.rs index 80e379ac5a..5867c582cf 100644 --- a/openhcl/underhill_init/src/lib.rs +++ b/openhcl/underhill_init/src/lib.rs @@ -204,7 +204,15 @@ fn setup( use_host_entropy().context("use host entropy")?; - for setup in &options.setup_script { + Ok(()) +} + +/// Run the setup scripts provided in the options. +/// +/// # Safety +/// The caller is responsible for ensuring that this process is currently single-threaded. +unsafe fn run_setup_scripts(scripts: &[String]) -> anyhow::Result<()> { + for setup in scripts { log::info!("Running provided setup script {}", setup); let result = Command::new("/bin/sh") @@ -226,10 +234,14 @@ fn setup( .and_then(|line| line.split_once('=')) { log::info!("setting env var {}={}", key, value); - std::env::set_var(key, value); + // SAFETY: Caller has guaranteed we are single threaded. + unsafe { + std::env::set_var(key, value); + } } } } + Ok(()) } @@ -423,10 +435,10 @@ fn timestamp() -> u64 { fn do_main() -> anyhow::Result<()> { let boot_time = timestamp(); - std::env::set_var("KERNEL_BOOT_TIME", boot_time.to_string()); init_logging(); + log::info!("kernel boot time {}", boot_time); log::info!( "Initial process: crate_name={}, crate_revision={}, crate_branch={}", env!("CARGO_PKG_NAME"), @@ -550,6 +562,10 @@ fn do_main() -> anyhow::Result<()> { ]; setup(&stat_files, &options, writes, &filesystems)?; + // SAFETY: We have not spawned any threads yet. + unsafe { + run_setup_scripts(&options.setup_script)?; + } if matches!( std::env::var("OPENHCL_NVME_VFIO").as_deref(), From 01c0e70e08e02eb0f8a3e0c8afdb2e468b67374a Mon Sep 17 00:00:00 2001 From: Steven Malis Date: Tue, 17 Dec 2024 13:45:29 -0500 Subject: [PATCH 2/3] Better solution, no unsafety --- openhcl/underhill_init/src/lib.rs | 25 ++++++++----------------- 1 file changed, 8 insertions(+), 17 deletions(-) diff --git a/openhcl/underhill_init/src/lib.rs b/openhcl/underhill_init/src/lib.rs index 5867c582cf..7b16940381 100644 --- a/openhcl/underhill_init/src/lib.rs +++ b/openhcl/underhill_init/src/lib.rs @@ -207,11 +207,8 @@ fn setup( Ok(()) } -/// Run the setup scripts provided in the options. -/// -/// # Safety -/// The caller is responsible for ensuring that this process is currently single-threaded. -unsafe fn run_setup_scripts(scripts: &[String]) -> anyhow::Result<()> { +fn run_setup_scripts(scripts: &[String]) -> anyhow::Result> { + let mut new_env = Vec::new(); for setup in scripts { log::info!("Running provided setup script {}", setup); @@ -234,21 +231,18 @@ unsafe fn run_setup_scripts(scripts: &[String]) -> anyhow::Result<()> { .and_then(|line| line.split_once('=')) { log::info!("setting env var {}={}", key, value); - // SAFETY: Caller has guaranteed we are single threaded. - unsafe { - std::env::set_var(key, value); - } + new_env.push((key.into(), value.into())); } } } - - Ok(()) + Ok(new_env) } -fn run(options: &Options) -> anyhow::Result<()> { +fn run(options: &Options, env: impl IntoIterator) -> anyhow::Result<()> { let mut command = Command::new(UNDERHILL_PATH); command.arg("--pid").arg("/run/underhill.pid"); command.args(&options.underhill_args); + command.envs(env); // Update the file descriptor limit for the main process, since large VMs // require lots of fds. There is no downside to a larger value except that @@ -562,10 +556,7 @@ fn do_main() -> anyhow::Result<()> { ]; setup(&stat_files, &options, writes, &filesystems)?; - // SAFETY: We have not spawned any threads yet. - unsafe { - run_setup_scripts(&options.setup_script)?; - } + let new_env = run_setup_scripts(&options.setup_script)?; if matches!( std::env::var("OPENHCL_NVME_VFIO").as_deref(), @@ -590,7 +581,7 @@ fn do_main() -> anyhow::Result<()> { } }); - run(&options) + run(&options, new_env) } pub fn main() -> ! { From 1cd57d0e9f6f102a95bdda98d6081f3418d8098f Mon Sep 17 00:00:00 2001 From: Steven Malis Date: Tue, 17 Dec 2024 14:06:09 -0500 Subject: [PATCH 3/3] Keep structured boot time log --- openhcl/underhill_core/src/worker.rs | 6 ++++++ openhcl/underhill_init/src/lib.rs | 4 ++-- 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/openhcl/underhill_core/src/worker.rs b/openhcl/underhill_core/src/worker.rs index af1421dd11..1ef0a468ec 100644 --- a/openhcl/underhill_core/src/worker.rs +++ b/openhcl/underhill_core/src/worker.rs @@ -1165,6 +1165,12 @@ async fn new_underhill_vm( control_send, } = params; + if let Ok(kernel_boot_time) = std::env::var("KERNEL_BOOT_TIME") { + if let Ok(kernel_boot_time_ns) = kernel_boot_time.parse::() { + tracing::info!(kernel_boot_time_ns, "kernel boot time"); + } + } + // Read the initial configuration from the IGVM parameters. let (runtime_params, measured_vtl2_info) = crate::loader::vtl2_config::read_vtl2_params().context("failed to read load parameters")?; diff --git a/openhcl/underhill_init/src/lib.rs b/openhcl/underhill_init/src/lib.rs index 7b16940381..3a5f116ccf 100644 --- a/openhcl/underhill_init/src/lib.rs +++ b/openhcl/underhill_init/src/lib.rs @@ -432,7 +432,6 @@ fn do_main() -> anyhow::Result<()> { init_logging(); - log::info!("kernel boot time {}", boot_time); log::info!( "Initial process: crate_name={}, crate_revision={}, crate_branch={}", env!("CARGO_PKG_NAME"), @@ -556,7 +555,8 @@ fn do_main() -> anyhow::Result<()> { ]; setup(&stat_files, &options, writes, &filesystems)?; - let new_env = run_setup_scripts(&options.setup_script)?; + let mut new_env = run_setup_scripts(&options.setup_script)?; + new_env.push(("KERNEL_BOOT_TIME".into(), boot_time.to_string())); if matches!( std::env::var("OPENHCL_NVME_VFIO").as_deref(),