From 5fca1fbaf2e7594496c854f4c3eef60bd3013697 Mon Sep 17 00:00:00 2001 From: =?utf-8?q?Ond=C5=99ej=20Va=C5=A1=C3=ADk?= Date: Thu, 3 Sep 2009 16:10:21 +0200 Subject: [PATCH] cp,mv: do preserve extended attributes even for read-only source files * src/copy.c (copy_reg): Set mode on file descriptor to 0600 for copying extended attributes to prevent failures when source file doesn't have write access rights. Reported by Ernest N. Mamikonyan. * tests/misc/xattr: Test that change. * NEWS (Bug fixes): Mention it. --- NEWS | 4 ++++ src/copy.c | 21 +++++++++++++++++---- tests/misc/xattr | 42 ++++++++++++++++++++++++++++-------------- 3 files changed, 49 insertions(+), 18 deletions(-) diff --git a/NEWS b/NEWS index 59270eb..8c511c6 100644 --- a/NEWS +++ b/NEWS @@ -15,6 +15,10 @@ GNU coreutils NEWS -*- outline -*- cp --preserve=xattr no longer leaks resources on each preservation failure. [bug introduced in coreutils-7.1] + cp --preserve=xattr and --archive now preserves extended attributes even + when the source file doesn't have write access rights + [bug introduced in coreutils-7.1] + dd now returns non-zero status if it encountered a write error while printing a summary to stderr. [bug introduced in coreutils-6.11] diff --git a/src/copy.c b/src/copy.c index e604ec5..9506fb4 100644 --- a/src/copy.c +++ b/src/copy.c @@ -850,10 +850,23 @@ 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; + /* to allow copying extended attributes on read-only files, change + destination file descriptor access rights to 0600 for a while. */ + if (x->preserve_xattr) + { + if (! fchmod_or_lchmod (dest_desc, dst_name, 0600)) + { + if (!x->reduce_diagnostics || x->require_preserve_xattr) + error (0, errno, + _("failed to set writable mode for %s, copying xattrs may fail"), + quote (dst_name)); + } + if (! copy_attr_by_fd (src_name, source_desc, dst_name, dest_desc, x) + && x->require_preserve_xattr) + return_val = false; + fchmod_or_lchmod (dest_desc, dst_name, dst_mode & ~omitted_permissions); + } + if (x->preserve_mode || x->move_mode) { diff --git a/tests/misc/xattr b/tests/misc/xattr index a27e1f6..404085f 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 rights +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 <&2 <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 <