Skip to content

Commit 38a01e2

Browse files
author
Roy Lin
committed
fix(runtime): rootfs-cache in-use guard prevents concurrent same-image corruption
RootfsCache::prune (called after a cache-miss put) evicted LRU entries with no in-use guard, so it could remove_dir_all a cache entry that a CONCURRENT box was using as its overlayfs lowerdir — that box's mount(2) then failed with ENOENT ('No such file or directory (os error 2)'), persisting through retries since the backing was gone. Two pipelines from the same image collapse onto one cache key, so this hit any concurrent same-image workload. Fix: the same in-use guard SnapshotStore::prune already applies to live CoW lowers. Each overlay box records the cache key it holds in <box_dir>/.rootfs- cache-key (removed with the box dir); prune skips any still-referenced key (prune_protecting, with prune as the empty-set wrapper). Found via a concurrent-pipeline chaos test driven through a3s-code; verified on a real /dev/kvm host (concurrency scenario: ~50% failure -> reliably green). 41 rootfs-cache unit tests + clippy clean.
1 parent 55136c6 commit 38a01e2

3 files changed

Lines changed: 116 additions & 5 deletions

File tree

CHANGELOG.md

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,21 @@ All notable changes to A3S Box will be documented in this file.
4141
(use `StepResult::combined()` for the old concatenated view). Breaking for
4242
direct `.logs` field access on the `a3s-box-sdk` pipeline API.
4343

44+
### Fixed
45+
46+
- **Concurrent same-image pipelines could corrupt each other's rootfs cache.**
47+
`RootfsCache::prune` (run after a cache-miss `put`) evicted least-recently-used
48+
entries with no in-use guard, so it could `remove_dir_all` a cache entry that
49+
another box was simultaneously using as its overlayfs **lowerdir** — the peer's
50+
`mount(2)` then failed with `No such file or directory (os error 2)`, and the
51+
failure persisted through retries (the backing was gone). Added the same in-use
52+
guard `SnapshotStore::prune` already applies to live copy-on-write lowers: each
53+
overlay box records the cache key it holds in a `<box_dir>/.rootfs-cache-key`
54+
marker (removed with the box dir), and `prune` skips any still-referenced key.
55+
Found via a concurrent-pipeline chaos test driven through a3s-code; root-caused
56+
and verified on a real `/dev/kvm` host (the concurrency scenario went from ~50%
57+
failure to reliably green).
58+
4459
## [2.6.0] — 2026-06-26
4560

4661
### Added

src/runtime/src/cache/rootfs_cache.rs

Lines changed: 66 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -190,11 +190,27 @@ impl RootfsCache {
190190
Ok(())
191191
}
192192

193-
/// Prune the cache to stay within the given entry count limit.
193+
/// Prune the cache to stay within the given entry count / byte limit.
194194
///
195-
/// Evicts least-recently-accessed entries first.
196-
/// Returns the number of entries evicted.
195+
/// Evicts least-recently-accessed entries first. Returns the number evicted.
197196
pub fn prune(&self, max_entries: usize, max_bytes: u64) -> Result<usize> {
197+
self.prune_protecting(max_entries, max_bytes, &std::collections::HashSet::new())
198+
}
199+
200+
/// Like [`RootfsCache::prune`], but never evicts an entry whose key is in
201+
/// `protected`. Such an entry is currently serving as a box's overlayfs
202+
/// **lowerdir**, and `remove_dir_all`-ing it out from under a concurrent box's
203+
/// `mount(2)` makes the mount fail with ENOENT ("No such file or directory").
204+
/// This is the same in-use guard [`crate::SnapshotStore::prune`] applies to
205+
/// live copy-on-write lowers — without it, two pipelines built from the same
206+
/// image (one cache-hit overlay box, one cache-miss box that prunes after its
207+
/// put) can race and corrupt each other.
208+
pub fn prune_protecting(
209+
&self,
210+
max_entries: usize,
211+
max_bytes: u64,
212+
protected: &std::collections::HashSet<String>,
213+
) -> Result<usize> {
198214
let mut entries = self.list_entries()?;
199215

200216
if entries.len() <= max_entries {
@@ -215,6 +231,11 @@ impl RootfsCache {
215231
if current_count <= max_entries && current_size <= max_bytes {
216232
break;
217233
}
234+
// Never evict an entry in use as a live overlay lower — deleting the
235+
// lowerdir under a concurrent box's mount(2) is the bug this guards.
236+
if protected.contains(&entry.key) {
237+
continue;
238+
}
218239
self.invalidate(&entry.key)?;
219240
current_count -= 1;
220241
current_size = current_size.saturating_sub(entry.size_bytes);
@@ -495,6 +516,48 @@ mod tests {
495516
assert_eq!(cache.entry_count().unwrap(), 1);
496517
}
497518

519+
#[test]
520+
fn prune_protecting_never_evicts_in_use_key() {
521+
let tmp = TempDir::new().unwrap();
522+
let cache = RootfsCache::new(tmp.path()).unwrap();
523+
for i in 0..4 {
524+
let src = tmp.path().join(format!("s{i}"));
525+
create_test_rootfs(&src, &[("f", "x")]);
526+
cache.put(&format!("k{i}"), &src, &format!("e{i}")).unwrap();
527+
std::thread::sleep(std::time::Duration::from_millis(10));
528+
}
529+
// k0 is the OLDEST (normally evicted first) but is in use as an overlay lower.
530+
let mut protected = std::collections::HashSet::new();
531+
protected.insert("k0".to_string());
532+
// keep=2 over 4 entries evicts two; the protected k0 is never one of them.
533+
// (last_accessed is second-resolution, so WHICH two unprotected entries go
534+
// is not asserted — only that the in-use lower survives.)
535+
let evicted = cache.prune_protecting(2, u64::MAX, &protected).unwrap();
536+
assert_eq!(evicted, 2, "two unprotected entries evicted to meet keep=2");
537+
assert!(
538+
cache.get("k0").unwrap().is_some(),
539+
"the in-use (protected) lower must survive prune"
540+
);
541+
assert_eq!(cache.entry_count().unwrap(), 2, "k0 + one unprotected remain");
542+
}
543+
544+
#[test]
545+
fn prune_protecting_keeps_all_when_all_in_use() {
546+
let tmp = TempDir::new().unwrap();
547+
let cache = RootfsCache::new(tmp.path()).unwrap();
548+
for i in 0..2 {
549+
let src = tmp.path().join(format!("s{i}"));
550+
create_test_rootfs(&src, &[("f", "x")]);
551+
cache.put(&format!("k{i}"), &src, "e").unwrap();
552+
}
553+
let protected: std::collections::HashSet<String> =
554+
["k0", "k1"].iter().map(|s| s.to_string()).collect();
555+
// Even asked to keep 0, nothing is evicted — every entry is a live lower.
556+
let evicted = cache.prune_protecting(0, 0, &protected).unwrap();
557+
assert_eq!(evicted, 0, "all in-use -> nothing evicted");
558+
assert_eq!(cache.entry_count().unwrap(), 2);
559+
}
560+
498561
#[test]
499562
fn test_rootfs_cache_metadata_persists() {
500563
let tmp = TempDir::new().unwrap();

src/runtime/src/vm/layout.rs

Lines changed: 35 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -156,6 +156,9 @@ impl VmManager {
156156
let cache_key = RootfsCache::compute_key(reference, &[], &[], &[]);
157157
if let Some(cached_path) = self.try_rootfs_cache_path(&cache_key)? {
158158
let rootfs_path = self.rootfs_provider.prepare(&box_dir, &cached_path)?;
159+
// Record that this box holds `cache_key` as its overlay lower, so a
160+
// concurrent box's cache prune won't evict it mid-mount (ENOENT).
161+
self.mark_rootfs_cache_key(&box_dir, &cache_key);
159162
let tee_instance_config = self.generate_tee_config(&box_dir)?;
160163
return Ok(BoxLayout {
161164
rootfs_path,
@@ -204,6 +207,9 @@ impl VmManager {
204207
prom.rootfs_cache_hits.inc();
205208
}
206209
let rootfs_path = self.rootfs_provider.prepare(&box_dir, &cached_path)?;
210+
// Record that this box holds `cache_key` as its overlay lower, so a
211+
// concurrent box's cache prune won't evict it mid-mount (ENOENT).
212+
self.mark_rootfs_cache_key(&box_dir, &cache_key);
207213

208214
if let Ok(guest_init_path) = Self::find_guest_init() {
209215
tracing::info!(
@@ -377,10 +383,14 @@ impl VmManager {
377383
description = %description,
378384
"Stored rootfs in cache"
379385
);
380-
// Prune if needed
381-
if let Err(e) = cache.prune(
386+
// Prune if needed — but never evict a cache entry that is in use as
387+
// a live overlay lower for a concurrent box (deleting the lowerdir
388+
// under its mount(2) is the same-image concurrency bug this guards).
389+
let protected = self.referenced_rootfs_cache_keys();
390+
if let Err(e) = cache.prune_protecting(
382391
self.config.cache.max_rootfs_entries,
383392
self.config.cache.max_cache_bytes,
393+
&protected,
384394
) {
385395
tracing::warn!(error = %e, "Failed to prune rootfs cache");
386396
}
@@ -391,6 +401,29 @@ impl VmManager {
391401
}
392402
}
393403

404+
/// Record which rootfs-cache key this box holds as its overlay lower, in a
405+
/// `<box_dir>/.rootfs-cache-key` marker (mirror of the snapshot store's
406+
/// `.snapshot-lower`). Read back by [`Self::referenced_rootfs_cache_keys`] so
407+
/// the cache prune never evicts a live lower. Best-effort; removed with box_dir.
408+
fn mark_rootfs_cache_key(&self, box_dir: &Path, cache_key: &str) {
409+
let _ = std::fs::write(box_dir.join(".rootfs-cache-key"), cache_key);
410+
}
411+
412+
/// Rootfs-cache keys currently in use as an overlay lower by some live box.
413+
/// Boxes live under `<home>/boxes/<id>/`; a removed box's marker is gone with
414+
/// its dir, so an evictable key is simply one no live box references.
415+
fn referenced_rootfs_cache_keys(&self) -> std::collections::HashSet<String> {
416+
let mut set = std::collections::HashSet::new();
417+
if let Ok(entries) = std::fs::read_dir(self.home_dir.join("boxes")) {
418+
for entry in entries.flatten() {
419+
if let Ok(k) = std::fs::read_to_string(entry.path().join(".rootfs-cache-key")) {
420+
set.insert(k.trim().to_string());
421+
}
422+
}
423+
}
424+
set
425+
}
426+
394427
/// Resolve the cache directory from config or default.
395428
pub(crate) fn resolve_cache_dir(&self) -> PathBuf {
396429
self.config

0 commit comments

Comments
 (0)