cp: open file only once (+32.93%) + add tests for mv & cp - #14272
Merged
Conversation
Merging this PR will improve performance by 32.93%
Performance Changes
Tip Curious why performance improved? Comment Comparing 
Footnotes
|
|
Binary size comparison: |
|
GNU testsuite comparison: |
sylvestre
marked this pull request as ready for review
August 30, 2026 17:58
Every regular-file copy on Linux opened the source three times: once in
check_for_data, once in check_sparse_detection, then again in the copy
itself. Open it once and thread the descriptor through the probe and the
copy, so the strategy and the bytes come from the same fd.
before: open("src", O_RDONLY) x3 + open("dst", O_WRONLY|O_CREAT) x1
after: open("src", O_RDONLY) x1 + open("dst", O_WRONLY|O_CREAT) x1
Copying 3000 small files, 50 runs: 199.9ms +/- 14.9 -> 171.7ms +/- 23.6.
The ratio's error bars overlap; the stable signal is system time,
177.3ms -> 146.9ms, with user time unchanged at ~18ms.
New util/check-open-once.sh straces cp and asserts the source path is opened exactly once, across the default, --sparse=always and --sparse=never paths. Verified it fails on the previous code with 'opened the source 3 times', so it guards the invariant rather than restating it. Kept out of check-toctou.sh: no exploit is known for the old window -- every open used the same credentials and flags -- and the security scripts should stay about attacker-exploitable splits. The new header says so and points at the two that do. Traces open(2) as well as openat(2): rustix issues the former on Linux, so an openat-only filter matches nothing and passes vacuously.
mv already holds the invariant: it has its own cross-device fallback rather than cp's copy machinery, so it never had the three-open problem. These pin it in place, since the pending fd-anchored rework of the EXDEV directory path (GHSA-pp2g) touches exactly this code. Three assertions: a same-filesystem move must open the source zero times (it is a rename, not a copy), and the cross-device file and directory fallbacks must open each source exactly once. Each was verified to fail by injecting a redundant File::open at the matching call site. The directory case matches any path ending in the source name opened O_RDONLY, so it keeps working once the traversal opens by bare name under a directory fd. Cross-device needs a second filesystem; /dev/shm supplies it, and the checks skip loudly when it shares a device with TEMP_DIR rather than passing vacuously.
sylvestre
force-pushed
the
cp-open-once
branch
from
August 30, 2026 17:58
84c9bff to
9ddecff
Compare
sylvestre
changed the title
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.
No description provided.