mv,cp: preserve symlink xattrs when copying across file systems

* src/copy.c (copy_internal): Include the copy_attr() call for symlinks.
This should not dereference symlinks, since llistxattr() is used
in attr_copy_file() in libattr, and so should copy all but the filtered
extended attributes.  Note we don't just move the copy_attr() call
before the set_owner() call, as that would break capabilities
for non symlinks.
* tests/cp/cp-mv-enotsup-xattr.sh: Add a test case.
* NEWS: Mention the bug fix.
Fixes http://bugs.gnu.org/16131

diff --git a/NEWS b/NEWS
index 50303f99ae2b7b039278bc0b81d64f6845924636..f7b511273e1e56d1c18abc79e56e445e4e7c2d9a 100644 (file)
--- a/NEWS
+++ b/NEWS
@@ -17,6 +17,9 @@ GNU coreutils NEWS                                    -*- outline -*-
   when reading the SELinux context for a file.
   [bug introduced in coreutils-8.22]
 
+  cp -a and mv now preserve xattrs of symlinks copied across file systems.
+  [bug introduced with extended attribute preservation feature in coreutils-7.1]
+
   date could crash or go into an infinite loop when parsing a malformed TZ="".
   [bug introduced with the --date='TZ="" ..' parsing feature in coreutils-5.3.0]
 

diff --git a/src/copy.c b/src/copy.c
index d471a77a047d5d8166dd5b6951da526b2b6f3855..eee918a813b03e22e88a0b63b67720aad9e5eaf8 100644 (file)
--- a/src/copy.c
+++ b/src/copy.c
@@ -2677,12 +2677,8 @@ copy_internal (char const *src_name, char const *dst_name,
         }
     }
 
-  /* The operations beyond this point may dereference a symlink.  */
-  if (dest_is_symlink)
-    return delayed_ok;
-
   /* Avoid calling chown if we know it's not necessary.  */
-  if (x->preserve_ownership
+  if (!dest_is_symlink && x->preserve_ownership
       && (new_dst || !SAME_OWNER_AND_GROUP (src_sb, dst_sb)))
     {
       switch (set_owner (x, dst_name, -1, &src_sb, new_dst, &dst_sb))
@@ -2696,12 +2692,17 @@ copy_internal (char const *src_name, char const *dst_name,
         }
     }
 
-  set_author (dst_name, -1, &src_sb);
-
+  /* Set xattrs after ownership as changing owners will clear capabilities.  */
   if (x->preserve_xattr && ! copy_attr (src_name, -1, dst_name, -1, x)
       && x->require_preserve_xattr)
     return false;
 
+  /* The operations beyond this point may dereference a symlink.  */
+  if (dest_is_symlink)
+    return delayed_ok;
+
+  set_author (dst_name, -1, &src_sb);
+
   if (x->preserve_mode || x->move_mode)
     {
       if (copy_acl (src_name, -1, dst_name, -1, src_mode) != 0

diff --git a/tests/cp/cp-mv-enotsup-xattr.sh b/tests/cp/cp-mv-enotsup-xattr.sh
index f8cdd455128fd5a48003d6b662ba1dfa1e335bce..11809892a0b6bb0c09fa0b68831d0676de024e7a 100755 (executable)
--- a/tests/cp/cp-mv-enotsup-xattr.sh
+++ b/tests/cp/cp-mv-enotsup-xattr.sh
@@ -106,4 +106,24 @@ mv xattr/a noxattr/ 2>err || fail=1
 test -s noxattr/a         || fail=1  # destination file must not be empty
 test -s err               && fail=1  # there must be no stderr output
 
+# This should pass and copy xattrs of the symlink
+# since the xattrs used here are not in the 'user.' namespace.
+# Up to and including coreutils-8.22 xattrs of symlinks
+# were not copied across file systems.
+ln -s 'foo' xattr/symlink || framework_failure_
+# Note 'user.' namespace is only supported on regular files/dirs
+# so use the 'trusted.' namespace here
+txattr='trusted.overlay.whiteout'
+if setfattr -hn "$txattr" -v y xattr/symlink; then
+  # Note only root can read the 'trusted.' namespace
+  if getfattr -h -m- -d xattr/symlink | grep -F "$txattr"; then
+    mv xattr/symlink noxattr/ || fail=1
+    getfattr -h -m- -d noxattr/symlink | grep -F "$txattr" || fail=1
+  else
+    echo "failed to get '$txattr' xattr. skipping symlink check" >&2
+  fi
+else
+  echo "failed to set '$txattr' xattr. skipping symlink check" >&2
+fi
+
 Exit $fail