cp,mv: preserve extended attributes even for read-only files

* src/copy.c (copy_reg): Temporarily set u+rw on the destination file
to allow GNU/Linux to set xattrs.
* tests/misc/xattr: Test that change.
* NEWS (Bug fixes): Mention it.
Reported by Ernest N. Mamikonyan.

3 files changed, 52 insertions, 19 deletions

diff --git a/NEWS b/NEWS
index 980fb543f..30f314449 100644
--- a/NEWS
+++ b/NEWS
@@ -2,6 +2,12 @@ GNU coreutils NEWS                                    -*- outline -*-
 
 * Noteworthy changes in release ?.? (????-??-??) [?]
 
+** Bug fixes
+
+  cp --preserve=xattr and --archive now preserve extended attributes even
+  when the source file doesn't have write access.
+  [bug introduced in coreutils-7.1]
+
 ** Changes in behavior
 
   id no longer prints SELinux " context=..." when the POSIXLY_CORRECT
diff --git a/src/copy.c b/src/copy.c
index f3ff5a286..ad2060b9c 100644
--- a/src/copy.c
+++ b/src/copy.c
@@ -834,6 +834,24 @@ copy_reg (char const *src_name, char const *dst_name,
         }
     }
 
+  /* To allow copying xattrs on read-only files, temporarily chmod u+rw.
+     This workaround is required as an inode permission check is done
+     by xattr_permission() in fs/xattr.c of the GNU/Linux kernel tree.  */
+  if (x->preserve_xattr)
+    {
+      bool access_changed = true;
+
+      if (!(sb.st_mode & S_IWUSR) && geteuid() != 0)
+        access_changed = fchmod_or_lchmod (dest_desc, dst_name, 0600) == 0;
+
+      if (!copy_attr_by_fd (src_name, source_desc, dst_name, dest_desc, x)
+          && x->require_preserve_xattr)
+        return_val = false;
+
+      if (access_changed)
+        fchmod_or_lchmod (dest_desc, dst_name, dst_mode & ~omitted_permissions);
+    }
+
   if (x->preserve_ownership && ! SAME_OWNER_AND_GROUP (*src_sb, sb))
     {
       switch (set_owner (x, dst_name, dest_desc, src_sb, *new_dst, &sb))
@@ -850,11 +868,6 @@ copy_reg (char const *src_name, char const *dst_name,
 
   set_author (dst_name, dest_desc, src_sb);
 
-  if (x->preserve_xattr && ! copy_attr_by_fd (src_name, source_desc,
-                                              dst_name, dest_desc, x)
-      && x->require_preserve_xattr)
-    return_val = false;
-
   if (x->preserve_mode || x->move_mode)
     {
       if (copy_acl (src_name, source_desc, dst_name, dest_desc, src_mode) != 0
diff --git a/tests/misc/xattr b/tests/misc/xattr
index a27e1f6d9..fcf7ceb79 100755
--- a/tests/misc/xattr
+++ b/tests/misc/xattr
@@ -29,7 +29,7 @@ fi
 
 # Skip this test if cp was built without xattr support:
 touch src dest || framework_failure
-cp --preserve=xattr -n src dest 2>/dev/null \
+cp --preserve=xattr -n src dest \
   || skip_test_ "coreutils built without xattr support"
 
 # this code was taken from test mv/backup-is-src
@@ -46,13 +46,13 @@ xattr_pair="$xattr_name=\"$xattr_value\""
 # create new file and check its xattrs
 touch a || framework_failure
 getfattr -d a >out_a || skip_test_ "failed to get xattr of file"
-grep -F "$xattr_pair" out_a >/dev/null && framework_failure
+grep -F "$xattr_pair" out_a && framework_failure
 
 # try to set user xattr on file
 setfattr -n "$xattr_name" -v "$xattr_value" a >out_a \
   || skip_test_ "failed to set xattr of file"
 getfattr -d a >out_a || skip_test_ "failed to get xattr of file"
-grep -F "$xattr_pair" out_a >/dev/null \
+grep -F "$xattr_pair" out_a \
   || skip_test_ "failed to set xattr of file"
 
 fail=0
@@ -60,36 +60,50 @@ fail=0
 # cp should not preserve xattr by default
 cp a b || fail=1
 getfattr -d b >out_b || skip_test_ "failed to get xattr of file"
-grep -F "$xattr_pair" out_b >/dev/null && fail=1
+grep -F "$xattr_pair" out_b && fail=1
 
 # test if --preserve=xattr option works
 cp --preserve=xattr a b || fail=1
 getfattr -d b >out_b || skip_test_ "failed to get xattr of file"
-grep -F "$xattr_pair" out_b >/dev/null || fail=1
+grep -F "$xattr_pair" out_b || fail=1
 
 #test if --preserve=all option works
 cp --preserve=all a c || fail=1
 getfattr -d c >out_c || skip_test_ "failed to get xattr of file"
-grep -F "$xattr_pair" out_c >/dev/null || fail=1
+grep -F "$xattr_pair" out_c || fail=1
 
 #test if -a option works without any diagnostics
 cp -a a d 2>err && test -s err && fail=1
 getfattr -d d >out_d || skip_test_ "failed to get xattr of file"
-grep -F "$xattr_pair" out_d >/dev/null || fail=1
+grep -F "$xattr_pair" out_d || fail=1
+
+#test if --preserve=xattr works even for files without write access
+chmod a-w a || framework_failure
+rm -f e
+cp --preserve=xattr a e || fail=1
+getfattr -d e >out_e || skip_test_ "failed to get xattr of file"
+grep -F "$xattr_pair" out_e || fail=1
+
+#Ensure that permission bits are preserved, too.
+src_perm=$(stat --format=%a a)
+dst_perm=$(stat --format=%a e)
+test "$dst_perm" = "$src_perm" || fail=1
+
+chmod u+w a || framework_failure
 
 rm b || framework_failure
 
 # install should never preserve xattr
 ginstall a b || fail=1
 getfattr -d b >out_b || skip_test_ "failed to get xattr of file"
-grep -F "$xattr_pair" out_b >/dev/null && fail=1
+grep -F "$xattr_pair" out_b && fail=1
 
 # mv should preserve xattr when renaming within a file system.
 # This is implicitly done by rename () and doesn't need explicit
 # xattr support in mv.
 mv a b || fail=1
 getfattr -d b >out_b || skip_test_ "failed to get xattr of file"
-grep -F "$xattr_pair" out_b >/dev/null || cat >&2 <<EOF
+grep -F "$xattr_pair" out_b || cat >&2 <<EOF
 =================================================================
 $0: WARNING!!!
 rename () does not preserve extended attributes
@@ -99,18 +113,18 @@ EOF
 # try to set user xattr on file on other partition
 test_mv=1
 touch "$b_other" || framework_failure
-setfattr -n "$xattr_name" -v "$xattr_value" "$b_other" >out_a 2>/dev/null \
+setfattr -n "$xattr_name" -v "$xattr_value" "$b_other" >out_a \
   || test_mv=0
-getfattr -d "$b_other" >out_b 2>/dev/null || test_mv=0
-grep -F "$xattr_pair" out_b >/dev/null || test_mv=0
+getfattr -d "$b_other" >out_b || test_mv=0
+grep -F "$xattr_pair" out_b || test_mv=0
 rm -f "$b_other" || framework_failure
 
 if test $test_mv -eq 1; then
   # mv should preserve xattr when copying content from one partition to another
   mv b "$b_other" || fail=1
-  getfattr -d "$b_other" >out_b 2>/dev/null ||
+  getfattr -d "$b_other" >out_b ||
     skip_test_ "failed to get xattr of file"
-  grep -F "$xattr_pair" out_b >/dev/null || fail=1
+  grep -F "$xattr_pair" out_b || fail=1
 else
   cat >&2 <<EOF
 =================================================================