diff --git a/make/test.mk b/make/test.mk index 964ea93d..757cacee 100644 --- a/make/test.mk +++ b/make/test.mk @@ -1,5 +1,11 @@ PHONY_TARGETS += test +# Allow duplicate symbols from libkrun.a (contains Rust stdlib symbols). +# This is needed when krun feature is enabled for tests (Linux only). +ifeq ($(shell uname -s),Linux) +export RUSTFLAGS := -C link-arg=-Wl,--allow-multiple-definition +endif + # Default test target runs only changed components. test: @$(MAKE) test:changed diff --git a/src/boxlite/src/runtime/rt_impl.rs b/src/boxlite/src/runtime/rt_impl.rs index a58506c2..ed0352b5 100644 --- a/src/boxlite/src/runtime/rt_impl.rs +++ b/src/boxlite/src/runtime/rt_impl.rs @@ -1055,7 +1055,7 @@ impl RuntimeImpl { /// Recover boxes from persistent storage on runtime startup. fn recover_boxes(&self) -> BoxliteResult<()> { - use crate::util::{is_process_alive, is_same_process}; + use crate::util::{is_boxlite_shim, is_process_alive}; // Check for system reboot and reset active boxes self.box_manager.check_and_handle_reboot()?; @@ -1196,7 +1196,7 @@ impl RuntimeImpl { if pid_file.exists() { match crate::util::read_pid_file(&pid_file) { Ok(pid) => { - if is_process_alive(pid) && is_same_process(pid, box_id.as_str()) { + if is_process_alive(pid) && is_boxlite_shim(pid) { // Process is alive and it's our boxlite-shim - box stays Running state.set_pid(Some(pid)); state.set_status(BoxStatus::Running); diff --git a/src/boxlite/src/util/mod.rs b/src/boxlite/src/util/mod.rs index 600d5b81..d6644a7f 100644 --- a/src/boxlite/src/util/mod.rs +++ b/src/boxlite/src/util/mod.rs @@ -13,7 +13,7 @@ use tracing_subscriber::util::SubscriberInitExt; use tracing_subscriber::{EnvFilter, fmt}; pub use process::{ - ProcessExit, ProcessMonitor, is_process_alive, is_same_process, kill_process, read_pid_file, + ProcessExit, ProcessMonitor, is_boxlite_shim, is_process_alive, kill_process, read_pid_file, }; #[cfg(any(target_os = "linux", target_os = "macos"))] diff --git a/src/boxlite/src/util/process.rs b/src/boxlite/src/util/process.rs index 45815a67..3144440b 100644 --- a/src/boxlite/src/util/process.rs +++ b/src/boxlite/src/util/process.rs @@ -244,31 +244,31 @@ fn is_process_zombie_macos(pid: u32) -> bool { info.pbi_status == libc::SZOMB } -/// Verify that a PID belongs to a boxlite-shim process for the given box. +/// Check whether a PID belongs to a boxlite-shim process. /// -/// This prevents PID reuse attacks where a PID is recycled for a different process. +/// Used during recovery to verify that a PID from the PID file is still our +/// shim process (not a PID recycled by the OS for a different program). /// /// # Implementation -/// * **Linux**: Read `/proc/{pid}/cmdline` and check for "boxlite-shim" + box_id +/// * **Linux**: Read `/proc/{pid}/cmdline` and check for "boxlite-shim" binary name. +/// The shim receives config via stdin (not CLI args), so the box_id is not in cmdline. /// * **macOS**: Use `sysinfo` crate to get process name and check for "boxlite-shim" /// /// # Arguments /// * `pid` - Process ID to verify -/// * `box_id` - Expected box ID in the command line /// /// # Returns -/// * `true` - PID is our boxlite-shim process -/// * `false` - PID is different process or doesn't exist -pub fn is_same_process(pid: u32, box_id: &str) -> bool { +/// * `true` - PID is a boxlite-shim process +/// * `false` - PID is a different process or doesn't exist +pub fn is_boxlite_shim(pid: u32) -> bool { #[cfg(target_os = "linux")] { - is_same_process_linux(pid, box_id) + is_boxlite_shim_linux(pid) } #[cfg(target_os = "macos")] { - let _ = box_id; // Unused on macOS - is_same_process_macos(pid) + is_boxlite_shim_macos(pid) } #[cfg(not(any(target_os = "linux", target_os = "macos")))] @@ -280,25 +280,25 @@ pub fn is_same_process(pid: u32, box_id: &str) -> bool { } #[cfg(target_os = "linux")] -fn is_same_process_linux(pid: u32, box_id: &str) -> bool { +fn is_boxlite_shim_linux(pid: u32) -> bool { use std::fs; let cmdline_path = format!("/proc/{}/cmdline", pid); match fs::read_to_string(&cmdline_path) { Ok(cmdline) => { - // cmdline is null-separated, split by \0 for proper parsing + // cmdline is null-separated, split by \0 for proper parsing. + // The first arg is the binary path (e.g., /path/to/boxlite-shim). + // The shim is launched with no CLI args — config is sent via stdin. let args: Vec<&str> = cmdline.split('\0').collect(); - - // Check if any arg contains "boxlite-shim" and cmdline contains box_id - args.iter().any(|arg| arg.contains("boxlite-shim")) && cmdline.contains(box_id) + args.iter().any(|arg| arg.contains("boxlite-shim")) } Err(_) => false, // Process doesn't exist or no permission } } #[cfg(target_os = "macos")] -fn is_same_process_macos(pid: u32) -> bool { +fn is_boxlite_shim_macos(pid: u32) -> bool { use sysinfo::{Pid, System}; let mut sys = System::new(); @@ -381,11 +381,11 @@ mod tests { } #[test] - fn test_is_same_process_current() { + fn test_is_boxlite_shim_current() { let current_pid = std::process::id(); // Current process is not boxlite-shim, so should return false - let result = is_same_process(current_pid, "test123"); + let result = is_boxlite_shim(current_pid); // On non-Linux/macOS systems, this will return true (fallback) #[cfg(any(target_os = "linux", target_os = "macos"))] @@ -393,10 +393,10 @@ mod tests { } #[test] - fn test_is_same_process_invalid() { + fn test_is_boxlite_shim_invalid() { // Invalid PID should return false - assert!(!is_same_process(0, "test123")); - assert!(!is_same_process(u32::MAX, "test123")); + assert!(!is_boxlite_shim(0)); + assert!(!is_boxlite_shim(u32::MAX)); } #[test] diff --git a/src/boxlite/tests/pid_file.rs b/src/boxlite/tests/pid_file.rs index be736c26..f7f1558f 100644 --- a/src/boxlite/tests/pid_file.rs +++ b/src/boxlite/tests/pid_file.rs @@ -3,7 +3,7 @@ //! Tests the PID file as single source of truth for process tracking: //! - PID file creation, correctness, and deletion //! - Cleanup on stop, force remove, and box directory removal -//! - Process validation via is_same_process +//! - Process validation via is_boxlite_shim mod common; @@ -11,7 +11,7 @@ use boxlite::BoxliteRuntime; use boxlite::litebox::BoxCommand; use boxlite::runtime::options::BoxliteOptions; use boxlite::runtime::types::BoxStatus; -use boxlite::util::{is_process_alive, is_same_process, read_pid_file}; +use boxlite::util::{is_boxlite_shim, is_process_alive, read_pid_file}; use std::path::{Path, PathBuf}; // ============================================================================ @@ -76,10 +76,9 @@ async fn pid_file_contains_correct_pid() { // Verify it's our boxlite-shim assert!( - is_same_process(pid_from_file, handle.id().as_str()), - "PID {} should belong to boxlite-shim for box {}", - pid_from_file, - handle.id() + is_boxlite_shim(pid_from_file), + "PID {} should belong to boxlite-shim", + pid_from_file ); // Cleanup @@ -262,7 +261,7 @@ async fn box_directory_cleanup_includes_pid_file() { // ============================================================================ #[tokio::test] -async fn is_same_process_validates_boxlite_shim() { +async fn is_boxlite_shim_validates_shim_process() { let home = boxlite_test_utils::home::PerTestBoxHome::new(); let runtime = BoxliteRuntime::new(BoxliteOptions { home_dir: home.path.clone(), @@ -279,14 +278,14 @@ async fn is_same_process_validates_boxlite_shim() { // Should be true for actual shim assert!( - is_same_process(pid, handle.id().as_str()), - "is_same_process should return true for actual shim process" + is_boxlite_shim(pid), + "is_boxlite_shim should return true for actual shim process" ); // Should be false for current test process assert!( - !is_same_process(std::process::id(), handle.id().as_str()), - "is_same_process should return false for non-shim process" + !is_boxlite_shim(std::process::id()), + "is_boxlite_shim should return false for non-shim process" ); // Cleanup