On 12/13/2013 04:43 PM, A. Wan wrote: > I wonder if this is a bug or I just did it the wrong way. I can't seem to > copy extended attributes of symbolic links. Example is to populate the > upperdir of an overlayfs in Linux. > > coreutils 8.21 > libattr 2.4.47 > > I can only use mv to preserve extended attributes of symbolic links, > and only when the destination is in the same filesystem. (It looks like > mv summons copy when destination is in a different filesystem) > > Steps to reproduce: (The same happens in either ext2 or tmpfs) > > --- cut here --- > touch a > ln -sv '(overlay-whiteout)' b > > | ‘b’ -> ‘(overlay-whiteout)’ > > setfattr -hn trusted.overlay.whiteout -v y a > setfattr -hn trusted.overlay.whiteout -v y b > getfattr -hn trusted.overlay.whiteout a > > | # file: a > | trusted.overlay.whiteout="y" > > getfattr -hn trusted.overlay.whiteout b > > | # file: b > | trusted.overlay.whiteout="y" > > cp -vaP a c > > | ‘a’ -> ‘c’ > > cp -vaP b d > > | ‘b’ -> ‘d’ > > getfattr -hn trusted.overlay.whiteout c > > | # file: c > | trusted.overlay.whiteout="y" > > getfattr -hn trusted.overlay.whiteout d > > | d: trusted.overlay.whiteout: No such attribute > > --- cut here ---
Thanks for the detailed reproduction. The combinations are tricky here. First, some general notes I've noticed on Linux. - xattrs in the user. namespace are not allowed on symlinks, only on regular files and dirs. - xattrs in the trusted. namespace are only listed for root user Therefore this would only be significant for attributes in non user. namespace, and only for the root user with the trusted. namespace at least. So should cp/mv be copying these xattrs across file system boundaries? Since they're already preserved with a rename, then it would be more consistent to copy them. Currently we don't attempt this, but the patch would be fairly trivial, since attr_copy_file() doesn't reference symlinks (since 2004). Note security context and capabilities etc. are also stored in xattrs, but we already filter selinux context at least in our attr_copy_file() wrapper, so that `mv -Z` for example won't be impacted. Also in the attacjed patch I was careful to maintain the order of set_owner() and copy_attr() so that capabilities weren't inadvertently cleared. I'll add a root only test later along the lines of the above. thanks, Pádraig.
>From f118bda19b4ab2e395613163a140b65f08791119 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?P=C3=A1draig=20Brady?= <[email protected]> Date: Fri, 2 May 2014 22:54:32 +0100 Subject: [PATCH] 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. * NEWS: Mention the bug fix. Fixes http://bugs.gnu.org/16131 --- NEWS | 3 +++ src/copy.c | 15 ++++++++------- 2 files changed, 11 insertions(+), 7 deletions(-) diff --git a/NEWS b/NEWS index 50303f9..f7b5112 100644 --- 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 d471a77..eee918a 100644 --- 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 -- 1.7.7.6
