--- 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
*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
--- 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.
0001-ln-replace-destination-links-more-atomically.patch
Description: Text Data
--- End Message ---