mirror of
https://github.com/openai/codex.git
synced 2026-10-28 18:58:46 +00:00
Handle Btrfs device mismatches when masking daemon sockets (#47968)
## Why Btrfs subvolumes can report an `st_dev` that differs from the device in `/proc/self/mountinfo`, causing daemon socket mount validation to reject valid mounts. ## What changed Allow the mismatch only when the opened directory and its uniquely identified mount are both verified as Btrfs. Discover socket directory aliases using both device identities while retaining checks that reject unsafe directory and socket bind mounts. ## Testing Add regression tests for aliases under both device numbers, unverified or inconsistent filesystems, unavailable or missing mount IDs, duplicate mount IDs, and unsafe bind mounts. GitOrigin-RevId: 921d6465b7a97022573a42412606a26560cc086a
This commit is contained in:
@@ -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<BTreeSet<PathBuf>> {
|
||||
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::<io::Result<Vec<_>>>()?;
|
||||
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);
|
||||
|
||||
@@ -10,10 +10,91 @@ fn check_mounts(
|
||||
mountinfo: &[u8],
|
||||
) -> io::Result<BTreeSet<PathBuf>> {
|
||||
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<BTreeSet<PathBuf>> {
|
||||
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"),
|
||||
|
||||
Reference in New Issue
Block a user