Skip to content

Commit 1cb9bb8

Browse files
author
Roy Lin
committed
fix(box): single-file bind mount no longer clobbers parent dir
A '-v /host/file:/etc/app.conf' mount destroyed the entire target parent directory (/etc lost hostname/passwd/etc.). The shim correctly converts a file mount to a temp-dir virtio-fs share, but the guest mounted that share *over the parent dir* (/etc), and the two sides decided file-vs-dir with disagreeing heuristics (shim: host_path.is_file(); guest: dot-in-name). - Host (spec.rs) is now authoritative: emits an explicit ':file' flag on BOX_VOL_<i> when the source path stats as a file. - Guest (init) obeys the flag: mounts the share at a private mountpoint, then bind-mounts only the file onto guest_path, preserving the parent. Honors :ro via an MS_REMOUNT pass. Dir mounts unchanged. - Shim: per-mount temp dir keyed by tag so two file mounts sharing a basename don't collide. Verified on KVM: file content present at target; /etc intact (37 entries, hostname present) vs clobbered (1 entry) before; :ro rejects writes and leaves host file unchanged; dir mounts still read+write-back. Known limit: rw single-file writes are guest-isolated (share is a copy), not written back to the host file.
1 parent 493bb07 commit 1cb9bb8

3 files changed

Lines changed: 81 additions & 52 deletions

File tree

src/guest/init/src/main.rs

Lines changed: 66 additions & 50 deletions
Original file line numberDiff line numberDiff line change
@@ -692,71 +692,87 @@ fn mount_user_volumes() -> Result<(), Box<dyn std::error::Error>> {
692692

693693
let tag = parts[0];
694694
let guest_path = parts[1];
695-
let read_only = parts.get(2).map(|&m| m == "ro").unwrap_or(false);
696-
697-
// Check if guest_path is a file (has an extension) or a directory
698-
// virtio-fs can only mount directories, so if guest_path is a file,
699-
// we need to mount at the parent directory instead
700-
let mount_path: &str;
701-
let file_name: Option<&str>;
702-
703-
if guest_path
704-
.rsplit('/')
705-
.next()
706-
.map(|s| s.contains('.'))
707-
.unwrap_or(false)
708-
{
709-
// guest_path looks like a file (has extension)
710-
// Extract parent directory and file name
695+
// Flags after the guest path may appear in any order: "ro", "file".
696+
// The host decides "file" (it can stat the source); the guest obeys.
697+
let read_only = parts[2..].iter().any(|&m| m == "ro");
698+
let is_file = parts[2..].iter().any(|&m| m == "file");
699+
700+
let flags = if read_only {
701+
MsFlags::MS_RDONLY
702+
} else {
703+
MsFlags::empty()
704+
};
705+
706+
if is_file {
707+
// Single-file bind mount. The shim shares a temp DIRECTORY
708+
// containing the file (virtio-fs cannot share a bare file), so
709+
// mount that share at a private location and bind just the file
710+
// onto guest_path. This preserves the target's parent directory
711+
// (e.g. /etc) instead of clobbering it with the share.
712+
let file_name = guest_path.rsplit('/').next().unwrap_or(guest_path);
713+
let private_mp = format!("/run/.a3s-filemounts/{}", index);
714+
std::fs::create_dir_all(&private_mp)?;
715+
mount(
716+
Some(tag),
717+
private_mp.as_str(),
718+
Some("virtiofs"),
719+
MsFlags::empty(),
720+
None::<&str>,
721+
)?;
722+
723+
let src = format!("{}/{}", private_mp, file_name);
724+
if !std::path::Path::new(&src).exists() {
725+
warn!("File mount source {} missing in share {}", src, tag);
726+
}
727+
728+
// Ensure the target parent and an (empty) target file exist so
729+
// the bind has somewhere to land.
711730
if let Some(last_slash) = guest_path.rfind('/') {
712-
mount_path = &guest_path[..last_slash];
713-
file_name = Some(&guest_path[last_slash + 1..]);
714-
} else {
715-
// No slash, just a filename - mount at current directory
716-
mount_path = ".";
717-
file_name = Some(guest_path);
731+
let parent = &guest_path[..last_slash];
732+
if !parent.is_empty() {
733+
std::fs::create_dir_all(parent)?;
734+
}
735+
}
736+
if !std::path::Path::new(guest_path).exists() {
737+
std::fs::File::create(guest_path)?;
738+
}
739+
740+
// Bind the file, then remount read-only if requested (a bind
741+
// mount needs a separate MS_REMOUNT pass to apply MS_RDONLY).
742+
mount(
743+
Some(src.as_str()),
744+
guest_path,
745+
None::<&str>,
746+
MsFlags::MS_BIND,
747+
None::<&str>,
748+
)?;
749+
if read_only {
750+
mount(
751+
None::<&str>,
752+
guest_path,
753+
None::<&str>,
754+
MsFlags::MS_BIND | MsFlags::MS_REMOUNT | MsFlags::MS_RDONLY,
755+
None::<&str>,
756+
)?;
718757
}
719758
info!(
720759
tag = tag,
721760
guest_path = guest_path,
722-
mount_path = mount_path,
723-
file_name = file_name.unwrap_or(""),
724761
read_only = read_only,
725-
"Mounting user volume (file mount detected, will mount parent directory)"
762+
"Mounted file volume (bind; parent directory preserved)"
726763
);
727764
} else {
728-
// guest_path is a directory
729-
mount_path = guest_path;
730-
file_name = None;
765+
// Directory mount: mount the virtio-fs share directly at guest_path.
766+
std::fs::create_dir_all(guest_path)?;
767+
mount(Some(tag), guest_path, Some("virtiofs"), flags, None::<&str>)?;
731768
info!(
732769
tag = tag,
733770
guest_path = guest_path,
734771
read_only = read_only,
735-
"Mounting user volume"
772+
"Mounted user volume"
736773
);
737774
}
738775

739-
// Ensure mount point exists (parent directory for file mounts)
740-
std::fs::create_dir_all(mount_path)?;
741-
742-
let flags = if read_only {
743-
MsFlags::MS_RDONLY
744-
} else {
745-
MsFlags::empty()
746-
};
747-
mount(Some(tag), mount_path, Some("virtiofs"), flags, None::<&str>)?;
748-
749-
// For file mounts, verify the file exists in the mounted directory
750-
if let Some(name) = file_name {
751-
let mounted_file = format!("{}/{}", mount_path, name);
752-
if !std::path::Path::new(&mounted_file).exists() {
753-
warn!(
754-
"Expected file {} after mount but it does not exist",
755-
mounted_file
756-
);
757-
}
758-
}
759-
760776
index += 1;
761777
}
762778
Err(_) => break,

src/runtime/src/vm/spec.rs

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -161,9 +161,19 @@ impl VmManager {
161161
} else {
162162
""
163163
};
164+
// Mark single-file bind mounts so the guest binds the file onto
165+
// guest_path instead of mounting the virtio-fs share over its
166+
// parent directory (which would clobber e.g. /etc). The host is
167+
// authoritative here (it can stat the path); the guest must not
168+
// re-guess from the guest path's shape.
169+
let file_flag = if std::path::Path::new(parts[0]).is_file() {
170+
":file"
171+
} else {
172+
""
173+
};
164174
env.push((
165175
format!("BOX_VOL_{}", i),
166-
format!("vol{}:{}{}", i, guest_path, mode),
176+
format!("vol{}:{}{}{}", i, guest_path, mode, file_flag),
167177
));
168178
}
169179
}

src/shim/src/main.rs

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -455,7 +455,10 @@ unsafe fn configure_and_start_vm(spec: &InstanceSpec) -> Result<()> {
455455

456456
let mount_path: std::path::PathBuf = if host_path.is_file() {
457457
// Create a temporary directory to hold the file
458-
let temp_dir = std::env::temp_dir().join(format!("a3s-fs-mount-{}", spec.box_id));
458+
// Per-mount temp dir (keyed by tag) so two file mounts sharing a
459+
// basename (e.g. two app.conf to different targets) don't collide.
460+
let temp_dir =
461+
std::env::temp_dir().join(format!("a3s-fs-mount-{}-{}", spec.box_id, mount.tag));
459462
let file_name = host_path.file_name().unwrap();
460463
let temp_file_path = temp_dir.join(file_name);
461464

0 commit comments

Comments
 (0)