GNU bug report logs - #29961
[PATCH] mv: document the missing atomicity of 'mv -n'

Previous Next

Full log

Message #20 received at 29961 <at> debbugs.gnu.org (full text, mbox):

From: Kamil Dudka <kdudka <at> redhat.com>
To: Pádraig Brady <P <at> draigBrady.com>
Cc: 29961 <at> debbugs.gnu.org
Subject: [PATCH] mv -n: do not overwrite the destination
Date: Thu,  4 Jan 2018 10:00:03 +0100

... if it is created by another process after mv has checked its
non-existence.

* src/copy.c (copy_internal): Use renameat2 (..., RENAME_NOREPLACE)
if called by mv -n.  If it fails with EEXIST in that case, pretend
successful rename as if the existing destination file was detected
by the preceding lstat call.
* NEWS: Mention the fix.
Fixes https://bugs.gnu.org/29961
---
 NEWS       |  3 +++
 src/copy.c | 17 ++++++++++++++++-
 2 files changed, 19 insertions(+), 1 deletion(-)

diff --git a/NEWS b/NEWS
index b89254f7e..e463ce6db 100644
--- a/NEWS
+++ b/NEWS
@@ -18,6 +18,9 @@ GNU coreutils NEWS                                    -*- outline -*-
   df no longer hangs when given a fifo argument.
   [bug introduced in coreutils-7.3]
 
+  mv -n no longer overwrites the destination if it is created by another
+  process after mv has checked its non-existence.
+
   ptx -S no longer infloops for a pattern which returns zero-length matches.
   [the bug dates back to the initial implementation]
 
diff --git a/src/copy.c b/src/copy.c
index 2a804945e..be4e357a8 100644
--- a/src/copy.c
+++ b/src/copy.c
@@ -53,6 +53,7 @@
 #include "ignore-value.h"
 #include "ioblksize.h"
 #include "quote.h"
+#include "renameat2.h"
 #include "root-uid.h"
 #include "same.h"
 #include "savedir.h"
@@ -2319,7 +2320,12 @@ copy_internal (char const *src_name, char const *dst_name,
 
   if (x->move_mode)
     {
-      if (rename (src_name, dst_name) == 0)
+      int flags = 0;
+      if (x->interactive == I_ALWAYS_NO)
+        /* do not replace DST_NAME if it was created since our last check */
+        flags = RENAME_NOREPLACE;
+
+      if (renameat2 (AT_FDCWD, src_name, AT_FDCWD, dst_name, flags) == 0)
         {
           if (x->verbose)
             {
@@ -2351,6 +2357,15 @@ copy_internal (char const *src_name, char const *dst_name,
           return true;
         }
 
+      if ((flags & RENAME_NOREPLACE) && (errno == EEXIST))
+        {
+          /* Pretend the rename succeeded, so the caller (mv)
+             doesn't end up removing the source file.  */
+          if (rename_succeeded)
+            *rename_succeeded = true;
+          return true;
+        }
+
       /* FIXME: someday, consider what to do when moving a directory into
          itself but when source and destination are on different devices.  */
 
-- 
2.13.6

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.