Re: [PATCH 0/3] cifs: some random patches for 2.6.31

On Sun, 2009-05-24 at 21:05 -0400, simo 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...


Bah, mixed up numbers :-)
Associate comments as follow:

1/3 -> cifs: make overriding of ownership conditional on new mount
options

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


2/3 and 3/3 -> cifs: tighten up default file_mode/dir_mode

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

And 100% agree with: cifs: fix artificial limit on reading symlinks

Simo.

-- 
Simo Sorce
Samba Team GPL Compliance Officer <[email protected]>
Principal Software Engineer at Red Hat, Inc. <[email protected]>