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

[Jeff Layton]

On Wed, 23 Jan 2008 12:39:56 -0600
"Steve French (smfltc)" <smfltc at us.ibm.com> wrote:

> Jeff Layton wrote:
> 
> >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.
> >  
> >
> If we want to treat the mode on the mount like a second mask, that is 
> fine, but we may want
> to make that configurable.
> 
> I would expect a lot of applications (not just ltp tests) would fail
> to mounts to Windows server if they create files with various
> different modes and they all end up getting the same mode (due to
> only using the mode on mount) even taking into account that we can
> handle removing write mode via +R dos attribute.
> 

Most don't care too much about the mode as long as they have
permissions to read or write as expected. There may be some that will
fail, but those really can't be expected to work anyway. Eventually the
inode info will be refreshed, or the fs will get remounted. At that
point these modes vanish.

> I have been assuming that the new cifsacl mount option(CIFS acl to
> mode mapping)  could be too slow for some workloads (and it is still 
> experimental), but it does fix the issue of having the right mode
> that the application asks for.  Do we need another mount option to
> indicate whether we want to use the mode specified on the mount or as
> we do today use the specified mode on lookup of existing inodes but
> do our best effort on chmod/mkdir/create?
> 

Maybe a mount option or other tunable is the best thing for now as a
transition mechanism. That said, this is really an implementation
detail, and trying to describe to users when they should use it may be
a challenge. Once cifsacl is more mature, perhaps we can make it the
default.

> >>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.
> >
> >  
> >
> It should not relate to when the metadata times out - if we set the
> mode locally (but can't set
> it remotely) we should never overwrite the mode for the same 
> in-memory-inode from the remote
> server.  We may have a bug here.
> 

Sorry, that's not correct. I meant that eventually the inode will get
flushed out of the inode cache (either due to memory pressure or for
other reasons). At that point we'll poll the server for the attrs and
this mode will go away.

> >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:
> >
> >  
> >
> This is worth investigating/exploring...
> 

Trying to preserve the uid/gid info sounds rather scary as well. I
think we have to be concerned about consistency here. These modes can
flip to the default at any time and we can't predict when it will
occur. Non-deterministic behavior is worse than something that reliably
fails, IMO.

Eliminating these ephemeral modes is really the best option. vfat, for
instance, doesn't try to keep modes like this and people use it all the
time. There may be some apps that break, but presumably those users will
eventually be able to transition to cifsacl support.

-- 
Jeff Layton <jlayton at redhat.com>