GNU bug report logs - #25680
[PATCH] copy: Avoid race when creating hard link over recently-created file

Previous Next

Full log

View this message in rfc822 format

From: help-debbugs <at> gnu.org (GNU bug Tracking System)
To: Mike Crowe <mac <at> mcrowe.com>
Subject: bug#25680: closed (Re: bug#25680: [PATCH] copy: Avoid race when
 creating hard link over recently-created file)
Date: Sun, 12 Feb 2017 07:21:02 +0000

[Message part 1 (text/plain, inline)]

Your bug report

#25680: [PATCH] copy: Avoid race when creating hard link over recently-created file

which was filed against the coreutils package, has been closed.

The explanation is attached below, along with your original report.
If you require more details, please reply to 25680 <at> debbugs.gnu.org.

-- 
25680: http://debbugs.gnu.org/cgi/bugreport.cgi?bug=25680
GNU Bug Tracking System
Contact help-debbugs <at> gnu.org with problems

[Message part 2 (message/rfc822, inline)]

From: Paul Eggert <eggert <at> cs.ucla.edu>
To: Mike Crowe <mac <at> mcrowe.com>
Cc: 25680-done <at> debbugs.gnu.org
Subject: Re: bug#25680: [PATCH] copy: Avoid race when creating hard link over
 recently-created file
Date: Sat, 11 Feb 2017 23:20:24 -0800

[Message part 3 (text/plain, inline)]

Mike Crowe wrote:
> Do you think that if I added such a limit and diagnostic then the patch
> would be acceptable?

I'd rather fix the underlying problem more systematically. How about the 
attached patch instead? I've installed this on Savannah so you can give it a 
whirl easily. It fixes the problem for me so I'm boldly marking this bug report 
as done; we can unmark it later if it turns out that I'm wrong.

[0001-ln-replace-destination-links-more-atomically.patch (text/x-diff, attachment)]

[Message part 5 (message/rfc822, inline)]

From: Mike Crowe <mac <at> mcrowe.com>
To: bug-coreutils <at> gnu.org
Cc: Mike Crowe <mac <at> mcrowe.com>
Subject: [PATCH] copy: Avoid race when creating hard link over
 recently-created file
Date: Fri, 10 Feb 2017 19:18:08 +0000

Ensure that create_hard_link is permitted to overwrite the destination
file if --force is passed to cp. It is possible for the file to appear
between checking and creating the hard link. This may happen multiple
times, so also loop inside create_hard_link each time it fails.

The race is made _much_ easier to reproduce by inserting a sleep(1) as
the line immediately prior to the call to modified create_hard_link call
in copy_internal.

Steps to reproduce (assuming the aforementioned sleep has been added):

 mkdir a
 date > a/test.txt
 rm -rf b && \
  cp -afl a b & sleep 0.2 && \
  cp -afl a b & sleep 0.4 && \
  cp -afl a b

One of the copies fails with:

 cp: cannot create hard link 'b/a/test.txt' to 'a/test.txt': File exists

I found this happening in the wild without the sleep multiple times in
the deep-dark depths of a build system.

It's not obvious to me that this fix is the correct one, but it does
seem to work. There is at least some risk that the while loop in
create_hard_link might spin forever, but something else would have to be
recreating the file again rather quickly. It might make sense for me to
add a maximum number of retries.

If this patch is acceptable then I believe that it is trivial enough
that it does not require copyright assignment.
---
 src/copy.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/copy.c b/src/copy.c
index e3832c23a..b72020ccf 100644
--- a/src/copy.c
+++ b/src/copy.c
@@ -1789,7 +1789,7 @@ create_hard_link (char const *src_name, char const *dst_name,
 
   /* If the link failed because of an existing destination,
      remove that file and then call link again.  */
-  if (link_failed && replace && errno == EEXIST)
+  while (link_failed && replace && errno == EEXIST)
     {
       if (unlink (dst_name) != 0)
         {
@@ -2617,7 +2617,7 @@ copy_internal (char const *src_name, char const *dst_name,
            && !(! CAN_HARDLINK_SYMLINKS && S_ISLNK (src_mode)
                 && x->dereference == DEREF_NEVER))
     {
-      if (! create_hard_link (src_name, dst_name, false, false, dereference))
+      if (! create_hard_link (src_name, dst_name, x->unlink_dest_after_failed_open, false, dereference))
         goto un_backup;
     }
   else if (S_ISREG (src_mode)
-- 
2.12.0.rc0

Previous Next

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