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

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 ?

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

Simo.

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