| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <524f69650706061723i3e065d0y83739e50d986a06f@mail.gmail.com>
Date: Wed, 6 Jun 2007 19:23:43 -0500
From: "Steve French" <smfrench@...il.com>
To: "Matt Keenan" <matt@...ode-solutions.com>
Cc: "Steve French" <sfrench@...ba.org>,
"Jeremy Allison" <jra@...ba.org>,
LKML <linux-kernel@...r.kernel.org>,
linux-cifs-client@...ts.samba.org
Subject: Re: [PATCH] CIFS should honour umask
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 <matt@...ode-solutions.com> 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 <matt@...ode-solutions.com>
>
> --
> 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 majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists