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

[Mike Crowe]

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