From 00abe3015b10ca47404293d2f649e1a5b69e84c3 Mon Sep 17 00:00:00 2001 From: Pedro Henrique Penna Date: Wed, 28 Oct 2026 17:45:04 -0700 Subject: [PATCH 1/3] nvx-rs: tell lock files apart by their 128-bit file IDs on Windows Once it holds a lock, the sandbox store keeps it only if the lock file's path still names the locked file, as the template store does, and a holder that leaves a directory without its caller's configuration record removes the lock file only if the path names the held one. These checks compared the identities of host::platform: on Windows, the volume serial number and the 64-bit file index of GetFileInformationByHandle, which ReFS does not keep unique. ReFS identifies a file by the 128 bits of FILE_ID_INFO, the ID of its directory and its ID within that directory, which it packs into the 64-bit index only while they fit; MS-FSCC gives every file without a unique 64-bit ID the same index, 0xFFFFFFFFFFFFFFFF. Two lock files of one volume could then pass for one file, so that a transition could keep the lock of a replaced lock file while another locked its replacement, and a holder could remove a lock file that another transition had created. The checks now compare the identity that a file's FileSeal holds, as the store's directory identities already do: the device and inode on Unix, and on Windows the volume serial number and the 128-bit file ID of FILE_ID_INFO, or those of BY_HANDLE_FILE_INFORMATION on a volume that has no file ID information. store::regular::identity_of identifies the held lock file, and the new identity_at the file that the path names, following links. On Windows, identity_at opens the file without access rights, as platform::file_identity does, so that the open succeeds while another process holds the file without sharing it, and fails for a lock file whose deletion is pending, which the lock then takes for a replaced one. The checks compare two files at one moment, so neither a record nor a public signature changes. The test of a lock file replaced while a waiter waits compares these identities. Part of #923. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- nvx-rs/src/host/store/lifecycle.rs | 5 ++-- nvx-rs/src/host/store/mod.rs | 5 ++-- nvx-rs/src/host/store/regular.rs | 38 ++++++++++++++++++++++++++++-- nvx-rs/src/host/store/tests.rs | 7 +++--- nvx-rs/src/host/template/store.rs | 5 ++-- 5 files changed, 49 insertions(+), 11 deletions(-) diff --git a/nvx-rs/src/host/store/lifecycle.rs b/nvx-rs/src/host/store/lifecycle.rs index 0310337b3..be0faa414 100644 --- a/nvx-rs/src/host/store/lifecycle.rs +++ b/nvx-rs/src/host/store/lifecycle.rs @@ -10,6 +10,7 @@ use std::path::{Path, PathBuf}; use std::time::Instant; use super::caller::{is_report_staging, is_store_temporary}; +use super::regular; use super::{ CLONE_BINDING_NAME, CallerFiles, FILE_NAMES, JOURNAL_NAME, LOCK_NAME, LockedStore, RUNTIME_NAME, Races, SandboxStore, Taken, absolute, pause, transition_in_progress, @@ -246,8 +247,8 @@ impl LockedStore { return; } let path = self.path(LOCK_NAME); - let held = platform::opened_file_identity(&self.lock); - if matches!((held, platform::file_identity(&path)), (Ok(held), Ok(current)) if held == current) + let held = regular::identity_of(&self.lock); + if matches!((held, regular::identity_at(&path)), (Ok(held), Ok(current)) if held == current) { let _ = fs::remove_file(&path); } diff --git a/nvx-rs/src/host/store/mod.rs b/nvx-rs/src/host/store/mod.rs index 0bef346a1..4d97664fd 100644 --- a/nvx-rs/src/host/store/mod.rs +++ b/nvx-rs/src/host/store/mod.rs @@ -250,6 +250,7 @@ pub use self::lifecycle::{Lingering, RemovalError}; pub use self::record::{ LaunchId, LaunchJournal, LaunchState, LogClaim, MAX_ENDPOINT_LEN, RuntimeRecord, }; +use self::regular::{identity_at, identity_of}; use super::filesystem::FileIdentity; use super::platform; use super::process::{BootId, Process, ProcessRef}; @@ -595,8 +596,8 @@ impl SandboxStore { } }, } - let locked = platform::opened_file_identity(&file).map_err(failed)?; - match platform::file_identity(&path) { + let locked = identity_of(&file).map_err(failed)?; + match identity_at(&path) { Ok(current) if current == locked => Ok(Taken::Locked(file)), Ok(_) => Ok(Taken::Replaced), Err(error) if error.kind() == io::ErrorKind::NotFound => Ok(Taken::Replaced), diff --git a/nvx-rs/src/host/store/regular.rs b/nvx-rs/src/host/store/regular.rs index 771c18ab0..e4bbd96c0 100644 --- a/nvx-rs/src/host/store/regular.rs +++ b/nvx-rs/src/host/store/regular.rs @@ -1,6 +1,7 @@ //! The files that the template modules read, and that a sandbox's store reads as its clone -//! binding: regular files that are not links, which they read within a bound, and the identity of -//! a file that a template module holds open. +//! binding: regular files that are not links, which they read within a bound, and the identities +//! by which the template modules and the stores tell files apart: of a file that one holds open, +//! and of the file that a path names. use std::fs::{File, OpenOptions}; use std::io::{self, Read}; @@ -90,3 +91,36 @@ pub(crate) fn identity_of(file: &File) -> io::Result<(u64, u128)> { let seal = FileSeal::of(file)?; Ok((seal.device(), seal.file_id())) } + +/// Returns the identity of the file or directory that `path` names, following links, as +/// [`identity_of`] gives it for the object open. +/// +/// On Windows, the object is opened without access rights, as [`platform::file_identity`] opens +/// it, which succeeds even while another process holds it open without sharing it. A file whose +/// deletion is pending, which a file system without POSIX deletion keeps under its name until its +/// last handle closes, refuses even that open, with a denied access. +pub(crate) fn identity_at(path: &Path) -> io::Result<(u64, u128)> { + #[cfg(unix)] + { + use std::os::unix::fs::MetadataExt; + + let metadata = std::fs::metadata(path)?; + Ok((metadata.dev(), u128::from(metadata.ino()))) + } + #[cfg(windows)] + { + use std::os::windows::fs::OpenOptionsExt; + + /// `FILE_FLAG_BACKUP_SEMANTICS`, which opens a directory too. + const BACKUP_SEMANTICS: u32 = 0x0200_0000; + let file = OpenOptions::new() + .access_mode(0) + .custom_flags(BACKUP_SEMANTICS) + .open(path)?; + identity_of(&file) + } + #[cfg(not(any(unix, windows)))] + { + platform::file_identity(path).map(|(device, index)| (device, u128::from(index))) + } +} diff --git a/nvx-rs/src/host/store/tests.rs b/nvx-rs/src/host/store/tests.rs index 44300b42d..f3f9c0a1e 100644 --- a/nvx-rs/src/host/store/tests.rs +++ b/nvx-rs/src/host/store/tests.rs @@ -322,9 +322,10 @@ fn a_lock_file_replaced_while_waiting_locks_nothing() { let store = store.clone(); move || { let locked = store.lock(None).unwrap(); - // The waiter holds the lock of the file that the path names now. - let current = platform::file_identity(&locked.path(LOCK_NAME)).unwrap(); - let held = platform::opened_file_identity(&locked.lock).unwrap(); + // The waiter holds the lock of the file that the path names now, by the identities + // that the lock compares. + let current = identity_at(&locked.path(LOCK_NAME)).unwrap(); + let held = identity_of(&locked.lock).unwrap(); assert_eq!(current, held); } }); diff --git a/nvx-rs/src/host/template/store.rs b/nvx-rs/src/host/template/store.rs index 8dd9cf4dd..af8aa6302 100644 --- a/nvx-rs/src/host/template/store.rs +++ b/nvx-rs/src/host/template/store.rs @@ -25,6 +25,7 @@ use crate::host::platform::{self, parse_hex_id}; use crate::host::process::BootId; use crate::host::store::files::{open_lock, write_private}; use crate::host::store::lease::{ID_LEN, hex, read_private, read_record, store_error}; +use crate::host::store::regular::{identity_at, identity_of}; pub use self::capture::{CAPTURE_RECORD_NAME, CAPTURES_NAME, Capture, Created, TRANSACTION_NAME}; pub use self::leases::{Acquired, LEASES_NAME, Retirement, TOMBSTONES_NAME}; @@ -878,8 +879,8 @@ fn take_lock(dir: &Path, deadline: Option) -> Result { } }, } - let locked = platform::opened_file_identity(&file).map_err(failed)?; - match platform::file_identity(&path) { + let locked = identity_of(&file).map_err(failed)?; + match identity_at(&path) { Ok(current) if current == locked => return Ok(file), Ok(_) => {} Err(error) if error.kind() == io::ErrorKind::NotFound => {} From 4fc064c0dfe0e04c4eb95873ee61f5937d512e90 Mon Sep 17 00:00:00 2001 From: Pedro Henrique Penna Date: Wed, 28 Oct 2026 17:45:28 -0700 Subject: [PATCH 2/3] nvx-rs: check a removal's lock file by its 128-bit file ID on Windows A removal unbinds a clone's sandbox through one handle of the directory that the sandbox's path names, and refuses unless that directory's lock file is the one whose lock the removal holds, so that a link in the path that changes once the lock is taken cannot redirect the unbinding to another sandbox, whose clone may still need its binding. The check compared the identities of host::platform, which on Windows were the volume serial number and the 64-bit file index that ReFS does not keep unique, so a collision between the lock files of two sandboxes of one ReFS volume reopened that race. platform::Directory::entry_identity now returns the identity that the entry's FileSeal holds: the device and inode on Linux, and on Windows the volume serial number and the 128-bit file ID of FILE_ID_INFO, or those of BY_HANDLE_FILE_INFORMATION on a volume that has no file ID information. The removal compares it with the identity of the held lock file that store::regular::identity_of gives. The entry is still opened through the directory's handle, without following a link or other reparse point, and the checks for the launch journal and the runtime record still only look for their entries. Directory::entry_identity has no other caller, so no record or public signature changes. The test that a removal makes the binding's absence durable compares the lock file of the directory that it syncs by identity_at. Part of #923. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- nvx-rs/src/host/platform/linux.rs | 8 ++++---- nvx-rs/src/host/platform/other.rs | 2 +- nvx-rs/src/host/platform/windows.rs | 16 +++++++++++----- nvx-rs/src/host/store/lifecycle.rs | 2 +- nvx-rs/src/host/store/lifecycle/tests.rs | 2 +- 5 files changed, 18 insertions(+), 12 deletions(-) diff --git a/nvx-rs/src/host/platform/linux.rs b/nvx-rs/src/host/platform/linux.rs index c26a28516..8b9b94d01 100644 --- a/nvx-rs/src/host/platform/linux.rs +++ b/nvx-rs/src/host/platform/linux.rs @@ -655,14 +655,14 @@ impl Directory { .map(Self) } - /// Returns the device and inode of its entry `name`, not following a link, as - /// [`opened_file_identity`] gives them, or `None` if it has no such entry. - pub(crate) fn entry_identity(&self, name: &str) -> io::Result> { + /// Returns the device and inode of its entry `name`, not following a link, as the entry's + /// [`FileSeal`](crate::host::seal::FileSeal) holds them, or `None` if it has no such entry. + pub(crate) fn entry_identity(&self, name: &str) -> io::Result> { use nix::fcntl::AtFlags; use nix::sys::stat::fstatat; match fstatat(&self.0, name, AtFlags::AT_SYMLINK_NOFOLLOW) { - Ok(status) => Ok(Some((status.st_dev, status.st_ino))), + Ok(status) => Ok(Some((status.st_dev, u128::from(status.st_ino)))), Err(Errno::ENOENT) => Ok(None), Err(errno) => Err(errno.into()), } diff --git a/nvx-rs/src/host/platform/other.rs b/nvx-rs/src/host/platform/other.rs index 57af35f57..761abbcca 100644 --- a/nvx-rs/src/host/platform/other.rs +++ b/nvx-rs/src/host/platform/other.rs @@ -108,7 +108,7 @@ impl Directory { } /// Never called: no directory is open. - pub(crate) fn entry_identity(&self, _name: &str) -> io::Result> { + pub(crate) fn entry_identity(&self, _name: &str) -> io::Result> { match *self {} } diff --git a/nvx-rs/src/host/platform/windows.rs b/nvx-rs/src/host/platform/windows.rs index 287f4a566..15fa10b51 100644 --- a/nvx-rs/src/host/platform/windows.rs +++ b/nvx-rs/src/host/platform/windows.rs @@ -69,6 +69,7 @@ use windows_sys::Win32::System::WindowsProgramming::QueryInterruptTimePrecise; use super::Transport; use super::inherit::Inherited; +use crate::host::seal::FileSeal; use crate::hypervisor::Hypervisor; /// Whether this host can run NVX microVMs. @@ -1407,10 +1408,12 @@ impl Directory { .map(Self) } - /// Returns the volume serial number and file index of its entry `name`, not following a link - /// or other reparse point, as [`opened_file_identity`] gives them, or `None` if it has no such - /// entry. - pub(crate) fn entry_identity(&self, name: &str) -> io::Result> { + /// Returns the identity of its entry `name`, not following a link or other reparse point, as + /// the entry's [`FileSeal`] holds it, or `None` if it has no such entry: the volume serial + /// number and the 128-bit file ID of `FILE_ID_INFO`, which tell the files of ReFS volumes + /// apart where their 64-bit file indexes may not, or those of `BY_HANDLE_FILE_INFORMATION` on + /// a volume that has no file ID information. + pub(crate) fn entry_identity(&self, name: &str) -> io::Result> { use windows_sys::Wdk::Storage::FileSystem::{ FILE_OPEN_REPARSE_POINT, FILE_SYNCHRONOUS_IO_NONALERT, }; @@ -1422,7 +1425,10 @@ impl Directory { FILE_READ_ATTRIBUTES | SYNCHRONIZE, FILE_OPEN_REPARSE_POINT | FILE_SYNCHRONOUS_IO_NONALERT, ) { - Ok(entry) => handle_identity(raw(&entry)).map(Some), + Ok(entry) => { + let seal = FileSeal::of(&fs::File::from(entry))?; + Ok(Some((seal.device(), seal.file_id()))) + } Err(STATUS_OBJECT_NAME_NOT_FOUND) => Ok(None), Err(status) => Err(status_error(status)), } diff --git a/nvx-rs/src/host/store/lifecycle.rs b/nvx-rs/src/host/store/lifecycle.rs index be0faa414..bbb976d31 100644 --- a/nvx-rs/src/host/store/lifecycle.rs +++ b/nvx-rs/src/host/store/lifecycle.rs @@ -381,7 +381,7 @@ impl LockedStore { move |error: io::Error| RemovalError::Io { path, error } }; let directory = platform::Directory::open(dir).map_err(failed)?; - let held = platform::opened_file_identity(&self.lock).map_err(entry_failed(LOCK_NAME))?; + let held = regular::identity_of(&self.lock).map_err(entry_failed(LOCK_NAME))?; if directory .entry_identity(LOCK_NAME) .map_err(entry_failed(LOCK_NAME))? diff --git a/nvx-rs/src/host/store/lifecycle/tests.rs b/nvx-rs/src/host/store/lifecycle/tests.rs index cdafeaaf7..149a52699 100644 --- a/nvx-rs/src/host/store/lifecycle/tests.rs +++ b/nvx-rs/src/host/store/lifecycle/tests.rs @@ -430,7 +430,7 @@ fn a_removal_makes_the_absence_of_the_clone_binding_durable_before_it_removes_an assert_eq!( synced.into_inner(), [( - Some(platform::file_identity(&dir.join(LOCK_NAME)).unwrap()), + Some(regular::identity_at(&dir.join(LOCK_NAME)).unwrap()), sorted(&[BOOT, CONFIG, LOCK_NAME, LOG_NAME]) )] ); From 03b67cad3cda01de1903e061484c4f57cb681180 Mon Sep 17 00:00:00 2001 From: Pedro Henrique Penna Date: Wed, 28 Oct 2026 17:45:54 -0700 Subject: [PATCH 3/3] nvx-rs: test that the stores identify lock files as their seals do The lock checks compare a lock file's identity through the lock's handle, which store::regular::identity_of reads, through its path, which identity_at reads, and through its directory's entry, which platform::Directory::entry_identity reads. A new test checks that the three agree with the identity that the file's FileSeal holds, and that they tell two files of one directory apart: on Windows by the volume serial number and the 128-bit file ID of FILE_ID_INFO, which tell the files of ReFS volumes apart where their 64-bit file indexes may not. The test also fails on NTFS should one of these identities return to the platform's 64-bit one there: NTFS's 128-bit file ID is its 64-bit file index, but FILE_ID_INFO holds the 64-bit volume serial number, of which BY_HANDLE_FILE_INFORMATION holds only the low 32 bits. Part of #923. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- nvx-rs/src/host/store/lifecycle/tests.rs | 29 ++++++++++++++++++++++++ 1 file changed, 29 insertions(+) diff --git a/nvx-rs/src/host/store/lifecycle/tests.rs b/nvx-rs/src/host/store/lifecycle/tests.rs index 149a52699..532ea2f4c 100644 --- a/nvx-rs/src/host/store/lifecycle/tests.rs +++ b/nvx-rs/src/host/store/lifecycle/tests.rs @@ -728,6 +728,35 @@ fn directories_are_identified_as_their_seals_identify_them() { assert_eq!(hold_directory(directory.path()).unwrap().0, expected); } +#[cfg(any(target_os = "linux", windows))] +#[test] +fn lock_files_are_identified_as_their_seals_identify_them() { + use crate::host::seal::{FileSeal, open_metadata_only}; + + // The lock, the path, and the directory's entry identify a lock file alike, and two files of + // one directory apart: on Windows, by the 128-bit file ID of `FILE_ID_INFO`, which tells the + // files of ReFS volumes apart where their 64-bit file indexes may not. + let (_directory, dir) = sandbox(); + let other = "other.lock"; + fs::write(dir.join(other), b"").unwrap(); + let locked = SandboxStore::lock_existing(&dir, &files(), None).unwrap(); + let directory = platform::Directory::open(&dir).unwrap(); + let identities = [LOCK_NAME, other].map(|name| { + let path = dir.join(name); + let seal = FileSeal::of(&open_metadata_only(&path).unwrap()).unwrap(); + let expected = (seal.device(), seal.file_id()); + assert_eq!(regular::identity_at(&path).unwrap(), expected, "{name}"); + assert_eq!( + directory.entry_identity(name).unwrap(), + Some(expected), + "{name}" + ); + expected + }); + assert_ne!(identities[0], identities[1]); + assert_eq!(regular::identity_of(&locked.lock).unwrap(), identities[0]); +} + #[test] fn a_removal_needs_the_configuration_record() { let (_directory, dir) = sandbox();