[Top][All Lists]

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

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

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

Ondřej Vašík wrote:
> Pádraig Brady wrote:
>> Ondřej Vašík wrote:
>>> Pádraig Brady wrote:
>>>> To minimize side affects perhaps we should only do the chmod(600)
>>>> if (geteuid () != 0 && !access (src_name, W_OK)) ?
>>> Good idea, it would reduce possibility of security leak, playing with
>>> access rights is always a bit dangerous (although here we play with
>>> rights on destination descriptor, which is imho much more safe).
>>> Additionally - Jim is correct that for different owner 0600 rights are
>>> not sufficient for different owner of the file - and 0666 is too much
>>> devil-like ;) . Any idea?
>> preserve_xattr before preserve_ownership ?
> Good idea, moved there and used that (geteuid () != 0 && access
> (src_name, W_OK)) construction - additionally I tried to reduce those
> chmod calls (call for returning permissions only when the write_access 
> granting call was used) - so it should be safer now.
> Anyway, added comment that real problem is in libattr and this is just
> workaround and added FIXME. Better now?

That looks much better, thanks.
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.

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.


reply via email to

[Prev in Thread] Current Thread [Next in Thread]