From 6b4a8a50030a302bd163bff6a841e4a9d586f88b Mon Sep 17 00:00:00 2001 From: WeissonHan <112967923+WeissonHan@users.noreply.github.com> Date: Sun, 16 Aug 2026 00:42:45 +0800 Subject: [PATCH 1/2] fix(blaze): bind instance storage root Retain an exclusive descriptor owner for storage.instances_dir from startup planning through the file-provider lifetime. Descriptor-relative operations prevent path replacement from redirecting mutable slot work without removing the public StorageSlot paths used by backends. Keep storage.images_dir reads path-based. Leave slot inventory, startup recovery, and warm storage to follow-up work. Fixes: 8cf1df2680ee ("feat(blaze): implement FileStorageProvider with unit tests") Signed-off-by: Weisson --- docs/user-guide/en/runtime/blaze.md | 25 + docs/user-guide/zh/runtime/blaze.md | 17 + src/blaze/README.md | 10 + src/blaze/README_zh.md | 6 + src/blaze/crates/blaze-core/src/config.rs | 31 + src/blaze/crates/blaze-core/src/storage.rs | 9 +- src/blaze/crates/blazed/src/api.rs | 20 +- src/blaze/crates/blazed/src/daemon.rs | 205 +- src/blaze/crates/blazed/src/failpoint.rs | 21 + .../crates/blazed/src/failpoint_disabled.rs | 19 + src/blaze/crates/blazed/src/file_provider.rs | 2104 +++++++++++++++-- .../crates/blazed/src/sandbox/template.rs | 242 +- 12 files changed, 2499 insertions(+), 210 deletions(-) diff --git a/docs/user-guide/en/runtime/blaze.md b/docs/user-guide/en/runtime/blaze.md index 0da29b9155..20f7e9c3ff 100644 --- a/docs/user-guide/en/runtime/blaze.md +++ b/docs/user-guide/en/runtime/blaze.md @@ -158,6 +158,31 @@ Leave `listen.http_addr` disabled in production until Daemon shutdown also does not yet wait for every active HTTP handler or release all runtime owners, so an in-flight request may observe a closed connection. +## File Storage Compatibility and Safety Checks + +This release does not change the storage configuration fields, HTTP API, or +successful sandbox lifecycle. It tightens failure handling for unsafe file +storage layouts and ownership changes. + +The file provider rejects `storage.instances_dir` when its path contains `..` +or a symbolic-link component; when `storage.images_dir` and +`storage.instances_dir` resolve to overlapping filesystem locations, including +through a symbolic link or bind mount; when the instances root would own +`daemon.state_dir` or enter one of its sandbox UUID subtrees, directly or +through an alias; or when another daemon already owns the same instances root. +Resolve these errors by choosing distinct, stable directories; do not work +around them with path aliases. + +Blaze retains ownership of the instances root that it opened at startup. If +the configured pathname is later renamed or replaced, operations that must +create or reconstruct backend-visible paths fail closed instead of using the +replacement. This includes the reconstruction step that normally starts a +periodic synchronization attempt. Release always uses the original opened +root, and a synchronization attempt that completed reconstruction before the +replacement also finishes against that root. The replacement directory is not +modified accidentally. Restore the configured pathname to the original +directory before requesting new allocation, reconstruction, or synchronization. + ## Storage Artifact Synchronization Blaze can periodically persist the already-written host artifacts and directory diff --git a/docs/user-guide/zh/runtime/blaze.md b/docs/user-guide/zh/runtime/blaze.md index dae16f2f55..18675157ff 100644 --- a/docs/user-guide/zh/runtime/blaze.md +++ b/docs/user-guide/zh/runtime/blaze.md @@ -134,6 +134,23 @@ read 响应过大时返回 HTTP 502 和 保持 `listen.http_addr` 关闭。Daemon 停止时也不会等待全部 HTTP handler 或 释放所有 runtime owner，因此正在执行的请求可能看到连接关闭。 +## 文件存储兼容性与安全检查 + +本次变更不修改存储配置字段、HTTP API 或正常的 sandbox 生命周期，只收紧不安全 +文件存储布局和所有权变化时的失败处理。 + +以下情况会导致文件存储提供程序拒绝 `storage.instances_dir`：路径包含 `..` 或 +符号链接组件；`storage.images_dir` 和 `storage.instances_dir` 直接或通过符号链接、 +绑定挂载解析到重叠的文件系统位置；实例根会覆盖 `daemon.state_dir` 或进入其 +sandbox UUID 子树（包括通过别名重叠）；同一实例根已经由另一个 daemon 占用。 +遇到这些错误时，应选择相互独立且稳定的目录，不要使用路径别名绕过检查。 + +Blaze 会保留启动时打开的实例根所有权。如果配置路径随后被重命名或替换，必须创建 +或重建后端可见路径的操作会失败关闭，而不会使用替换目录；周期同步通常也会在开始时 +执行这一步重建。释放操作始终使用原先打开的存储根；如果同步操作在路径被替换前已经 +完成重建，也会在该存储根上完成，因此不会意外修改替换目录。再次请求新的存储分配、 +重建或同步前，应将配置路径恢复为原目录。 + ## 存储制品同步 Blaze 可以定期持久化 running sandbox 中已经写入的宿主机制品和目录元数据。 diff --git a/src/blaze/README.md b/src/blaze/README.md index 7c1c03aa3b..e97fc8e383 100644 --- a/src/blaze/README.md +++ b/src/blaze/README.md @@ -115,6 +115,16 @@ sync_timeout = "30s" # Maximum scheduler wait for reconstruction plus arti ``` The `file` provider uses standard filesystem operations for sandbox storage. The `auto` provider probes available backends in priority order (currently equivalent to `file`). Unrecognized values will log a warning and fall back to `file`. +Existing storage configuration fields, HTTP APIs, and successful lifecycle +behavior are unchanged. The file provider now rejects an instances-root path +that contains `..` or a symbolic-link component; storage roots that resolve to +overlapping locations; an instances root that would own the daemon state +directory or enter a sandbox UUID subtree, directly or through an alias; or a +root already owned by another daemon. If the configured pathname is replaced +while Blaze is running, new allocation and reconstruction fail closed. Release, +and any synchronization attempt that already completed reconstruction, +continue against the original storage root. See [File storage compatibility and safety checks](../../docs/user-guide/en/runtime/blaze.md#file-storage-compatibility-and-safety-checks) +for the complete behavior. When periodic synchronization is enabled, a completed provider failure is isolated from later sandboxes. If a provider cannot stop its filesystem work at the deadline, that work keeps the sandbox operation lock and the single diff --git a/src/blaze/README_zh.md b/src/blaze/README_zh.md index dad6c0e123..9abf634771 100644 --- a/src/blaze/README_zh.md +++ b/src/blaze/README_zh.md @@ -110,6 +110,12 @@ sync_timeout = "30s" # scheduler 等待 slot 重建与制品同步的最 ``` `file` provider 使用标准文件系统操作管理 sandbox 存储。`auto` 按优先级探测可用 provider（当前等同于 `file`）。无法识别的值将记录告警并回退到 `file`。 +现有存储配置字段、HTTP API 和正常生命周期行为保持不变。文件存储提供程序现在会 +拒绝包含 `..` 或符号链接组件的实例根路径、解析后位置重叠的存储根、会覆盖 daemon +状态目录或进入 sandbox UUID 子树的实例根（包括通过别名重叠），以及已经由另一个 +daemon 占用的实例根。如果 Blaze 运行期间配置路径被替换，新的存储分配和重建会 +失败关闭。释放操作以及已经完成重建的同步操作仍作用于启动时打开的原存储根。 +完整行为见[文件存储兼容性与安全检查](../../docs/user-guide/zh/runtime/blaze.md#文件存储兼容性与安全检查)。 启用周期同步后，已经返回的 provider 失败不会中断后续 sandbox。如果 provider 在 deadline 到达时仍无法停止文件系统操作，该操作会继续持有 sandbox operation lock 和唯一的同步许可直至完成；后续同步会被推迟而不会不断累积。service loop diff --git a/src/blaze/crates/blaze-core/src/config.rs b/src/blaze/crates/blaze-core/src/config.rs index 376061cf91..0832d50e57 100644 --- a/src/blaze/crates/blaze-core/src/config.rs +++ b/src/blaze/crates/blaze-core/src/config.rs @@ -264,6 +264,11 @@ impl DaemonConfig { /// Validate cross-field invariants that serde cannot express. pub fn validate(&self) -> Result<()> { validate_storage_paths(&self.storage.images_dir, &self.storage.instances_dir)?; + validate_state_boundary( + &self.storage.instances_dir, + "storage.instances_dir", + &self.daemon.state_dir, + )?; self.storage.sync_schedule()?; self.storage.sync_timeout_duration()?; let template_boundaries = [ @@ -805,4 +810,30 @@ mod tests { total.template.max_total_bytes = 0; assert!(total.validate().is_err()); } + + #[test] + fn rejects_instances_root_inside_daemon_lifecycle_state() { + let state = PathBuf::from("/srv/blaze-state"); + for instances in [ + state.clone(), + state + .join("86b59faf-3b91-46e4-9db0-2468b8336eb6") + .join("slot"), + ] { + let mut config = DaemonConfig::default(); + config.daemon.state_dir = state.clone(); + config.storage.images_dir = PathBuf::from("/srv/blaze-images"); + config.storage.instances_dir = instances; + config.template.dir = PathBuf::from("/srv/blaze-templates"); + config.template.import_root = Some(PathBuf::from("/srv/blaze-imports")); + config.policy.dir = PathBuf::from("/srv/blaze-policies"); + config.daemon.socket = PathBuf::from("/srv/blaze-run/api.sock"); + + let error = config + .validate() + .expect_err("instances root must not enter lifecycle state"); + + assert!(error.to_string().contains("storage.instances_dir")); + } + } } diff --git a/src/blaze/crates/blaze-core/src/storage.rs b/src/blaze/crates/blaze-core/src/storage.rs index 051a45a9f4..9646f63368 100644 --- a/src/blaze/crates/blaze-core/src/storage.rs +++ b/src/blaze/crates/blaze-core/src/storage.rs @@ -55,9 +55,10 @@ pub struct AcquireOpts { /// Storage allocation failure with an optional residual slot owner. /// -/// A provider returns `residual` only when rollback could not remove resources -/// that were created for this request. The caller must retain the stable slot -/// ID until a later release succeeds. +/// A provider returns `residual` when rollback could not remove resources that +/// were created for this request, or when a blocking allocation transaction's +/// completion is unknown. The caller must retain the stable slot ID until a +/// later release succeeds. #[derive(Debug, Error)] #[error("{source}")] pub struct StorageAcquireError { @@ -75,7 +76,7 @@ impl StorageAcquireError { } } - /// Build a failure that transfers residual slot ownership to the caller. + /// Build a failure that transfers possible residual ownership to the caller. pub fn with_residual(source: BlazeError, residual: StorageSlot) -> Self { Self { source, diff --git a/src/blaze/crates/blazed/src/api.rs b/src/blaze/crates/blazed/src/api.rs index 32c478f2ec..475b2a2a57 100644 --- a/src/blaze/crates/blazed/src/api.rs +++ b/src/blaze/crates/blazed/src/api.rs @@ -2479,11 +2479,15 @@ mod tests { cleanup_count: policy_cleanups.clone(), }), ); - let restarted_storage: Arc = - Arc::new(FileStorageProvider::with_images( + let restarted_storage: Arc = Arc::new( + FileStorageProvider::reopen_after_simulated_restart( config.storage.images_dir.clone(), instances_dir.clone(), - )); + std::time::Duration::from_secs(1), + ) + .await + .expect("restart storage instances root"), + ); let restarted = build_test_state( config, test_policy(BackendKind::Firecracker, false), @@ -3430,11 +3434,15 @@ mod tests { drop(initial_state); let cleanup_count = Arc::new(AtomicUsize::new(0)); - let restarted_storage: Arc = - Arc::new(FileStorageProvider::with_images( + let restarted_storage: Arc = Arc::new( + FileStorageProvider::reopen_after_simulated_restart( config.storage.images_dir.clone(), instances_dir.clone(), - )); + std::time::Duration::from_secs(1), + ) + .await + .expect("restart storage instances root"), + ); let restarted = build_test_state( config, test_policy(BackendKind::Mock, false), diff --git a/src/blaze/crates/blazed/src/daemon.rs b/src/blaze/crates/blazed/src/daemon.rs index 6ccdc8040b..fb449f13fa 100644 --- a/src/blaze/crates/blazed/src/daemon.rs +++ b/src/blaze/crates/blazed/src/daemon.rs @@ -21,9 +21,10 @@ use tokio::signal::unix::{SignalKind, signal}; use crate::api; use crate::error::{BlazeDaemonError, Result}; +use crate::file_provider::{FileStorageProvider, PlannedFileStorageProvider}; use crate::sandbox::StorageSyncLoop; use crate::sandbox::template::{ - PinnedConfigSource, PolicyLoadDisposition, TemplateCatalog, + PinnedConfigSource, PolicyLoadDisposition, TemplateCatalog, preflight_storage_root_boundaries, validate_template_roots_with_policy_mode, }; use crate::spawner::{ @@ -65,7 +66,10 @@ fn absolutize_backend_paths(config: &mut DaemonConfig) -> Result<()> { } async fn run_loaded_config(loaded: LoadedDaemonConfig) -> Result<()> { - let LoadedDaemonConfig { config, source } = loaded; + let LoadedDaemonConfig { mut config, source } = loaded; + // Resolve and inspect the instances root before any daemon-owned path is + // materialized. The provider later consumes this exact retained plan. + let planned_storage = plan_storage_instances(&mut config)?; let sync_schedule = config.storage.sync_schedule()?; let sync_timeout = config.storage.sync_timeout_duration()?; @@ -81,6 +85,7 @@ async fn run_loaded_config(loaded: LoadedDaemonConfig) -> Result<()> { config.policy.on_load_error, )?; let policy_load = template_roots.policy_load_disposition(); + let prepared_storage = FileStorageProvider::prepare(planned_storage)?; let template_catalog = TemplateCatalog::open_validated(&config.template, template_roots)?; ensure_dirs(&config)?; // Retain the accepted state-root object before policy, backend, and @@ -113,11 +118,8 @@ async fn run_loaded_config(loaded: LoadedDaemonConfig) -> Result<()> { use crate::file_provider::FileStorageProvider; // Keep immutable images and provider-owned runtime slots separate. tokio::fs::create_dir_all(&config.storage.images_dir).await?; - tokio::fs::create_dir_all(&config.storage.instances_dir).await?; - let fp = FileStorageProvider::with_images( - config.storage.images_dir.clone(), - config.storage.instances_dir.clone(), - ); + let fp = + FileStorageProvider::from_prepared(config.storage.images_dir.clone(), prepared_storage); match fp.probe().await { Ok(true) => { tracing::info!(dir = %config.storage.images_dir.display(), "storage provider ready"); @@ -186,16 +188,24 @@ fn ensure_dirs(cfg: &DaemonConfig) -> Result<()> { // TemplateCatalog::open_validated creates and retains the accepted catalog // object. Reopening its configured path here would discard that binding. std::fs::create_dir_all(&cfg.storage.images_dir)?; - std::fs::create_dir_all(&cfg.storage.instances_dir)?; - let images_dir = std::fs::canonicalize(&cfg.storage.images_dir)?; - let instances_dir = std::fs::canonicalize(&cfg.storage.instances_dir)?; - blaze_core::config::validate_storage_paths(&images_dir, &instances_dir)?; if let Some(parent) = cfg.daemon.socket.parent() { std::fs::create_dir_all(parent)?; } Ok(()) } +fn plan_storage_instances(config: &mut DaemonConfig) -> Result { + let planned = FileStorageProvider::plan(config.storage.instances_dir.clone())?; + config.storage.instances_dir = FileStorageProvider::planned_instances_dir(&planned).into(); + config.validate()?; + preflight_storage_root_boundaries( + &config.storage.images_dir, + &config.storage.instances_dir, + &config.daemon.state_dir, + )?; + Ok(planned) +} + /// Build the [`crate::spawner::BackendSpawner`] implementations used by API /// handlers. Probes /// `[backends]` for known backends in priority order: @@ -465,6 +475,164 @@ mod tests { use super::*; + #[test] + fn instances_plan_does_not_materialize() { + let temp = tempfile::tempdir().expect("tempdir"); + let configured = temp.path().join("missing/instances"); + let mut config = DaemonConfig::default(); + config.storage.instances_dir = configured.clone(); + + let _planned = plan_storage_instances(&mut config).expect("instances plan"); + + assert_eq!(config.storage.instances_dir, configured); + assert!(!configured.exists()); + } + + #[test] + fn instances_plan_rejects_lifecycle_subtree_without_materializing() { + let temp = tempfile::tempdir().expect("tempdir"); + let state = temp.path().join("state"); + std::fs::create_dir(&state).expect("state root"); + let instances = state + .join("86b59faf-3b91-46e4-9db0-2468b8336eb6") + .join("instances"); + let mut config = DaemonConfig::default(); + config.daemon.state_dir = state; + config.storage.instances_dir = instances.clone(); + + let error = match plan_storage_instances(&mut config) { + Ok(_) => panic!("instances root must not enter lifecycle state"), + Err(error) => error, + }; + + assert!(error.to_string().contains("storage.instances_dir")); + assert!(!instances.exists()); + } + + #[test] + fn instances_plan_rejects_an_images_alias_without_mutating_the_instances_root() { + let temp = tempfile::tempdir().expect("tempdir"); + let instances = temp.path().join("instances"); + let images_alias = temp.path().join("images-alias"); + let state = temp.path().join("state"); + std::fs::create_dir(&instances).expect("instances root"); + std::fs::create_dir(&state).expect("state root"); + std::fs::write(instances.join("sentinel"), b"retained").expect("instances sentinel"); + symlink(&instances, &images_alias).expect("images alias"); + let mut config = DaemonConfig::default(); + config.storage.images_dir = images_alias.clone(); + config.storage.instances_dir = instances.clone(); + config.daemon.state_dir = state; + + let error = match plan_storage_instances(&mut config) { + Ok(_) => panic!("resolved storage roots must be disjoint"), + Err(error) => error, + }; + + assert!(error.to_string().contains("storage.images_dir")); + assert!(error.to_string().contains("storage.instances_dir")); + assert_eq!( + std::fs::read(instances.join("sentinel")).expect("instances sentinel"), + b"retained" + ); + assert!(images_alias.is_symlink()); + } + + #[test] + fn instances_plan_rejects_a_dangling_images_alias_to_the_planned_root() { + let temp = tempfile::tempdir().expect("tempdir"); + let instances = temp.path().join("missing/instances"); + let images_alias = temp.path().join("images-alias"); + let state = temp.path().join("state"); + std::fs::create_dir(&state).expect("state root"); + symlink(&instances, &images_alias).expect("dangling images alias"); + let mut config = DaemonConfig::default(); + config.storage.images_dir = images_alias.clone(); + config.storage.instances_dir = instances.clone(); + config.daemon.state_dir = state; + + let error = match plan_storage_instances(&mut config) { + Ok(_) => panic!("future storage aliases must be rejected"), + Err(error) => error, + }; + + assert!(error.to_string().contains("storage.images_dir")); + assert!(error.to_string().contains("storage.instances_dir")); + assert!(!instances.exists()); + assert!(!instances.parent().expect("instances parent").exists()); + assert!(images_alias.is_symlink()); + assert_eq!( + std::fs::read_link(&images_alias).expect("images alias target"), + instances + ); + } + + #[test] + fn instances_plan_resolves_relative_dangling_aliases_below_symlinked_parents() { + let temp = tempfile::tempdir().expect("tempdir"); + let retained_parent = temp.path().join("retained/dir"); + let configured_parent = temp.path().join("configured-parent"); + let instances = temp.path().join("retained/instances"); + let images_alias = retained_parent.join("images-alias"); + let configured_images = configured_parent.join("images-alias/base/v1"); + let state = temp.path().join("state"); + std::fs::create_dir_all(&retained_parent).expect("retained parent"); + std::fs::create_dir(&state).expect("state root"); + symlink(&retained_parent, &configured_parent).expect("configured parent alias"); + symlink("../instances", &images_alias).expect("relative dangling images alias"); + let mut config = DaemonConfig::default(); + config.storage.images_dir = configured_images; + config.storage.instances_dir = instances.join("base"); + config.daemon.state_dir = state; + + let error = match plan_storage_instances(&mut config) { + Ok(_) => panic!("relative future storage aliases must be rejected"), + Err(error) => error, + }; + + assert!(error.to_string().contains("storage.images_dir")); + assert!(error.to_string().contains("storage.instances_dir")); + assert!(!instances.exists()); + assert_eq!( + std::fs::read_link(&images_alias).expect("relative images alias target"), + Path::new("../instances") + ); + } + + #[test] + fn instances_plan_resolves_parent_components_after_symlink_targets() { + let temp = tempfile::tempdir().expect("tempdir"); + let resolved_subtree = temp.path().join("resolved/x/y"); + let lexical_parent = temp.path().join("configured"); + let subalias = lexical_parent.join("subalias"); + let images_alias = lexical_parent.join("images-alias"); + let instances = temp.path().join("resolved/x/instances"); + let state = temp.path().join("state"); + std::fs::create_dir_all(&resolved_subtree).expect("resolved subtree"); + std::fs::create_dir(&lexical_parent).expect("configured parent"); + std::fs::create_dir(&state).expect("state root"); + symlink(&resolved_subtree, &subalias).expect("target component alias"); + symlink("subalias/../instances", &images_alias) + .expect("images alias with parent component"); + let mut config = DaemonConfig::default(); + config.storage.images_dir = images_alias.clone(); + config.storage.instances_dir = instances.clone(); + config.daemon.state_dir = state; + + let error = match plan_storage_instances(&mut config) { + Ok(_) => panic!("parent components after symbolic links must follow kernel semantics"), + Err(error) => error, + }; + + assert!(error.to_string().contains("storage.images_dir")); + assert!(error.to_string().contains("storage.instances_dir")); + assert!(!instances.exists()); + assert_eq!( + std::fs::read_link(&images_alias).expect("images alias target"), + Path::new("subalias/../instances") + ); + } + #[test] fn policy_boundary_fallback_prevents_a_later_directory_rescan() { let temp = tempfile::tempdir().expect("tempdir"); @@ -587,7 +755,7 @@ backend_priority = ["bubblewrap"] assert!(!config.template.dir.exists()); assert!(detached_parent.join("catalog").is_dir()); assert!(config.storage.images_dir.is_dir()); - assert!(config.storage.instances_dir.is_dir()); + assert!(!config.storage.instances_dir.exists()); assert!(config.daemon.state_dir.is_dir()); } @@ -863,8 +1031,7 @@ backend_priority = ["bubblewrap"] #[tokio::test] async fn every_configured_backend_path_is_checked_before_catalog_creation() { - let current_dir = std::env::current_dir().expect("current directory"); - let temp = tempfile::tempdir_in(¤t_dir).expect("tempdir below current directory"); + let temp = tempfile::tempdir().expect("tempdir"); let catalog = temp.path().join("catalog"); let import_root = temp.path().join("imports"); std::fs::create_dir(&import_root).expect("import root"); @@ -878,13 +1045,9 @@ backend_priority = ["bubblewrap"] config.daemon.state_dir = temp.path().join("state"); config.daemon.socket = temp.path().join("run/api.sock"); let missing_binary = catalog.join("future-backend"); - config.backends.insert( - "future-backend".to_string(), - missing_binary - .strip_prefix(¤t_dir) - .expect("backend path below current directory") - .to_path_buf(), - ); + config + .backends + .insert("future-backend".to_string(), missing_binary.clone()); let config_path = temp.path().join("config.toml"); std::fs::write( &config_path, diff --git a/src/blaze/crates/blazed/src/failpoint.rs b/src/blaze/crates/blazed/src/failpoint.rs index 15edb0bc92..cdc168b8c9 100644 --- a/src/blaze/crates/blazed/src/failpoint.rs +++ b/src/blaze/crates/blazed/src/failpoint.rs @@ -39,6 +39,10 @@ pub(crate) struct TestFailpoint { state: Arc, } +/// Captured task-local failpoint state for a blocking worker. +#[cfg(test)] +pub(crate) struct TestFailpointContext(Option>); + #[cfg(test)] struct TestFailpointScope { previous: Option>, @@ -93,6 +97,23 @@ impl TestFailpoint { } } +/// Capture the current test failpoints without triggering them. +#[cfg(test)] +pub(crate) fn capture_test_context() -> TestFailpointContext { + TestFailpointContext(TEST_FAILPOINTS.with(|current| current.borrow().clone())) +} + +/// Install captured test failpoints while one blocking operation runs. +#[cfg(test)] +pub(crate) fn with_test_context( + context: TestFailpointContext, + operation: impl FnOnce() -> T, +) -> T { + let previous = TEST_FAILPOINTS.with(|current| current.replace(context.0)); + let _scope = TestFailpointScope { previous }; + operation() +} + /// Log that a test-only binary is accepting failpoint configuration. pub(crate) fn announce() { tracing::warn!( diff --git a/src/blaze/crates/blazed/src/failpoint_disabled.rs b/src/blaze/crates/blazed/src/failpoint_disabled.rs index a43b1ef25f..66eb79a17d 100644 --- a/src/blaze/crates/blazed/src/failpoint_disabled.rs +++ b/src/blaze/crates/blazed/src/failpoint_disabled.rs @@ -16,6 +16,25 @@ pub(crate) fn storage(_name: &str) -> blaze_core::Result<()> { Ok(()) } +/// Empty test context used by the production no-op implementation. +#[cfg(test)] +pub(crate) struct TestFailpointContext; + +/// Capture an empty failpoint context in default-feature tests. +#[cfg(test)] +pub(crate) fn capture_test_context() -> TestFailpointContext { + TestFailpointContext +} + +/// Run a blocking operation unchanged in default-feature tests. +#[cfg(test)] +pub(crate) fn with_test_context( + _context: TestFailpointContext, + operation: impl FnOnce() -> T, +) -> T { + operation() +} + /// Leave guest operations unchanged in production builds. pub(crate) fn guest(_name: &str) -> crate::guest::Result<()> { Ok(()) diff --git a/src/blaze/crates/blazed/src/file_provider.rs b/src/blaze/crates/blazed/src/file_provider.rs index 6feabbc04c..5925b6e719 100644 --- a/src/blaze/crates/blazed/src/file_provider.rs +++ b/src/blaze/crates/blazed/src/file_provider.rs @@ -3,24 +3,46 @@ //! rootfs and memory files on a local filesystem. Base images and mutable //! instance slots use separate roots; runtime pooling is owned by the daemon. +use std::ffi::OsString; use std::path::{Path, PathBuf}; use std::sync::Arc; use async_trait::async_trait; -use rustix::fs::{Mode, OFlags, open, openat}; +use rustix::fs::{ + AtFlags, Dir, DirEntry, FileType, FlockOperation, Mode, OFlags, flock, fstat, fsync, mkdirat, + open, openat, statat, unlinkat, +}; +use rustix::io::Errno; use blaze_core::error::{BlazeError, Result}; use blaze_core::storage::{ AcquireOpts, PoolStatus, StorageAcquireError, StorageProvider, StorageSlot, }; +use crate::sandbox::template::opened_mount_id_for_owned_fd; + /// A filesystem-based provider that copies base artifacts when available and /// otherwise creates sparse rootfs and memory files at configured sizes. pub struct FileStorageProvider { images_dir: PathBuf, instances_dir: PathBuf, + instances_owner: Option>, #[cfg(test)] artifact_sync_open_hook: Option>, + #[cfg(test)] + acquire_blocking_hook: Option>, +} + +pub(crate) struct PreparedFileStorageProvider { + resolved_instances_dir: PathBuf, + owner: std::os::fd::OwnedFd, +} + +pub(crate) struct PlannedFileStorageProvider { + resolved_instances_dir: PathBuf, + parent: std::os::fd::OwnedFd, + parent_path: PathBuf, + missing: Vec, } #[cfg(test)] @@ -29,6 +51,51 @@ pub(crate) struct ArtifactSyncOpenHook { resume: tokio::sync::Notify, } +#[cfg(test)] +struct AcquireBlockingHook { + entered: tokio::sync::Notify, + finished: tokio::sync::Notify, + released: std::sync::Mutex, + release: std::sync::Condvar, +} + +#[cfg(test)] +impl AcquireBlockingHook { + fn new() -> Self { + Self { + entered: tokio::sync::Notify::new(), + finished: tokio::sync::Notify::new(), + released: std::sync::Mutex::new(false), + release: std::sync::Condvar::new(), + } + } + + fn pause_after_mkdir(&self) { + self.entered.notify_one(); + let mut released = self.released.lock().expect("acquire hook lock"); + while !*released { + released = self.release.wait(released).expect("acquire hook wait"); + } + } + + async fn wait_until_entered(&self) { + self.entered.notified().await; + } + + async fn wait_until_finished(&self) { + self.finished.notified().await; + } + + fn resume(&self) { + *self.released.lock().expect("acquire hook lock") = true; + self.release.notify_one(); + } + + fn finish(&self) { + self.finished.notify_one(); + } +} + #[cfg(test)] impl ArtifactSyncOpenHook { pub(crate) fn new() -> Self { @@ -48,27 +115,150 @@ impl ArtifactSyncOpenHook { } impl FileStorageProvider { + pub(crate) fn plan(instances_dir: PathBuf) -> Result { + plan_instances_owner(&instances_dir) + } + + pub(crate) fn planned_instances_dir(planned: &PlannedFileStorageProvider) -> &Path { + &planned.resolved_instances_dir + } + + pub(crate) fn prepare( + planned: PlannedFileStorageProvider, + ) -> Result { + revalidate_instances_owner_plan(&planned)?; + let resolved_instances_dir = planned.resolved_instances_dir; + let owner = + materialize_instances_owner(planned.parent, &planned.missing, &resolved_instances_dir)?; + Ok(PreparedFileStorageProvider { + resolved_instances_dir, + owner, + }) + } + + pub(crate) fn from_prepared( + images_dir: PathBuf, + prepared: PreparedFileStorageProvider, + ) -> Self { + let owner = Arc::new(prepared.owner); + Self { + images_dir, + instances_dir: prepared.resolved_instances_dir, + instances_owner: Some(owner), + #[cfg(test)] + artifact_sync_open_hook: None, + #[cfg(test)] + acquire_blocking_hook: None, + } + } + + /// Reopen a provider after a test has dropped every simulated-daemon owner. + /// + /// The parallel Rust test harness can fork a process-spawning sibling while + /// this test still owns the provider root. The child briefly inherits the + /// CLOEXEC flock until it calls exec, so a simulated restart may observe an + /// exact owner-contention error after the old Rust owner has been dropped. + /// Production startup remains single-attempt and fail-fast through + /// [`Self::prepare`]. + #[cfg(test)] + pub(crate) async fn reopen_after_simulated_restart( + images_dir: PathBuf, + instances_dir: PathBuf, + retry_for: std::time::Duration, + ) -> Result { + let deadline = std::time::Instant::now() + retry_for; + let mut planned = Self::plan(instances_dir.clone())?; + let expected_root = Self::planned_instances_dir(&planned).to_path_buf(); + let exact_contention = format!( + "storage instances root {} is already owned by another daemon", + expected_root.display() + ); + + loop { + match Self::prepare(planned) { + Ok(prepared) => return Ok(Self::from_prepared(images_dir, prepared)), + Err(error) => { + let is_exact_contention = matches!( + &error, + BlazeError::StorageError { msg } if msg == &exact_contention + ); + if !is_exact_contention || std::time::Instant::now() >= deadline { + return Err(error); + } + } + } + + tokio::time::sleep(std::time::Duration::from_millis(1)).await; + planned = Self::plan(instances_dir.clone())?; + if Self::planned_instances_dir(&planned) != expected_root.as_path() { + return Err(BlazeError::StorageError { + msg: format!( + "storage instances root {} changed while reopening a simulated daemon", + expected_root.display() + ), + }); + } + } + } + /// Create a provider with no separate image directory. /// /// This constructor is kept for focused tests. Daemon startup uses /// [`Self::with_images`] so immutable images and runtime slots cannot mix. #[cfg(test)] pub fn new(instances_dir: PathBuf) -> Self { + let owner = Some(Arc::new( + open( + &instances_dir, + OFlags::RDONLY | OFlags::DIRECTORY | OFlags::NOFOLLOW | OFlags::CLOEXEC, + Mode::empty(), + ) + .expect("test storage instances root"), + )); Self { images_dir: instances_dir.clone(), instances_dir, + // Focused tests use this constructor to exercise missing-root + // probing as well as normal slot operations. Production startup + // always goes through `prepare` and retains the exclusive owner. + instances_owner: owner, artifact_sync_open_hook: None, + acquire_blocking_hook: None, + } + } + + #[cfg(test)] + fn unavailable_for_test(instances_dir: PathBuf) -> Self { + Self { + images_dir: instances_dir.clone(), + instances_dir, + instances_owner: None, + artifact_sync_open_hook: None, + acquire_blocking_hook: None, } } /// Create a provider with distinct immutable image and runtime roots. + #[cfg(test)] pub fn with_images(images_dir: PathBuf, instances_dir: PathBuf) -> Self { - Self { + Self::try_with_images(images_dir, instances_dir).expect("storage instances root owner") + } + + #[cfg(test)] + fn try_with_images(images_dir: PathBuf, instances_dir: PathBuf) -> Result { + let planned = plan_instances_owner(&instances_dir)?; + let resolved = planned.resolved_instances_dir.clone(); + let owner = materialize_instances_owner(planned.parent, &planned.missing, &resolved)?; + let owner = Arc::new(owner); + Ok(Self { images_dir, - instances_dir, + instances_dir: resolved, + instances_owner: Some(owner), #[cfg(test)] artifact_sync_open_hook: None, - } + #[cfg(test)] + acquire_blocking_hook: None, + }) } #[cfg(test)] @@ -77,10 +267,17 @@ impl FileStorageProvider { instances_dir: PathBuf, hook: std::sync::Arc, ) -> Self { + let planned = plan_instances_owner(&instances_dir).expect("plan test storage root"); + let resolved = planned.resolved_instances_dir.clone(); + let owner = materialize_instances_owner(planned.parent, &planned.missing, &resolved) + .expect("test storage root owner"); + let owner = Arc::new(owner); Self { images_dir, - instances_dir, + instances_dir: resolved, + instances_owner: Some(owner), artifact_sync_open_hook: Some(hook), + acquire_blocking_hook: None, } } @@ -103,6 +300,173 @@ impl FileStorageProvider { } } +fn resolve_relative_instances_path(path: &Path, current_dir: &Path) -> PathBuf { + current_dir.join(path) +} + +fn plan_instances_owner(path: &Path) -> Result { + let absolute = if path.is_absolute() { + path.to_path_buf() + } else { + let current_dir = std::env::current_dir().map_err(|error| BlazeError::StorageError { + msg: format!("resolve storage instances root {}: {error}", path.display()), + })?; + resolve_relative_instances_path(path, ¤t_dir) + }; + if absolute.components().any(|component| { + matches!( + component, + std::path::Component::ParentDir | std::path::Component::Prefix(_) + ) + }) { + return Err(BlazeError::StorageError { + msg: format!( + "storage instances root {} contains an unsupported path component", + path.display() + ), + }); + } + let mut current = open( + "/", + OFlags::RDONLY | OFlags::DIRECTORY | OFlags::NOFOLLOW | OFlags::CLOEXEC, + Mode::empty(), + ) + .map_err(|error| BlazeError::StorageError { + msg: format!("open storage root directory: {error}"), + })?; + let components = absolute + .components() + .filter_map(|component| match component { + std::path::Component::Normal(name) => Some(name.to_os_string()), + _ => None, + }) + .collect::>(); + let mut missing = Vec::new(); + for (index, name) in components.iter().enumerate() { + match openat( + ¤t, + name, + OFlags::RDONLY | OFlags::DIRECTORY | OFlags::NOFOLLOW | OFlags::CLOEXEC, + Mode::empty(), + ) { + Ok(next) => current = next, + Err(Errno::NOENT) => { + missing.extend(components[index..].iter().cloned()); + break; + } + Err(error) => { + return Err(BlazeError::StorageError { + msg: format!("inspect storage instances root {}: {error}", path.display()), + }); + } + } + } + Ok(PlannedFileStorageProvider { + resolved_instances_dir: absolute, + parent: current, + parent_path: components[..components.len() - missing.len()] + .iter() + .fold(PathBuf::from("/"), |path, component| path.join(component)), + missing, + }) +} + +fn revalidate_instances_owner_plan(planned: &PlannedFileStorageProvider) -> Result<()> { + let current = open( + &planned.parent_path, + OFlags::RDONLY | OFlags::DIRECTORY | OFlags::NOFOLLOW | OFlags::CLOEXEC, + Mode::empty(), + ) + .map_err(|error| BlazeError::StorageError { + msg: format!( + "revalidate storage instances ancestor {}: {error}", + planned.parent_path.display() + ), + })?; + ensure_same_object( + &fstat(&planned.parent).map_err(std::io::Error::from)?, + ¤t, + "planned storage instances ancestor", + )?; + if let Some(component) = planned.missing.first() { + match statat(¤t, component, AtFlags::SYMLINK_NOFOLLOW) { + Err(Errno::NOENT) => {} + Ok(_) => { + return Err(BlazeError::StorageError { + msg: format!( + "planned missing storage instances component {component:?} now exists" + ), + }); + } + Err(error) => return Err(std::io::Error::from(error).into()), + } + } + Ok(()) +} + +fn materialize_instances_owner( + mut current: std::os::fd::OwnedFd, + missing: &[OsString], + resolved: &Path, +) -> Result { + let mut walked = resolved.to_path_buf(); + for _ in missing { + walked.pop(); + } + for name in missing { + walked.push(name); + match mkdirat(¤t, name, Mode::from_bits_truncate(0o750)) { + Ok(()) => {} + Err(Errno::EXIST) => { + return Err(BlazeError::StorageError { + msg: format!( + "storage instances path {} appeared after startup planning", + walked.display() + ), + }); + } + Err(error) => { + return Err(BlazeError::StorageError { + msg: format!( + "create storage instances path {}: {error}", + walked.display() + ), + }); + } + } + // Synchronize each newly created component before descending into it. + fsync(¤t).map_err(|error| BlazeError::StorageError { + msg: format!("synchronize storage parent {}: {error}", walked.display()), + })?; + current = openat( + ¤t, + name, + OFlags::RDONLY | OFlags::DIRECTORY | OFlags::NOFOLLOW | OFlags::CLOEXEC, + Mode::empty(), + ) + .map_err(|error| BlazeError::StorageError { + msg: format!("open storage instances path {}: {error}", walked.display()), + })?; + } + let directory = current; + if let Err(error) = flock(&directory, FlockOperation::NonBlockingLockExclusive) { + return Err(BlazeError::StorageError { + msg: if error == Errno::WOULDBLOCK { + format!( + "storage instances root {} is already owned by another daemon", + resolved.display() + ) + } else { + format!( + "lock storage instances root {}: {error}", + resolved.display() + ) + }, + }); + } + Ok(directory) +} + #[derive(Clone, Copy)] enum RequiredPathType { Directory, @@ -125,38 +489,10 @@ impl RequiredPathType { } } -async fn require_slot_path( - instance_id: &str, - path: &Path, - required_type: RequiredPathType, -) -> Result<()> { - match tokio::fs::symlink_metadata(path).await { - Ok(metadata) if required_type.matches(&metadata) => Ok(()), - Ok(_) => Err(BlazeError::StorageIncomplete { - instance_id: instance_id.to_string(), - path: path.to_path_buf(), - expected: required_type.description(), - }), - Err(error) if error.kind() == std::io::ErrorKind::NotFound => { - Err(BlazeError::StorageIncomplete { - instance_id: instance_id.to_string(), - path: path.to_path_buf(), - expected: required_type.description(), - }) - } - Err(error) => Err(BlazeError::StorageError { - msg: format!( - "reconstruct '{instance_id}': inspect {}: {error}", - path.display() - ), - }), - } -} - #[async_trait] impl StorageProvider for FileStorageProvider { async fn probe(&self) -> Result { - Ok(self.images_dir.exists() && self.instances_dir.exists()) + Ok(self.images_dir.exists() && self.instances_owner.is_some()) } async fn acquire( @@ -165,92 +501,90 @@ impl StorageProvider for FileStorageProvider { ) -> std::result::Result { crate::failpoint::storage("storage-acquire")?; let slot = self.slot_for_id(&opts.instance_id)?; - let instance_dir = slot.instance_dir.clone(); - // Atomic: create_dir fails with AlreadyExists if concurrent acquire races - match tokio::fs::create_dir(&instance_dir).await { - Ok(()) => {} - Err(e) if e.kind() == std::io::ErrorKind::AlreadyExists => { - return Err(StorageAcquireError::clean(BlazeError::StorageError { - msg: format!( - "acquire '{}': instance directory already exists", - opts.instance_id - ), - })); + let owner = self.instances_owner.as_ref().cloned().ok_or_else(|| { + StorageAcquireError::clean(BlazeError::StorageError { + msg: "storage provider does not retain an instances-root owner".to_string(), + }) + })?; + #[cfg(test)] + let failpoint_context = crate::failpoint::capture_test_context(); + #[cfg(test)] + let acquire_hook = self.acquire_blocking_hook.clone(); + #[cfg(test)] + let finish_hook = self.acquire_blocking_hook.clone(); + let after_mkdir = move || { + #[cfg(test)] + if let Some(hook) = acquire_hook { + hook.pause_after_mkdir(); } - Err(e) => { - return Err(StorageAcquireError::clean(BlazeError::StorageError { - msg: format!("acquire '{}': create dir: {}", opts.instance_id, e), - })); + }; + let instance_id = opts.instance_id.clone(); + let instances_dir = self.instances_dir.clone(); + let images_dir = self.images_dir.clone(); + let rootfs_size = opts.rootfs_size; + let mem_size = opts.mem_size; + let transaction_slot = slot.clone(); + let result = tokio::task::spawn_blocking(move || { + #[cfg(test)] + let result = crate::failpoint::with_test_context(failpoint_context, || { + acquire_slot_blocking( + owner, + transaction_slot, + instances_dir, + images_dir, + rootfs_size, + mem_size, + after_mkdir, + ) + }); + #[cfg(not(test))] + let result = acquire_slot_blocking( + owner, + transaction_slot, + instances_dir, + images_dir, + rootfs_size, + mem_size, + after_mkdir, + ); + #[cfg(test)] + if let Some(hook) = finish_hook { + hook.finish(); } - } - - // Create rootfs + mem; rollback dir on failure - let result = async { - create_or_copy( - &self.images_dir.join("rootfs.ext4"), - &slot.rootfs_path, - opts.rootfs_size, - ) - .await?; - create_or_copy( - &self.images_dir.join("mem.bin"), - &slot.mem_path, - opts.mem_size, - ) - .await?; - tokio::fs::File::create(&slot.mem_diff_path).await?; - tokio::fs::File::create(&slot.rootfs_diff_path).await?; - crate::failpoint::storage("storage-acquire-artifacts")?; - Ok::<(), BlazeError>(()) - } + result + }) .await; - - if let Err(e) = result { - let rollback = match crate::failpoint::storage("storage-acquire-rollback") { - Ok(()) => tokio::fs::remove_dir_all(&instance_dir) - .await - .map_err(BlazeError::from), - Err(error) => Err(error), - }; - let source = match rollback { - Ok(()) => BlazeError::StorageError { + match result { + Ok(Ok(())) => Ok(slot), + Ok(Err(error)) => Err(*error), + Err(error) => Err(StorageAcquireError::with_residual( + BlazeError::StorageError { msg: format!( - "acquire '{}': file setup failed, rolled back: {}", - opts.instance_id, e + "acquire '{instance_id}': blocking transaction failed to join: {error}; outcome is unknown" ), }, - Err(cleanup) => { - return Err(StorageAcquireError::with_residual( - BlazeError::StorageError { - msg: format!( - "acquire '{}': file setup failed ({e}); rollback failed for {}: {cleanup}", - opts.instance_id, - instance_dir.display() - ), - }, - slot, - )); - } - }; - return Err(StorageAcquireError::clean(source)); + slot, + )), } - - Ok(slot) } async fn release(&self, slot: StorageSlot) -> Result<()> { crate::failpoint::storage("storage-release")?; // Re-derive the canonical path from instances_dir + slot.id. Do not // trust path strings carried in a persisted or externally built slot. - let canonical_dir = self.slot_for_id(&slot.id)?.instance_dir; - if canonical_dir.exists() { - tokio::fs::remove_dir_all(&canonical_dir) - .await - .map_err(|e| BlazeError::StorageError { - msg: format!("release '{}': {}", slot.id, e), - })?; - } + validate_instance_id(&slot.id)?; + let Some(owner) = self.instances_owner.as_ref().cloned() else { + return Err(BlazeError::StorageError { + msg: "storage provider does not retain an instances-root owner".to_string(), + }); + }; + let id = slot.id.clone(); + tokio::task::spawn_blocking(move || remove_slot_tree(&owner, &id)) + .await + .map_err(|error| BlazeError::StorageError { + msg: format!("release '{}': join cleanup: {error}", slot.id), + })??; Ok(()) } @@ -261,15 +595,23 @@ impl StorageProvider for FileStorageProvider { async fn reconstruct(&self, instance_id: &str) -> Result { let slot = self.slot_for_id(instance_id)?; - require_slot_path(instance_id, &slot.instance_dir, RequiredPathType::Directory).await?; - for path in [ - &slot.rootfs_path, - &slot.mem_path, - &slot.mem_diff_path, - &slot.rootfs_diff_path, - ] { - require_slot_path(instance_id, path, RequiredPathType::File).await?; - } + let display_slot = slot.instance_dir.clone(); + let owner = + self.instances_owner + .as_ref() + .cloned() + .ok_or_else(|| BlazeError::StorageError { + msg: "storage provider does not retain an instances-root owner".to_string(), + })?; + let id = instance_id.to_string(); + let display_root = self.instances_dir.clone(); + tokio::task::spawn_blocking(move || { + validate_complete_slot(&owner, &id, &display_root, &display_slot) + }) + .await + .map_err(|error| BlazeError::StorageError { + msg: format!("reconstruct '{instance_id}': join validation: {error}"), + })??; Ok(slot) } @@ -278,14 +620,22 @@ impl StorageProvider for FileStorageProvider { // Never trust paths carried by a runtime or persisted slot. Rebuild // the complete provider-owned artifact set from the validated ID. let canonical = self.slot_for_id(&slot.id)?; - let instance_dir = canonical.instance_dir.clone(); + let owner = + self.instances_owner + .as_ref() + .cloned() + .ok_or_else(|| BlazeError::StorageError { + msg: "storage provider does not retain an instances-root owner".to_string(), + })?; + let slot_id = slot.id.clone(); let directory_fd = open_required_slot_path( &slot.id, &canonical.instance_dir, RequiredPathType::Directory, move || { - open( - &instance_dir, + openat( + &owner, + slot_id.as_str(), OFlags::RDONLY | OFlags::DIRECTORY | OFlags::NOFOLLOW | OFlags::CLOEXEC, Mode::empty(), ) @@ -370,23 +720,13 @@ where let join_path = task_path.clone(); tokio::task::spawn_blocking(move || { let file = open_path().map_err(|error| { - if matches!( + classify_required_slot_open_error( + "sync artifacts", + &task_instance_id, + &task_path, + required_type.description(), error, - rustix::io::Errno::NOENT | rustix::io::Errno::NOTDIR | rustix::io::Errno::LOOP - ) { - BlazeError::StorageIncomplete { - instance_id: task_instance_id.clone(), - path: task_path.clone(), - expected: required_type.description(), - } - } else { - BlazeError::StorageError { - msg: format!( - "sync artifacts '{task_instance_id}': open {}: {error}", - task_path.display() - ), - } - } + ) })?; let file = std::fs::File::from(file); let metadata = file.metadata().map_err(|error| BlazeError::StorageError { @@ -413,40 +753,1012 @@ where })? } -async fn create_or_copy( - source: &std::path::Path, - target: &std::path::Path, - size: u64, -) -> std::io::Result<()> { - if source.is_file() && source != target { - tokio::fs::copy(source, target).await?; - return Ok(()); +fn remove_slot_tree(root: &std::os::fd::OwnedFd, instance_id: &str) -> Result<()> { + remove_slot_tree_with_hooks( + root, + instance_id, + &mut || Ok(()), + &mut || crate::failpoint::storage("storage-release-after-entry"), + &mut || Ok(()), + &mut || crate::failpoint::storage("storage-release-before-root-sync"), + ) +} + +fn remove_slot_tree_with_hooks( + root: &std::os::fd::OwnedFd, + instance_id: &str, + after_inspect: &mut I, + after_entry: &mut E, + before_unlink: &mut U, + before_root_sync: &mut S, +) -> Result<()> +where + I: FnMut() -> Result<()>, + E: FnMut() -> Result<()>, + U: FnMut() -> Result<()>, + S: FnMut() -> Result<()>, +{ + let root_mount_id = + opened_mount_id_for_owned_fd(root).map_err(|error| BlazeError::StorageError { + msg: format!("release '{instance_id}': inspect instances-root mount: {error}"), + })?; + let inspected = match statat(root, instance_id, AtFlags::SYMLINK_NOFOLLOW) { + Ok(stat) if FileType::from_raw_mode(stat.st_mode) == FileType::Directory => stat, + Ok(_) => { + return Err(BlazeError::StorageError { + msg: format!("release '{instance_id}': refusing non-directory slot"), + }); + } + Err(Errno::NOENT) => { + sync_instances_root(root, instance_id, "missing slot", before_root_sync)?; + return Ok(()); + } + Err(error) => { + return Err(BlazeError::StorageError { + msg: format!("release '{instance_id}': inspect retained slot: {error}"), + }); + } + }; + after_inspect()?; + let directory = match openat( + root, + instance_id, + OFlags::RDONLY | OFlags::DIRECTORY | OFlags::NOFOLLOW | OFlags::CLOEXEC, + Mode::empty(), + ) { + Ok(directory) => directory, + Err(Errno::NOENT) => { + return Err(BlazeError::StorageError { + msg: format!("release '{instance_id}': slot disappeared after validation"), + }); + } + Err(error) => { + return Err(BlazeError::StorageError { + msg: format!("release '{instance_id}': open retained slot: {error}"), + }); + } + }; + ensure_same_object(&inspected, &directory, "provider slot root")?; + remove_directory_contents_on_mount( + &directory, + root_mount_id, + DirEntry::file_type, + opened_mount_id_for_owned_fd, + after_entry, + ) + .map_err(|error| BlazeError::StorageError { + msg: format!("release '{instance_id}': remove contents: {error}"), + })?; + before_unlink()?; + let linked = + statat(root, instance_id, AtFlags::SYMLINK_NOFOLLOW).map_err(std::io::Error::from)?; + ensure_same_metadata_object(&inspected, &linked, "provider slot root")?; + ensure_same_object(&linked, &directory, "provider slot root")?; + unlinkat(root, instance_id, AtFlags::REMOVEDIR).map_err(|error| BlazeError::StorageError { + msg: format!("release '{instance_id}': unlink slot: {error}"), + })?; + sync_instances_root(root, instance_id, "removed slot", before_root_sync)?; + Ok(()) +} + +fn sync_instances_root( + root: &std::os::fd::OwnedFd, + instance_id: &str, + state: &str, + before_root_sync: &mut S, +) -> Result<()> +where + S: FnMut() -> Result<()>, +{ + before_root_sync()?; + fsync(root).map_err(|error| BlazeError::StorageError { + msg: format!("release '{instance_id}': synchronize {state}: {error}"), + })?; + Ok(()) +} + +fn validate_complete_slot( + root: &std::os::fd::OwnedFd, + instance_id: &str, + display_root: &Path, + display_slot: &Path, +) -> Result<()> { + let inspected = inspect_required_slot_entry( + root, + instance_id, + display_slot, + "directory", + FileType::Directory, + )?; + let directory = openat( + root, + instance_id, + OFlags::RDONLY | OFlags::DIRECTORY | OFlags::NOFOLLOW | OFlags::CLOEXEC, + Mode::empty(), + ) + .map_err(|error| BlazeError::StorageError { + msg: format!( + "reconstruct '{instance_id}': open {} after validation: {error}", + display_slot.display() + ), + })?; + ensure_same_object(&inspected, &directory, "provider slot root")?; + for name in ["rootfs.ext4", "mem.bin", "mem.diff", "rootfs.diff"] { + let display_path = display_slot.join(name); + let inspected = inspect_required_slot_entry( + &directory, + instance_id, + &display_path, + "file", + FileType::RegularFile, + )?; + let file = openat( + &directory, + name, + OFlags::RDONLY | OFlags::NOFOLLOW | OFlags::CLOEXEC | OFlags::NONBLOCK, + Mode::empty(), + ) + .map_err(|error| BlazeError::StorageError { + msg: format!( + "reconstruct '{instance_id}': open {} after validation: {error}", + display_path.display() + ), + })?; + ensure_same_object(&inspected, &file, "provider slot file")?; + } + revalidate_backend_visible_root(root, display_root, "reconstruct")?; + Ok(()) +} + +fn revalidate_backend_visible_root( + retained: &std::os::fd::OwnedFd, + configured: &Path, + operation: &str, +) -> Result<()> { + let visible = open( + configured, + OFlags::RDONLY | OFlags::DIRECTORY | OFlags::NOFOLLOW | OFlags::CLOEXEC, + Mode::empty(), + ) + .map_err(|error| BlazeError::StorageError { + msg: format!( + "{operation}: configured instances root {} no longer opens the retained root: {error}", + configured.display() + ), + })?; + let retained_metadata = fstat(retained).map_err(|error| BlazeError::StorageError { + msg: format!( + "{operation}: inspect retained instances root {}: {error}", + configured.display() + ), + })?; + ensure_same_object( + &retained_metadata, + &visible, + "backend-visible storage instances root", + )?; + let retained_mount = + opened_mount_id_for_owned_fd(retained).map_err(|error| BlazeError::StorageError { + msg: format!( + "{operation}: inspect retained instances-root mount for {}: {error}", + configured.display() + ), + })?; + let visible_mount = + opened_mount_id_for_owned_fd(&visible).map_err(|error| BlazeError::StorageError { + msg: format!( + "{operation}: inspect configured instances-root mount for {}: {error}", + configured.display() + ), + })?; + if retained_mount != visible_mount { + return Err(BlazeError::StorageError { + msg: format!( + "{operation}: configured instances root {} changed mount identity {retained_mount}->{visible_mount}", + configured.display() + ), + }); + } + Ok(()) +} + +fn inspect_required_slot_entry( + directory: &std::os::fd::OwnedFd, + instance_id: &str, + display_path: &Path, + expected: &'static str, + expected_type: FileType, +) -> Result { + let name = display_path + .file_name() + .ok_or_else(|| BlazeError::StorageError { + msg: format!( + "reconstruct '{instance_id}': {} has no final component", + display_path.display() + ), + })?; + let metadata = statat(directory, name, AtFlags::SYMLINK_NOFOLLOW).map_err(|error| { + classify_required_slot_open_error("reconstruct", instance_id, display_path, expected, error) + })?; + if FileType::from_raw_mode(metadata.st_mode) != expected_type { + return Err(BlazeError::StorageIncomplete { + instance_id: instance_id.to_string(), + path: display_path.to_path_buf(), + expected, + }); + } + Ok(metadata) +} + +#[cfg(test)] +fn remove_directory_contents_with_type_hint( + directory: &std::os::fd::OwnedFd, + type_hint: F, +) -> Result<()> +where + F: Copy + Fn(&DirEntry) -> FileType, +{ + let mount_id = + opened_mount_id_for_owned_fd(directory).map_err(|error| BlazeError::StorageError { + msg: format!("inspect provider slot mount: {error}"), + })?; + remove_directory_contents_on_mount( + directory, + mount_id, + type_hint, + opened_mount_id_for_owned_fd, + &mut || crate::failpoint::storage("storage-release-after-entry"), + ) +} + +fn remove_directory_contents_on_mount( + directory: &std::os::fd::OwnedFd, + expected_mount_id: u64, + type_hint: F, + mount_id: M, + after_entry: &mut E, +) -> Result<()> +where + F: Copy + Fn(&DirEntry) -> FileType, + M: Copy + Fn(&std::os::fd::OwnedFd) -> std::io::Result, + E: FnMut() -> Result<()>, +{ + let observed_mount_id = mount_id(directory).map_err(|error| BlazeError::StorageError { + msg: format!("inspect provider slot mount: {error}"), + })?; + if observed_mount_id != expected_mount_id { + return Err(BlazeError::StorageError { + msg: format!( + "refusing to remove provider slot across mount boundary \ + {expected_mount_id}->{observed_mount_id}" + ), + }); + } + for entry in Dir::read_from(directory).map_err(std::io::Error::from)? { + let entry = entry.map_err(std::io::Error::from)?; + let name = entry.file_name(); + if matches!(name.to_bytes(), b"." | b"..") { + continue; + } + let metadata = + statat(directory, name, AtFlags::SYMLINK_NOFOLLOW).map_err(std::io::Error::from)?; + match file_type_from_metadata(type_hint(&entry), &metadata) { + FileType::Directory => { + let child = openat( + directory, + name, + OFlags::RDONLY | OFlags::DIRECTORY | OFlags::NOFOLLOW | OFlags::CLOEXEC, + Mode::empty(), + ) + .map_err(std::io::Error::from)?; + ensure_same_object(&metadata, &child, "provider slot directory")?; + remove_directory_contents_on_mount( + &child, + expected_mount_id, + type_hint, + mount_id, + after_entry, + )?; + let linked = statat(directory, name, AtFlags::SYMLINK_NOFOLLOW) + .map_err(std::io::Error::from)?; + ensure_same_metadata_object(&metadata, &linked, "provider slot directory")?; + unlinkat(directory, name, AtFlags::REMOVEDIR).map_err(std::io::Error::from)?; + } + FileType::RegularFile => { + if metadata.st_nlink != 1 { + return Err(BlazeError::StorageError { + msg: "refusing to remove provider slot file with multiple hard links" + .to_string(), + }); + } + let file = openat( + directory, + name, + OFlags::RDONLY | OFlags::NOFOLLOW | OFlags::CLOEXEC | OFlags::NONBLOCK, + Mode::empty(), + ) + .map_err(std::io::Error::from)?; + ensure_same_object(&metadata, &file, "provider slot file")?; + let observed_mount_id = + mount_id(&file).map_err(|error| BlazeError::StorageError { + msg: format!("inspect provider slot file mount: {error}"), + })?; + if observed_mount_id != expected_mount_id { + return Err(BlazeError::StorageError { + msg: format!( + "refusing to remove provider slot across mount boundary \ + {expected_mount_id}->{observed_mount_id}" + ), + }); + } + let linked = statat(directory, name, AtFlags::SYMLINK_NOFOLLOW) + .map_err(std::io::Error::from)?; + ensure_same_metadata_object(&metadata, &linked, "provider slot file")?; + ensure_same_object(&linked, &file, "provider slot file")?; + unlinkat(directory, name, AtFlags::empty()).map_err(std::io::Error::from)?; + } + kind => { + return Err(BlazeError::StorageError { + msg: format!( + "refusing to remove provider slot entry with unsupported type {kind:?}" + ), + }); + } + } + after_entry()?; + } + fsync(directory).map_err(std::io::Error::from)?; + Ok(()) +} + +fn file_type_from_metadata( + _directory_entry_hint: FileType, + metadata: &rustix::fs::Stat, +) -> FileType { + FileType::from_raw_mode(metadata.st_mode) +} + +fn ensure_same_object( + expected: &rustix::fs::Stat, + opened: &std::os::fd::OwnedFd, + label: &str, +) -> Result<()> { + let opened = fstat(opened).map_err(std::io::Error::from)?; + ensure_same_metadata_object(expected, &opened, label) +} + +fn ensure_same_metadata_object( + expected: &rustix::fs::Stat, + observed: &rustix::fs::Stat, + label: &str, +) -> Result<()> { + if expected.st_dev == observed.st_dev && expected.st_ino == observed.st_ino { + Ok(()) + } else { + Err(BlazeError::StorageError { + msg: format!("{label} changed identity while it was retained"), + }) + } +} + +fn acquire_slot_blocking( + owner: Arc, + slot: StorageSlot, + instances_dir: PathBuf, + images_dir: PathBuf, + rootfs_size: u64, + mem_size: u64, + after_create: F, +) -> std::result::Result<(), Box> +where + F: FnOnce(), +{ + let instance_id = slot.id.clone(); + match mkdirat( + &owner, + instance_id.as_str(), + Mode::from_bits_truncate(0o750), + ) { + Ok(()) => {} + Err(Errno::EXIST) => { + return Err(Box::new(StorageAcquireError::clean( + BlazeError::StorageError { + msg: format!("acquire '{instance_id}': instance directory already exists"), + }, + ))); + } + Err(error) => { + return Err(Box::new(StorageAcquireError::clean( + BlazeError::StorageError { + msg: format!("acquire '{instance_id}': create dir: {error}"), + }, + ))); + } + } + after_create(); + + let setup = (|| -> Result<()> { + crate::failpoint::storage("storage-acquire-retain-slot")?; + let slot_directory = openat( + &owner, + instance_id.as_str(), + OFlags::RDONLY | OFlags::DIRECTORY | OFlags::NOFOLLOW | OFlags::CLOEXEC, + Mode::empty(), + ) + .map_err(|error| BlazeError::StorageError { + msg: format!("acquire '{instance_id}': retain slot: {error}"), + })?; + create_or_copy_at_blocking( + &images_dir.join("rootfs.ext4"), + &slot_directory, + "rootfs.ext4", + rootfs_size, + )?; + create_or_copy_at_blocking( + &images_dir.join("mem.bin"), + &slot_directory, + "mem.bin", + mem_size, + )?; + create_empty_at_blocking(&slot_directory, "mem.diff")?; + create_empty_at_blocking(&slot_directory, "rootfs.diff")?; + crate::failpoint::storage("storage-acquire-artifacts")?; + crate::failpoint::storage("storage-acquire-before-root-sync")?; + fsync(&owner).map_err(|error| BlazeError::StorageError { + msg: format!("acquire '{instance_id}': synchronize instances root: {error}"), + })?; + // Backends still consume ordinary paths. Verify that those paths name + // the retained root immediately before returning them; provider-local + // cleanup remains descriptor-relative if this check fails. + revalidate_backend_visible_root(&owner, &instances_dir, "acquire")?; + Ok(()) + })(); + let Err(setup_error) = setup else { + return Ok(()); + }; + + let rollback_result = crate::failpoint::storage("storage-acquire-rollback") + .and_then(|_| remove_slot_tree(&owner, &instance_id)); + match rollback_result { + Ok(()) => Err(Box::new(StorageAcquireError::clean( + BlazeError::StorageError { + msg: format!( + "acquire '{instance_id}': slot setup failed, rolled back: {setup_error}" + ), + }, + ))), + Err(cleanup_error) => Err(Box::new(StorageAcquireError::with_residual( + BlazeError::StorageError { + msg: format!( + "acquire '{instance_id}': slot setup failed ({setup_error}); rollback failed for {}: {cleanup_error}", + slot.instance_dir.display() + ), + }, + slot, + ))), + } +} + +fn create_or_copy_at_blocking( + source: &Path, + directory: &std::os::fd::OwnedFd, + target_name: &str, + size: u64, +) -> std::io::Result<()> { + let target = openat( + directory, + target_name, + OFlags::WRONLY | OFlags::CREATE | OFlags::EXCL | OFlags::NOFOLLOW | OFlags::CLOEXEC, + Mode::from_bits_truncate(0o640), + ) + .map_err(std::io::Error::from)?; + let mut target = std::fs::File::from(target); + if source.is_file() { + let mut source = std::fs::File::open(source)?; + std::io::copy(&mut source, &mut target)?; + } else if size > 0 { + target.set_len(size)?; + } + Ok(()) +} + +fn create_empty_at_blocking( + directory: &std::os::fd::OwnedFd, + target_name: &str, +) -> std::io::Result<()> { + let file = openat( + directory, + target_name, + OFlags::WRONLY | OFlags::CREATE | OFlags::EXCL | OFlags::NOFOLLOW | OFlags::CLOEXEC, + Mode::from_bits_truncate(0o640), + ) + .map_err(std::io::Error::from)?; + drop(std::fs::File::from(file)); + Ok(()) +} + +fn classify_required_slot_open_error( + operation: &str, + instance_id: &str, + path: &Path, + expected: &'static str, + error: Errno, +) -> BlazeError { + if matches!(error, Errno::NOENT | Errno::NOTDIR | Errno::LOOP) { + BlazeError::StorageIncomplete { + instance_id: instance_id.to_string(), + path: path.to_path_buf(), + expected, + } + } else { + BlazeError::StorageError { + msg: format!( + "{operation} '{instance_id}': open {}: {error}", + path.display() + ), + } + } +} + +fn validate_instance_id(instance_id: &str) -> Result<()> { + if instance_id.is_empty() + || instance_id.contains('/') + || instance_id.contains('\\') + || instance_id == ".." + || instance_id == "." + || std::path::Path::new(instance_id).is_absolute() + { + return Err(BlazeError::StorageError { + msg: format!("invalid instance_id '{instance_id}': must be a single path component"), + }); + } + Ok(()) +} + +#[cfg(test)] +mod tests { + use super::*; + use uuid::Uuid; + + fn write_complete_replacement_slot(root: &Path, id: &str, marker: &[u8]) { + let slot = root.join(id); + std::fs::create_dir(&slot).expect("replacement slot"); + for name in ["rootfs.ext4", "mem.bin", "mem.diff", "rootfs.diff"] { + std::fs::write(slot.join(name), marker).expect("replacement artifact"); + } + } + + #[test] + fn plan_materializes_multiple_missing_components_without_rewalking() { + let temp = tempfile::tempdir().expect("tempdir"); + let target = temp.path().join("one/two/instances"); + let planned = FileStorageProvider::plan(target.clone()).expect("read-only plan"); + assert!(!target.exists()); + + let prepared = FileStorageProvider::prepare(planned).expect("materialize planned root"); + + assert_eq!(prepared.resolved_instances_dir, target); + assert!(target.is_dir()); + } + + #[test] + fn relative_instances_path_resolves_against_current_directory() { + let current = Path::new("/srv/blaze"); + let relative = Path::new("missing/instances"); + + assert_eq!( + resolve_relative_instances_path(relative, current), + current.join(relative) + ); + } + + #[test] + fn prepare_rejects_replaced_planned_ancestor_without_materializing() { + let temp = tempfile::tempdir().expect("tempdir"); + let ancestor = temp.path().join("ancestor"); + let detached = temp.path().join("ancestor-retained"); + let target = ancestor.join("one/two/instances"); + std::fs::create_dir(&ancestor).expect("ancestor"); + let planned = FileStorageProvider::plan(target.clone()).expect("read-only plan"); + std::fs::rename(&ancestor, &detached).expect("detach planned ancestor"); + std::fs::create_dir(&ancestor).expect("replacement ancestor"); + + assert!(FileStorageProvider::prepare(planned).is_err()); + + assert!(!ancestor.join("one").exists()); + assert!(!detached.join("one").exists()); + } + + #[test] + fn prepare_rejects_a_planned_missing_component_that_appears() { + let temp = tempfile::tempdir().expect("tempdir"); + let ancestor = temp.path().join("ancestor"); + let target = ancestor.join("one/two/instances"); + std::fs::create_dir(&ancestor).expect("ancestor"); + let planned = FileStorageProvider::plan(target).expect("read-only plan"); + let appeared = ancestor.join("one"); + std::fs::create_dir(&appeared).expect("appeared component"); + std::fs::write(appeared.join("sentinel"), b"unrelated").expect("sentinel"); + + assert!(FileStorageProvider::prepare(planned).is_err()); + + assert_eq!( + std::fs::read(appeared.join("sentinel")).expect("sentinel remains"), + b"unrelated" + ); + assert!(!appeared.join("two").exists()); + } + + #[test] + fn plan_rejects_parent_components_before_creation() { + let temp = tempfile::tempdir().expect("tempdir"); + let downstream = temp.path().join("created"); + let target = temp.path().join("missing/../created"); + + assert!(FileStorageProvider::plan(target).is_err()); + + assert!(!downstream.exists()); + } + + #[tokio::test] + async fn prepared_root_is_exclusive_until_provider_drop() { + let temp = tempfile::tempdir().expect("tempdir"); + let target = temp.path().join("instances"); + let first = FileStorageProvider::prepare( + FileStorageProvider::plan(target.clone()).expect("first plan"), + ) + .expect("first owner"); + let second_plan = FileStorageProvider::plan(target.clone()).expect("second plan"); + let error = match FileStorageProvider::prepare(second_plan) { + Ok(_) => panic!("live provider owner must remain exclusive"), + Err(error) => error, + }; + assert_eq!( + error.to_string(), + format!( + "storage error: storage instances root {} is already owned by another daemon", + target.display() + ) + ); + + drop(first); + FileStorageProvider::reopen_after_simulated_restart( + temp.path().join("images"), + target, + std::time::Duration::from_secs(1), + ) + .await + .expect("owner can be reacquired after drop"); + } + + #[test] + fn plan_rejects_symlink_or_non_directory_components_without_creating_descendants() { + use std::os::unix::fs::symlink; + + for kind in ["symlink", "file"] { + let temp = tempfile::tempdir().expect("tempdir"); + let component = temp.path().join("blocked"); + if kind == "symlink" { + let target = temp.path().join("target"); + std::fs::create_dir(&target).expect("target"); + symlink(&target, &component).expect("component link"); + } else { + std::fs::write(&component, b"not a directory").expect("component file"); + } + let descendant = component.join("one/two/instances"); + + assert!(FileStorageProvider::plan(descendant.clone()).is_err()); + assert!(!descendant.exists()); + } + } + + #[tokio::test] + async fn plan_rejects_symlink_alias_without_disturbing_provider_owner() { + use std::os::unix::fs::symlink; + + let temp = tempfile::tempdir().expect("tempdir"); + let target = temp.path().join("instances"); + std::fs::create_dir(&target).expect("instances"); + let alias = temp.path().join("instances-alias"); + symlink(&target, &alias).expect("alias"); + let first = FileStorageProvider::prepare( + FileStorageProvider::plan(target.clone()).expect("target plan"), + ) + .expect("first owner"); + + assert!(FileStorageProvider::plan(alias).is_err()); + assert!(target.is_dir()); + assert!( + target + .read_dir() + .expect("instances remains") + .next() + .is_none() + ); + drop(first); + FileStorageProvider::reopen_after_simulated_restart( + temp.path().join("images"), + target, + std::time::Duration::from_secs(1), + ) + .await + .expect("owner reacquired after drop"); + } + + #[tokio::test] + async fn backend_paths_fail_closed_after_prepared_root_replacement() { + let temp = tempfile::tempdir().expect("tempdir"); + let images = temp.path().join("images"); + let configured = temp.path().join("instances"); + let detached = temp.path().join("instances-retained"); + std::fs::create_dir(&images).expect("images"); + std::fs::write(images.join("rootfs.ext4"), b"rootfs").expect("rootfs image"); + std::fs::write(images.join("mem.bin"), b"memory").expect("memory image"); + let prepared = FileStorageProvider::prepare( + FileStorageProvider::plan(configured.clone()).expect("plan"), + ) + .expect("prepare"); + let provider = FileStorageProvider::from_prepared(images, prepared); + let id = Uuid::new_v4().to_string(); + let slot = provider + .acquire(&AcquireOpts { + instance_id: id.clone(), + rootfs_size: 0, + mem_size: 0, + }) + .await + .expect("acquire before root replacement"); + assert_eq!(slot.instance_dir, configured.join(&id)); + assert_eq!(slot.rootfs_path, configured.join(&id).join("rootfs.ext4")); + assert_eq!(slot.mem_path, configured.join(&id).join("mem.bin")); + assert_eq!(slot.mem_diff_path, configured.join(&id).join("mem.diff")); + assert_eq!( + slot.rootfs_diff_path, + configured.join(&id).join("rootfs.diff") + ); + + std::fs::rename(&configured, &detached).expect("detach prepared root"); + std::fs::create_dir(&configured).expect("replacement root"); + write_complete_replacement_slot(&configured, &id, b"replacement-existing"); + let reconstruct_error = provider + .reconstruct(&id) + .await + .expect_err("backend path must not escape the retained root"); + assert!(matches!( + &reconstruct_error, + BlazeError::StorageError { .. } + )); + assert!( + reconstruct_error + .to_string() + .contains("backend-visible storage instances root") + ); + provider + .sync_artifacts(&slot) + .await + .expect("provider synchronization remains descriptor-relative"); + + provider + .release(slot) + .await + .expect("provider release remains descriptor-relative"); + + assert!(!detached.join(&id).exists()); + assert_eq!( + std::fs::read(configured.join(&id).join("rootfs.ext4")) + .expect("replacement rootfs remains"), + b"replacement-existing" + ); + + let later_id = Uuid::new_v4().to_string(); + write_complete_replacement_slot(&configured, &later_id, b"replacement-later"); + let acquire_error = provider + .acquire(&AcquireOpts { + instance_id: later_id.clone(), + rootfs_size: 0, + mem_size: 0, + }) + .await + .expect_err("acquire must not return a path through a replacement root"); + let (source, residual) = acquire_error.into_parts(); + assert!(matches!(&source, BlazeError::StorageError { .. })); + assert!( + source + .to_string() + .contains("backend-visible storage instances root") + ); + assert!(residual.is_none()); + assert!(!detached.join(&later_id).exists()); + for name in ["rootfs.ext4", "mem.bin", "mem.diff", "rootfs.diff"] { + assert_eq!( + std::fs::read(configured.join(&later_id).join(name)) + .expect("replacement artifact remains"), + b"replacement-later" + ); + } + } + + #[tokio::test] + async fn acquire_always_creates_empty_diff_artifacts() { + let temp = tempfile::tempdir().expect("tempdir"); + let images = temp.path().join("images"); + let instances = temp.path().join("instances"); + std::fs::create_dir(&images).expect("images"); + std::fs::create_dir(&instances).expect("instances"); + std::fs::write(images.join("rootfs.ext4"), b"rootfs").expect("rootfs image"); + std::fs::write(images.join("mem.bin"), b"memory").expect("memory image"); + std::fs::write(images.join("mem.diff"), b"stale memory state").expect("stale memory diff"); + std::fs::write(images.join("rootfs.diff"), b"stale disk state").expect("stale rootfs diff"); + let provider = FileStorageProvider::with_images(images, instances); + + let slot = provider + .acquire(&AcquireOpts { + instance_id: "empty-diffs".into(), + rootfs_size: 0, + mem_size: 0, + }) + .await + .expect("acquire slot"); + + assert_eq!(std::fs::read(&slot.rootfs_path).expect("rootfs"), b"rootfs"); + assert_eq!(std::fs::read(&slot.mem_path).expect("memory"), b"memory"); + assert!( + std::fs::read(&slot.mem_diff_path) + .expect("memory diff") + .is_empty() + ); + assert!( + std::fs::read(&slot.rootfs_diff_path) + .expect("rootfs diff") + .is_empty() + ); + } + + #[test] + fn unknown_directory_entry_hints_use_descriptor_relative_metadata() { + use std::os::unix::fs::symlink; + + let temp = tempfile::tempdir().expect("tempdir"); + let id = Uuid::new_v4().to_string(); + let slot = temp.path().join(&id); + std::fs::create_dir_all(slot.join("nested")).expect("nested slot"); + std::fs::write(slot.join("nested/artifact"), b"artifact").expect("artifact"); + let root = open( + temp.path(), + OFlags::RDONLY | OFlags::DIRECTORY | OFlags::NOFOLLOW | OFlags::CLOEXEC, + Mode::empty(), + ) + .expect("root owner"); + + let slot_owner = openat( + &root, + id.as_str(), + OFlags::RDONLY | OFlags::DIRECTORY | OFlags::NOFOLLOW | OFlags::CLOEXEC, + Mode::empty(), + ) + .expect("slot owner"); + remove_directory_contents_with_type_hint(&slot_owner, |_| FileType::Unknown) + .expect("recursive cleanup ignores unknown d_type"); + assert!(slot.read_dir().expect("empty slot").next().is_none()); + + let target = temp.path().join("outside"); + std::fs::create_dir(&target).expect("outside"); + let link = slot.join("linked"); + symlink(&target, &link).expect("slot link"); + remove_directory_contents_with_type_hint(&slot_owner, |_| FileType::Unknown) + .expect_err("symlink still fails closed when d_type is unknown"); + assert!( + std::fs::symlink_metadata(&link) + .expect("link remains") + .file_type() + .is_symlink() + ); + assert!(target.is_dir()); } - let file = tokio::fs::File::create(target).await?; - if size > 0 { - file.set_len(size).await?; + + #[test] + fn recursive_cleanup_rejects_hard_linked_files() { + let temp = tempfile::tempdir().expect("tempdir"); + let outside = temp.path().join("outside"); + let slot = temp.path().join("slot"); + std::fs::write(&outside, b"outside").expect("outside file"); + std::fs::create_dir(&slot).expect("slot"); + std::fs::hard_link(&outside, slot.join("linked")).expect("linked file"); + let slot_owner = open( + &slot, + OFlags::RDONLY | OFlags::DIRECTORY | OFlags::NOFOLLOW | OFlags::CLOEXEC, + Mode::empty(), + ) + .expect("slot owner"); + + let error = remove_directory_contents_with_type_hint(&slot_owner, DirEntry::file_type) + .expect_err("hard-linked file must be retained"); + + assert!(error.to_string().contains("multiple hard links")); + assert_eq!( + std::fs::read(&outside).expect("outside remains"), + b"outside" + ); + assert_eq!( + std::fs::read(slot.join("linked")).expect("slot link remains"), + b"outside" + ); } - Ok(()) -} -fn validate_instance_id(instance_id: &str) -> Result<()> { - if instance_id.is_empty() - || instance_id.contains('/') - || instance_id.contains('\\') - || instance_id == ".." - || instance_id == "." - || std::path::Path::new(instance_id).is_absolute() - { - return Err(BlazeError::StorageError { - msg: format!("invalid instance_id '{instance_id}': must be a single path component"), - }); + #[test] + fn recursive_cleanup_rejects_file_mount_id_before_unlink() { + let temp = tempfile::tempdir().expect("tempdir"); + let slot = temp.path().join("slot"); + let artifact = slot.join("artifact"); + std::fs::create_dir(&slot).expect("slot"); + std::fs::write(&artifact, b"artifact").expect("artifact"); + let slot_owner = open( + &slot, + OFlags::RDONLY | OFlags::DIRECTORY | OFlags::NOFOLLOW | OFlags::CLOEXEC, + Mode::empty(), + ) + .expect("slot owner"); + let artifact_inode = + statat(&slot_owner, "artifact", AtFlags::SYMLINK_NOFOLLOW).expect("artifact identity"); + + let error = remove_directory_contents_on_mount( + &slot_owner, + 41, + DirEntry::file_type, + |object| { + let inode = fstat(object).map_err(std::io::Error::from)?.st_ino; + Ok(if inode == artifact_inode.st_ino { + 42 + } else { + 41 + }) + }, + &mut || Ok(()), + ) + .expect_err("file mount must be retained"); + + assert!(error.to_string().contains("across mount boundary 41->42")); + assert_eq!( + std::fs::read(&artifact).expect("artifact remains"), + b"artifact" + ); } - Ok(()) -} -#[cfg(test)] -mod tests { - use super::*; + #[test] + fn recursive_cleanup_rejects_nested_mount_id_before_mutation() { + let temp = tempfile::tempdir().expect("tempdir"); + let slot = temp.path().join("slot"); + std::fs::create_dir_all(slot.join("nested")).expect("nested slot"); + let artifact = slot.join("nested/artifact"); + std::fs::write(&artifact, b"artifact").expect("artifact"); + let slot_owner = open( + &slot, + OFlags::RDONLY | OFlags::DIRECTORY | OFlags::NOFOLLOW | OFlags::CLOEXEC, + Mode::empty(), + ) + .expect("slot owner"); + let nested = + statat(&slot_owner, "nested", AtFlags::SYMLINK_NOFOLLOW).expect("nested identity"); + + let error = remove_directory_contents_on_mount( + &slot_owner, + 41, + DirEntry::file_type, + |directory| { + let inode = fstat(directory).map_err(std::io::Error::from)?.st_ino; + Ok(if inode == nested.st_ino { 42 } else { 41 }) + }, + &mut || Ok(()), + ) + .expect_err("nested mount must be retained"); + + assert!(error.to_string().contains("across mount boundary 41->42")); + assert_eq!( + std::fs::read(&artifact).expect("artifact remains"), + b"artifact" + ); + assert!(slot.join("nested").is_dir()); + } #[tokio::test] async fn probe_existing_dir_returns_true() { @@ -457,8 +1769,9 @@ mod tests { #[tokio::test] async fn probe_missing_dir_returns_false() { - let provider = - FileStorageProvider::new(PathBuf::from("/nonexistent/blaze-test-storage-probe")); + let provider = FileStorageProvider::unavailable_for_test(PathBuf::from( + "/nonexistent/blaze-test-storage-probe", + )); assert!(!provider.probe().await.unwrap()); } @@ -487,6 +1800,85 @@ mod tests { ); } + #[cfg(feature = "test-failpoints")] + #[tokio::test] + async fn acquire_rolls_back_when_slot_open_fails_after_mkdir() { + let temporary = tempfile::tempdir().expect("temporary directory"); + let provider = FileStorageProvider::new(temporary.path().to_path_buf()); + let opts = AcquireOpts { + instance_id: "retain-failure".into(), + rootfs_size: 64, + mem_size: 32, + }; + let failpoint = crate::failpoint::TestFailpoint::new(&["storage-acquire-retain-slot"]); + + let error = failpoint + .run(provider.acquire(&opts)) + .await + .expect_err("slot retain failure"); + let (_source, residual) = error.into_parts(); + + assert!(residual.is_none()); + assert!(!temporary.path().join(&opts.instance_id).exists()); + } + + #[cfg(feature = "test-failpoints")] + #[tokio::test] + async fn acquire_returns_residual_when_open_failure_rollback_fails() { + let temporary = tempfile::tempdir().expect("temporary directory"); + let provider = FileStorageProvider::new(temporary.path().to_path_buf()); + let opts = AcquireOpts { + instance_id: "retain-residual".into(), + rootfs_size: 64, + mem_size: 32, + }; + let failpoint = crate::failpoint::TestFailpoint::new(&[ + "storage-acquire-retain-slot", + "storage-acquire-rollback", + ]); + + let error = failpoint + .run(provider.acquire(&opts)) + .await + .expect_err("slot retain and rollback failure"); + let (_source, residual) = error.into_parts(); + let residual = residual.expect("residual slot ownership"); + + assert_eq!(residual.id, opts.instance_id); + assert!(temporary.path().join(&residual.id).is_dir()); + provider + .release(residual) + .await + .expect("release residual slot"); + } + + #[cfg(feature = "test-failpoints")] + #[tokio::test] + async fn acquire_rolls_back_when_the_instances_root_sync_fails() { + let temporary = tempfile::tempdir().expect("temporary directory"); + let provider = FileStorageProvider::new(temporary.path().to_path_buf()); + let opts = AcquireOpts { + instance_id: "root-sync-failure".into(), + rootfs_size: 64, + mem_size: 32, + }; + let failpoint = crate::failpoint::TestFailpoint::new(&["storage-acquire-before-root-sync"]); + + let error = failpoint + .run(provider.acquire(&opts)) + .await + .expect_err("instances-root synchronization failure"); + let (source, residual) = error.into_parts(); + + assert!( + source + .to_string() + .contains("storage-acquire-before-root-sync") + ); + assert!(residual.is_none()); + assert!(!temporary.path().join(&opts.instance_id).exists()); + } + #[tokio::test] async fn release_removes_instance_dir() { let tmp = tempfile::TempDir::new().unwrap(); @@ -503,6 +1895,208 @@ mod tests { assert!(!dir.exists()); } + #[tokio::test] + async fn release_by_id_recovers_missing_and_partial_slots() { + let tmp = tempfile::TempDir::new().unwrap(); + let provider = FileStorageProvider::new(tmp.path().to_path_buf()); + let id = Uuid::new_v4().to_string(); + let missing_id = Uuid::new_v4().to_string(); + provider.release_by_id(&missing_id).await.unwrap(); + provider.release_by_id(&missing_id).await.unwrap(); + let partial = tmp.path().join(&id); + tokio::fs::create_dir(&partial).await.unwrap(); + tokio::fs::write(partial.join("rootfs.ext4"), b"partial") + .await + .unwrap(); + + provider.release_by_id(&id).await.unwrap(); + provider.release_by_id(&id).await.unwrap(); + + assert!(!partial.exists()); + } + + #[tokio::test] + async fn release_by_id_retries_after_a_partial_recursive_delete() { + let tmp = tempfile::TempDir::new().unwrap(); + let provider = FileStorageProvider::new(tmp.path().to_path_buf()); + let id = Uuid::new_v4().to_string(); + let slot = tmp.path().join(&id); + std::fs::create_dir_all(slot.join("nested")).expect("nested slot"); + std::fs::write(slot.join("00-first"), b"first").expect("first artifact"); + std::fs::write(slot.join("nested/second"), b"second").expect("second artifact"); + let owner = provider.instances_owner.as_ref().expect("instances owner"); + let mut fail_after_first_entry = true; + + remove_slot_tree_with_hooks( + owner, + &id, + &mut || Ok(()), + &mut || { + if std::mem::take(&mut fail_after_first_entry) { + Err(BlazeError::StorageError { + msg: "injected recursive cleanup failure".into(), + }) + } else { + Ok(()) + } + }, + &mut || Ok(()), + &mut || Ok(()), + ) + .expect_err("first recursive cleanup is interrupted"); + assert!(slot.exists()); + assert!( + !slot.join("00-first").exists() || !slot.join("nested/second").exists(), + "the injected failure must follow at least one unlink" + ); + + provider + .release_by_id(&id) + .await + .expect("retry partial slot"); + provider.release_by_id(&id).await.expect("missing retry"); + assert!(!slot.exists()); + } + + #[tokio::test] + async fn release_retries_the_instances_root_sync_after_unlink() { + let temporary = tempfile::tempdir().expect("temporary directory"); + let provider = FileStorageProvider::new(temporary.path().to_path_buf()); + let id = Uuid::new_v4().to_string(); + let slot = temporary.path().join(&id); + std::fs::create_dir(&slot).expect("slot"); + std::fs::write(slot.join("artifact"), b"artifact").expect("artifact"); + let owner = provider.instances_owner.as_ref().expect("instances owner"); + let mut fail_first_sync = true; + + remove_slot_tree_with_hooks( + owner, + &id, + &mut || Ok(()), + &mut || Ok(()), + &mut || Ok(()), + &mut || { + if std::mem::take(&mut fail_first_sync) { + Err(BlazeError::StorageError { + msg: "injected instances-root sync failure".into(), + }) + } else { + Ok(()) + } + }, + ) + .expect_err("root synchronization failure must be visible"); + assert!(!slot.exists(), "the failure follows the slot unlink"); + + provider + .release_by_id(&id) + .await + .expect("missing-slot retry crosses the root durability boundary"); + provider + .release_by_id(&id) + .await + .expect("durable missing slot is idempotent"); + } + + #[test] + fn release_rejects_slot_replacement_between_inspect_and_open() { + let temporary = tempfile::tempdir().expect("temporary directory"); + let provider = FileStorageProvider::new(temporary.path().to_path_buf()); + let id = Uuid::new_v4().to_string(); + let slot = temporary.path().join(&id); + let detached = temporary.path().join(format!("{id}-retained")); + std::fs::create_dir(&slot).expect("slot"); + std::fs::write(slot.join("original"), b"original").expect("original sentinel"); + let owner = provider.instances_owner.as_ref().expect("instances owner"); + + let error = remove_slot_tree_with_hooks( + owner, + &id, + &mut || { + std::fs::rename(&slot, &detached).expect("detach inspected slot"); + std::fs::create_dir(&slot).expect("replacement slot"); + std::fs::write(slot.join("replacement"), b"replacement") + .expect("replacement sentinel"); + Ok(()) + }, + &mut || Ok(()), + &mut || Ok(()), + &mut || Ok(()), + ) + .expect_err("replacement must not match the inspected slot"); + + assert!(error.to_string().contains("changed identity")); + assert_eq!( + std::fs::read(detached.join("original")).expect("original remains"), + b"original" + ); + assert_eq!( + std::fs::read(slot.join("replacement")).expect("replacement remains"), + b"replacement" + ); + } + + #[test] + fn release_rejects_slot_replacement_before_unlink() { + let temporary = tempfile::tempdir().expect("temporary directory"); + let provider = FileStorageProvider::new(temporary.path().to_path_buf()); + let id = Uuid::new_v4().to_string(); + let slot = temporary.path().join(&id); + let detached = temporary.path().join(format!("{id}-retained")); + std::fs::create_dir(&slot).expect("slot"); + let owner = provider.instances_owner.as_ref().expect("instances owner"); + + let error = remove_slot_tree_with_hooks( + owner, + &id, + &mut || Ok(()), + &mut || Ok(()), + &mut || { + std::fs::rename(&slot, &detached).expect("detach retained slot"); + std::fs::create_dir(&slot).expect("replacement slot"); + std::fs::write(slot.join("replacement"), b"replacement") + .expect("replacement sentinel"); + Ok(()) + }, + &mut || Ok(()), + ) + .expect_err("replacement must not be unlinked"); + + assert!(error.to_string().contains("changed identity")); + assert!(detached.is_dir()); + assert_eq!( + std::fs::read(slot.join("replacement")).expect("replacement remains"), + b"replacement" + ); + } + + #[cfg(unix)] + #[tokio::test] + async fn release_by_id_rejects_non_directory_and_symlink_slots() { + use std::os::unix::fs::symlink; + + let tmp = tempfile::TempDir::new().unwrap(); + let provider = FileStorageProvider::new(tmp.path().to_path_buf()); + let id = Uuid::new_v4().to_string(); + let slot_path = tmp.path().join(&id); + tokio::fs::write(&slot_path, b"not a directory") + .await + .unwrap(); + + let file_error = provider.release_by_id(&id).await.unwrap_err(); + assert!(file_error.to_string().contains("refusing non-directory")); + assert!(slot_path.is_file()); + + tokio::fs::remove_file(&slot_path).await.unwrap(); + let target = tempfile::TempDir::new().unwrap(); + symlink(target.path(), &slot_path).unwrap(); + + let symlink_error = provider.release_by_id(&id).await.unwrap_err(); + assert!(symlink_error.to_string().contains("refusing non-directory")); + assert!(std::fs::symlink_metadata(&slot_path).unwrap().is_symlink()); + assert!(target.path().is_dir()); + } + #[tokio::test] async fn pool_status_returns_defaults() { let tmp = tempfile::TempDir::new().unwrap(); @@ -649,6 +2243,19 @@ mod tests { )); } + #[test] + fn required_slot_open_errors_preserve_transient_failures() { + let path = Path::new("/configured/instances/example/rootfs.ext4"); + let missing = + classify_required_slot_open_error("reconstruct", "example", path, "file", Errno::NOENT); + assert!(matches!(missing, BlazeError::StorageIncomplete { .. })); + + let transient = + classify_required_slot_open_error("reconstruct", "example", path, "file", Errno::MFILE); + assert!(matches!(transient, BlazeError::StorageError { .. })); + assert!(transient.to_string().contains("Too many open files")); + } + #[cfg(unix)] #[tokio::test] async fn reconstruct_rejects_a_linked_slot_root() { @@ -728,6 +2335,173 @@ mod tests { assert!(external.is_file()); } + #[cfg(unix)] + #[tokio::test] + async fn reconstruct_classifies_a_socket_artifact_as_incomplete() { + use std::os::unix::fs::FileTypeExt; + use std::os::unix::net::UnixListener; + + let temp = tempfile::TempDir::new().unwrap(); + let provider = FileStorageProvider::new(temp.path().to_path_buf()); + let slot = provider + .acquire(&AcquireOpts { + instance_id: "socket-artifact".into(), + rootfs_size: 64, + mem_size: 32, + }) + .await + .unwrap(); + tokio::fs::remove_file(&slot.mem_diff_path).await.unwrap(); + let listener = UnixListener::bind(&slot.mem_diff_path).expect("socket artifact"); + + let error = provider + .reconstruct("socket-artifact") + .await + .expect_err("socket artifact must invalidate the slot"); + + assert!(matches!( + error, + BlazeError::StorageIncomplete { + ref instance_id, + ref path, + expected: "file", + } if instance_id == "socket-artifact" && path == &slot.mem_diff_path + )); + drop(listener); + assert!( + std::fs::symlink_metadata(&slot.mem_diff_path) + .unwrap() + .file_type() + .is_socket() + ); + } + + #[tokio::test(flavor = "current_thread")] + async fn slot_creation_does_not_block_async_runtime() -> Result<()> { + use std::sync::atomic::{AtomicBool, Ordering}; + use std::sync::mpsc; + use std::time::Duration; + + let temp = tempfile::tempdir()?; + let mut provider = FileStorageProvider::new(temp.path().to_path_buf()); + let hook = Arc::new(AcquireBlockingHook::new()); + provider.acquire_blocking_hook = Some(Arc::clone(&hook)); + let (watchdog_cancel_tx, watchdog_cancel_rx) = mpsc::channel(); + let watchdog_fired = Arc::new(AtomicBool::new(false)); + let watchdog_state = Arc::clone(&watchdog_fired); + let watchdog_hook = Arc::clone(&hook); + let watchdog = std::thread::spawn(move || { + if watchdog_cancel_rx + .recv_timeout(Duration::from_secs(2)) + .is_err() + { + watchdog_state.store(true, Ordering::SeqCst); + watchdog_hook.resume(); + } + }); + + let opts = AcquireOpts { + instance_id: "runtime-progress".to_string(), + rootfs_size: 64, + mem_size: 32, + }; + let acquire_future = provider.acquire(&opts); + let progress_hook = Arc::clone(&hook); + let runtime_progress = async { + let entered = + tokio::time::timeout(Duration::from_secs(4), progress_hook.wait_until_entered()) + .await; + assert!( + entered.is_ok(), + "blocking acquire transaction did not start" + ); + tokio::task::yield_now().await; + assert!( + !watchdog_fired.load(Ordering::SeqCst), + "blocking acquire transaction stalled the current-thread runtime" + ); + progress_hook.resume(); + assert!( + watchdog_cancel_tx.send(()).is_ok(), + "cancel acquire watchdog" + ); + }; + + let (acquire_result, ()) = tokio::join!(acquire_future, runtime_progress); + assert!(watchdog.join().is_ok(), "acquire watchdog panicked"); + assert!( + !watchdog_fired.load(Ordering::SeqCst), + "acquire watchdog released a blocked runtime" + ); + let slot = acquire_result.map_err(|error| error.into_parts().0)?; + assert_eq!(std::fs::metadata(&slot.rootfs_path)?.len(), 64); + assert_eq!(std::fs::metadata(&slot.mem_path)?.len(), 32); + assert!(slot.mem_diff_path.is_file()); + assert!(slot.rootfs_diff_path.is_file()); + provider.release(slot).await?; + Ok(()) + } + + #[tokio::test(flavor = "current_thread")] + async fn cancelled_acquire_finishes_without_a_partial_slot() -> Result<()> { + use std::sync::mpsc; + use std::time::Duration; + + let temp = tempfile::tempdir()?; + let mut provider = FileStorageProvider::new(temp.path().to_path_buf()); + let hook = Arc::new(AcquireBlockingHook::new()); + provider.acquire_blocking_hook = Some(Arc::clone(&hook)); + let watchdog_hook = Arc::clone(&hook); + let (watchdog_cancel_tx, watchdog_cancel_rx) = mpsc::channel(); + let watchdog = std::thread::spawn(move || { + if watchdog_cancel_rx + .recv_timeout(Duration::from_secs(2)) + .is_err() + { + watchdog_hook.resume(); + } + }); + let provider = Arc::new(provider); + let task_provider = Arc::clone(&provider); + let task = tokio::spawn(async move { + task_provider + .acquire(&AcquireOpts { + instance_id: "cancelled-acquire".to_string(), + rootfs_size: 64, + mem_size: 32, + }) + .await + }); + + tokio::time::timeout(Duration::from_secs(4), hook.wait_until_entered()) + .await + .expect("blocking acquire transaction started"); + task.abort(); + hook.resume(); + tokio::time::timeout(Duration::from_secs(4), hook.wait_until_finished()) + .await + .expect("blocking acquire transaction finished"); + assert!( + watchdog_cancel_tx.send(()).is_ok(), + "cancel acquire watchdog" + ); + assert!(watchdog.join().is_ok(), "acquire watchdog panicked"); + assert!( + task.await + .expect_err("acquire task was cancelled") + .is_cancelled(), + "acquire task must report cancellation" + ); + + let slot = provider.reconstruct("cancelled-acquire").await?; + assert_eq!(std::fs::metadata(&slot.rootfs_path)?.len(), 64); + assert_eq!(std::fs::metadata(&slot.mem_path)?.len(), 32); + assert!(slot.mem_diff_path.is_file()); + assert!(slot.rootfs_diff_path.is_file()); + provider.release(slot).await?; + Ok(()) + } + #[tokio::test(flavor = "current_thread")] async fn slot_open_does_not_block_async_runtime() -> Result<()> { use std::sync::atomic::{AtomicBool, Ordering}; diff --git a/src/blaze/crates/blazed/src/sandbox/template.rs b/src/blaze/crates/blazed/src/sandbox/template.rs index 0d96d8bd0d..e9573380b6 100644 --- a/src/blaze/crates/blazed/src/sandbox/template.rs +++ b/src/blaze/crates/blazed/src/sandbox/template.rs @@ -62,10 +62,18 @@ struct FilesystemLocation { path: PathBuf, } +// Non-Linux builds retain the shared boundary types, but load no mount table. +#[derive(Clone, Debug, Eq, PartialEq)] +#[cfg_attr(not(target_os = "linux"), allow(dead_code))] +enum MountRoot { + Hierarchical(PathBuf), + Opaque(OsString), +} + #[derive(Clone, Debug)] struct MountEntry { device: (u64, u64), - root: PathBuf, + root: MountRoot, mount_point: PathBuf, } @@ -1829,6 +1837,15 @@ fn opened_mount_id(file: &File) -> io::Result { Ok(file.metadata()?.dev()) } +/// Return the mount identity for an already-opened filesystem object. +/// +/// Provider cleanup uses the catalog's mount-ID seam so recursive deletion +/// can reject nested mounts without reopening the configured instances path. +pub(crate) fn opened_mount_id_for_owned_fd(object: &std::os::fd::OwnedFd) -> io::Result { + let duplicate = rustix::io::dup(object).map_err(io::Error::from)?; + opened_mount_id(&File::from(duplicate)) +} + fn validate_catalog_mount_id(mount_id: u64, boundary: CatalogBoundary, label: &Path) -> Result<()> { if mount_id != boundary.mount_id { return Err(BlazeDaemonError::RecoveryRequired(format!( @@ -2186,6 +2203,55 @@ pub(crate) fn validate_template_roots_with_policy_mode( ) } +/// Validate the resolved storage roots before creating either root. +pub(crate) fn preflight_storage_root_boundaries( + images_dir: &Path, + instances_dir: &Path, + state_dir: &Path, +) -> Result<()> { + let mounts = MountTable::load()?; + validate_storage_root_disjointness(images_dir, instances_dir, &mounts)?; + let instances = resolve_existing_prefix(instances_dir)?; + let configured_state = normalize_startup_path(state_dir)?; + let state = resolve_existing_prefix(state_dir)?; + validate_lifecycle_boundary( + "storage.instances_dir", + instances_dir, + &instances, + &configured_state, + &mounts, + )?; + if state != configured_state { + validate_lifecycle_boundary( + "storage.instances_dir", + instances_dir, + &instances, + &state, + &mounts, + )?; + } + Ok(()) +} + +fn validate_storage_root_disjointness( + images_dir: &Path, + instances_dir: &Path, + mounts: &MountTable, +) -> Result<()> { + let images = resolve_existing_prefix(images_dir)?; + let instances = resolve_existing_prefix(instances_dir)?; + if paths_overlap_across_mounts(&images, &instances, mounts)? { + return Err(BlazeDaemonError::Core(BlazeError::ConfigError { + source: ConfigErrorSource::InvalidValue(format!( + "storage.images_dir ({}) and storage.instances_dir ({}) resolve to overlapping filesystem locations", + images_dir.display(), + instances_dir.display() + )), + })); + } + Ok(()) +} + // Keep the test-only transition hook adjacent to the complete boundary set so // the production wrapper and deterministic replacement test exercise one path. #[allow(clippy::too_many_arguments)] @@ -2670,7 +2736,20 @@ fn resolve_existing_prefix(path: &Path) -> Result { // Anchor them before walking missing suffixes so a fresh single-component // path still has the working directory as an existing ancestor. let absolute = normalize_startup_path(path)?; - let mut existing = absolute.as_path(); + resolve_existing_prefix_absolute(&absolute, 0) +} + +fn resolve_existing_prefix_absolute(path: &Path, symlink_depth: usize) -> Result { + const MAX_SYMLINK_DEPTH: usize = 40; + if symlink_depth > MAX_SYMLINK_DEPTH { + return Err(io::Error::new( + io::ErrorKind::InvalidData, + "startup path exceeds the symbolic-link resolution limit", + ) + .into()); + } + + let mut existing = path; let mut missing = Vec::new(); loop { match std::fs::canonicalize(existing) { @@ -2681,6 +2760,38 @@ fn resolve_existing_prefix(path: &Path) -> Result { return Ok(resolved); } Err(error) if error.kind() == io::ErrorKind::NotFound => { + match std::fs::symlink_metadata(existing) { + Ok(metadata) if metadata.file_type().is_symlink() => { + let target = std::fs::read_link(existing)?; + let target = if target.is_absolute() { + target + } else { + let parent = existing.parent().ok_or_else(|| { + io::Error::new( + io::ErrorKind::InvalidData, + "symbolic link has no parent directory", + ) + })?; + std::fs::canonicalize(parent)?.join(target) + }; + if !target.is_absolute() { + return Err(io::Error::new( + io::ErrorKind::InvalidData, + "resolved symbolic-link target must be absolute", + ) + .into()); + } + let mut resolved = + resolve_existing_prefix_absolute(&target, symlink_depth + 1)?; + for component in missing.iter().rev() { + resolved.push(component); + } + return Ok(resolved); + } + Ok(_) => return Err(error.into()), + Err(metadata_error) if metadata_error.kind() == io::ErrorKind::NotFound => {} + Err(metadata_error) => return Err(metadata_error.into()), + } let name = existing.file_name().ok_or(error)?; missing.push(name.to_os_string()); existing = existing.parent().ok_or_else(|| { @@ -2733,15 +2844,20 @@ impl MountTable { let device = parse_mount_device(fields[2])?; let root = decode_mount_path(fields[3])?; let mount_point = decode_mount_path(fields[4])?; - if !root.is_absolute() || !mount_point.is_absolute() { + if !mount_point.is_absolute() { return Err(io::Error::new( io::ErrorKind::InvalidData, - "mountinfo root and mount point must be absolute", + "mountinfo mount point must be absolute", )); } + let root = if root.is_absolute() { + MountRoot::Hierarchical(normalize_absolute_path(&root)?) + } else { + MountRoot::Opaque(root.into_os_string()) + }; entries.push(MountEntry { device, - root: normalize_absolute_path(&root)?, + root, mount_point: normalize_absolute_path(&mount_point)?, }); } @@ -2764,9 +2880,22 @@ impl MountTable { "mount point stopped containing the resolved path", ) })?; + let root = match &entry.root { + MountRoot::Hierarchical(root) => root, + MountRoot::Opaque(root) => { + return Err(io::Error::new( + io::ErrorKind::InvalidData, + format!( + "mount point {} has non-hierarchical root {}; filesystem location cannot be translated", + entry.mount_point.display(), + Path::new(root).display() + ), + )); + } + }; Ok(Some(FilesystemLocation { device: entry.device, - path: normalize_absolute_path(&entry.root.join(relative))?, + path: normalize_absolute_path(&root.join(relative))?, })) } } @@ -4059,6 +4188,91 @@ mod tests { ); } + #[test] + fn storage_roots_reject_a_bind_alias_before_materialization() { + let mounts = MountTable::parse( + b"24 1 8:1 / / rw - ext4 /dev/root rw\n\ + 25 24 8:1 /srv/instances/slot-1 /mnt/images rw - ext4 /dev/root rw\n", + ) + .expect("mount table"); + + let error = validate_storage_root_disjointness( + Path::new("/mnt/images"), + Path::new("/srv/instances"), + &mounts, + ) + .expect_err("bind aliases must be rejected"); + + assert!(error.to_string().contains("storage.images_dir")); + assert!(error.to_string().contains("storage.instances_dir")); + validate_storage_root_disjointness( + Path::new("/mnt/other-images"), + Path::new("/srv/instances"), + &mounts, + ) + .expect("disjoint storage roots"); + } + + #[test] + fn mount_table_treats_an_opaque_nsfs_root_as_a_mapping_boundary() { + let mounts = MountTable::parse( + b"24 1 8:1 / / rw - ext4 /dev/root rw\n\ + 25 24 8:1 /srv/storage /mnt/catalog rw - ext4 /dev/root rw\n\ + 90 43 0:4 net:[4026537325] /run/netns/ns-2 rw,nosuid,nodev,noexec,relatime shared:51 - nsfs nsfs rw\n", + ) + .expect("mount table with an nsfs entry"); + + assert_eq!( + mounts.entries[2].root, + MountRoot::Opaque(OsString::from("net:[4026537325]")) + ); + let error = mounts + .location(Path::new("/run/netns/ns-2")) + .expect_err("opaque mount point lookup must fail closed"); + assert_eq!(error.kind(), io::ErrorKind::InvalidData); + assert!(error.to_string().contains("non-hierarchical root")); + assert!( + mounts + .location(Path::new("/run/netns/ns-2/child")) + .expect_err("opaque mount subtree lookup must fail closed") + .to_string() + .contains("non-hierarchical root") + ); + assert_eq!( + mounts + .location(Path::new("/run/netns/other")) + .expect("parent mount lookup") + .expect("parent mount location") + .path, + Path::new("/run/netns/other") + ); + assert!( + paths_overlap_across_mounts( + Path::new("/srv/storage"), + Path::new("/mnt/catalog/templates/base"), + &mounts, + ) + .expect("bind alias remains detectable") + ); + let error = paths_overlap_across_mounts( + Path::new("/srv/storage"), + Path::new("/run/netns/ns-2"), + &mounts, + ) + .expect_err("boundary preflight must reject an opaque mount root"); + assert!(error.to_string().contains("non-hierarchical root")); + } + + #[test] + fn mount_table_rejects_a_relative_mount_point() { + let error = + MountTable::parse(b"90 43 0:4 net:[4026537325] run/netns/ns-2 rw - nsfs nsfs rw\n") + .expect_err("relative mount point"); + + assert_eq!(error.kind(), io::ErrorKind::InvalidData); + assert_eq!(error.to_string(), "mountinfo mount point must be absolute"); + } + #[test] fn root_validation_rejects_bind_alias_to_storage() { let temp = tempfile::tempdir().expect("tempdir"); @@ -4082,12 +4296,12 @@ mod tests { entries: vec![ MountEntry { device: (8, 1), - root: PathBuf::from("/"), + root: MountRoot::Hierarchical(PathBuf::from("/")), mount_point: PathBuf::from("/"), }, MountEntry { device: (8, 1), - root: images.clone(), + root: MountRoot::Hierarchical(images.clone()), mount_point: catalog.clone(), }, ], @@ -4139,12 +4353,12 @@ mod tests { entries: vec![ MountEntry { device: (8, 1), - root: PathBuf::from("/"), + root: MountRoot::Hierarchical(PathBuf::from("/")), mount_point: PathBuf::from("/"), }, MountEntry { device: (8, 1), - root: backend_root, + root: MountRoot::Hierarchical(backend_root), mount_point: catalog.clone(), }, ], @@ -4416,12 +4630,12 @@ mod tests { entries: vec![ MountEntry { device: (8, 1), - root: PathBuf::from("/"), + root: MountRoot::Hierarchical(PathBuf::from("/")), mount_point: PathBuf::from("/"), }, MountEntry { device: (8, 1), - root: network_root, + root: MountRoot::Hierarchical(network_root), mount_point: catalog.clone(), }, ], @@ -4637,12 +4851,12 @@ mod tests { entries: vec![ MountEntry { device: (8, 1), - root: PathBuf::from("/"), + root: MountRoot::Hierarchical(PathBuf::from("/")), mount_point: PathBuf::from("/"), }, MountEntry { device: (8, 1), - root: network_root, + root: MountRoot::Hierarchical(network_root), mount_point: owner.clone(), }, ], From 03616fc0a18d06ec5d170df344e2dc2465e1ec10 Mon Sep 17 00:00:00 2001 From: WeissonHan <112967923+WeissonHan@users.noreply.github.com> Date: Sun, 16 Aug 2026 00:43:15 +0800 Subject: [PATCH 2/2] fix(blaze): close storage publication races Publish missing storage components and sandbox slots through private, descriptor-retained staging directories and no-replace renames. Reject path chains whose permissions allow untrusted replacement while keeping first-start creation. Keep storage acquisition under detached per-sandbox supervision so request cancellation cannot let destroy overtake filesystem work. Carry cleanup disposition explicitly and suppress stable-name cleanup when identity cannot be proven. A process crash before staging publication can leave an empty hidden directory. Root and same-effective-user path mutation remains an administrative trust boundary. Signed-off-by: Weisson --- docs/user-guide/en/runtime/blaze.md | 23 + docs/user-guide/zh/runtime/blaze.md | 20 +- src/blaze/README.md | 20 +- src/blaze/README_zh.md | 15 +- src/blaze/crates/blaze-core/src/storage.rs | 48 +- src/blaze/crates/blazed/src/api.rs | 294 ++++++ src/blaze/crates/blazed/src/failpoint.rs | 137 ++- .../crates/blazed/src/failpoint_disabled.rs | 14 +- src/blaze/crates/blazed/src/file_provider.rs | 864 ++++++++++++++++-- .../crates/blazed/src/sandbox/manager.rs | 192 +++- 10 files changed, 1522 insertions(+), 105 deletions(-) diff --git a/docs/user-guide/en/runtime/blaze.md b/docs/user-guide/en/runtime/blaze.md index 20f7e9c3ff..eb2e5f3cbf 100644 --- a/docs/user-guide/en/runtime/blaze.md +++ b/docs/user-guide/en/runtime/blaze.md @@ -173,6 +173,29 @@ through an alias; or when another daemon already owns the same instances root. Resolve these errors by choosing distinct, stable directories; do not work around them with path aliases. +Blaze still creates a missing `storage.instances_dir` on first start. Every +directory into which Blaze publishes a missing component or sandbox slot, and +the final instances root, must be owned by root or the daemon's effective user +and must not be writable by group or other users. A shared writable ancestor +in the already-existing path is accepted only when it is sticky and the next +component is owned by root or the daemon's effective user. A sticky publication +parent is still rejected because another user could reserve a not-yet-published +name. These checks preserve normal first-start behavior while rejecting layouts +where an untrusted user could exchange or reserve a directory. Root and +processes running as the same effective user as the daemon remain within the +host administration trust boundary. Stop Blaze before an administrator changes +its storage paths. + +Missing path components and sandbox slots are published without replacing an +existing name. If the target name is occupied at atomic publication, Blaze +rejects the operation and leaves that directory unchanged. +If a published sandbox slot changes identity before allocation completes, +creation enters recovery handling and automatic cleanup by the stable sandbox +identifier is suppressed for that daemon process so that the replacement is +not removed. This safeguard does not claim to isolate Blaze from concurrent +changes made by root or the daemon's own effective user across a process restart. +Inspect or restore the storage path before restarting after such an intervention. + Blaze retains ownership of the instances root that it opened at startup. If the configured pathname is later renamed or replaced, operations that must create or reconstruct backend-visible paths fail closed instead of using the diff --git a/docs/user-guide/zh/runtime/blaze.md b/docs/user-guide/zh/runtime/blaze.md index 18675157ff..b7fe193ce4 100644 --- a/docs/user-guide/zh/runtime/blaze.md +++ b/docs/user-guide/zh/runtime/blaze.md @@ -136,15 +136,31 @@ read 响应过大时返回 HTTP 502 和 ## 文件存储兼容性与安全检查 -本次变更不修改存储配置字段、HTTP API 或正常的 sandbox 生命周期，只收紧不安全 +本次变更不修改存储配置字段、HTTP API 或正常的沙箱生命周期，只收紧不安全 文件存储布局和所有权变化时的失败处理。 以下情况会导致文件存储提供程序拒绝 `storage.instances_dir`：路径包含 `..` 或 符号链接组件；`storage.images_dir` 和 `storage.instances_dir` 直接或通过符号链接、 绑定挂载解析到重叠的文件系统位置；实例根会覆盖 `daemon.state_dir` 或进入其 -sandbox UUID 子树（包括通过别名重叠）；同一实例根已经由另一个 daemon 占用。 +沙箱 UUID 子树（包括通过别名重叠）；同一实例根已经由另一个守护进程占用。 遇到这些错误时，应选择相互独立且稳定的目录，不要使用路径别名绕过检查。 +Blaze 首次启动时仍会自动创建缺失的 `storage.instances_dir`。用于发布缺失组件或 +沙箱存储槽的每个目录，以及最终实例根，必须由超级用户或守护进程的有效用户所有， +并且不得允许组用户或其他用户写入。已有路径中的共享可写祖先只有在设置粘滞位、且 +下一级组件由超级用户或守护进程的有效用户所有时才会被接受。用于发布新名称的父目录 +即使设置了粘滞位也会被拒绝，因为其他用户仍可能抢占尚未发布的名称。这些检查保留 +正常的首次启动行为，同时拒绝不受信任用户能够在发布期间交换或抢占目录的布局。 +超级用户和与守护进程使用相同有效用户的进程仍属于主机管理信任边界。管理员更改 +存储路径前应停止 Blaze。 + +缺失路径组件和沙箱存储槽会以不覆盖已有名称的方式发布。如果原子发布时目标名称 +已被占用，Blaze 会拒绝操作并保持该目录不变。如果已发布的沙箱存储槽在 +分配完成前改变身份，创建会进入恢复处理；在当前守护进程存活期间，系统会禁止按 +稳定沙箱标识自动清理存储，避免删除替换目录。该保护不承诺隔离超级用户或守护进程 +自身有效用户跨进程重启实施的并发路径更改。发生此类管理操作后，重启前必须检查或 +恢复存储路径。 + Blaze 会保留启动时打开的实例根所有权。如果配置路径随后被重命名或替换，必须创建 或重建后端可见路径的操作会失败关闭，而不会使用替换目录；周期同步通常也会在开始时 执行这一步重建。释放操作始终使用原先打开的存储根；如果同步操作在路径被替换前已经 diff --git a/src/blaze/README.md b/src/blaze/README.md index e97fc8e383..d26d2e4792 100644 --- a/src/blaze/README.md +++ b/src/blaze/README.md @@ -120,10 +120,22 @@ behavior are unchanged. The file provider now rejects an instances-root path that contains `..` or a symbolic-link component; storage roots that resolve to overlapping locations; an instances root that would own the daemon state directory or enter a sandbox UUID subtree, directly or through an alias; or a -root already owned by another daemon. If the configured pathname is replaced -while Blaze is running, new allocation and reconstruction fail closed. Release, -and any synchronization attempt that already completed reconstruction, -continue against the original storage root. See [File storage compatibility and safety checks](../../docs/user-guide/en/runtime/blaze.md#file-storage-compatibility-and-safety-checks) +root already owned by another daemon. First-start creation remains supported, +but every publication parent and the resulting instances root must be owned by +root or the daemon's effective user and must not be writable by group or other +users. A shared writable ancestor in an existing path is accepted only when it +is sticky and the next component is owned by root or the daemon's effective +user. If the target name is occupied at atomic publication, or if a published +slot is replaced before allocation completes, the operation fails without +adopting or removing the replacement during that daemon process. This cleanup +suppression does not survive a daemon restart; inspect or restore the path +before restarting. Root and processes running as the daemon's effective user +remain within the host administration trust boundary; stop Blaze before +changing its storage paths. +If the configured pathname is replaced while Blaze is running, new allocation +and reconstruction fail closed. Release, and any synchronization attempt that +already completed reconstruction, continue against the original storage root. +See [File storage compatibility and safety checks](../../docs/user-guide/en/runtime/blaze.md#file-storage-compatibility-and-safety-checks) for the complete behavior. When periodic synchronization is enabled, a completed provider failure is isolated from later sandboxes. If a provider cannot stop its filesystem work at diff --git a/src/blaze/README_zh.md b/src/blaze/README_zh.md index 9abf634771..684fb2ac87 100644 --- a/src/blaze/README_zh.md +++ b/src/blaze/README_zh.md @@ -111,10 +111,17 @@ sync_timeout = "30s" # scheduler 等待 slot 重建与制品同步的最 `file` provider 使用标准文件系统操作管理 sandbox 存储。`auto` 按优先级探测可用 provider（当前等同于 `file`）。无法识别的值将记录告警并回退到 `file`。 现有存储配置字段、HTTP API 和正常生命周期行为保持不变。文件存储提供程序现在会 -拒绝包含 `..` 或符号链接组件的实例根路径、解析后位置重叠的存储根、会覆盖 daemon -状态目录或进入 sandbox UUID 子树的实例根（包括通过别名重叠），以及已经由另一个 -daemon 占用的实例根。如果 Blaze 运行期间配置路径被替换，新的存储分配和重建会 -失败关闭。释放操作以及已经完成重建的同步操作仍作用于启动时打开的原存储根。 +拒绝包含 `..` 或符号链接组件的实例根路径、解析后位置重叠的存储根、会覆盖守护进程 +状态目录或进入沙箱 UUID 子树的实例根（包括通过别名重叠），以及已经由另一个 +守护进程占用的实例根。首次启动时仍会自动创建缺失的实例根，但用于发布目录的每个 +父目录及最终实例根必须由超级用户或守护进程的有效用户所有，并且不得允许组用户或 +其他用户写入。已有路径中的共享可写祖先只有在设置粘滞位、且下一级组件由超级用户或 +守护进程的有效用户所有时才会被接受。如果原子发布时目标名称已被占用，或者已发布的 +存储槽在分配完成前被替换，操作会失败；当前守护进程不会采用或删除替换目录。该清理 +禁令不会跨守护进程重启保留，重启前必须检查或恢复路径。超级用户和与守护进程使用 +相同有效用户的进程属于主机管理信任边界；更改存储路径前应停止 Blaze。 +如果 Blaze 运行期间配置路径被替换，新的存储分配和重建会失败关闭。释放操作以及 +已经完成重建的同步操作仍作用于启动时打开的原存储根。 完整行为见[文件存储兼容性与安全检查](../../docs/user-guide/zh/runtime/blaze.md#文件存储兼容性与安全检查)。 启用周期同步后，已经返回的 provider 失败不会中断后续 sandbox。如果 provider 在 deadline 到达时仍无法停止文件系统操作，该操作会继续持有 sandbox operation diff --git a/src/blaze/crates/blaze-core/src/storage.rs b/src/blaze/crates/blaze-core/src/storage.rs index 9646f63368..990a001040 100644 --- a/src/blaze/crates/blaze-core/src/storage.rs +++ b/src/blaze/crates/blaze-core/src/storage.rs @@ -53,18 +53,28 @@ pub struct AcquireOpts { pub mem_size: u64, } -/// Storage allocation failure with an optional residual slot owner. +/// Cleanup responsibility after a storage allocation failure. +#[derive(Debug)] +pub enum StorageAcquireDisposition { + /// The provider proved that the failed request left no resources. + Clean, + /// The caller owns a provider object that can be retried by stable slot ID. + Residual(StorageSlot), + /// Stable-name cleanup is unsafe and requires manual inspection. + ManualCleanupRequired, +} + +/// Storage allocation failure with an explicit cleanup disposition. /// -/// A provider returns `residual` when rollback could not remove resources that -/// were created for this request, or when a blocking allocation transaction's -/// completion is unknown. The caller must retain the stable slot ID until a -/// later release succeeds. +/// A provider returns `Residual` when rollback could not remove resources that +/// were created for this request. `ManualCleanupRequired` means that the stable +/// slot name cannot safely identify the provider object. #[derive(Debug, Error)] #[error("{source}")] pub struct StorageAcquireError { #[source] source: BlazeError, - residual: Option, + disposition: StorageAcquireDisposition, } impl StorageAcquireError { @@ -72,7 +82,7 @@ impl StorageAcquireError { pub fn clean(source: BlazeError) -> Self { Self { source, - residual: None, + disposition: StorageAcquireDisposition::Clean, } } @@ -80,13 +90,29 @@ impl StorageAcquireError { pub fn with_residual(source: BlazeError, residual: StorageSlot) -> Self { Self { source, - residual: Some(residual), + disposition: StorageAcquireDisposition::Residual(residual), } } - /// Split the original provider error from any residual slot owner. - pub fn into_parts(self) -> (BlazeError, Option) { - (self.source, self.residual) + /// Build a failure that cannot be cleaned up safely by stable slot name. + pub fn with_manual_cleanup_required(source: BlazeError) -> Self { + Self { + source, + disposition: StorageAcquireDisposition::ManualCleanupRequired, + } + } + + /// Whether automatic cleanup by stable slot name must be suppressed. + pub fn requires_manual_cleanup(&self) -> bool { + matches!( + self.disposition, + StorageAcquireDisposition::ManualCleanupRequired + ) + } + + /// Split the provider error from the required cleanup action. + pub fn into_parts(self) -> (BlazeError, StorageAcquireDisposition) { + (self.source, self.disposition) } } diff --git a/src/blaze/crates/blazed/src/api.rs b/src/blaze/crates/blazed/src/api.rs index 475b2a2a57..2f0e0b3cc9 100644 --- a/src/blaze/crates/blazed/src/api.rs +++ b/src/blaze/crates/blazed/src/api.rs @@ -1337,6 +1337,58 @@ mod tests { release_count: Arc, } + struct PanickingAcquireStorage { + entered: tokio::sync::Notify, + resume: tokio::sync::Notify, + release_count: Arc, + } + + #[async_trait] + impl StorageProvider for PanickingAcquireStorage { + async fn probe(&self) -> blaze_core::Result { + Ok(true) + } + + async fn acquire( + &self, + _opts: &AcquireOpts, + ) -> std::result::Result { + self.entered.notify_one(); + self.resume.notified().await; + panic!("controlled storage acquire panic"); + } + + async fn release(&self, _slot: StorageSlot) -> blaze_core::Result<()> { + self.release_count.fetch_add(1, Ordering::AcqRel); + Ok(()) + } + + async fn release_by_id(&self, _instance_id: &str) -> blaze_core::Result<()> { + self.release_count.fetch_add(1, Ordering::AcqRel); + Ok(()) + } + + async fn reconstruct(&self, instance_id: &str) -> blaze_core::Result { + Err(BlazeError::StorageIncomplete { + instance_id: instance_id.to_string(), + path: PathBuf::from(instance_id), + expected: "provider-owned slot", + }) + } + + async fn sync_artifacts(&self, _slot: &StorageSlot) -> blaze_core::Result<()> { + Ok(()) + } + + fn pool_status(&self) -> PoolStatus { + PoolStatus::default() + } + + async fn drain_pool(&self) -> blaze_core::Result { + Ok(0) + } + } + #[async_trait] impl StorageProvider for CountingStorage { async fn probe(&self) -> blaze_core::Result { @@ -3372,6 +3424,248 @@ mod tests { .expect("destroy residual slot"); } + #[cfg(feature = "test-failpoints")] + struct AcquireFailpointReleaseGuard<'a>(&'a crate::failpoint::TestFailpoint); + + #[cfg(feature = "test-failpoints")] + impl Drop for AcquireFailpointReleaseGuard<'_> { + fn drop(&mut self) { + self.0.release(); + } + } + + #[cfg(feature = "test-failpoints")] + async fn assert_cancelled_acquire_keeps_destroy_waiting( + failpoint: &'static str, + slot_is_published: bool, + ) { + let temp = tempfile::tempdir().expect("temp"); + let state = mock_state(&temp, false); + let hook = crate::failpoint::TestFailpoint::new(&[failpoint]); + let release_guard = AcquireFailpointReleaseGuard(&hook); + let create_state = Arc::clone(&state); + let create_hook = hook.clone(); + let create = tokio::spawn(async move { + create_hook + .run(create_instance(&create_state, &test_request())) + .await + }); + tokio::time::timeout(std::time::Duration::from_secs(2), hook.wait_until_paused()) + .await + .expect("storage acquire reached deterministic boundary"); + + let instance = state + .instances + .lock() + .expect("instances") + .values() + .next() + .cloned() + .expect("write-ahead instance"); + let id = instance.id; + let slot = temp.path().join("instances").join(id.to_string()); + assert_eq!(slot.exists(), slot_is_published); + + create.abort(); + assert!( + create + .await + .expect_err("create task aborted") + .is_cancelled() + ); + assert!(state.manager.operation_lock(id).try_lock().is_err()); + + let destroy_state = Arc::clone(&state); + let (destroy_started_tx, destroy_started_rx) = tokio::sync::oneshot::channel(); + let destroy = tokio::spawn(async move { + destroy_started_tx.send(()).expect("mark destroy started"); + destroy_instance(&destroy_state, &id.to_string()).await + }); + tokio::time::timeout(std::time::Duration::from_secs(2), destroy_started_rx) + .await + .expect("destroy task was scheduled") + .expect("destroy start signal"); + tokio::task::yield_now().await; + assert!( + !destroy.is_finished(), + "destroy must wait for supervised storage acquisition" + ); + assert_eq!(slot.exists(), slot_is_published); + + hook.release(); + drop(release_guard); + tokio::time::timeout(std::time::Duration::from_secs(2), destroy) + .await + .expect("destroy completed after storage supervision") + .expect("destroy task") + .expect("destroy response"); + assert!(!slot.exists()); + assert_eq!( + state.instances.lock().expect("instances")[&id].state, + SandboxState::Destroyed + ); + } + + #[cfg(feature = "test-failpoints")] + #[tokio::test] + async fn cancelled_acquire_before_publication_keeps_destroy_waiting() { + assert_cancelled_acquire_keeps_destroy_waiting( + "storage-acquire-before-slot-publish", + false, + ) + .await; + } + + #[cfg(feature = "test-failpoints")] + #[tokio::test] + async fn cancelled_acquire_after_publication_keeps_destroy_waiting() { + assert_cancelled_acquire_keeps_destroy_waiting("storage-acquire-after-slot-publish", true) + .await; + } + + #[cfg(feature = "test-failpoints")] + #[tokio::test] + async fn cancelled_replaced_slot_disables_automatic_cleanup_before_unlocking() { + let temp = tempfile::tempdir().expect("temp"); + let state = mock_state(&temp, false); + let hook = crate::failpoint::TestFailpoint::new(&["storage-acquire-after-slot-publish"]); + let release_guard = AcquireFailpointReleaseGuard(&hook); + let create_state = Arc::clone(&state); + let create_hook = hook.clone(); + let create = tokio::spawn(async move { + create_hook + .run(create_instance(&create_state, &test_request())) + .await + }); + tokio::time::timeout(std::time::Duration::from_secs(2), hook.wait_until_paused()) + .await + .expect("storage acquire reached post-publication boundary"); + + let instance = state + .instances + .lock() + .expect("instances") + .values() + .next() + .cloned() + .expect("write-ahead instance"); + let id = instance.id; + let instances = temp.path().join("instances"); + let slot = instances.join(id.to_string()); + let retained = instances.join(format!("{id}.retained")); + std::fs::rename(&slot, &retained).expect("detach provider-owned slot"); + std::fs::create_dir(&slot).expect("replacement slot"); + std::fs::write(slot.join("sentinel"), b"replacement").expect("replacement sentinel"); + + create.abort(); + assert!( + create + .await + .expect_err("create task aborted") + .is_cancelled() + ); + assert!(state.manager.operation_lock(id).try_lock().is_err()); + let destroy_state = Arc::clone(&state); + let (destroy_started_tx, destroy_started_rx) = tokio::sync::oneshot::channel(); + let destroy = tokio::spawn(async move { + destroy_started_tx.send(()).expect("mark destroy started"); + destroy_instance(&destroy_state, &id.to_string()).await + }); + tokio::time::timeout(std::time::Duration::from_secs(2), destroy_started_rx) + .await + .expect("destroy task was scheduled") + .expect("destroy start signal"); + tokio::task::yield_now().await; + assert!( + !destroy.is_finished(), + "destroy must wait while the provider classifies the replacement" + ); + assert_eq!( + std::fs::read(slot.join("sentinel")).expect("replacement remains while paused"), + b"replacement" + ); + + hook.release(); + drop(release_guard); + let error = tokio::time::timeout(std::time::Duration::from_secs(2), destroy) + .await + .expect("destroy completed after storage supervision") + .expect("destroy task") + .expect_err("replacement requires manual inspection"); + assert!(matches!(error, BlazeDaemonError::RecoveryRequired(_))); + assert_eq!( + std::fs::read(slot.join("sentinel")).expect("replacement remains"), + b"replacement" + ); + assert!(retained.is_dir()); + assert_eq!( + state.instances.lock().expect("instances")[&id].state, + SandboxState::RecoveryRequired + ); + } + + #[tokio::test] + async fn cancelled_create_blocks_cleanup_when_storage_acquire_panics() { + let temp = tempfile::tempdir().expect("temp"); + let config = test_config(&temp); + let release_count = Arc::new(AtomicUsize::new(0)); + let storage = Arc::new(PanickingAcquireStorage { + entered: tokio::sync::Notify::new(), + resume: tokio::sync::Notify::new(), + release_count: Arc::clone(&release_count), + }); + let state = build_test_state( + config, + test_policy(BackendKind::Mock, false), + spawners(BackendKind::Mock, Arc::new(MockSpawner)), + BackendKind::Mock, + storage.clone(), + ); + let create_state = Arc::clone(&state); + let create = + tokio::spawn(async move { create_instance(&create_state, &test_request()).await }); + tokio::time::timeout( + std::time::Duration::from_secs(2), + storage.entered.notified(), + ) + .await + .expect("storage provider entered acquire"); + + let instance = state + .instances + .lock() + .expect("instances") + .values() + .next() + .cloned() + .expect("write-ahead instance"); + let id = instance.id; + create.abort(); + assert!( + tokio::time::timeout(std::time::Duration::from_secs(2), create) + .await + .expect("create cancellation completed") + .expect_err("create task aborted") + .is_cancelled() + ); + assert!(state.manager.operation_lock(id).try_lock().is_err()); + + storage.resume.notify_one(); + let error = tokio::time::timeout( + std::time::Duration::from_secs(2), + destroy_instance(&state, &id.to_string()), + ) + .await + .expect("destroy completed after acquire panic") + .expect_err("unknown acquire outcome requires manual inspection"); + assert!(matches!(error, BlazeDaemonError::RecoveryRequired(_))); + assert_eq!(release_count.load(Ordering::Acquire), 0); + assert_eq!( + state.instances.lock().expect("instances")[&id].state, + SandboxState::RecoveryRequired + ); + } + #[cfg(feature = "test-failpoints")] #[tokio::test] async fn acquired_slot_is_destroyable_after_restart_before_start_commit() { diff --git a/src/blaze/crates/blazed/src/failpoint.rs b/src/blaze/crates/blazed/src/failpoint.rs index cdc168b8c9..85c4d972c3 100644 --- a/src/blaze/crates/blazed/src/failpoint.rs +++ b/src/blaze/crates/blazed/src/failpoint.rs @@ -1,8 +1,6 @@ // SPDX-License-Identifier: Apache-2.0 //! Feature-gated fault hooks for daemon-level integration verification. -#![allow(dead_code)] // Call sites land with their owning lifecycle commits. - #[cfg(test)] use std::cell::RefCell; #[cfg(test)] @@ -17,9 +15,14 @@ use tokio::sync::Notify; const FAILPOINTS_ENV: &str = "BLAZE_TEST_FAILPOINTS"; const FAILPOINT_FILE_ENV: &str = "BLAZE_TEST_FAILPOINT_FILE"; +#[cfg(test)] +tokio::task_local! { + static TEST_FAILPOINTS: Option>; +} + #[cfg(test)] thread_local! { - static TEST_FAILPOINTS: RefCell>> = + static BLOCKING_TEST_FAILPOINTS: RefCell>> = const { RefCell::new(None) }; } @@ -44,14 +47,14 @@ pub(crate) struct TestFailpoint { pub(crate) struct TestFailpointContext(Option>); #[cfg(test)] -struct TestFailpointScope { +struct BlockingTestFailpointScope { previous: Option>, } #[cfg(test)] -impl Drop for TestFailpointScope { +impl Drop for BlockingTestFailpointScope { fn drop(&mut self) { - TEST_FAILPOINTS.with(|current| { + BLOCKING_TEST_FAILPOINTS.with(|current| { current.replace(self.previous.take()); }); } @@ -74,9 +77,9 @@ impl TestFailpoint { /// Run one future with this failpoint set in its test-thread context. pub(crate) async fn run(&self, future: F) -> F::Output { - let previous = TEST_FAILPOINTS.with(|current| current.replace(Some(self.state.clone()))); - let _scope = TestFailpointScope { previous }; - future.await + TEST_FAILPOINTS + .scope(Some(self.state.clone()), future) + .await } /// Wait until the scoped future reaches a pause failpoint. @@ -100,7 +103,10 @@ impl TestFailpoint { /// Capture the current test failpoints without triggering them. #[cfg(test)] pub(crate) fn capture_test_context() -> TestFailpointContext { - TestFailpointContext(TEST_FAILPOINTS.with(|current| current.borrow().clone())) + TestFailpointContext( + task_test_context() + .or_else(|| BLOCKING_TEST_FAILPOINTS.with(|current| current.borrow().clone())), + ) } /// Install captured test failpoints while one blocking operation runs. @@ -109,11 +115,31 @@ pub(crate) fn with_test_context( context: TestFailpointContext, operation: impl FnOnce() -> T, ) -> T { - let previous = TEST_FAILPOINTS.with(|current| current.replace(context.0)); - let _scope = TestFailpointScope { previous }; + let previous = BLOCKING_TEST_FAILPOINTS.with(|current| current.replace(context.0)); + let _scope = BlockingTestFailpointScope { previous }; operation() } +// Preserve the active unit-test failpoint context in detached supervision. +#[cfg(test)] +pub(crate) fn spawn(future: F) -> tokio::task::JoinHandle +where + F: Future + Send + 'static, + R: Send + 'static, +{ + let context = task_test_context(); + tokio::spawn(TEST_FAILPOINTS.scope(context, future)) +} + +#[cfg(not(test))] +pub(crate) fn spawn(future: F) -> tokio::task::JoinHandle +where + F: std::future::Future + Send + 'static, + R: Send + 'static, +{ + tokio::spawn(future) +} + /// Log that a test-only binary is accepting failpoint configuration. pub(crate) fn announce() { tracing::warn!( @@ -190,6 +216,29 @@ pub(crate) async fn pause(name: &str) { } } +/// Hold a blocking durability operation at a test-only boundary. +pub(crate) fn pause_blocking(name: &str) { + #[cfg(test)] + if let Some(state) = test_state(name) { + state.paused.store(true, Ordering::Release); + state.paused_notify.notify_waiters(); + tracing::warn!(failpoint = name, "test failpoint paused"); + while !state.released.load(Ordering::Acquire) { + std::thread::sleep(std::time::Duration::from_millis(1)); + } + tracing::warn!(failpoint = name, "test failpoint released"); + return; + } + + if armed(name) { + tracing::warn!(failpoint = name, "test failpoint paused"); + while armed(name) { + std::thread::sleep(std::time::Duration::from_millis(10)); + } + tracing::warn!(failpoint = name, "test failpoint released"); + } +} + fn hit(name: &str) -> bool { if armed(name) { tracing::warn!(failpoint = name, "test failpoint triggered"); @@ -213,11 +262,19 @@ fn armed(name: &str) -> bool { #[cfg(test)] fn test_state(name: &str) -> Option> { - TEST_FAILPOINTS - .with(|current| current.borrow().clone()) + task_test_context() + .or_else(|| BLOCKING_TEST_FAILPOINTS.with(|current| current.borrow().clone())) .filter(|state| !state.released.load(Ordering::Acquire) && state.names.contains(&name)) } +#[cfg(test)] +fn task_test_context() -> Option> { + TEST_FAILPOINTS + .try_with(|current| current.clone()) + .ok() + .flatten() +} + fn configured(name: &str, inline: &str, file: &str) -> bool { inline .split(|character: char| character == ',' || character.is_whitespace()) @@ -230,10 +287,62 @@ fn configured(name: &str, inline: &str, file: &str) -> bool { mod tests { use super::configured; + struct DetachedFailpointReleaseGuard<'a>(&'a super::TestFailpoint); + + impl Drop for DetachedFailpointReleaseGuard<'_> { + fn drop(&mut self) { + self.0.release(); + } + } + #[test] fn configuration_matches_complete_tokens_from_both_sources() { assert!(configured("before-publish", "start, before-publish", "")); assert!(configured("after-publish", "", "start\nafter-publish")); assert!(!configured("publish", "before-publish", "after-publish")); } + + #[tokio::test] + async fn detached_spawn_keeps_failpoint_context_after_parent_abort() { + let timeout = std::time::Duration::from_secs(2); + let hook = super::TestFailpoint::new(&["detached-boundary"]); + let release_guard = DetachedFailpointReleaseGuard(&hook); + let parent_hook = hook.clone(); + let (continue_tx, continue_rx) = tokio::sync::oneshot::channel(); + let (child_tx, child_rx) = tokio::sync::oneshot::channel(); + let parent = tokio::spawn(async move { + parent_hook + .run(async move { + let child = super::spawn(async { + continue_rx.await.expect("release detached child"); + super::pause("detached-boundary").await; + }); + child_tx.send(child).expect("send detached child"); + std::future::pending::<()>().await; + }) + .await; + }); + let child = tokio::time::timeout(timeout, child_rx) + .await + .expect("parent spawned detached child") + .expect("receive detached child"); + parent.abort(); + assert!( + tokio::time::timeout(timeout, parent) + .await + .expect("parent cancellation completed") + .expect_err("parent task aborted") + .is_cancelled() + ); + continue_tx.send(()).expect("continue detached child"); + tokio::time::timeout(timeout, hook.wait_until_paused()) + .await + .expect("detached child retained failpoint context"); + hook.release(); + drop(release_guard); + tokio::time::timeout(timeout, child) + .await + .expect("detached child completed in time") + .expect("detached child completed"); + } } diff --git a/src/blaze/crates/blazed/src/failpoint_disabled.rs b/src/blaze/crates/blazed/src/failpoint_disabled.rs index 66eb79a17d..7c0472b521 100644 --- a/src/blaze/crates/blazed/src/failpoint_disabled.rs +++ b/src/blaze/crates/blazed/src/failpoint_disabled.rs @@ -1,8 +1,6 @@ // SPDX-License-Identifier: Apache-2.0 //! No-op hooks used when daemon verification support is disabled. -#![allow(dead_code)] // Call sites land with their owning lifecycle commits. - /// Keep daemon startup independent from verification-only configuration. pub(crate) fn announce() {} @@ -48,6 +46,18 @@ pub(crate) fn state(_name: &str) -> crate::error::Result<()> { /// Never pause production requests. pub(crate) async fn pause(_name: &str) {} +// Spawn detached supervision in production builds. +pub(crate) fn spawn(future: F) -> tokio::task::JoinHandle +where + F: std::future::Future + Send + 'static, + R: Send + 'static, +{ + tokio::spawn(future) +} + +/// Never pause production blocking operations. +pub(crate) fn pause_blocking(_name: &str) {} + #[cfg(test)] mod tests { #[tokio::test] diff --git a/src/blaze/crates/blazed/src/file_provider.rs b/src/blaze/crates/blazed/src/file_provider.rs index 5925b6e719..00a36c3e40 100644 --- a/src/blaze/crates/blazed/src/file_provider.rs +++ b/src/blaze/crates/blazed/src/file_provider.rs @@ -3,16 +3,17 @@ //! rootfs and memory files on a local filesystem. Base images and mutable //! instance slots use separate roots; runtime pooling is owned by the daemon. -use std::ffi::OsString; +use std::ffi::{OsStr, OsString}; use std::path::{Path, PathBuf}; use std::sync::Arc; use async_trait::async_trait; use rustix::fs::{ - AtFlags, Dir, DirEntry, FileType, FlockOperation, Mode, OFlags, flock, fstat, fsync, mkdirat, - open, openat, statat, unlinkat, + AtFlags, Dir, DirEntry, FileType, FlockOperation, Mode, OFlags, RenameFlags, fchmod, flock, + fstat, fsync, mkdirat, open, openat, renameat_with, statat, unlinkat, }; use rustix::io::Errno; +use uuid::Uuid; use blaze_core::error::{BlazeError, Result}; use blaze_core::storage::{ @@ -70,7 +71,7 @@ impl AcquireBlockingHook { } } - fn pause_after_mkdir(&self) { + fn pause_after_publish(&self) { self.entered.notify_one(); let mut released = self.released.lock().expect("acquire hook lock"); while !*released { @@ -349,7 +350,14 @@ fn plan_instances_owner(path: &Path) -> Result { OFlags::RDONLY | OFlags::DIRECTORY | OFlags::NOFOLLOW | OFlags::CLOEXEC, Mode::empty(), ) { - Ok(next) => current = next, + Ok(next) => { + ensure_trusted_existing_path_component(¤t, name, &next).map_err(|error| { + BlazeError::StorageError { + msg: format!("inspect storage instances root {}: {error}", path.display()), + } + })?; + current = next; + } Err(Errno::NOENT) => { missing.extend(components[index..].iter().cloned()); break; @@ -404,6 +412,400 @@ fn revalidate_instances_owner_plan(planned: &PlannedFileStorageProvider) -> Resu Ok(()) } +enum DirectoryPublicationError { + TargetExists, + Clean(String), + UnaddressableResidual(String), + PublishedOwnedResidual(String), +} + +struct RetainedDirectoryIdentity { + metadata: rustix::fs::Stat, + mount_id: u64, +} + +struct PublishedDirectory { + directory: std::os::fd::OwnedFd, + identity: RetainedDirectoryIdentity, +} + +fn verify_linked_directory_identity( + parent: &std::os::fd::OwnedFd, + name: &OsStr, + expected: &RetainedDirectoryIdentity, + label: &str, +) -> std::result::Result<(), String> { + let linked = openat( + parent, + name, + OFlags::RDONLY | OFlags::DIRECTORY | OFlags::NOFOLLOW | OFlags::CLOEXEC, + Mode::empty(), + ) + .map_err(|error| format!("open {label}: {error}"))?; + let metadata = fstat(&linked).map_err(|error| format!("inspect {label}: {error}"))?; + if metadata.st_dev != expected.metadata.st_dev || metadata.st_ino != expected.metadata.st_ino { + return Err(format!("{label} changed filesystem identity")); + } + let mount_id = opened_mount_id_for_owned_fd(&linked) + .map_err(|error| format!("inspect {label} mount identity: {error}"))?; + if mount_id != expected.mount_id { + return Err(format!( + "{label} changed mount identity {}->{mount_id}", + expected.mount_id + )); + } + Ok(()) +} + +fn slot_cleanup_is_retryable_by_id( + parent: &std::os::fd::OwnedFd, + name: &str, + expected: &RetainedDirectoryIdentity, +) -> bool { + if matches!( + statat(parent, name, AtFlags::SYMLINK_NOFOLLOW), + Err(Errno::NOENT) + ) { + // The directory was removed but its parent fsync may have failed. + // release_by_id can safely retry that durability step. + return true; + } + verify_linked_directory_identity( + parent, + OsStr::new(name), + expected, + "provider-owned slot directory", + ) + .is_ok() +} + +fn ensure_trusted_existing_path_component( + parent: &std::os::fd::OwnedFd, + name: &OsStr, + child: &std::os::fd::OwnedFd, +) -> std::result::Result<(), String> { + let parent_metadata = + fstat(parent).map_err(|error| format!("inspect path-component parent: {error}"))?; + let effective_uid = unsafe { libc::geteuid() }; + let trusted_parent_owner = + parent_metadata.st_uid == 0 || parent_metadata.st_uid == effective_uid; + let shared_write = parent_metadata.st_mode & (libc::S_IWGRP | libc::S_IWOTH) != 0; + let sticky = parent_metadata.st_mode & libc::S_ISVTX != 0; + if !trusted_parent_owner || (shared_write && !sticky) { + return Err(format!( + "path-component parent has unsafe owner or permissions (uid {}, mode {:04o})", + parent_metadata.st_uid, + parent_metadata.st_mode & 0o7777 + )); + } + + let child_metadata = fstat(child) + .map_err(|error| format!("inspect existing path component {name:?}: {error}"))?; + if shared_write && child_metadata.st_uid != 0 && child_metadata.st_uid != effective_uid { + return Err(format!( + "existing path component {name:?} in a shared sticky parent is owned by untrusted uid {}", + child_metadata.st_uid + )); + } + let linked = statat(parent, name, AtFlags::SYMLINK_NOFOLLOW) + .map_err(|error| format!("revalidate existing path component {name:?}: {error}"))?; + if linked.st_dev != child_metadata.st_dev || linked.st_ino != child_metadata.st_ino { + return Err(format!( + "existing path component {name:?} changed identity while it was opened" + )); + } + Ok(()) +} + +fn ensure_trusted_publication_parent( + parent: &std::os::fd::OwnedFd, +) -> std::result::Result<(), String> { + let metadata = fstat(parent).map_err(|error| format!("inspect publication parent: {error}"))?; + if FileType::from_raw_mode(metadata.st_mode) != FileType::Directory { + return Err("publication parent is not a directory".to_string()); + } + + let effective_uid = unsafe { libc::geteuid() }; + let trusted_owner = metadata.st_uid == 0 || metadata.st_uid == effective_uid; + let shared_write = metadata.st_mode & (libc::S_IWGRP | libc::S_IWOTH) != 0; + // A sticky directory protects existing entries, but another user can still + // reserve a not-yet-published sandbox name. Reject shared writers so later + // recovery can never mistake such a reservation for provider-owned storage. + if !trusted_owner || shared_write { + return Err(format!( + "publication parent has unsafe owner or permissions (uid {}, mode {:04o}); it must be owned by root or uid {} and must not be writable by group or other users", + metadata.st_uid, + metadata.st_mode & 0o7777, + effective_uid + )); + } + Ok(()) +} + +fn cleanup_owned_empty_directory_link( + parent: &std::os::fd::OwnedFd, + name: &OsStr, + expected: &rustix::fs::Stat, +) -> std::result::Result<(), String> { + let observed = match statat(parent, name, AtFlags::SYMLINK_NOFOLLOW) { + Ok(observed) => observed, + Err(Errno::NOENT) => return Err(format!("{name:?} disappeared before cleanup")), + Err(error) => return Err(format!("inspect {name:?} before cleanup: {error}")), + }; + if observed.st_dev != expected.st_dev || observed.st_ino != expected.st_ino { + return Err(format!("{name:?} changed identity before cleanup")); + } + unlinkat(parent, name, AtFlags::REMOVEDIR) + .map_err(|error| format!("remove {name:?}: {error}"))?; + fsync(parent) + .map_err(|error| format!("synchronize parent after removing {name:?}: {error}"))?; + Ok(()) +} + +fn staging_publication_failure( + parent: &std::os::fd::OwnedFd, + staging_name: &OsStr, + staging_identity: &rustix::fs::Stat, + error: String, +) -> DirectoryPublicationError { + match cleanup_owned_empty_directory_link(parent, staging_name, staging_identity) { + Ok(()) => DirectoryPublicationError::Clean(error), + Err(cleanup_error) => DirectoryPublicationError::UnaddressableResidual(format!( + "{error}; private staging cleanup could not be confirmed: {cleanup_error}" + )), + } +} + +enum PublishedDirectoryCleanupError { + Unaddressable(String), + Addressable(String), +} + +fn cleanup_published_empty_directory_link( + parent: &std::os::fd::OwnedFd, + name: &OsStr, + expected: &RetainedDirectoryIdentity, +) -> std::result::Result<(), PublishedDirectoryCleanupError> { + verify_linked_directory_identity(parent, name, expected, "published directory") + .map_err(PublishedDirectoryCleanupError::Unaddressable)?; + let linked = statat(parent, name, AtFlags::SYMLINK_NOFOLLOW).map_err(|error| { + PublishedDirectoryCleanupError::Unaddressable(format!( + "inspect {name:?} before unlink: {error}" + )) + })?; + if linked.st_dev != expected.metadata.st_dev || linked.st_ino != expected.metadata.st_ino { + return Err(PublishedDirectoryCleanupError::Unaddressable(format!( + "{name:?} changed identity before unlink" + ))); + } + unlinkat(parent, name, AtFlags::REMOVEDIR).map_err(|error| { + PublishedDirectoryCleanupError::Addressable(format!("remove {name:?}: {error}")) + })?; + fsync(parent).map_err(|error| { + PublishedDirectoryCleanupError::Addressable(format!( + "synchronize parent after removing {name:?}: {error}" + )) + })?; + Ok(()) +} + +fn published_directory_failure( + parent: &std::os::fd::OwnedFd, + target_name: &OsStr, + directory_identity: &RetainedDirectoryIdentity, + error: String, +) -> DirectoryPublicationError { + match cleanup_published_empty_directory_link(parent, target_name, directory_identity) { + Ok(()) => DirectoryPublicationError::Clean(error), + Err(PublishedDirectoryCleanupError::Unaddressable(cleanup_error)) => { + DirectoryPublicationError::UnaddressableResidual(format!( + "{error}; published directory cleanup was unsafe: {cleanup_error}" + )) + } + Err(PublishedDirectoryCleanupError::Addressable(cleanup_error)) => { + DirectoryPublicationError::PublishedOwnedResidual(format!( + "{error}; published directory cleanup could not be confirmed: {cleanup_error}" + )) + } + } +} + +fn publish_new_directory_at( + parent: &std::os::fd::OwnedFd, + target_name: &OsStr, + final_mode: Mode, + after_publish: F, +) -> std::result::Result +where + F: FnOnce(), +{ + ensure_trusted_publication_parent(parent).map_err(DirectoryPublicationError::Clean)?; + + let staging_name = loop { + let candidate = OsString::from(format!(".blaze-dir-{}.tmp", Uuid::new_v4())); + if candidate == target_name { + continue; + } + match mkdirat(parent, &candidate, Mode::from_bits_truncate(0o700)) { + Ok(()) => break candidate, + Err(Errno::EXIST) => continue, + Err(error) => { + return Err(DirectoryPublicationError::Clean(format!( + "create private staging directory: {error}" + ))); + } + } + }; + let directory = match openat( + parent, + &staging_name, + OFlags::RDONLY | OFlags::DIRECTORY | OFlags::NOFOLLOW | OFlags::CLOEXEC, + Mode::empty(), + ) { + Ok(directory) => directory, + Err(error) => { + let staging_identity = match statat(parent, &staging_name, AtFlags::SYMLINK_NOFOLLOW) { + Ok(identity) => identity, + Err(_) => { + return Err(DirectoryPublicationError::UnaddressableResidual(format!( + "retain private staging directory: {error}; private staging cleanup could not be confirmed" + ))); + } + }; + return Err(staging_publication_failure( + parent, + &staging_name, + &staging_identity, + format!("retain private staging directory: {error}"), + )); + } + }; + let directory_metadata = match fstat(&directory) { + Ok(identity) => identity, + Err(error) => { + let fallback_identity = match statat(parent, &staging_name, AtFlags::SYMLINK_NOFOLLOW) { + Ok(identity) => identity, + Err(_) => { + return Err(DirectoryPublicationError::UnaddressableResidual(format!( + "inspect private staging directory: {error}; private staging cleanup could not be confirmed" + ))); + } + }; + return Err(staging_publication_failure( + parent, + &staging_name, + &fallback_identity, + format!("inspect private staging directory: {error}"), + )); + } + }; + let effective_uid = unsafe { libc::geteuid() }; + if FileType::from_raw_mode(directory_metadata.st_mode) != FileType::Directory + || directory_metadata.st_uid != effective_uid + || directory_metadata.st_mode & (libc::S_IWGRP | libc::S_IWOTH) != 0 + { + return Err(staging_publication_failure( + parent, + &staging_name, + &directory_metadata, + format!( + "private staging directory has unexpected owner or permissions (uid {}, mode {:04o})", + directory_metadata.st_uid, + directory_metadata.st_mode & 0o7777 + ), + )); + } + let mount_id = match opened_mount_id_for_owned_fd(&directory) { + Ok(mount_id) => mount_id, + Err(error) => { + return Err(staging_publication_failure( + parent, + &staging_name, + &directory_metadata, + format!("inspect private staging mount identity: {error}"), + )); + } + }; + let directory_identity = RetainedDirectoryIdentity { + metadata: directory_metadata, + mount_id, + }; + if let Err(error) = fchmod(&directory, final_mode) { + return Err(staging_publication_failure( + parent, + &staging_name, + &directory_identity.metadata, + format!("set private staging directory permissions: {error}"), + )); + } + if let Err(error) = fsync(&directory) { + return Err(staging_publication_failure( + parent, + &staging_name, + &directory_identity.metadata, + format!("synchronize private staging directory: {error}"), + )); + } + + match renameat_with( + parent, + &staging_name, + parent, + target_name, + RenameFlags::NOREPLACE, + ) { + Ok(()) => {} + Err(Errno::EXIST) => { + let cleanup = cleanup_owned_empty_directory_link( + parent, + &staging_name, + &directory_identity.metadata, + ); + return match cleanup { + Ok(()) => Err(DirectoryPublicationError::TargetExists), + Err(cleanup_error) => { + Err(DirectoryPublicationError::UnaddressableResidual(format!( + "target already exists and private staging cleanup could not be confirmed: {cleanup_error}" + ))) + } + }; + } + Err(error) => { + return Err(staging_publication_failure( + parent, + &staging_name, + &directory_identity.metadata, + format!("publish directory without replacement: {error}"), + )); + } + } + + if let Err(error) = fsync(parent) { + return Err(published_directory_failure( + parent, + target_name, + &directory_identity, + format!("synchronize publication parent: {error}"), + )); + } + after_publish(); + + match verify_linked_directory_identity( + parent, + target_name, + &directory_identity, + "published directory", + ) { + Ok(()) => Ok(PublishedDirectory { + directory, + identity: directory_identity, + }), + Err(error) => Err(DirectoryPublicationError::UnaddressableResidual(format!( + "published directory identity could not be verified: {error}" + ))), + } +} + fn materialize_instances_owner( mut current: std::os::fd::OwnedFd, missing: &[OsString], @@ -415,9 +817,14 @@ fn materialize_instances_owner( } for name in missing { walked.push(name); - match mkdirat(¤t, name, Mode::from_bits_truncate(0o750)) { - Ok(()) => {} - Err(Errno::EXIST) => { + current = match publish_new_directory_at( + ¤t, + name, + Mode::from_bits_truncate(0o750), + || {}, + ) { + Ok(published) => published.directory, + Err(DirectoryPublicationError::TargetExists) => { return Err(BlazeError::StorageError { msg: format!( "storage instances path {} appeared after startup planning", @@ -425,7 +832,11 @@ fn materialize_instances_owner( ), }); } - Err(error) => { + Err( + DirectoryPublicationError::Clean(error) + | DirectoryPublicationError::UnaddressableResidual(error) + | DirectoryPublicationError::PublishedOwnedResidual(error), + ) => { return Err(BlazeError::StorageError { msg: format!( "create storage instances path {}: {error}", @@ -433,22 +844,15 @@ fn materialize_instances_owner( ), }); } - } - // Synchronize each newly created component before descending into it. - fsync(¤t).map_err(|error| BlazeError::StorageError { - msg: format!("synchronize storage parent {}: {error}", walked.display()), - })?; - current = openat( - ¤t, - name, - OFlags::RDONLY | OFlags::DIRECTORY | OFlags::NOFOLLOW | OFlags::CLOEXEC, - Mode::empty(), - ) - .map_err(|error| BlazeError::StorageError { - msg: format!("open storage instances path {}: {error}", walked.display()), - })?; + }; } let directory = current; + ensure_trusted_publication_parent(&directory).map_err(|error| BlazeError::StorageError { + msg: format!( + "storage instances root {} cannot safely publish slots: {error}", + resolved.display() + ), + })?; if let Err(error) = flock(&directory, FlockOperation::NonBlockingLockExclusive) { return Err(BlazeError::StorageError { msg: if error == Errno::WOULDBLOCK { @@ -513,10 +917,10 @@ impl StorageProvider for FileStorageProvider { let acquire_hook = self.acquire_blocking_hook.clone(); #[cfg(test)] let finish_hook = self.acquire_blocking_hook.clone(); - let after_mkdir = move || { + let after_publish = move || { #[cfg(test)] if let Some(hook) = acquire_hook { - hook.pause_after_mkdir(); + hook.pause_after_publish(); } }; let instance_id = opts.instance_id.clone(); @@ -535,7 +939,7 @@ impl StorageProvider for FileStorageProvider { images_dir, rootfs_size, mem_size, - after_mkdir, + after_publish, ) }); #[cfg(not(test))] @@ -546,7 +950,7 @@ impl StorageProvider for FileStorageProvider { images_dir, rootfs_size, mem_size, - after_mkdir, + after_publish, ); #[cfg(test)] if let Some(hook) = finish_hook { @@ -558,13 +962,12 @@ impl StorageProvider for FileStorageProvider { match result { Ok(Ok(())) => Ok(slot), Ok(Err(error)) => Err(*error), - Err(error) => Err(StorageAcquireError::with_residual( + Err(error) => Err(StorageAcquireError::with_manual_cleanup_required( BlazeError::StorageError { msg: format!( "acquire '{instance_id}': blocking transaction failed to join: {error}; outcome is unknown" ), }, - slot, )), } } @@ -764,6 +1167,22 @@ fn remove_slot_tree(root: &std::os::fd::OwnedFd, instance_id: &str) -> Result<() ) } +fn remove_slot_tree_matching( + root: &std::os::fd::OwnedFd, + instance_id: &str, + expected: &RetainedDirectoryIdentity, +) -> Result<()> { + remove_slot_tree_with_identity_and_hooks( + root, + instance_id, + Some(expected), + &mut || Ok(()), + &mut || crate::failpoint::storage("storage-release-after-entry"), + &mut || Ok(()), + &mut || crate::failpoint::storage("storage-release-before-root-sync"), + ) +} + fn remove_slot_tree_with_hooks( root: &std::os::fd::OwnedFd, instance_id: &str, @@ -772,6 +1191,32 @@ fn remove_slot_tree_with_hooks( before_unlink: &mut U, before_root_sync: &mut S, ) -> Result<()> +where + I: FnMut() -> Result<()>, + E: FnMut() -> Result<()>, + U: FnMut() -> Result<()>, + S: FnMut() -> Result<()>, +{ + remove_slot_tree_with_identity_and_hooks( + root, + instance_id, + None, + after_inspect, + after_entry, + before_unlink, + before_root_sync, + ) +} + +fn remove_slot_tree_with_identity_and_hooks( + root: &std::os::fd::OwnedFd, + instance_id: &str, + expected: Option<&RetainedDirectoryIdentity>, + after_inspect: &mut I, + after_entry: &mut E, + before_unlink: &mut U, + before_root_sync: &mut S, +) -> Result<()> where I: FnMut() -> Result<()>, E: FnMut() -> Result<()>, @@ -799,6 +1244,9 @@ where }); } }; + if let Some(expected) = expected { + ensure_same_metadata_object(&expected.metadata, &inspected, "provider-owned slot root")?; + } after_inspect()?; let directory = match openat( root, @@ -819,6 +1267,21 @@ where } }; ensure_same_object(&inspected, &directory, "provider slot root")?; + if let Some(expected) = expected { + ensure_same_object(&expected.metadata, &directory, "provider-owned slot root")?; + let mount_id = + opened_mount_id_for_owned_fd(&directory).map_err(|error| BlazeError::StorageError { + msg: format!("release '{instance_id}': inspect owned slot mount: {error}"), + })?; + if mount_id != expected.mount_id { + return Err(BlazeError::StorageError { + msg: format!( + "release '{instance_id}': provider-owned slot changed mount identity {}->{mount_id}", + expected.mount_id + ), + }); + } + } remove_directory_contents_on_mount( &directory, root_mount_id, @@ -1148,46 +1611,59 @@ fn acquire_slot_blocking( images_dir: PathBuf, rootfs_size: u64, mem_size: u64, - after_create: F, + after_publish: F, ) -> std::result::Result<(), Box> where F: FnOnce(), { let instance_id = slot.id.clone(); - match mkdirat( + crate::failpoint::pause_blocking("storage-acquire-before-slot-publish"); + let published = match publish_new_directory_at( &owner, - instance_id.as_str(), + OsStr::new(&instance_id), Mode::from_bits_truncate(0o750), + || {}, ) { - Ok(()) => {} - Err(Errno::EXIST) => { + Ok(published) => published, + Err(DirectoryPublicationError::TargetExists) => { return Err(Box::new(StorageAcquireError::clean( BlazeError::StorageError { msg: format!("acquire '{instance_id}': instance directory already exists"), }, ))); } - Err(error) => { + Err(DirectoryPublicationError::Clean(error)) => { return Err(Box::new(StorageAcquireError::clean( BlazeError::StorageError { msg: format!("acquire '{instance_id}': create dir: {error}"), }, ))); } - } - after_create(); + Err(DirectoryPublicationError::PublishedOwnedResidual(error)) => { + return Err(Box::new(StorageAcquireError::with_residual( + BlazeError::StorageError { + msg: format!("acquire '{instance_id}': create dir: {error}"), + }, + slot, + ))); + } + Err(DirectoryPublicationError::UnaddressableResidual(error)) => { + return Err(Box::new(StorageAcquireError::with_manual_cleanup_required( + BlazeError::StorageError { + msg: format!("acquire '{instance_id}': create dir: {error}"), + }, + ))); + } + }; + let PublishedDirectory { + directory: slot_directory, + identity: slot_identity, + } = published; + after_publish(); + crate::failpoint::pause_blocking("storage-acquire-after-slot-publish"); let setup = (|| -> Result<()> { crate::failpoint::storage("storage-acquire-retain-slot")?; - let slot_directory = openat( - &owner, - instance_id.as_str(), - OFlags::RDONLY | OFlags::DIRECTORY | OFlags::NOFOLLOW | OFlags::CLOEXEC, - Mode::empty(), - ) - .map_err(|error| BlazeError::StorageError { - msg: format!("acquire '{instance_id}': retain slot: {error}"), - })?; create_or_copy_at_blocking( &images_dir.join("rootfs.ext4"), &slot_directory, @@ -1211,6 +1687,15 @@ where // the retained root immediately before returning them; provider-local // cleanup remains descriptor-relative if this check fails. revalidate_backend_visible_root(&owner, &instances_dir, "acquire")?; + verify_linked_directory_identity( + &owner, + OsStr::new(&instance_id), + &slot_identity, + "published slot directory", + ) + .map_err(|error| BlazeError::StorageError { + msg: format!("acquire '{instance_id}': {error}"), + })?; Ok(()) })(); let Err(setup_error) = setup else { @@ -1218,7 +1703,7 @@ where }; let rollback_result = crate::failpoint::storage("storage-acquire-rollback") - .and_then(|_| remove_slot_tree(&owner, &instance_id)); + .and_then(|_| remove_slot_tree_matching(&owner, &instance_id, &slot_identity)); match rollback_result { Ok(()) => Err(Box::new(StorageAcquireError::clean( BlazeError::StorageError { @@ -1227,14 +1712,26 @@ where ), }, ))), - Err(cleanup_error) => Err(Box::new(StorageAcquireError::with_residual( + Err(cleanup_error) + if slot_cleanup_is_retryable_by_id(&owner, &instance_id, &slot_identity) => + { + Err(Box::new(StorageAcquireError::with_residual( + BlazeError::StorageError { + msg: format!( + "acquire '{instance_id}': slot setup failed ({setup_error}); rollback failed for {}: {cleanup_error}", + slot.instance_dir.display() + ), + }, + slot, + ))) + } + Err(cleanup_error) => Err(Box::new(StorageAcquireError::with_manual_cleanup_required( BlazeError::StorageError { msg: format!( - "acquire '{instance_id}': slot setup failed ({setup_error}); rollback failed for {}: {cleanup_error}", + "acquire '{instance_id}': slot setup failed ({setup_error}); rollback could not safely address {}: {cleanup_error}", slot.instance_dir.display() ), }, - slot, ))), } } @@ -1318,7 +1815,7 @@ fn validate_instance_id(instance_id: &str) -> Result<()> { #[cfg(test)] mod tests { use super::*; - use uuid::Uuid; + use blaze_core::storage::StorageAcquireDisposition; fn write_complete_replacement_slot(root: &Path, id: &str, marker: &[u8]) { let slot = root.join(id); @@ -1352,6 +1849,174 @@ mod tests { ); } + #[test] + fn directory_publication_never_replaces_an_existing_target() { + let temp = tempfile::tempdir().expect("tempdir"); + let target = temp.path().join("instances"); + std::fs::create_dir(&target).expect("existing target"); + std::fs::write(target.join("sentinel"), b"existing").expect("existing sentinel"); + let parent = open( + temp.path(), + OFlags::RDONLY | OFlags::DIRECTORY | OFlags::NOFOLLOW | OFlags::CLOEXEC, + Mode::empty(), + ) + .expect("publication parent"); + + let error = match publish_new_directory_at( + &parent, + OsStr::new("instances"), + Mode::from_bits_truncate(0o750), + || {}, + ) { + Ok(_) => panic!("publication must not replace an existing target"), + Err(error) => error, + }; + + assert!(matches!(error, DirectoryPublicationError::TargetExists)); + assert_eq!( + std::fs::read(target.join("sentinel")).expect("existing sentinel remains"), + b"existing" + ); + assert!( + temp.path() + .read_dir() + .expect("publication parent") + .all(|entry| !entry + .expect("directory entry") + .file_name() + .to_string_lossy() + .starts_with(".blaze-dir-")) + ); + } + + #[test] + fn directory_publication_rejects_a_post_publish_replacement() { + let temp = tempfile::tempdir().expect("tempdir"); + let target = temp.path().join("instances"); + let detached = temp.path().join("instances-retained"); + let parent = open( + temp.path(), + OFlags::RDONLY | OFlags::DIRECTORY | OFlags::NOFOLLOW | OFlags::CLOEXEC, + Mode::empty(), + ) + .expect("publication parent"); + + let error = match publish_new_directory_at( + &parent, + OsStr::new("instances"), + Mode::from_bits_truncate(0o750), + || { + std::fs::rename(&target, &detached).expect("detach published directory"); + std::fs::create_dir(&target).expect("replacement directory"); + std::fs::write(target.join("sentinel"), b"replacement") + .expect("replacement sentinel"); + }, + ) { + Ok(_) => panic!("publication must not retain a replacement directory"), + Err(error) => error, + }; + + assert!(matches!( + error, + DirectoryPublicationError::UnaddressableResidual(ref message) + if message.contains("changed filesystem identity") + )); + assert!(detached.is_dir()); + assert_eq!( + std::fs::read(target.join("sentinel")).expect("replacement remains"), + b"replacement" + ); + } + + #[test] + fn prepare_rejects_a_shared_writable_instances_root() { + use std::os::unix::fs::PermissionsExt; + + let temp = tempfile::tempdir().expect("tempdir"); + let target = temp.path().join("instances"); + std::fs::create_dir(&target).expect("instances root"); + std::fs::set_permissions(&target, std::fs::Permissions::from_mode(0o777)) + .expect("unsafe shared permissions"); + let planned = FileStorageProvider::plan(target.clone()).expect("plan instances root"); + + let error = match FileStorageProvider::prepare(planned) { + Ok(_) => panic!("unsafe shared parent must be rejected"), + Err(error) => error, + }; + + assert!( + error + .to_string() + .contains("must not be writable by group or other users") + ); + } + + #[test] + fn prepare_rejects_a_sticky_shared_writable_instances_root() { + use std::os::unix::fs::PermissionsExt; + + let temp = tempfile::tempdir().expect("tempdir"); + let target = temp.path().join("instances"); + std::fs::create_dir(&target).expect("instances root"); + std::fs::set_permissions(&target, std::fs::Permissions::from_mode(0o1777)) + .expect("sticky shared permissions"); + let planned = FileStorageProvider::plan(target).expect("plan instances root"); + + let error = match FileStorageProvider::prepare(planned) { + Ok(_) => panic!("sticky shared parent must be rejected"), + Err(error) => error, + }; + + assert!( + error + .to_string() + .contains("must not be writable by group or other users") + ); + } + + #[test] + fn plan_rejects_a_non_sticky_shared_writable_existing_ancestor() { + use std::os::unix::fs::PermissionsExt; + + let temp = tempfile::tempdir().expect("tempdir"); + let shared = temp.path().join("shared"); + let owned = shared.join("owned"); + std::fs::create_dir(&shared).expect("shared ancestor"); + std::fs::set_permissions(&shared, std::fs::Permissions::from_mode(0o777)) + .expect("unsafe shared permissions"); + std::fs::create_dir(&owned).expect("owned child"); + + let error = match FileStorageProvider::plan(owned.join("instances")) { + Ok(_) => panic!("unsafe existing ancestor must be rejected"), + Err(error) => error, + }; + + assert!( + error + .to_string() + .contains("path-component parent has unsafe owner or permissions") + ); + } + + #[test] + fn plan_accepts_an_owned_component_below_a_sticky_system_parent() { + use std::os::unix::fs::PermissionsExt; + + let temp = tempfile::tempdir().expect("tempdir"); + let shared = temp.path().join("shared"); + let owned = shared.join("owned"); + std::fs::create_dir(&shared).expect("shared ancestor"); + std::fs::set_permissions(&shared, std::fs::Permissions::from_mode(0o1777)) + .expect("sticky shared permissions"); + std::fs::create_dir(&owned).expect("owned child"); + + let target = owned.join("instances"); + let planned = FileStorageProvider::plan(target.clone()).expect("safe existing chain"); + let prepared = FileStorageProvider::prepare(planned).expect("publish below owned child"); + + assert_eq!(prepared.resolved_instances_dir, target); + } + #[test] fn prepare_rejects_replaced_planned_ancestor_without_materializing() { let temp = tempfile::tempdir().expect("tempdir"); @@ -1560,14 +2225,14 @@ mod tests { }) .await .expect_err("acquire must not return a path through a replacement root"); - let (source, residual) = acquire_error.into_parts(); + let (source, disposition) = acquire_error.into_parts(); assert!(matches!(&source, BlazeError::StorageError { .. })); assert!( source .to_string() .contains("backend-visible storage instances root") ); - assert!(residual.is_none()); + assert!(matches!(disposition, StorageAcquireDisposition::Clean)); assert!(!detached.join(&later_id).exists()); for name in ["rootfs.ext4", "mem.bin", "mem.diff", "rootfs.diff"] { assert_eq!( @@ -1816,9 +2481,9 @@ mod tests { .run(provider.acquire(&opts)) .await .expect_err("slot retain failure"); - let (_source, residual) = error.into_parts(); + let (_source, disposition) = error.into_parts(); - assert!(residual.is_none()); + assert!(matches!(disposition, StorageAcquireDisposition::Clean)); assert!(!temporary.path().join(&opts.instance_id).exists()); } @@ -1841,8 +2506,10 @@ mod tests { .run(provider.acquire(&opts)) .await .expect_err("slot retain and rollback failure"); - let (_source, residual) = error.into_parts(); - let residual = residual.expect("residual slot ownership"); + let (_source, disposition) = error.into_parts(); + let StorageAcquireDisposition::Residual(residual) = disposition else { + panic!("residual slot ownership must be transferred"); + }; assert_eq!(residual.id, opts.instance_id); assert!(temporary.path().join(&residual.id).is_dir()); @@ -1852,6 +2519,37 @@ mod tests { .expect("release residual slot"); } + #[cfg(feature = "test-failpoints")] + #[tokio::test] + async fn acquire_returns_a_retryable_residual_after_unlink_sync_failure() { + let temporary = tempfile::tempdir().expect("temporary directory"); + let provider = FileStorageProvider::new(temporary.path().to_path_buf()); + let opts = AcquireOpts { + instance_id: "removed-residual".into(), + rootfs_size: 64, + mem_size: 32, + }; + let failpoint = crate::failpoint::TestFailpoint::new(&[ + "storage-acquire-artifacts", + "storage-release-before-root-sync", + ]); + + let error = failpoint + .run(provider.acquire(&opts)) + .await + .expect_err("root synchronization failure"); + let (_source, disposition) = error.into_parts(); + let StorageAcquireDisposition::Residual(residual) = disposition else { + panic!("removed slot must retain a retryable cleanup owner"); + }; + + assert!(!temporary.path().join(&opts.instance_id).exists()); + provider + .release_by_id(&residual.id) + .await + .expect("retry missing-slot parent synchronization"); + } + #[cfg(feature = "test-failpoints")] #[tokio::test] async fn acquire_rolls_back_when_the_instances_root_sync_fails() { @@ -1868,14 +2566,14 @@ mod tests { .run(provider.acquire(&opts)) .await .expect_err("instances-root synchronization failure"); - let (source, residual) = error.into_parts(); + let (source, disposition) = error.into_parts(); assert!( source .to_string() .contains("storage-acquire-before-root-sync") ); - assert!(residual.is_none()); + assert!(matches!(disposition, StorageAcquireDisposition::Clean)); assert!(!temporary.path().join(&opts.instance_id).exists()); } @@ -2376,6 +3074,54 @@ mod tests { ); } + #[tokio::test(flavor = "current_thread")] + async fn acquire_rejects_a_slot_replaced_after_publication() -> Result<()> { + use std::time::Duration; + + let temp = tempfile::tempdir()?; + let mut provider = FileStorageProvider::new(temp.path().to_path_buf()); + let hook = Arc::new(AcquireBlockingHook::new()); + provider.acquire_blocking_hook = Some(Arc::clone(&hook)); + let provider = Arc::new(provider); + let task_provider = Arc::clone(&provider); + let task = tokio::spawn(async move { + task_provider + .acquire(&AcquireOpts { + instance_id: "publication-race".to_string(), + rootfs_size: 64, + mem_size: 32, + }) + .await + }); + + tokio::time::timeout(Duration::from_secs(4), hook.wait_until_entered()) + .await + .expect("slot publication reached deterministic boundary"); + let published = temp.path().join("publication-race"); + let detached = temp.path().join("publication-race-retained"); + std::fs::rename(&published, &detached).expect("detach published slot"); + std::fs::create_dir(&published).expect("replacement slot"); + std::fs::write(published.join("sentinel"), b"replacement").expect("replacement sentinel"); + hook.resume(); + + let acquire_error = task + .await + .expect("acquire task completed") + .expect_err("replacement must fail acquisition"); + let (source, disposition) = acquire_error.into_parts(); + assert!(source.to_string().contains("changed identity")); + assert!(matches!( + disposition, + StorageAcquireDisposition::ManualCleanupRequired + )); + assert!(detached.is_dir()); + assert_eq!( + std::fs::read(published.join("sentinel")).expect("replacement remains"), + b"replacement" + ); + Ok(()) + } + #[tokio::test(flavor = "current_thread")] async fn slot_creation_does_not_block_async_runtime() -> Result<()> { use std::sync::atomic::{AtomicBool, Ordering}; diff --git a/src/blaze/crates/blazed/src/sandbox/manager.rs b/src/blaze/crates/blazed/src/sandbox/manager.rs index d40d76e1ea..796ec95291 100644 --- a/src/blaze/crates/blazed/src/sandbox/manager.rs +++ b/src/blaze/crates/blazed/src/sandbox/manager.rs @@ -80,6 +80,7 @@ pub struct SandboxManager { instances: Arc>>, backend_instances: Arc>>, operation_locks: Mutex>>>, + storage_cleanup_blocked: Arc>>, pub(super) storage_sync_inflight: Arc>>, pub(super) storage_sync_permits: Arc, pool: Arc>, @@ -147,6 +148,7 @@ impl SandboxManager { instances, backend_instances, operation_locks: Mutex::new(operation_locks), + storage_cleanup_blocked: Arc::new(Mutex::new(HashSet::new())), storage_sync_inflight: Arc::new(Mutex::new(HashSet::new())), // The periodic worker is sequential. Retain that bound when a // timed-out provider operation has to finish in the background. @@ -285,7 +287,7 @@ impl SandboxManager { request.decision.policy_name.clone(), ); let operation_lock = self.operation_lock(instance.id); - let _operation = operation_lock.lock().await; + let operation = operation_lock.lock_owned().await; instance.transition(SandboxState::Creating)?; instance.begin_operation(OperationKind::Create); @@ -319,19 +321,52 @@ impl SandboxManager { ))); } - let storage = match self - .storage - .acquire(&AcquireOpts { + let acquire = crate::failpoint::spawn(supervise_storage_acquire( + Arc::clone(&self.storage), + Arc::clone(&self.storage_cleanup_blocked), + operation, + instance.id, + AcquireOpts { instance_id: instance.id.to_string(), rootfs_size: self.rootfs_size, mem_size: self.mem_size, - }) - .await - { + }, + )); + let (operation, acquire_result) = match acquire.await { + Ok(result) => result, + Err(error) => { + let recovery = self.mark_instance_recovery(instance.clone()).err(); + return Err(BlazeDaemonError::RecoveryRequired(format!( + "create {}: storage acquisition supervisor failed: {error}; automatic storage cleanup is disabled{}", + instance.id, + recovery + .map(|error| format!("; recovery state persistence failed: {error}")) + .unwrap_or_default() + ))); + } + }; + let _operation = operation; + let storage = match acquire_result { Ok(storage) => storage, Err(error) => { - let (source, residual) = error.into_parts(); - return Err(self.retain_failed_acquire(&mut instance, residual, source.into())); + let (source, disposition) = error.into_parts(); + match disposition { + blaze_core::storage::StorageAcquireDisposition::Clean => { + return Err(self.retain_failed_acquire(&mut instance, None, source.into())); + } + blaze_core::storage::StorageAcquireDisposition::Residual(residual) => { + return Err(self.retain_failed_acquire( + &mut instance, + Some(residual), + source.into(), + )); + } + blaze_core::storage::StorageAcquireDisposition::ManualCleanupRequired => { + return Err( + self.retain_manual_storage_failure(&mut instance, source.into()) + ); + } + } } }; crate::failpoint::pause("create-after-storage-acquire").await; @@ -732,6 +767,11 @@ impl SandboxManager { let _ = self.mark_recovery(id); return; } + if let Err(error) = self.ensure_automatic_storage_cleanup_allowed(id) { + tracing::error!(instance = %id, %error, "quarantined storage cleanup suppressed"); + let _ = self.mark_recovery(id); + return; + } if let Err(error) = self.storage.release_by_id(&id.to_string()).await { tracing::error!(instance = %id, %error, "quarantined storage cleanup failed"); let _ = self.mark_recovery(id); @@ -862,6 +902,15 @@ impl SandboxManager { ))); } + if let Err(error) = self.ensure_automatic_storage_cleanup_allowed(id) { + let recovery = self.mark_recovery(id).err(); + return Err(BlazeDaemonError::RecoveryRequired(format!( + "destroy {id}: {error}; storage retained for manual inspection{}", + recovery + .map(|error| format!("; recovery state persistence failed: {error}")) + .unwrap_or_default() + ))); + } if let Err(error) = self.storage.release_by_id(&id.to_string()).await { let recovery = self.mark_recovery(id).err(); return Err(BlazeDaemonError::RecoveryRequired(format!( @@ -1130,6 +1179,49 @@ impl SandboxManager { } } + fn retain_manual_storage_failure( + &self, + instance: &mut SandboxInstance, + original: BlazeDaemonError, + ) -> BlazeDaemonError { + let mut errors = Vec::new(); + if instance.state != SandboxState::RecoveryRequired + && let Err(error) = instance.transition(SandboxState::RecoveryRequired) + { + errors.push(format!("recovery state update failed: {error}")); + } + if let Err(error) = self.state_store.persist(instance) { + errors.push(format!("recovery state persistence failed: {error}")); + } + if let Some(error) = self.retain_instance(instance.clone()) { + errors.push(error); + } + let suffix = if errors.is_empty() { + "automatic cleanup by sandbox ID is disabled; inspect storage manually".to_string() + } else { + format!( + "automatic cleanup by sandbox ID is disabled; recovery recording also failed: {}", + errors.join("; ") + ) + }; + BlazeDaemonError::RecoveryRequired(format!( + "{original}; instance {}: {suffix}", + instance.id + )) + } + + fn ensure_automatic_storage_cleanup_allowed(&self, id: Uuid) -> Result<()> { + match self.storage_cleanup_blocked.lock() { + Ok(blocked) if !blocked.contains(&id) => Ok(()), + Ok(_) => Err(BlazeDaemonError::RecoveryRequired(format!( + "automatic storage cleanup for {id} is disabled because the provider could not prove that the stable slot name still identifies its object" + ))), + Err(_) => Err(BlazeDaemonError::RecoveryRequired(format!( + "automatic storage cleanup for {id} is disabled because the cleanup-safety registry is poisoned" + ))), + } + } + /// Commit a fully compensated create as terminal without losing the /// operation record when that terminal commit itself fails. fn commit_create_rollback(&self, instance: &mut SandboxInstance) -> Vec { @@ -1218,6 +1310,88 @@ impl SandboxManager { } } +struct StorageAcquireSupervision { + operation: Option>, + cleanup_blocked: Arc>>, + instance_id: Uuid, + armed: bool, +} + +impl StorageAcquireSupervision { + fn new( + operation: OwnedMutexGuard<()>, + cleanup_blocked: Arc>>, + instance_id: Uuid, + ) -> Self { + Self { + operation: Some(operation), + cleanup_blocked, + instance_id, + armed: true, + } + } + + fn finish(mut self, manual_cleanup: bool) -> OwnedMutexGuard<()> { + if manual_cleanup { + self.block_cleanup(); + } + let operation = self.operation.take().expect("operation guard"); + self.armed = false; + operation + } + + fn block_cleanup(&self) { + match self.cleanup_blocked.lock() { + Ok(mut blocked) => { + blocked.insert(self.instance_id); + } + Err(poisoned) => { + poisoned.into_inner().insert(self.instance_id); + } + } + } +} + +impl Drop for StorageAcquireSupervision { + fn drop(&mut self) { + if self.armed { + self.block_cleanup(); + } + } +} + +async fn supervise_storage_acquire( + storage: Arc, + cleanup_blocked: Arc>>, + operation: OwnedMutexGuard<()>, + instance_id: Uuid, + opts: AcquireOpts, +) -> ( + OwnedMutexGuard<()>, + std::result::Result, +) { + let supervision = StorageAcquireSupervision::new(operation, cleanup_blocked, instance_id); + let worker = crate::failpoint::spawn(async move { storage.acquire(&opts).await }); + let result = match worker.await { + Ok(result) => result, + Err(error) => Err( + blaze_core::storage::StorageAcquireError::with_manual_cleanup_required( + BlazeError::StorageError { + msg: format!( + "acquire '{instance_id}': supervised provider task failed: {error}; outcome is unknown" + ), + }, + ), + ), + }; + let manual_cleanup = result + .as_ref() + .err() + .is_some_and(|error| error.requires_manual_cleanup()); + let operation = supervision.finish(manual_cleanup); + (operation, result) +} + fn poisoned(name: &str) -> BlazeDaemonError { BlazeDaemonError::Internal(format!("{name} lock poisoned")) }