Storing ownership / device nodes without root

Wayne Davison wayned at
Fri Oct 27 22:58:38 GMT 2006

On Fri, Oct 27, 2006 at 01:37:45PM +0200, Wesley W. Terpstra wrote:
> ... fake-super is never sent from the client...?

That code is just to prevent mischief from someone that might tweak
their client to send the option to a daemon.

> Your chmod change broke symlinks. do_chmod needs to see if the file  
> is a symlink or not, but you clobbered the high bits:

Thanks for pointing that out.  It's fixed in the latest patch.

> In do_*stat, I would consider a failure to get_stat_xattr to be  
> critical. That doesn't mean a setting was missing, it meant a setting  
> was corrupt!

Yeah, the error checking is not up-to-snuff yet.

> When I use lsetxattr on a symlink, I get EPERM:

Me too.  I currently have symlinks disabled so that we never try to set
the xattr-stat value.  It might be better to just silence the error in
the future.

> When trying to set the xattr on a read-only file, I get EACESS.
> In your patch, you moved the set_stat_xattr to after the chmod -->  
> breaks if umask omits write access.

Which I had been assuming was not a supported scenario.  The latest code
ensures that the umask can't ask to restrict the file-owner's
permissions, so this will not happen (and I went ahead and moved the
set_stat_xattr() call up higher, just in case the ACL settings mess
things up).

> I think the whole chmod handling is broken right now. What was your  
> intention with that change? Make all files have the same file-system  
> mode? If so, remove the difference test before the chmod.

Yes, I didn't get that code quite right.  The idea is as follows:  I
want to keep all the files readable and writable by the owning user,
which emulates the ability of root to read and write any file.  I had
thought that I would therefor force all files to be 0600 and all dirs
to be 0700, but I thought it would be more flexible to allow some way
for the user to choose what group/world bits they want on the files.
I also don't want any files to have their execute bits set, because
the files may not be in the right state to be used (if an setuid or
setgid file was in the transfer, for instance).

So, the current code should get this right.  It should make sure that
all dirs always have u+rwx and all files have u+rw, and it lets the
umask determine what group/world bits get set.

> Finally, the biggest bug: the permissions/device info/etc doesn't  
> make it back!

That was probably the temporary bug I had in the code where it was still
checking scanf() == 4 after having increased the number of scanned items
to 5.

> Presumably this is because you moved the reading into do_(l)stat, but  
> link_stat doesn't get affected (nor does fstat).

I don't understand this -- link_stat() calls do_stat().  Affecting
fstat() is a good point, and is something that the current code should
do.  I also changed how the code interfaces to the xattr-stat reading
(since I didn't like my choice of putting it down in do_stat()).

Thanks for the feedback, and please feel free to offer more hints,
criticisms, and/or patches.


More information about the rsync mailing list