[linux-cifs-client] Re: [PATCH] [CIFS] when creating new inodes, use file_mode/dir_mode on mount without unix extensions

[Jeff Layton]

On Tue, 22 Jan 2008 18:39:10 -0600
"Steve French (smfltc)" <smfltc at us.ibm.com> wrote:

> Jeff Layton wrote:
> 
> >Hi Steve,
> >   I originally sent this patch in late September. We discussed it a
> >bit and you thought it was best to wait until most of the cifsacl
> >patches were in. Now that they are, I'd like to have this patch
> >reconsidered. It just makes it so that file and dir modes are not
> >temporarily overridden when a new inode is created.
> >
> >I've tested it using my modified cthon04 test on a CIFS mount to
> >samba with unix extensions disabled. Everything seems to work as long
> >as you pick sensible file_mode/dir_mode settings. The default
> >file_mode makes it fail, however -- it seems that cthon04 doesn't
> >care for mandatory locking.
> >
> >Thoughts?
> >  
> >
> This seems risky in a few ways.  With your patch an application which 
> wants to restrict the mode of a new file which is creates, ie to less 
> than the default mode for the share, would no longer be able to.  In
> the local case you may have a mask that lets you get a different mode
> than you requested, but the result would be less than what you ask
> for (never more than what you asked for on the create).


Right, but it's also possible that the reverse is true -- someone could
specify a restrictive file_mode/dir_mode and assume that when they
create a file that they'll get that mode regardless of what option is
mode is specified in the create call.

> It also
> seems like we go from a case in which the mode is correct for quite a
> while (but then could revert back to the default for the share when
> the inode is flushed/reloaded), to a case in which the mode is never
> right.   The original idea was that revalidate was not supposed to
> change the mode for a file if the application had set a specific mode 
> (create/mkdir/chmod).  There were cases in which the inode info would 
> get flushed because e.g. the file were not still open and there were 
> memory pressure - and in these cases the inode's mode would look 
> different on next lookup, but at least it would be correct for a long 
> time (and this is no different than what Unix applications deal with 
> everyday - a file's mode could change at any time on the server so 
> applications have to deal with this already).

It all comes down to what is the behavior of least surprise. IMO,
if someone specifies a file_mode/dir_mode then that's what they should
expect in all cases, not just when the cached inode metadata times out.

The main problem with this is that we're just talking about the mode.
What about the uid/gid? While the mode is respected here, the uid/gid
of the user creating the file is not. This causes problems like this:

Consider a nounix mount that has uid/gid of user1, but a
file_mode/dir_mode of 0777. user2 comes along and does this:

$ umask 022; mkdir /mnt/cifs/foo; touch /mnt/cifs/foo/bar
touch: cannot touch `/mnt/cifs/foo/bar': Permission denied

(wtf? I can't create a file in the directory I just made?)

Eventually the inode metadata on "foo" will time out and user2 can
create this file. This behavior seems completely unintuitive.

The other problem is that this behavior allows you to create a file
that's owned by the uid/gid of the mount with an arbitrary mode (even a
setuid file). While I've not been able to come up with a way to turn
that into a local exploit, it seems dangerous to me.

-- 
Jeff Layton <jlayton at redhat.com>