Skip to content

mv: -n (--no-clobber) is racy, unlike GNU mv #7791

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Open
kotauskas opened this issue Apr 19, 2025 · 3 comments · May be fixed by #7832
Open

mv: -n (--no-clobber) is racy, unlike GNU mv #7791

kotauskas opened this issue Apr 19, 2025 · 3 comments · May be fixed by #7832
Labels
U - mv

Comments

@kotauskas
Copy link

kotauskas commented Apr 19, 2025 •
edited
Loading

GNU mv makes its best attempt to use renameat2 with the RENAME_NOREPLACE flag, falling back via gnulib to the accessat + renameat (or via glibc to access + rename on even older kernels) if that isn't possible. uutils mv does not use renameat2, performing a racy check which is essentially equivalent to the access + rename construction. This introduces a low but non-zero risk of data loss when -n is relied upon. (For reference, BusyBox mv has the same deficiency, and, like uutils, does not bother with renameat and accessat, only making use of access and rename instead.)
@jtracey
Copy link
Contributor

jtracey commented Apr 19, 2025 •
edited
Loading

Please don't link to GNU source code, we can't look at it for licensing reasons.

It looks like nix already exposes renameat2, so this should be an easy fix.

Edit for clarity: That is to say, nix exposes a safe wrapper for the libc function, so at least according to the man page, glibc will handle fallback behavior for us. We might want to test what happens without glibc before relying on that though.

@kotauskas
Copy link
Author

glibc will handle fallback behavior for us

There's fallback for renameat → rename, but not the actually important part:

RENAME_NOREPLACE requires support from the underlying filesystem.
[…]
The following additional errors can occur for renameat2():
EINVAL The filesystem does not support one of the flags in flags.

libcs (including glibc) will simply forward this EINVAL. What uutils mv should have is a wrapper around renameat2 that tries real renameat2(RENAME_NOREPLACE) and falls back to accessat + renameat if that EINVALs.

@jtracey
Copy link
Contributor

jtracey commented Apr 19, 2025

Whoops, good catch. Still should be fairly simple, but not as simple as I was thinking.

@jtracey jtracey linked a pull request Apr 24, 2025 that will close this issue
Draft
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
U - mv
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

mv: use RENAME_NOREPLACE when we can jtracey/coreutils
3 participants
@cakebaker @jtracey @kotauskas