[linux-cifs-client] [PATCH 0/3] cifs: some random patches for 2.6.31

[Jeff Layton]

On Sun, 24 May 2009 21:05:42 -0400
simo <idra at samba.org> wrote:

> On Sun, 2009-05-24 at 18:45 -0400, Jeff Layton wrote:
> > These are some patches that I'd like considered for 2.6.31. Two of them
> > are patches that I posted a while back but that didn't get taken for
> > 2.6.30. The third patch is a new one to fix a bug that I found recently
> > when dealing with long symlinks.
> > 
> > They're fairly simple patches, so please let me know if you see any
> > issue with taking them for 2.6.31 so we can get the problems ironed
> > out within the merge window.
> > 
> > Thanks...
> > 
> > Jeff Layton (3):
> >   cifs: make overriding of ownership conditional on new mount options
> >   cifs: tighten up default file_mode/dir_mode
> >   cifs: fix artificial limit on reading symlinks
> > 
> >  fs/cifs/cifssmb.c |    3 +--
> >  fs/cifs/connect.c |   14 +++++++-------
> >  2 files changed, 8 insertions(+), 9 deletions(-)
> 
> 
> +1 on all three conceptually, but...
> 
> 1/3) Seem fine for servers with unix extensions, are there any negative
> effects if force[u|g]id are not explicitly passed for normal windows
> servers ? Or do we always use vol->linux_[u|g]id in that case ?
>

There should be none. In the case that cifs can't determine a different
owner of an inode (which is only possible today via unix extensions),
then it should just fall back to the using the default owner of the
mount (value of the uid=/gid= options). IOW, this shouldn't have any
effect when unix extensions are disabled.

In the future, if/when we implement id mapping in some form. These
options could also be used to override that if needed.
 
> 2/3) I agree 101% with this one, making the default mount secure is
> certainly a good idea.
> 
> 3/3) You are going from a very broad set of permission to a very
> restrictive one, on one side I think this is better security wise, on
> the other side I wonder if we should at least give RX to group/other by
> default and rely on the umask to be more restrictive when the user
> creates files ?

The problem there is that relying on the umask won't work on a mount
w/o unix extensions. We can't enforce these modes permanently, so we
don't try to set them. You can use "dynperm" to get this behavior (ugh)
but as soon as the inode cache is flushed, any mode that's different
from the default file_mode/dir_mode will vanish.

Even if that weren't the case though, I still think it's reasonable and
appropriate to default to a very restrictive mode. The admin can always
override it, and I think it's better that that is done as a conscious step.
 
> Also on the mandatory issue, I wonder if applications
> ever check it in the real world. If not, then either way is fine,
> otherwise having the bit set would give precious hints.
> (Although I prefer the patch as it is rather than leaving permission as
> open as they are now).
>

I doubt applications ever look at this. If they do, then they also need
to pay attention to the "mand" mount option and I'm pretty sure that
checking that wouldn't be portable anyway. This change is really about
whether we want the kernel's VFS layer to enforce mandatory locking on
these files.

-- 
Jeff Layton <jlayton at redhat.com>