Jim Meyering wrote: > Unfortunately, with this patch, "make check" fails the cp/link-heap test. > Investigating why, I discovered that it introduces a huge leak into > cp -a, at least on F10. 80MB worth in this case. > > I'll let you investigate that one ;-)
Leak is in libattr - added by 2.4.43 (~8 months ago) - reported as
RHBZ #485473 . Kamil Dudka already proposed patch for it. Leak doesn't
occur for older libattr or libattrs with that patch.
> Also, please fold the following into your patch for next time.
> Without the first hunk, with -Werror (via ./configure --enable-gcc-warnings)
> your new code doesn't even compile.
Actually, I dropped new ignore_attr_error() function completely - it
should be safe to pass just NULL instead of &ctx to ignore error
messages. Other mentioned "cosmetic" indentation/ChangeLog format issues
fixed. New version attached.
Greetings,
Ondrej
From 45c8ee820c3e9781433ef3b81d5fb9befebc164e Mon Sep 17 00:00:00 2001 From: =?utf-8?q?Ond=C5=99ej=20Va=C5=A1=C3=ADk?= <[email protected]> Date: Mon, 9 Feb 2009 15:44:00 +0100 Subject: [PATCH] cp: make -a option preserve SELinux ctx and xattrs with reduced diagnostics * copy.c (copy_reg): Reduce SELinux context diagnostics for 'cp -a'. (copy_internal): Likewise (copy_attr_by_fd): Reduce xattr diagnostics for 'cp -a'. (copy_attr_by_name): Likewise. * copy.h (cp_options): Add boolean reduce_diagnostics. * cp.c (usage): Mention that --archive (-a) behaves like -dR --preserve=all. (cp_option_init): Initialize added reduce_diagnostics. (main): Add reduce_diagnostics for -a option, preserve xattrs and SELinux context, if possible. * mv.c (cp_options_init): Set initial value for added booleans in cp_options. * install.c (cp_option_init): Likewise. * NEWS: Mention those behaviour changes. * doc/coreutils.texi: Document --preserve=context, document that diagnostics are not shown for failures of non-mandatory attributes (xattr,SELinux). * tests/cp/cp-a-selinux: Check not only failures, but succesful use of preserving SELinux context in cp. * tests/misc/xattr: Add tests for 'cp --preserve=all' and 'cp -a'. --- NEWS | 6 ++++- doc/coreutils.texi | 14 ++++++++++-- src/copy.c | 50 ++++++++++++++++++++++++++++-------------------- src/copy.h | 6 +++++ src/cp.c | 9 ++++++- src/install.c | 2 + src/mv.c | 2 + tests/cp/cp-a-selinux | 22 +++++++++++++++++--- tests/misc/xattr | 15 ++++++++++++- 9 files changed, 93 insertions(+), 33 deletions(-) diff --git a/NEWS b/NEWS index 8eae7cc..9e8e85e 100644 --- a/NEWS +++ b/NEWS @@ -6,7 +6,8 @@ GNU coreutils NEWS -*- outline -*- Add extended attribute support available on certain filesystems like ext2 and XFS. - cp: Tries to copy xattrs when --preserve=xattr specified + cp: Tries to copy xattrs when --preserve=xattr or --preserve=all specified, + -a option tries to preserve xattrs but doesn't inform about failure mv: Always tries to copy xattrs install: Never copies xattrs @@ -27,6 +28,9 @@ GNU coreutils NEWS -*- outline -*- cp uses much less memory in some situations + cp -a now correctly tries to preserve SELinux context (announced in 6.9.90), + doesn't inform about failure unlike with --preserve=all + du --files0-from=FILE no longer reads all of FILE into RAM before processing the first file name diff --git a/doc/coreutils.texi b/doc/coreutils.texi index c3a1164..3d52390 100644 --- a/doc/coreutils.texi +++ b/doc/coreutils.texi @@ -7253,8 +7253,11 @@ The program accepts the following options. Also see @ref{Common options}. Preserve as much as possible of the structure and attributes of the original files in the copy (but do not attempt to preserve internal directory structure; i.e., @samp{ls -U} may list the entries in a copied -directory in a different order). -Equivalent to @option{-dpR}. +directory in a different order). Try to preserve SELinux security context +and extended attributes (xattrs) as well, but do not fail when this is not +succesful. Diagnostic errors are not displayed for those non-guaranteed +attributes. +Equivalent to @option{-dR --preserve=all}. @item -b @itemx @w...@kbd{--backup}[=@var{method}]} @@ -7396,6 +7399,9 @@ Preserve in the destination files any links between corresponding source files. @c Give examples illustrating how hard links are preserved. @c Also, show how soft links map to hard links with -L and -H. +...@itemx context +Preserve SELinux security context of the file. @command{cp} will fail +if the preserving of SELinux security context is not succesful. @itemx xattr Preserve extended attributes if @command{cp} is built with xattr support, and xattrs are supported and enabled on your file system. @@ -7403,7 +7409,9 @@ If SELinux context and/or ACLs are implemented using xattrs, they are preserved by this option as well. @itemx all Preserve all file attributes. -Equivalent to specifying all of the above. +Equivalent to specifying all of the above. Ignore failure +to preserve SELinux security context or extended attributes, +but show diagnostic messages about failures. @end table Using @option{--preserve} with no @var{attribute_list} is equivalent diff --git a/src/copy.c b/src/copy.c index a6ca9dd..99befa5 100644 --- a/src/copy.c +++ b/src/copy.c @@ -152,13 +152,13 @@ copy_attr_quote (struct error_context *ctx ATTRIBUTE_UNUSED, char const *str) static void copy_attr_free (struct error_context *ctx ATTRIBUTE_UNUSED, - char const *str ATTRIBUTE_UNUSED) + char const *str ATTRIBUTE_UNUSED) { } static bool copy_attr_by_fd (char const *src_path, int src_fd, - char const *dst_path, int dst_fd) + char const *dst_path, int dst_fd, const struct cp_options *x) { struct error_context ctx = { @@ -166,11 +166,13 @@ copy_attr_by_fd (char const *src_path, int src_fd, .quote = copy_attr_quote, .quote_free = copy_attr_free }; - return 0 == attr_copy_fd (src_path, src_fd, dst_path, dst_fd, 0, &ctx); + return 0 == attr_copy_fd (src_path, src_fd, dst_path, dst_fd, 0, + x->reduce_diagnostics ? NULL : &ctx); } static bool -copy_attr_by_name (char const *src_path, char const *dst_path) +copy_attr_by_name (char const *src_path, char const *dst_path, + const struct cp_options *x) { struct error_context ctx = { @@ -178,19 +180,21 @@ copy_attr_by_name (char const *src_path, char const *dst_path) .quote = copy_attr_quote, .quote_free = copy_attr_free }; - return 0 == attr_copy_file (src_path, dst_path, 0, &ctx); + return 0 == attr_copy_file (src_path, dst_path, 0, + x-> reduce_diagnostics ? NULL :&ctx); } #else /* USE_XATTR */ static bool copy_attr_by_fd (char const *src_path, int src_fd, - char const *dst_path, int dst_fd) + char const *dst_path, int dst_fd, const struct cp_options *x) { return true; } static bool -copy_attr_by_name (char const *src_path, char const *dst_path) +copy_attr_by_name (char const *src_path, char const *dst_path, + const struct cp_options *x) { return true; } @@ -450,7 +454,8 @@ copy_reg (char const *src_name, char const *dst_name, security_context_t con = NULL; if (getfscreatecon (&con) < 0) { - error (0, errno, _("failed to get file system create context")); + if (!x->reduce_diagnostics) + error (0, errno, _("failed to get file system create context")); if (x->require_preserve_context) { return_val = false; @@ -462,9 +467,10 @@ copy_reg (char const *src_name, char const *dst_name, { if (fsetfilecon (dest_desc, con) < 0) { - error (0, errno, - _("failed to set the security context of %s to %s"), - quote_n (0, dst_name), quote_n (1, con)); + if (!x->reduce_diagnostics) + error (0, errno, + _("failed to set the security context of %s to %s"), + quote_n (0, dst_name), quote_n (1, con)); if (x->require_preserve_context) { return_val = false; @@ -472,7 +478,7 @@ copy_reg (char const *src_name, char const *dst_name, goto close_src_and_dst_desc; } } - freecon(con); + freecon (con); } } @@ -495,7 +501,7 @@ copy_reg (char const *src_name, char const *dst_name, if (*new_dst) { int open_flags = O_WRONLY | O_CREAT | O_BINARY; - dest_desc = open (dst_name, open_flags | O_EXCL , + dest_desc = open (dst_name, open_flags | O_EXCL, dst_mode & ~omitted_permissions); dest_errno = errno; @@ -755,7 +761,7 @@ 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) + dst_name, dest_desc, x) && x->require_preserve_xattr) return false; @@ -1721,9 +1727,10 @@ copy_internal (char const *src_name, char const *dst_name, { if (setfscreatecon (con) < 0) { - error (0, errno, - _("failed to set default file creation context to %s"), - quote (con)); + if (!x->reduce_diagnostics) + error (0, errno, + _("failed to set default file creation context to %s"), + quote (con)); if (x->require_preserve_context) { freecon (con); @@ -1736,9 +1743,10 @@ copy_internal (char const *src_name, char const *dst_name, { if (errno != ENOTSUP && errno != ENODATA) { - error (0, errno, - _("failed to get security context of %s"), - quote (src_name)); + if (!x->reduce_diagnostics) + error (0, errno, + _("failed to get security context of %s"), + quote (src_name)); if (x->require_preserve_context) return false; } @@ -2063,7 +2071,7 @@ copy_internal (char const *src_name, char const *dst_name, set_author (dst_name, -1, &src_sb); - if (x->preserve_xattr && ! copy_attr_by_name (src_name, dst_name) + if (x->preserve_xattr && ! copy_attr_by_name (src_name, dst_name, x) && x->require_preserve_xattr) return false; diff --git a/src/copy.h b/src/copy.h index 0cdf16b..ac980f4 100644 --- a/src/copy.h +++ b/src/copy.h @@ -186,6 +186,12 @@ struct cp_options this flag is "true", while with "cp --preserve=all", it is false. */ bool require_preserve_xattr; + /* Used as difference boolean between cp -a and cp -dR --preserve=ALL. + If true, non-mandatory failure diagnostics are not displayed. This + should prevent poluting cp -a output. + */ + bool reduce_diagnostics; + /* If true, copy directories recursively and copy special files as themselves rather than copying their contents. */ bool recursive; diff --git a/src/cp.c b/src/cp.c index 9171fa6..1ad6b13 100644 --- a/src/cp.c +++ b/src/cp.c @@ -160,7 +160,7 @@ Copy SOURCE to DEST, or multiple SOURCE(s) to DIRECTORY.\n\ Mandatory arguments to long options are mandatory for short options too.\n\ "), stdout); fputs (_("\ - -a, --archive same as -dpR\n\ + -a, --archive same as -dR --preserve=all\n\ --backup[=CONTROL] make a backup of each existing destination file\n\ -b like --backup but does not accept an argument\n\ --copy-contents copy contents of special files when recursive\n\ @@ -766,6 +766,7 @@ cp_option_init (struct cp_options *x) x->preserve_security_context = false; x->require_preserve_context = false; x->preserve_xattr = false; + x->reduce_diagnostics = false; x->require_preserve_xattr = false; x->require_preserve = false; @@ -921,13 +922,17 @@ main (int argc, char **argv) sparse_type_string, sparse_type); break; - case 'a': /* Like -dpR. */ + case 'a': /* Like -dR --preserve=all with reduced failure diagnostics. */ x.dereference = DEREF_NEVER; x.preserve_links = true; x.preserve_ownership = true; x.preserve_mode = true; x.preserve_timestamps = true; x.require_preserve = true; + if (selinux_enabled) + x.preserve_security_context = true; + x.preserve_xattr = true; + x.reduce_diagnostics = true; x.recursive = true; break; diff --git a/src/install.c b/src/install.c index 9bf9eee..f985b95 100644 --- a/src/install.c +++ b/src/install.c @@ -183,8 +183,10 @@ cp_option_init (struct cp_options *x) x->preserve_links = false; x->preserve_mode = false; x->preserve_timestamps = false; + x->reduce_diagnostics=false; x->require_preserve = false; x->require_preserve_context = false; + x->require_preserve_xattr = false; x->recursive = false; x->sparse_mode = SPARSE_AUTO; x->symbolic_link = false; diff --git a/src/mv.c b/src/mv.c index db9207b..77ad2fb 100644 --- a/src/mv.c +++ b/src/mv.c @@ -122,9 +122,11 @@ cp_option_init (struct cp_options *x) x->preserve_mode = true; x->preserve_timestamps = true; x->preserve_security_context = selinux_enabled; + x->reduce_diagnostics = false; x->require_preserve = false; /* FIXME: maybe make this an option */ x->require_preserve_context = false; x->preserve_xattr = true; + x->require_preserve_xattr = false; x->recursive = true; x->sparse_mode = SPARSE_AUTO; /* FIXME: maybe make this an option */ x->symbolic_link = false; diff --git a/tests/cp/cp-a-selinux b/tests/cp/cp-a-selinux index 2f4af35..8b7cc4d 100755 --- a/tests/cp/cp-a-selinux +++ b/tests/cp/cp-a-selinux @@ -1,8 +1,10 @@ #!/bin/sh # Ensure that cp -a and cp --preserve=context work properly. # In particular, test on a writable NFS partition. +# Check also locally if --preserve=context, -a and --preserve=all +# does work -# Copyright (C) 2007-2008 Free Software Foundation, Inc. +# Copyright (C) 2007-2009 Free Software Foundation, Inc. # This program is free software: you can redistribute it and/or modify # it under the terms of the GNU General Public License as published by @@ -29,6 +31,21 @@ require_selinux_ cwd=`pwd` cleanup_() { cd /; umount "$cwd/mnt"; } +# This context is special: it works even when mcstransd isn't running. +ctx=root:object_r:tmp_t:s0 + +# Check basic functionality - before check on fixed context mount +touch c || framework_failure +chcon $ctx c || framework_failure +cp -a c d 2>err || framework_failure +cp --preserve=context c e || framework_failure +cp --preserve=all c f || framework_failure +ls -Z d | grep $ctx || fail=1 +test -s err && fail=1 #there must be no stderr output for -a +ls -Z e | grep $ctx || fail=1 +ls -Z f | grep $ctx || fail=1 + + # Create a file system, then mount it with the context=... option. dd if=/dev/zero of=blob bs=8192 count=200 > /dev/null 2>&1 \ || framework_failure @@ -36,9 +53,6 @@ mkdir mnt || framework_failure mkfs -t ext2 -F blob || skip_test_ "failed to create an ext2 file system" -# This context is special: it works even when mcstransd isn't running. -ctx=root:object_r:tmp_t:s0 - mount -oloop,context=$ctx blob mnt || framework_failure cd mnt || framework_failure diff --git a/tests/misc/xattr b/tests/misc/xattr index 4137c53..f067ff5 100755 --- a/tests/misc/xattr +++ b/tests/misc/xattr @@ -1,6 +1,7 @@ #!/bin/sh -# Ensure that cp --preserve=xattr and mv preserve extended attributes and -# install does not preserve extended attributes. +# Ensure that cp --preserve=xattr, cp --preserve=all and mv preserve extended +# attributes and install does not preserve extended attributes. +# cp -a should preserve xattr, error diagnostics should not be displayed # Copyright (C) 2009 Free Software Foundation, Inc. @@ -66,6 +67,16 @@ 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 +#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 + +#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 + rm b || framework_failure # install should never preserve xattr -- 1.5.6.1.156.ge903b
signature.asc
Description: Toto je digitálně podepsaná část zprávy
_______________________________________________ Bug-coreutils mailing list [email protected] http://lists.gnu.org/mailman/listinfo/bug-coreutils
