No longer set S_ISVTX when mounted to Windows

No longer set S_ISVTX when mounted to Windows

[Steve French]

Not sure why the cifs code sets the S_ISVTX bit when mounted to
Windows servers.  I think the intent was to turn off all of the high
bits except that (S_ISGID) needed to indicate mandatory locking
(mandatory locking is set by having S_ISGID on and group execute,
S_IXGRP, off).  When reviewing the cifsacl code (to map
CIFS/Windows/NTFS ACLs to mode bits I noticed that we were setting the
sticky bit (S_ISVTX) on files (this does not happen when a user
specifies a default mode on mount) and probably shouldn't.  Does
anyone see any problem with turning this bit off in the default file
mode when mounted to Windows?

diff --git a/fs/cifs/connect.c b/fs/cifs/connect.c
index 19ee11f..380ee99 100644
--- a/fs/cifs/connect.c
+++ b/fs/cifs/connect.c
@@ -793,7 +793,7 @@ cifs_parse_mount_options(char *options, const char *devname,
        vol->linux_gid = current->gid;
        vol->dir_mode = S_IRWXUGO;
        /* 2767 perms indicate mandatory locking support */
-       vol->file_mode = S_IALLUGO & ~(S_ISUID | S_IXGRP);
+       vol->file_mode = (S_IRWXUGO | S_ISGID) & (~S_IXGRP);

        /* vol->retry default is 0 (i.e. "soft" limited retry not hard retry) */
        vol->rw = TRUE;

-- 
Thanks,

Steve

Re: No longer set S_ISVTX when mounted to Windows

[Jeff Layton]

On Thu, 1 Nov 2007 11:08:33 -0500
"Steve French" <smfrench@gmail.com> wrote:

> Not sure why the cifs code sets the S_ISVTX bit when mounted to
> Windows servers.  I think the intent was to turn off all of the high
> bits except that (S_ISGID) needed to indicate mandatory locking
> (mandatory locking is set by having S_ISGID on and group execute,
> S_IXGRP, off).  When reviewing the cifsacl code (to map
> CIFS/Windows/NTFS ACLs to mode bits I noticed that we were setting the
> sticky bit (S_ISVTX) on files (this does not happen when a user
> specifies a default mode on mount) and probably shouldn't.  Does
> anyone see any problem with turning this bit off in the default file
> mode when mounted to Windows?
> 
> diff --git a/fs/cifs/connect.c b/fs/cifs/connect.c
> index 19ee11f..380ee99 100644
> --- a/fs/cifs/connect.c
> +++ b/fs/cifs/connect.c
> @@ -793,7 +793,7 @@ cifs_parse_mount_options(char *options, const
> char *devname, vol->linux_gid = current->gid;
>         vol->dir_mode = S_IRWXUGO;
>         /* 2767 perms indicate mandatory locking support */
> -       vol->file_mode = S_IALLUGO & ~(S_ISUID | S_IXGRP);
> +       vol->file_mode = (S_IRWXUGO | S_ISGID) & (~S_IXGRP);
> 
>         /* vol->retry default is 0 (i.e. "soft" limited retry not
> hard retry) */ vol->rw = TRUE;
> 
> -- 
> Thanks,
> 
> Steve

ACK :-)

Looks pretty much like the patch I sent to the list on Oct 16. At the
time I think you emailed JRA to ask why we were setting the sticky bit
but I don't believe he ever replied.

AFAICT, the sticky bit on a regular file doesn't mean anything on
Linux. Having it set is harmless, but I could see problems if someone
were for instance, to tar up some files on a CIFS share and untar them
on an OS where it does have meaning.

-- 
Jeff Layton <jlayton@redhat.com>