diff --git a/codex-rs/linux-sandbox/src/daemon_mounts.rs b/codex-rs/linux-sandbox/src/daemon_mounts.rs index e0f9d66ea6..1ad026f1b5 100644 --- a/codex-rs/linux-sandbox/src/daemon_mounts.rs +++ b/codex-rs/linux-sandbox/src/daemon_mounts.rs @@ -1,8 +1,10 @@ //! Find host mount aliases that need the privileged socket directory mask. //! Mount roots describe filesystem identity; canonical paths alone miss bind mounts. +//! Btrfs subvolume device numbers can differ from the containing mount's device. use rustix::fs::AtFlags; use rustix::fs::StatxFlags; +use rustix::fs::fstatfs; use rustix::fs::statx; use std::collections::BTreeSet; use std::fs; @@ -13,12 +15,24 @@ use std::os::unix::fs::MetadataExt; use std::path::Path; use std::path::PathBuf; +#[derive(Clone, Copy, PartialEq, Eq)] +enum SocketFilesystem { + Btrfs, + Other, +} + pub(crate) fn daemon_socket_mask_paths( directory: &Path, masked_root: Option<&Path>, ) -> io::Result> { let directory_file = fs::File::open(directory)?; let device = directory_file.metadata()?.dev(); + let filesystem = + if fstatfs(&directory_file).is_ok_and(|stat| stat.f_type == libc::BTRFS_SUPER_MAGIC) { + SocketFilesystem::Btrfs + } else { + SocketFilesystem::Other + }; let mount_id = fs::read_to_string(format!("/proc/self/fdinfo/{}", directory_file.as_raw_fd())) .ok() .and_then(|fdinfo| { @@ -39,6 +53,7 @@ pub(crate) fn daemon_socket_mask_paths( check_mounts( directory, &format!("{}:{}", libc::major(device), libc::minor(device)), + filesystem, mount_id.as_deref(), &fs::read("/proc/self/mountinfo")?, masked_root, @@ -48,6 +63,7 @@ pub(crate) fn daemon_socket_mask_paths( fn check_mounts( directory: &Path, device: &str, + filesystem: SocketFilesystem, mount_id: Option<&str>, mountinfo: &[u8], masked_root: Option<&Path>, @@ -58,30 +74,66 @@ fn check_mounts( .split(|byte| *byte == b'\n') .filter(|line| !line.is_empty()) { - let fields: Vec<_> = line.split(|byte| *byte == b' ').take(5).collect(); - let [id, parent, mount_device, root, destination] = fields.as_slice() else { + let mut fields = line.split(|byte| *byte == b' '); + let prefix: Vec<_> = fields.by_ref().take(5).collect(); + let [id, parent, mount_device, root, destination] = prefix.as_slice() else { return Err(invalid()); }; let destination = mount_path(destination)?; - // Only roots on the socket filesystem can identify aliases. Other - // filesystems can use non-path roots such as nsfs `mnt:[inode]`, but - // their destinations still matter for ancestry and nested-mount checks. - let root = (*mount_device == device.as_bytes()) - .then(|| mount_path(root)) - .transpose()?; - mounts.push((*id, *parent, *mount_device, root, destination)); + let mount_filesystem = fields + .skip_while(|field| *field != b"-") + .nth(1) + .ok_or_else(invalid)?; + mounts.push(( + *id, + *parent, + *mount_device, + *root, + destination, + mount_filesystem, + )); } - let (location, containing_mount) = if let Some(mount_id) = mount_id { + let selected_index = mount_id + .map(|mount_id| { + let mut matching = mounts + .iter() + .enumerate() + .filter(|(_, (id, ..))| *id == mount_id.as_bytes()); + let (index, (_, _, mount_device, _, _, mount_filesystem)) = + matching.next().ok_or_else(invalid)?; + if matching.next().is_some() + || (*mount_device != device.as_bytes() + && (filesystem != SocketFilesystem::Btrfs || *mount_filesystem != b"btrfs")) + { + return Err(invalid()); + } + Ok(index) + }) + .transpose()?; + // Btrfs reports a per-subvolume st_dev, while mountinfo uses the superblock + // device. Only the verified descriptor's mount ID can authorize that mismatch. + // Retain both device identities when looking for aliases. + let mount_device = selected_index + .map(|index| mounts[index].2) + .unwrap_or(device.as_bytes()); + let mounts = mounts + .into_iter() + .map(|(id, parent, candidate_device, root, destination, _)| { + // Only roots on the socket filesystem can identify aliases. Other + // filesystems can use non-path roots such as nsfs `mnt:[inode]`, but + // their destinations still matter for ancestry and nested-mount checks. + let root = (candidate_device == device.as_bytes() || candidate_device == mount_device) + .then(|| mount_path(root)) + .transpose()?; + Ok((id, parent, candidate_device, root, destination)) + }) + .collect::>>()?; + let (location, containing_mount) = if let Some((mount_id, index)) = mount_id.zip(selected_index) + { // fdinfo/statx identifies the opened mount, which may have been covered // by another mount before we read mountinfo. - let selected = mounts - .iter() - .find(|(id, ..)| *id == mount_id.as_bytes()) - .ok_or_else(invalid)?; - let (_, _, mount_device, root, destination) = selected; - if *mount_device != device.as_bytes() { - return Err(invalid()); - } + let selected = &mounts[index]; + let (_, _, _, root, destination) = selected; let root = root.as_ref().ok_or_else(invalid)?; let relative = directory.strip_prefix(destination).map_err(|_| invalid())?; let mut current = Some(selected); diff --git a/codex-rs/linux-sandbox/src/daemon_mounts_tests.rs b/codex-rs/linux-sandbox/src/daemon_mounts_tests.rs index 55fb1f6a4c..c332cad2f5 100644 --- a/codex-rs/linux-sandbox/src/daemon_mounts_tests.rs +++ b/codex-rs/linux-sandbox/src/daemon_mounts_tests.rs @@ -10,10 +10,91 @@ fn check_mounts( mountinfo: &[u8], ) -> io::Result> { super::check_mounts( - directory, device, mount_id, mountinfo, /*masked_root*/ None, + directory, + device, + SocketFilesystem::Other, + mount_id, + mountinfo, + /*masked_root*/ None, ) } +fn check_btrfs_mounts(mount_id: &str, mountinfo: &[u8]) -> io::Result> { + super::check_mounts( + Path::new("/tmp/codex-daemon-1000"), + "0:2", + SocketFilesystem::Btrfs, + Some(mount_id), + mountinfo, + /*masked_root*/ None, + ) +} + +#[test] +fn btrfs_bind_mount_masks_aliases_for_both_device_numbers() { + let mounts = b"1 0 0:1 / / rw - btrfs disk rw\n\ + 2 1 0:1 /var/lib/system-tmp /tmp rw shared:1 - btrfs disk rw\n\ + 3 1 0:1 /var/lib/system-tmp /mount-device-alias rw - btrfs disk rw\n\ + 4 1 0:2 /var/lib/system-tmp /stat-device-alias rw - btrfs disk rw\n\ + 5 1 0:3 mnt:[1234] /run/example.mnt rw - nsfs nsfs rw\n"; + assert_eq!( + check_btrfs_mounts("2", mounts).unwrap(), + BTreeSet::from([ + PathBuf::from("/tmp/codex-daemon-1000"), + PathBuf::from("/var/lib/system-tmp/codex-daemon-1000"), + PathBuf::from("/mount-device-alias/codex-daemon-1000"), + PathBuf::from("/stat-device-alias/codex-daemon-1000"), + ]), + ); +} + +#[test_case(SocketFilesystem::Other, "btrfs", Some("1"); "unverified descriptor filesystem")] +#[test_case(SocketFilesystem::Btrfs, "ext4", Some("1"); "inconsistent mount filesystem")] +#[test_case(SocketFilesystem::Btrfs, "btrfs", None; "unavailable mount id")] +#[test_case(SocketFilesystem::Btrfs, "btrfs", Some("missing"); "missing mount id")] +fn device_mismatch_requires_verified_btrfs_mount( + filesystem: SocketFilesystem, + mount_filesystem: &str, + mount_id: Option<&str>, +) { + let mounts = format!("1 0 0:1 / / rw - {mount_filesystem} disk rw\n"); + assert_eq!( + super::check_mounts( + Path::new("/tmp/codex-daemon-1000"), + "0:2", + filesystem, + mount_id, + mounts.as_bytes(), + /*masked_root*/ None, + ) + .map_err(|error| error.kind()), + Err(io::ErrorKind::Other), + ); +} + +#[test_case("0:1", "/@/tmp/codex-daemon-1000", "/alias"; "mount device directory alias")] +#[test_case("0:2", "/@/tmp/codex-daemon-1000/rpc.sock", "/alias.sock"; "stat device socket alias")] +fn btrfs_device_mismatch_still_rejects_unsafe_mounts(device: &str, root: &str, destination: &str) { + let mounts = format!( + "1 0 0:1 /@ / rw - btrfs disk rw\n\ + 2 1 {device} {root} {destination} rw - btrfs disk rw\n" + ); + assert_eq!( + check_btrfs_mounts("1", mounts.as_bytes()).map_err(|error| error.kind()), + Err(io::ErrorKind::PermissionDenied), + ); +} + +#[test] +fn btrfs_device_mismatch_rejects_duplicate_mount_id() { + let mounts = b"1 0 0:1 /@ / rw - btrfs disk rw\n\ + 1 0 0:1 /@ / rw - btrfs disk rw\n"; + assert_eq!( + check_btrfs_mounts("1", mounts).map_err(|error| error.kind()), + Err(io::ErrorKind::Other), + ); +} + #[test_case("/tmp", "/host-tmp", true; "ancestor alias")] #[test_case("/tmp/codex-daemon-1000", "/alias", false; "directory alias")] #[test_case("/tmp/codex-daemon-1000/rpc.sock", "/alias.sock", false; "socket alias")] @@ -213,11 +294,27 @@ fn masked_wslg_alias_does_not_skip_other_socket_masks() { ]) ); assert_eq!( - super::check_mounts(directory, "0:1", mount_id, mounts.as_bytes(), mask).unwrap(), + super::check_mounts( + directory, + "0:1", + SocketFilesystem::Other, + mount_id, + mounts.as_bytes(), + mask + ) + .unwrap(), BTreeSet::from([directory.to_path_buf()]) ); assert_eq!( - super::check_mounts(directory, "0:1", mount_id, exposed.as_bytes(), mask).unwrap(), + super::check_mounts( + directory, + "0:1", + SocketFilesystem::Other, + mount_id, + exposed.as_bytes(), + mask + ) + .unwrap(), BTreeSet::from([ directory.to_path_buf(), PathBuf::from("/host-tmp/codex-daemon-1000"),