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

simo idra at samba.org
Mon May 25 16:42:42 GMT 2009


On Mon, 2009-05-25 at 06:14 -0400, Jeff Layton wrote:
> 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.

make sense, +1

> > 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.

uhm right

> 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.

yes, make sense, +1
 
> > 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.

Yes if apps never look at it (as I believe too) then it is probably just
useless to set the locking as mandatory, although in that case shouldn't
we prevent sending advisory locks to the server if unix extensions are
not negotiated ? (do we already do that?)

Simo.

-- 
Simo Sorce
Samba Team GPL Compliance Officer <simo at samba.org>
Principal Software Engineer at Red Hat, Inc. <simo at redhat.com>



More information about the linux-cifs-client mailing list