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 & S_IWUSR) 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 user_xattr.
>> 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.
