Hello,
I have found a bug in "cp -rfs".
Steps to reproduce:
1. Given "path1" and "path2" are on different devices.
2. $ touch "path1/file"
3. $ cd path2/; ln -s path1/file
4. $ cp --symbolic-link --force --recursive path1/file .
Expected:
The link is overwritten with an exact copy.
Actual result:
cp shows an error:
cp: 'path1/file' and './file' are the same file
This bug was introduced in
http://git.savannah.gnu.org/cgit/coreutils.git/commit/?id=376967889ed7ed561e46ff6d88a66779db62737a
Specifically this hunk:
diff --git a/src/copy.c b/src/copy.c
index e3832c2..9dbd536 100644
--- a/src/copy.c
<http://git.savannah.gnu.org/cgit/coreutils.git/tree/src/copy.c?id=2f69dba5df8caaf9eda658c1808b1379e9949f22>
+++ b/src/copy.c
<http://git.savannah.gnu.org/cgit/coreutils.git/tree/src/copy.c?id=376967889ed7ed561e46ff6d88a66779db62737a>
@@ -46,6 +46,7 @@
#include "file-set.h"
#include "filemode.h"
#include "filenamecat.h"
+#include "force-link.h"
#include "full-write.h"
#include "hash.h"
#include "hash-triple.h"
@@ -1623,11 +1624,13 @@ same_file_ok (char const *src_name, struct stat
const *src_sb,
}
}
- /* It's ok to remove a destination symlink. But that works only when we
- unlink before opening the destination and when the source and destination
- files are on the same partition. */
- if (x->unlink_dest_before_opening
- && S_ISLNK (dst_sb_link->st_mode))
+ /* It's ok to remove a destination symlink. But that works only
+ when creating symbolic links, or when the source and destination
+ are on the same file system and when creating hard links or when
+ unlinking before opening the destination. */
+ if (x->symbolic_link
+ || ((x->hard_link || x->unlink_dest_before_opening)
+ && S_ISLNK (dst_sb_link->st_mode)))
return dst_sb_link->st_dev == src_sb_link->st_dev;
if (x->dereference == DEREF_NEVER)
Two patches that fix the issue are attached.
They are against the current master in
https://github.com/coreutils/coreutils
The changes are also here:
https://github.com/coreutils/coreutils/compare/master...ilya-bobyr:master
Thank you, Illia Bobyr
From 51d42245674dc864ac0dbb2f160a55cd38fc2458 Mon Sep 17 00:00:00 2001
From: Illia Bobyr <[email protected]>
Date: Fri, 4 May 2018 09:41:27 -0700
Subject: [PATCH 2/2] cp: Overwrite symlinks on another device
When target of a copy operation is a symlink, it is not the same file as
the source, so it is safe to overwrite it, regardless of if it is the
same device as the source or a different device.
Previous check only allowed overwrite on the same device and produced an
error when run across different file systems.
---
src/copy.c | 15 ++++++++++-----
1 file changed, 10 insertions(+), 5 deletions(-)
diff --git a/src/copy.c b/src/copy.c
index dbfabf215..b35f9568b 100644
--- a/src/copy.c
+++ b/src/copy.c
@@ -1627,11 +1627,16 @@ same_file_ok (char const *src_name, struct stat const *src_sb,
}
}
- /* It's ok to remove a destination symlink. But that works only
- when creating symbolic links, or when the source and destination
- are on the same file system and creating hard links. */
- if (x->symbolic_link
- || (x->hard_link && S_ISLNK (dst_sb_link->st_mode)))
+ /* It's ok to remove a destination symlink - does not affect the source. */
+ if (x->symbolic_link)
+ return true;
+
+ /* When creating hard links it OK when the source and destination are on the
+ same file system. TODO: The S_ISLNK() check is probably redundant, as we
+ should not be here if the source and the destination are the same file and
+ the destination is not a symbolic link. The case when the source and the
+ destination are the same due to being hard links is checked above. */
+ if (x->hard_link && S_ISLNK (dst_sb_link->st_mode))
return dst_sb_link->st_dev == src_sb_link->st_dev;
if (x->dereference == DEREF_NEVER)
--
2.17.0.441.gb46fe60e1d-goog
From a2b21f29edaa2c3448d6542870279d6217c43508 Mon Sep 17 00:00:00 2001
From: Illia Bobyr <[email protected]>
Date: Thu, 3 May 2018 16:38:40 -0700
Subject: [PATCH 1/2] cp: No dup check for unlink_dest_after_failed_open
A case when unlink_dest_after_failed_open is set and the destination is
a symbolic link is checked in same_file_ok() in lines 1565-1572. It
does not make sense to check again.
---
src/copy.c | 6 ++----
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/src/copy.c b/src/copy.c
index 4998c83e6..dbfabf215 100644
--- a/src/copy.c
+++ b/src/copy.c
@@ -1629,11 +1629,9 @@ same_file_ok (char const *src_name, struct stat const *src_sb,
/* It's ok to remove a destination symlink. But that works only
when creating symbolic links, or when the source and destination
- are on the same file system and when creating hard links or when
- unlinking before opening the destination. */
+ are on the same file system and creating hard links. */
if (x->symbolic_link
- || ((x->hard_link || x->unlink_dest_before_opening)
- && S_ISLNK (dst_sb_link->st_mode)))
+ || (x->hard_link && S_ISLNK (dst_sb_link->st_mode)))
return dst_sb_link->st_dev == src_sb_link->st_dev;
if (x->dereference == DEREF_NEVER)
--
2.17.0.441.gb46fe60e1d-goog
