Re: [PATCH] cp, mv: do preserve extended attributes even for read-only s (original) (raw)

[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

From: Pádraig Brady
Subject: Re: [PATCH] cp, mv: do preserve extended attributes even for read-only source files
Date: Mon, 14 Sep 2009 10:56:56 +0100
User-agent: Thunderbird 2.0.0.6 (X11/20071008)

Ondřej Vašík wrote:

Pádraig Brady wrote: > Since we're only doing u+rw, and we've already stat'd it's > probably better to just (sb.mode & SIWUSR) rather than access(...). > Also a couple of the if statements are indented too far.

Hopefully ok with the attached patch.

> This should now be safer but as Jim says it > only effects file systems mounted userxattr. > Perhaps we should wait until coreutils-7.7 and > also feedback from libattr devs so as we can put > an accurate comment in the code.

As libattr feedback is already here at http://lists.gnu.org/archive/html/bug-coreutils/2009-09/msg00166.html and it seems it is not a bug in libattr (just strange requirement by kernel), I modified the comment about workaround - as the culprit is probably in kernel.

Yes I agree that the change is required. I've tweaked it so that the geteuid() syscall is only called if readonly files. Also I removed the error message on chmod failure as the user will still get an error message if the copy_xattr fails. Also I ran it through indent and did s/write access rights/write access/. The crux of the patch is now:

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);
}

I'll push it soon if there are no objections.

cheers, Pádraig.