Ernest N. Mamikonyan wrote:
[returning mailing list to CC, please keep it there]

> On Tue, 01 Sep 2009 02:16:28 -0400, Ondřej Vašík <ova...@redhat.com> wrote:
> > Ernest N. Mamikonyan wrote:
> >> Cp(1) doesn't correctly copy extended attributes for read-only files:
> >>
> >> touch foo
> >> setfattr -n user.key -v value foo
> >> chmod a-w foo
> >> cp --preserve=xattr foo bar
> >> cp: setting attribute `user.key' for `user.key': Permission denied
> >
> > This error message is not produced by coreutils sources, but by libattr
> > - all messages about extended attributes are generated there. I'm not
> > sure why they are trying to set attributes for source file - maybe they
> > are not and access rights for destination file are more relevant.
> > Strace/ltrace of the failure could be helpful as well.
> The problem is quite simple! Cp(1) tries to change the xattrs of a file
> with mode 0400 (see attached trace). Is opening the destination file with
> initial an mode of 0600 a security (or some other) risk? I suppose that's
> what needs to be done.

Sorry, I got confused ... it's obvious - not sure about the risk, so not
trying to fix this now. What do you think, Jim/Pádraig?

Additionally it looks like there is a leak (about 36k per file for
failing xattr preserve) in copy_reg() - as the return false; should be
changed to return_val=false; - sending patch for this... but it's not
fixing the reported issue.

> >> If one uses "cp -a" instead, it simply strips the metadata and doesn't
> >> complain.
> >
> > cp -a shouldn't complain, but the xattrs are likely not preserved in
> > this case - just error message from xattr preservation is suppressed and
> > not preserving extended attributes is not considered as error in that
> > case.
> Well, the manpage says that "cp -a" is the same as "cp -dR --preserve=all."

But the manpage also says that relevant source of information is info
documentation. And info documentation mention that little difference in
behaviour. In fact cp -a behaves like "cp -dR --preserve=all", but is
silent about the failures of preserving SELinux context and/or extended
attributes.

> PS. I forgot to mention the version; it's Coreutils 7.5.
> 
> Thanks
> Ernest N. Mamikonyan     


Greetings,
          Ondřej Vašík
execve("/bin/cp", ["cp", "--preserve=all", "foo", "bar"], [/* 16 vars */]) = 0
brk(0)                                  = 0x8f37000
access("/etc/ld.so.preload", R_OK)      = -1 ENOENT (No such file or directory)
open("/etc/ld.so.cache", O_RDONLY)      = 3
fstat64(3, {st_mode=S_IFREG|0644, st_size=89435, ...}) = 0
mmap2(NULL, 89435, PROT_READ, MAP_PRIVATE, 3, 0) = 0xb7f39000
close(3)                                = 0
open("/lib/libattr.so.1", O_RDONLY)     = 3
read(3, "\177ELF\1\1\1\0\0\0\0\0\0\0\0\0\3\0\3\0\1\0\0\0P\20\0\0004\0\0\0h"..., 512) = 512
fstat64(3, {st_mode=S_IFREG|0755, st_size=17784, ...}) = 0
mmap2(NULL, 4096, PROT_READ|PROT_WRITE, MAP_PRIVATE|MAP_ANONYMOUS, -1, 0) = 0xb7f38000
mmap2(NULL, 20652, PROT_READ|PROT_EXEC, MAP_PRIVATE|MAP_DENYWRITE, 3, 0) = 0xb7f32000
mmap2(0xb7f36000, 8192, PROT_READ|PROT_WRITE, MAP_PRIVATE|MAP_FIXED|MAP_DENYWRITE, 3, 0x3) = 0xb7f36000
close(3)                                = 0
open("/lib/libc.so.6", O_RDONLY)        = 3
read(3, "\177ELF\1\1\1\0\0\0\0\0\0\0\0\0\3\0\3\0\1\0\0\0\360k\1\0004\0\0\0\244"..., 512) = 512
fstat64(3, {st_mode=S_IFREG|0755, st_size=1478940, ...}) = 0
mmap2(NULL, 1489192, PROT_READ|PROT_EXEC, MAP_PRIVATE|MAP_DENYWRITE, 3, 0) = 0xb7dc6000
mprotect(0xb7f2b000, 4096, PROT_NONE)   = 0
mmap2(0xb7f2c000, 12288, PROT_READ|PROT_WRITE, MAP_PRIVATE|MAP_FIXED|MAP_DENYWRITE, 3, 0x165) = 0xb7f2c000
mmap2(0xb7f2f000, 10536, PROT_READ|PROT_WRITE, MAP_PRIVATE|MAP_FIXED|MAP_ANONYMOUS, -1, 0) = 0xb7f2f000
close(3)                                = 0
mmap2(NULL, 4096, PROT_READ|PROT_WRITE, MAP_PRIVATE|MAP_ANONYMOUS, -1, 0) = 0xb7dc5000
set_thread_area({entry_number:-1 -> 6, base_addr:0xb7dc56c0, limit:1048575, seg_32bit:1, contents:0, read_exec_only:0, limit_in_pages:1, seg_not_present:0, useable:1}) = 0
mprotect(0xb7f2c000, 8192, PROT_READ)   = 0
mprotect(0xb7f36000, 4096, PROT_READ)   = 0
mprotect(0x805e000, 4096, PROT_READ)    = 0
mprotect(0xb7f6f000, 4096, PROT_READ)   = 0
munmap(0xb7f39000, 89435)               = 0
brk(0)                                  = 0x8f37000
brk(0x8f58000)                          = 0x8f58000
open("/usr/lib/locale/locale-archive", O_RDONLY|O_LARGEFILE) = 3
fstat64(3, {st_mode=S_IFREG|0644, st_size=1821680, ...}) = 0
mmap2(NULL, 1821680, PROT_READ, MAP_PRIVATE, 3, 0) = 0xb7c08000
close(3)                                = 0
geteuid32()                             = 100
stat64("bar", 0xbf924ed4)               = -1 ENOENT (No such file or directory)
stat64("foo", {st_mode=S_IFREG|0444, st_size=0, ...}) = 0
stat64("bar", 0xbf924cfc)               = -1 ENOENT (No such file or directory)
open("foo", O_RDONLY|O_LARGEFILE)       = 3
fstat64(3, {st_mode=S_IFREG|0444, st_size=0, ...}) = 0
open("bar", O_WRONLY|O_CREAT|O_EXCL|O_LARGEFILE, 0400) = 5
fstat64(5, {st_mode=S_IFREG|0400, st_size=0, ...}) = 0
read(3, ""..., 32768)                   = 0
utimensat(5, NULL, {{1251817700, 0}, {1251817700, 0}}, 0) = 0
flistxattr(3, (nil), 0)                 = 9
flistxattr(3, 0xbf924ad0, 9)            = 9
open("/etc/xattr.conf", O_RDONLY|O_LARGEFILE) = -1 ENOENT (No such file or directory)
fgetxattr(3, "user.key", 0x0, 0)        = 5
fgetxattr(3, "user.key", "value", 5)    = 5
fsetxattr(5, "user.key", "value", 5, 0) = -1 EACCES (Permission denied)
write(2, "cp: "..., 4cp: )                  = 4
write(2, "setting attribute `user.key' for "..., 43setting attribute `user.key' for `user.key') = 43
open("/usr/share/locale/locale.alias", O_RDONLY) = 6
fstat64(6, {st_mode=S_IFREG|0644, st_size=2570, ...}) = 0
mmap2(NULL, 4096, PROT_READ|PROT_WRITE, MAP_PRIVATE|MAP_ANONYMOUS, -1, 0) = 0xb7f4e000
read(6, "# Locale name alias data base.\n# "..., 4096) = 2570
read(6, ""..., 4096)                    = 0
close(6)                                = 0
munmap(0xb7f4e000, 4096)                = 0
open("/usr/share/locale/en_US.ISO-8859-15/LC_MESSAGES/libc.mo", O_RDONLY) = -1 ENOENT (No such file or directory)
open("/usr/share/locale/en_US.iso885915/LC_MESSAGES/libc.mo", O_RDONLY) = -1 ENOENT (No such file or directory)
open("/usr/share/locale/en_US/LC_MESSAGES/libc.mo", O_RDONLY) = -1 ENOENT (No such file or directory)
open("/usr/share/locale/en.ISO-8859-15/LC_MESSAGES/libc.mo", O_RDONLY) = -1 ENOENT (No such file or directory)
open("/usr/share/locale/en.iso885915/LC_MESSAGES/libc.mo", O_RDONLY) = -1 ENOENT (No such file or directory)
open("/usr/share/locale/en/LC_MESSAGES/libc.mo", O_RDONLY) = -1 ENOENT (No such file or directory)
write(2, ": Permission denied"..., 19: Permission denied)  = 19
write(2, "\n"..., 1
)                    = 1
fchmod(5, 0100444)                      = 0
close(5)                                = 0
close(3)                                = 0
close(0)                                = 0
close(1)                                = 0
close(2)                                = 0
exit_group(0)                           = ?
From 53899ca4b2d15852cefe12dfdc1b60228ea52543 Mon Sep 17 00:00:00 2001
From: =?utf-8?q?Ond=C5=99ej=20Va=C5=A1=C3=ADk?= <ova...@redhat.com>
Date: Wed, 2 Sep 2009 09:52:34 +0200
Subject: [PATCH] cp: Do not leak memory for failed preservation of extended attributes

src/copy.c (copy_reg): do not exit from the function after unsuccesful
  and required preservation of extended attributes
NEWS: mention that fix
---
 NEWS       |    2 ++
 src/copy.c |    2 +-
 2 files changed, 3 insertions(+), 1 deletions(-)

diff --git a/NEWS b/NEWS
index 6df0d65..adcbefd 100644
--- a/NEWS
+++ b/NEWS
@@ -4,6 +4,8 @@ GNU coreutils NEWS                                    -*- outline -*-
 
 ** Bug fixes
 
+  cp no more leaks memory for failed preservation of extended attributes.
+
   dd's oflag=direct option now works even when the size of the input
   is not a multiple of e.g., 512 bytes.
 
diff --git a/src/copy.c b/src/copy.c
index bed90c4..2004a03 100644
--- a/src/copy.c
+++ b/src/copy.c
@@ -835,7 +835,7 @@ copy_reg (char const *src_name, char const *dst_name,
   if (x->preserve_xattr && ! copy_attr_by_fd (src_name, source_desc,
 					      dst_name, dest_desc, x)
       && x->require_preserve_xattr)
-    return false;
+    return_val = false;
 
   if (x->preserve_mode || x->move_mode)
     {
-- 
1.5.6.1.156.ge903b

Attachment: signature.asc
Description: Toto je digitálně podepsaná část zprávy

Reply via email to