Repository navigation
nvx-rs: compare lock files by their 128-bit file IDs on Windows - #984
Closed
Pedro Henrique Penna (ppenna) wants to merge 3 commits into
Closed
Pedro Henrique Penna (ppenna) wants to merge 3 commits into
Pedro Henrique Penna (ppenna) wants to merge 3 commits into
Conversation
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>
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>
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>
Contributor
There was a problem hiding this comment.
🟢 Approval recommended
No active review issues remain for the lock-file identity changes supporting ReFS.
No open issues 🎉
What changed in this PR
This PR addresses #923 by using FileSeal identities for internal lock-file comparisons, allowing NVX’s sandbox and template stores to distinguish files on ReFS using 128-bit Windows file IDs. Public APIs and persisted record formats remain unchanged.
Changes:
- Use consistent handle and path identities for lock acquisition, release, and sandbox unbinding.
- Widen directory-entry identities while preserving Unix behavior and the Windows fallback.
- Add identity-consistency coverage and update existing lock tests.
| File | Description |
|---|---|
nvx-rs/src/host/template/store.rs |
Use seal-based identities when validating template locks. |
nvx-rs/src/host/store/tests.rs |
Update replacement-lock assertions to use the new identities. |
nvx-rs/src/host/store/regular.rs |
Add a cross-platform path identity helper. |
nvx-rs/src/host/store/mod.rs |
Use consistent identities when validating sandbox locks. |
nvx-rs/src/host/store/lifecycle/tests.rs |
Test identity consistency and distinguish separate lock files. |
nvx-rs/src/host/store/lifecycle.rs |
Update lock removal and unbinding identity checks. |
nvx-rs/src/host/platform/windows.rs |
Return seal-based directory-entry identities with 128-bit file IDs. |
nvx-rs/src/host/platform/other.rs |
Align the unsupported-platform identity signature. |
nvx-rs/src/host/platform/linux.rs |
Widen the inode return type without changing identity semantics. |
Review effort: Balanced
Contributor
Author
|
Superseded by #949, which landed e9d7531 ("nvx-rs: tell the files of a store apart by their 128-bit IDs on Windows") on Closing. #923 records the validation of |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #923
On Windows, the checks that tell whether two handles or paths reach the same lock file compared the volume serial number and the 64-bit file index of
GetFileInformationByHandle, which is not guaranteed unique on ReFS. They now compare the identity that the file'sFileSealholds, as the store's directory identities already do: the volume serial number and the 128-bit file ID ofFILE_ID_INFO, or those ofBY_HANDLE_FILE_INFORMATIONon a volume without file ID information, and the device and inode on Unix.take_lock, and the release that removes a holder's lock file, identify the held lock file withstore::regular::identity_of, and the file that the path names with the newstore::regular::identity_at, which follows links and on Windows opens the file without access rights, asplatform::file_identitydoes. A lock file whose deletion is pending still refuses that open, and counts as replaced.platform::Directory::entry_identity, whose only caller isLockedStore::unbind, returns the entry'sFileSealidentity, which the removal compares with the held lock file's.lock_files_are_identified_as_their_seals_identify_themchecks that the lock's handle, the path, and the directory's entry identify a lock file as itsFileSealdoes, and tell two files of one directory apart. It also fails on NTFS if one of them returns to the 64-bit identity, whose 32-bit volume serial number differs fromFILE_ID_INFO's 64-bit one. Two existing tests now compare the new identities.The checks compare two files at one moment, so no record and no public signature changes. As the issue scopes it,
FileIdentity, which the launch journal records for its claimed log, and the publicplatform::file_identitykeep their 64-bit indexes.Both sides of each comparison must use the same scheme: with a 128-bit identity on one side and a 64-bit one on the other, every lock file looks replaced, and a lock without a deadline retries forever, which a deliberate mismatch reproduced locally.
How ReFS forms the 64-bit index. A probe on a ReFS 3.14 volume (Windows Server 2025) showed that the 128-bit ID holds the file's directory ID in its high half and the file's ID within that directory in its low half. ReFS packs them into the 64-bit index as the number of hexadecimal digits of the directory ID, then the file's ID, then the directory ID:
0000000000001a92_0000000000000002becomes4000000000021a92. Among 10,000 files, one in each of 5,000 directories and 5,000 in one more, no two indexes collided. When the packing does not fit, MS-FSCC §2.1.9 requires0xFFFFFFFFFFFFFFFFfor every file that has no unique 64-bit ID, and those files then collide.Validation
cargo fmt --all --check; Clippy with-D warningsfor--all-targets --all-features,--lib, and--lib --no-default-features, both natively and with--target x86_64-unknown-linux-gnu;cargo doc --no-deps --all-featureswith-D warnings;cargo test --all-features,cargo test, andcargo test --lib --no-default-features;cargo +1.89 check --all-targets --all-features; andcargo check --all-targets --all-features --target aarch64-apple-darwin. Intests/runtime-adapter: format, Clippy for the protocol-only and host-only configurations and for the guest-only configuration with--target x86_64-unknown-linux-musl, the host-only tests, and the 1.89 check.--all-targets --all-features, and passes thehost::storeandhost::template::storetests on Windows (git rebase -x) and on Linux.nvx-whp-test, Windows Server 2025, a temporary ReFS 3.14 VHDX):cargo test --all-features --lib --testswithTMPandTEMPon the ReFS volume, at the base and at the head: 981 and 982 library tests passed, and every integration test too. A run withTMPon a missing drive confirmed that the test directories followTMP. The head also passes on NTFS on that host.nvx-kvm-test, Azure Linux 3, kernel 6.6):cargo test(all 829 library tests),cargo test --lib --no-default-features,cargo doc, the runtime adapter's protocol-only and host-only tests, and its musl guest build. Incargo test --all-features, allhost::tests pass. Threeguest::tests fail identically at the base commit on that host, as an unprivileged user on its kernel:guest::entry::tests::runtime_binds_need_a_mount_root_unless_there_are_nonewithEOPNOTSUPP, and twoguest::exectests that expectESRCHand getEINVAL.