[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

[debbugs-tracker] bug#25680: closed ([PATCH] copy: Avoid race when creat

From: GNU bug Tracking System
Subject: [debbugs-tracker] bug#25680: closed ([PATCH] copy: Avoid race when creating hard link over recently-created file)
Date: Sun, 12 Feb 2017 07:21:02 +0000

Your message dated Sat, 11 Feb 2017 23:20:24 -0800
with message-id <address@hidden>
and subject line Re: bug#25680: [PATCH] copy: Avoid race when creating hard 
link over recently-created file
has caused the debbugs.gnu.org bug report #25680,
regarding [PATCH] copy: Avoid race when creating hard link over 
recently-created file
to be marked as done.

(If you believe you have received this mail in error, please contact

25680: http://debbugs.gnu.org/cgi/bugreport.cgi?bug=25680
GNU Bug Tracking System
Contact address@hidden with problems
--- Begin Message --- 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 
   /* 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)

--- End Message ---
--- Begin Message --- 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 User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.7.0
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.

Attachment: 0001-ln-replace-destination-links-more-atomically.patch
Description: Text Data

--- End Message ---

reply via email to

[Prev in Thread] Current Thread [Next in Thread]