Skip to content

mv: fix the remaining TOCTOU#12170

Open
sylvestre wants to merge 4 commits intouutils:mainfrom
sylvestre:toctou-mv
Open

mv: fix the remaining TOCTOU#12170
sylvestre wants to merge 4 commits intouutils:mainfrom
sylvestre:toctou-mv

Conversation

@sylvestre
Copy link
Copy Markdown
Contributor

No description provided.

@github-actions
Copy link
Copy Markdown

github-actions Bot commented May 6, 2026

GNU testsuite comparison:

Skip an intermittent issue tests/pr/bounded-memory (fails in this run but passes in the 'main' branch)
Congrats! The gnu test tests/mv/meta-to-xpart is no longer failing!
Note: The gnu test tests/tail/tail-n0f is now being skipped but was previously passing.

@sylvestre sylvestre changed the title Toctou mv mv: fix the remaining TOCTOU May 6, 2026
@sylvestre sylvestre force-pushed the toctou-mv branch 2 times, most recently from eb476d8 to 5104ad4 Compare May 6, 2026 23:08
sylvestre added 4 commits May 7, 2026 08:42
uutils#10010)

EXDEV fallback now catches AlreadyExists and replaces the destination
via temp-name + fs::rename. Matches GNU mv.
Add fsxattr::copy_xattrs_fd; mv's EXDEV fallback opens src/dst with
O_NOFOLLOW via safe_copy and copies content + xattrs on the live fds,
pinning both inodes against a concurrent renamer.
Rely on fs::copy's O_TRUNC to overwrite an existing regular file in
place, like GNU mv. The symlink-destination unlink is kept.
Restore strict ENOTSUP/EOPNOTSUPP propagation on copy_xattrs and
copy_xattrs_fd; add copy_xattrs_ignore_unsupported and
copy_xattrs_fd_ignore_unsupported wrappers for best-effort callers.
mv uses the wrappers; cp --preserve=xattr exits 1 on ENOTSUP again.
@sylvestre sylvestre requested review from RenjiSann and cakebaker May 7, 2026 06:53
@sylvestre sylvestre marked this pull request as ready for review May 7, 2026 06:53
Comment thread tests/by-util/test_mv.rs
Comment on lines +2640 to +2641
/// and producing no diagnostic), so we report "unable to remove target"
/// instead.
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The we report "unable to remove target" instead part doesn't match the current behavior of the implementation as there is no such message shown (and this test doesn't check for the message).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants