Re: [PATCH] CIFS should honour umask

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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.

diff --git a/fs/cifs/dir.c b/fs/cifs/dir.c
index f085db9..8e86aac 100644
--- a/fs/cifs/dir.c
+++ b/fs/cifs/dir.c
@@ -208,7 +208,8 @@ cifs_create(struct inode *inode, struct
               /* 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)) {
+                       mode &= ~current->fs->umask;
                       if (cifs_sb->mnt_cifs_flags & CIFS_MOUNT_SET_UID) {
                               CIFSSMBUnixSetPerms(xid, pTcon, full_path, mode,
                                       (__u64)current->fsuid,
@@ -226,7 +227,7 @@ cifs_create(struct inode *inode, struct
                                       cifs_sb->mnt_cifs_flags &
                                               CIFS_MOUNT_MAP_SPECIAL_CHR);
                       }
-               else {
+               } else {
                       /* BB implement mode setting via Windows security
                          descriptors e.g. */
                       /* CIFSSMBWinSetPerms(xid,pTcon,path,mode,-1,-1,nls);*/
@@ -336,6 +337,7 @@ int cifs_mknod(struct inode *inode, stru
       if (full_path == NULL)
               rc = -ENOMEM;
       else if (pTcon->ses->capabilities & CAP_UNIX) {
+               mode &= ~current->fs->umask;
               if (cifs_sb->mnt_cifs_flags & CIFS_MOUNT_SET_UID) {
                       rc = CIFSSMBUnixSetPerms(xid, pTcon, full_path,
                               mode, (__u64)current->fsuid,
diff --git a/fs/cifs/inode.c b/fs/cifs/inode.c
index 3e87dad..f0ff12b 100644
--- a/fs/cifs/inode.c
+++ b/fs/cifs/inode.c
@@ -986,7 +986,8 @@ mkdir_get_info:
                 * failed to get it from the server or was set bogus */
               if ((direntry->d_inode) && (direntry->d_inode->i_nlink < 2))
                               direntry->d_inode->i_nlink = 2;
-               if (cifs_sb->tcon->ses->capabilities & CAP_UNIX)
+               if (cifs_sb->tcon->ses->capabilities & CAP_UNIX) {
+                       mode &= ~current->fs->umask;
                       if (cifs_sb->mnt_cifs_flags & CIFS_MOUNT_SET_UID) {
                               CIFSSMBUnixSetPerms(xid, pTcon, full_path,
                                                   mode,
@@ -1004,7 +1005,7 @@ mkdir_get_info:
                                                   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); */


On 6/6/07, Matt Keenan <[email protected]> wrote:
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
--- linux-2.6.22-rc4/fs/cifs/dir.c      2007-06-06 08:34:03.000000000 +0100
+++ linux-2.6.22-rc4.cifs-umask-fix/fs/cifs/dir.c       2007-06-06 19:27:00.000000000 +0100
@@ -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
--- linux-2.6.22-rc4/fs/cifs/inode.c    2007-06-06 08:34:03.000000000 +0100
+++ linux-2.6.22-rc4.cifs-umask-fix/fs/cifs/inode.c     2007-06-06 19:26:26.000000000 +0100
@@ -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); */




--
Thanks,

Steve
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[Index of Archives]     [Kernel Newbies]     [Netfilter]     [Bugtraq]     [Photo]     [Stuff]     [Gimp]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Video 4 Linux]     [Linux for the blind]     [Linux Resources]
  Powered by Linux