CIFS should honour umask

This patch makes CIFS honour a process' umask like other filesystems. Of course the server is still free to munge the permissions if it wants to; but the client will send the "right" permissions to begin with.

A few caveats;

1) It only applies to filesystems that have CAP_UNIX (aka support unix extensions)
2) It applies the correct mode to the follow up CIFSSMBUnixSetPerms() after remote creation (I can write a new patch if you want with the "right" mode at actual creation time; however the "right" perms will still need to be given to the follow up CIFSSMBUnixSetPerms() anyway).
3) It will probably work best with Samba 3.0.25a or newer (ie with this patch applied
http://lists.samba.org/archive/linux-cifs-client/2007-January/001697.html)
4) It has been compiled, and tested on 2.6.22-rc4 / Samba 3.0.25a (Ubuntu Dapper with a few custom backports), and with a bit of testing seems to work just fine. (it also incidentally side steps bugs in thunderbird and openoffice (the apps don't check the permissions on files they create, they assume they will open() the way that have asked them to be created xref open(O_WRONLY|O_CREAT) => valid fd then mmap(fd,PROT_READ) => EFAULT).

I am going to give this patch a more thorough test tomorrow with ltp. Comments, corrections, et al are welcome.

Matt

--
Matt Keenan
OpCode Solutions

Signed-off-by Matt Keenan <[email protected]>

--
diff -urN linux-2.6.22-rc4/fs/cifs/dir.c linux-2.6.22-rc4.cifs-umask-fix/fs/cifs/dir.c

fs/cifs/dir.c 2007-06-06 19:27:00.000000000 +0100 renamed from fs/cifs/dir.c 2007-06-06 08:34:03.000000000 +0100 +6 -2

@@ -206,7 +206,11 @@
                /* If Open reported that we actually created a file
                then we now have to set the mode if possible */
                if ((cifs_sb->tcon->ses->capabilities & CAP_UNIX) &&
-                       (oplock & CIFS_CREATE_ACTION))
+                       (oplock & CIFS_CREATE_ACTION)) {
+                       /* respect umask settings like other filesystems;
+                        * if the server wants to munge the bits let it, but the client
+                        * should "Do The Right Thing" (tm) */
+                       mode &= ~current->fs->umask;
                        if(cifs_sb->mnt_cifs_flags & CIFS_MOUNT_SET_UID) {
                                CIFSSMBUnixSetPerms(xid, pTcon, full_path, mode,
                                        (__u64)current->fsuid,

@@ -224,7 +228,7 @@
                                        cifs_sb->mnt_cifs_flags &
                                                CIFS_MOUNT_MAP_SPECIAL_CHR);
                        }
-               else {
+               } else {
                        /* BB implement mode setting via Windows security descriptors */
                        /* eg CIFSSMBWinSetPerms(xid,pTcon,full_path,mode,-1,-1,local_nls);*/
                        /* could set r/o dos attribute if mode & 0222 == 0 */

diff -urN linux-2.6.22-rc4/fs/cifs/inode.c linux-2.6.22-rc4.cifs-umask-fix/fs/cifs/inode.c

fs/cifs/inode.c 2007-06-06 19:26:26.000000000 +0100 renamed from fs/cifs/inode.c 2007-06-06 08:34:03.000000000 +0100 +6 -1

@@ -987,6 +987,11 @@
                if ((direntry->d_inode) && (direntry->d_inode->i_nlink < 2))
                                direntry->d_inode->i_nlink = 2;
                if (cifs_sb->tcon->ses->capabilities & CAP_UNIX)
+               {
+                       /* respect umask settings like other filesystems;
+                        * if the server wants to munge the bits let it, but the client
+                        * should "Do The Right Thing" (tm) */
+                       mode &= ~current->fs->umask;
                        if (cifs_sb->mnt_cifs_flags & CIFS_MOUNT_SET_UID) {
                                CIFSSMBUnixSetPerms(xid, pTcon, full_path,
                                                    mode,

@@ -1004,7 +1009,7 @@
                                                    cifs_sb->mnt_cifs_flags &
                                                    CIFS_MOUNT_MAP_SPECIAL_CHR);
                        }
-               else {
+               } else {
                        /* BB to be implemented via Windows secrty descriptors
                           eg CIFSSMBWinSetPerms(xid, pTcon, full_path, mode,
                                                 -1, -1, local_nls); */

Quoted 5 lines · Steve French

Thanks - it looks almost right but you missed mknod case and your patch had some whitespace/formatting problems.

Could you try the following and make sure it works for you? If so will merge.

For the non-unix case (e.g. Windows servers) the mode will be taken from the default specified on the mount. I am not sure if we also should add code to also honor umask in that case.

I am not sure how common it is to change umask to different values in different processes which would access the same mount.