[PATCH 0/3] Add O_DENY* flags to fcntl and cifs

[PATCH 0/3] Add O_DENY* flags to fcntl and cifs

[Pavel Shilovsky]

Network filesystems CIFS, SMB2.0, SMB3.0 and NFSv4 have such flags - this change can benefit cifs and nfs modules. While this change is ok for network filesystems, itsn't not targeted for local filesystems due security problems (e.g. when a user process can deny root to delete a file).

Share flags are used by Windows applications and WINE have to deal with them too. While WINE can process open share flags itself on local filesystems, it can't do it if a file stored on a network share and is used by several clients. This patchset makes it possible for CIFS/SMB2.0/SMB3.0.

Pavel Shilovsky (3):
  fcntl: Introduce new O_DENY* open flags for network filesystems
  CIFS: Add O_DENY* open flags support
  CIFS: Use NT_CREATE_ANDX command for forcemand mounts

 fs/cifs/cifsacl.c                |   10 ++++----
 fs/cifs/cifsglob.h               |   11 ++++++++-
 fs/cifs/cifsproto.h              |    9 ++++----
 fs/cifs/cifssmb.c                |   47 ++++++++++++++++++++------------------
 fs/cifs/dir.c                    |   14 ++++++++----
 fs/cifs/file.c                   |   18 ++++++++++-----
 fs/cifs/inode.c                  |   11 +++++----
 fs/cifs/link.c                   |   10 ++++----
 fs/cifs/readdir.c                |    2 +-
 fs/cifs/smb1ops.c                |   15 ++++++------
 fs/cifs/smb2file.c               |   10 ++++----
 fs/cifs/smb2inode.c              |    4 ++--
 fs/cifs/smb2ops.c                |   10 ++++----
 fs/cifs/smb2pdu.c                |    6 ++---
 fs/cifs/smb2proto.h              |   14 +++++++-----
 fs/fcntl.c                       |    5 ++--
 include/uapi/asm-generic/fcntl.h |   11 +++++++++
 17 files changed, 125 insertions(+), 82 deletions(-)

-- 
1.7.10.4

[PATCH 1/3] fcntl: Introduce new O_DENY* open flags for network filesystems

[Pavel Shilovsky]

This patch adds 3 flags:
1) O_DENYREAD that doesn't permit read access
2) O_DENYWRITE that doesn't permit write access
3) O_DENYDELETE that doesn't permit delete or rename

Network filesystems CIFS, SMB2.0, SMB3.0 and NFSv4 have such flags -
this change can benefit cifs and nfs modules. While this change is ok
for network filesystems, itsn't not targeted for local filesystems due
to security problems (e.g. when a user process can deny root to delete
a file).

Signed-off-by: Pavel Shilovsky <piastry@etersoft.ru>
---
 fs/fcntl.c                       |    5 +++--
 include/uapi/asm-generic/fcntl.h |   11 +++++++++++
 2 files changed, 14 insertions(+), 2 deletions(-)

diff --git a/fs/fcntl.c b/fs/fcntl.c
index 71a600a..7abce5a 100644
--- a/fs/fcntl.c
+++ b/fs/fcntl.c
@@ -730,14 +730,15 @@ static int __init fcntl_init(void)
 	 * Exceptions: O_NONBLOCK is a two bit define on parisc; O_NDELAY
 	 * is defined as O_NONBLOCK on some platforms and not on others.
 	 */
-	BUILD_BUG_ON(19 - 1 /* for O_RDONLY being 0 */ != HWEIGHT32(
+	BUILD_BUG_ON(22 - 1 /* for O_RDONLY being 0 */ != HWEIGHT32(
 		O_RDONLY	| O_WRONLY	| O_RDWR	|
 		O_CREAT		| O_EXCL	| O_NOCTTY	|
 		O_TRUNC		| O_APPEND	| /* O_NONBLOCK	| */
 		__O_SYNC	| O_DSYNC	| FASYNC	|
 		O_DIRECT	| O_LARGEFILE	| O_DIRECTORY	|
 		O_NOFOLLOW	| O_NOATIME	| O_CLOEXEC	|
-		__FMODE_EXEC	| O_PATH
+		__FMODE_EXEC	| O_PATH	| O_DENYREAD	|
+		O_DENYWRITE	| O_DENYDELETE
 		));
 
 	fasync_cache = kmem_cache_create("fasync_cache",
diff --git a/include/uapi/asm-generic/fcntl.h b/include/uapi/asm-generic/fcntl.h
index a48937d..5ac0d49 100644
--- a/include/uapi/asm-generic/fcntl.h
+++ b/include/uapi/asm-generic/fcntl.h
@@ -84,6 +84,17 @@
 #define O_PATH		010000000
 #endif
 
+#ifndef O_DENYREAD
+#define O_DENYREAD	020000000	/* Do not permit read access */
+#endif
+#ifndef O_DENYWRITE
+#define O_DENYWRITE	040000000	/* Do not permit write access */
+#endif
+/* FMODE_NONOTIFY	0100000000 */
+#ifndef O_DENYDELETE
+#define O_DENYDELETE	0200000000	/* Do not permit delete or rename */
+#endif
+
 #ifndef O_NDELAY
 #define O_NDELAY	O_NONBLOCK
 #endif
-- 
1.7.10.4

[PATCH 2/3] CIFS: Add O_DENY* open flags support

[Pavel Shilovsky]

Make CIFSSMBOpen take share_flags as a parm that allows us
to pass new O_DENY* flags to the server.

Signed-off-by: Pavel Shilovsky <piastry@etersoft.ru>
---
 fs/cifs/cifsacl.c   |   10 ++++++----
 fs/cifs/cifsglob.h  |   11 ++++++++++-
 fs/cifs/cifsproto.h |    9 +++++----
 fs/cifs/cifssmb.c   |   47 +++++++++++++++++++++++++----------------------
 fs/cifs/dir.c       |   13 ++++++++-----
 fs/cifs/file.c      |   12 ++++++++----
 fs/cifs/inode.c     |   11 ++++++-----
 fs/cifs/link.c      |   10 +++++-----
 fs/cifs/readdir.c   |    2 +-
 fs/cifs/smb1ops.c   |   15 ++++++++-------
 fs/cifs/smb2file.c  |   10 +++++-----
 fs/cifs/smb2inode.c |    4 ++--
 fs/cifs/smb2ops.c   |   10 ++++++----
 fs/cifs/smb2pdu.c   |    6 +++---
 fs/cifs/smb2proto.h |   14 ++++++++------
 15 files changed, 106 insertions(+), 78 deletions(-)

diff --git a/fs/cifs/cifsacl.c b/fs/cifs/cifsacl.c
index 9c3ebbd..86981df 100644
--- a/fs/cifs/cifsacl.c
+++ b/fs/cifs/cifsacl.c
@@ -895,8 +895,9 @@ static struct cifs_ntsd *get_cifs_acl_by_path(struct cifs_sb_info *cifs_sb,
 		create_options |= CREATE_OPEN_BACKUP_INTENT;
 
 	rc = CIFSSMBOpen(xid, tcon, path, FILE_OPEN, READ_CONTROL,
-			create_options, &fid, &oplock, NULL, cifs_sb->local_nls,
-			cifs_sb->mnt_cifs_flags & CIFS_MOUNT_MAP_SPECIAL_CHR);
+			 FILE_SHARE_ALL, create_options, &fid, &oplock,
+			 NULL, cifs_sb->local_nls,
+			 cifs_sb->mnt_cifs_flags & CIFS_MOUNT_MAP_SPECIAL_CHR);
 	if (!rc) {
 		rc = CIFSSMBGetCIFSACL(xid, tcon, fid, &pntsd, pacllen);
 		CIFSSMBClose(xid, tcon, fid);
@@ -956,8 +957,9 @@ int set_cifs_acl(struct cifs_ntsd *pnntsd, __u32 acllen,
 		access_flags = WRITE_DAC;
 
 	rc = CIFSSMBOpen(xid, tcon, path, FILE_OPEN, access_flags,
-			create_options, &fid, &oplock, NULL, cifs_sb->local_nls,
-			cifs_sb->mnt_cifs_flags & CIFS_MOUNT_MAP_SPECIAL_CHR);
+			 FILE_SHARE_ALL, create_options, &fid, &oplock,
+			 NULL, cifs_sb->local_nls,
+			 cifs_sb->mnt_cifs_flags & CIFS_MOUNT_MAP_SPECIAL_CHR);
 	if (rc) {
 		cERROR(1, "Unable to open file to set ACL");
 		goto out;
diff --git a/fs/cifs/cifsglob.h b/fs/cifs/cifsglob.h
index 74a07b6..2ca6f7d 100644
--- a/fs/cifs/cifsglob.h
+++ b/fs/cifs/cifsglob.h
@@ -310,7 +310,7 @@ struct smb_version_operations {
 			       struct cifs_sb_info *);
 	/* open a file for non-posix mounts */
 	int (*open)(const unsigned int, struct cifs_tcon *, const char *, int,
-		    int, int, struct cifs_fid *, __u32 *, FILE_ALL_INFO *,
+		    int, int, int, struct cifs_fid *, __u32 *, FILE_ALL_INFO *,
 		    struct cifs_sb_info *);
 	/* set fid protocol-specific info */
 	void (*set_fid)(struct cifsFileInfo *, struct cifs_fid *, __u32);
@@ -944,6 +944,15 @@ struct cifsFileInfo {
 	struct work_struct oplock_break; /* work for oplock breaks */
 };
 
+#define CIFS_DENY_RW_FLAGS_SHIFT	22
+#define CIFS_DENY_DEL_FLAG_SHIFT	23
+
+static inline int cifs_get_share_flags(unsigned int flags)
+{
+	return ((~(flags >> CIFS_DENY_RW_FLAGS_SHIFT)) & 3) |
+	       ((~(flags >> CIFS_DENY_DEL_FLAG_SHIFT)) & 4);
+}
+
 struct cifs_io_parms {
 	__u16 netfid;
 #ifdef CONFIG_CIFS_SMB2
diff --git a/fs/cifs/cifsproto.h b/fs/cifs/cifsproto.h
index fa81130..d086c4a 100644
--- a/fs/cifs/cifsproto.h
+++ b/fs/cifs/cifsproto.h
@@ -361,10 +361,11 @@ extern int CIFSSMBQueryReparseLinkInfo(const unsigned int xid,
 			const struct nls_table *nls_codepage);
 #endif /* temporarily unused until cifs_symlink fixed */
 extern int CIFSSMBOpen(const unsigned int xid, struct cifs_tcon *tcon,
-			const char *fileName, const int disposition,
-			const int access_flags, const int omode,
-			__u16 *netfid, int *pOplock, FILE_ALL_INFO *,
-			const struct nls_table *nls_codepage, int remap);
+			const char *file_name, const int disposition,
+			const int access_flags, const int share_flags,
+			const int omode, __u16 *netfid, int *oplock,
+			FILE_ALL_INFO *, const struct nls_table *nls_codepage,
+			int remap);
 extern int SMBLegacyOpen(const unsigned int xid, struct cifs_tcon *tcon,
 			const char *fileName, const int disposition,
 			const int access_flags, const int omode,
diff --git a/fs/cifs/cifssmb.c b/fs/cifs/cifssmb.c
index 76d0d29..9c4632f 100644
--- a/fs/cifs/cifssmb.c
+++ b/fs/cifs/cifssmb.c
@@ -1289,10 +1289,11 @@ OldOpenRetry:
 
 int
 CIFSSMBOpen(const unsigned int xid, struct cifs_tcon *tcon,
-	    const char *fileName, const int openDisposition,
-	    const int access_flags, const int create_options, __u16 *netfid,
-	    int *pOplock, FILE_ALL_INFO *pfile_info,
-	    const struct nls_table *nls_codepage, int remap)
+	    const char *file_name, const int disposition,
+	    const int access_flags, const int share_flags,
+	    const int create_options, __u16 *netfid, int *oplock,
+	    FILE_ALL_INFO *file_info, const struct nls_table *nls_codepage,
+	    int remap)
 {
 	int rc = -EACCES;
 	OPEN_REQ *pSMB = NULL;
@@ -1313,26 +1314,28 @@ openRetry:
 		count = 1;	/* account for one byte pad to word boundary */
 		name_len =
 		    cifsConvertToUTF16((__le16 *) (pSMB->fileName + 1),
-				       fileName, PATH_MAX, nls_codepage, remap);
+				       file_name, PATH_MAX, nls_codepage,
+				       remap);
 		name_len++;	/* trailing null */
 		name_len *= 2;
 		pSMB->NameLength = cpu_to_le16(name_len);
 	} else {		/* BB improve check for buffer overruns BB */
 		count = 0;	/* no pad */
-		name_len = strnlen(fileName, PATH_MAX);
+		name_len = strnlen(file_name, PATH_MAX);
 		name_len++;	/* trailing null */
 		pSMB->NameLength = cpu_to_le16(name_len);
-		strncpy(pSMB->fileName, fileName, name_len);
+		strncpy(pSMB->fileName, file_name, name_len);
 	}
-	if (*pOplock & REQ_OPLOCK)
+	if (*oplock & REQ_OPLOCK)
 		pSMB->OpenFlags = cpu_to_le32(REQ_OPLOCK);
-	else if (*pOplock & REQ_BATCHOPLOCK)
+	else if (*oplock & REQ_BATCHOPLOCK)
 		pSMB->OpenFlags = cpu_to_le32(REQ_BATCHOPLOCK);
 	pSMB->DesiredAccess = cpu_to_le32(access_flags);
 	pSMB->AllocationSize = 0;
-	/* set file as system file if special file such
-	   as fifo and server expecting SFU style and
-	   no Unix extensions */
+	/*
+	 * set file as system file if special file such as fifo and server
+	 * expecting SFU style and no Unix extensions
+	 */
 	if (create_options & CREATE_OPTION_SPECIAL)
 		pSMB->FileAttributes = cpu_to_le32(ATTR_SYSTEM);
 	else
@@ -1347,8 +1350,8 @@ openRetry:
 	if (create_options & CREATE_OPTION_READONLY)
 		pSMB->FileAttributes |= cpu_to_le32(ATTR_READONLY);
 
-	pSMB->ShareAccess = cpu_to_le32(FILE_SHARE_ALL);
-	pSMB->CreateDisposition = cpu_to_le32(openDisposition);
+	pSMB->ShareAccess = cpu_to_le32(share_flags);
+	pSMB->CreateDisposition = cpu_to_le32(disposition);
 	pSMB->CreateOptions = cpu_to_le32(create_options & CREATE_OPTIONS_MASK);
 	/* BB Expirement with various impersonation levels and verify */
 	pSMB->ImpersonationLevel = cpu_to_le32(SECURITY_IMPERSONATION);
@@ -1366,20 +1369,20 @@ openRetry:
 	if (rc) {
 		cFYI(1, "Error in Open = %d", rc);
 	} else {
-		*pOplock = pSMBr->OplockLevel; /* 1 byte no need to le_to_cpu */
+		*oplock = pSMBr->OplockLevel; /* 1 byte no need to le_to_cpu */
 		*netfid = pSMBr->Fid;	/* cifs fid stays in le */
 		/* Let caller know file was created so we can set the mode. */
 		/* Do we care about the CreateAction in any other cases? */
 		if (cpu_to_le32(FILE_CREATE) == pSMBr->CreateAction)
-			*pOplock |= CIFS_CREATE_ACTION;
-		if (pfile_info) {
-			memcpy((char *)pfile_info, (char *)&pSMBr->CreationTime,
+			*oplock |= CIFS_CREATE_ACTION;
+		if (file_info) {
+			memcpy((char *)file_info, (char *)&pSMBr->CreationTime,
 				36 /* CreationTime to Attributes */);
 			/* the file_info buf is endian converted by caller */
-			pfile_info->AllocationSize = pSMBr->AllocationSize;
-			pfile_info->EndOfFile = pSMBr->EndOfFile;
-			pfile_info->NumberOfLinks = cpu_to_le32(1);
-			pfile_info->DeletePending = 0;
+			file_info->AllocationSize = pSMBr->AllocationSize;
+			file_info->EndOfFile = pSMBr->EndOfFile;
+			file_info->NumberOfLinks = cpu_to_le32(1);
+			file_info->DeletePending = 0;
 		}
 	}
 
diff --git a/fs/cifs/dir.c b/fs/cifs/dir.c
index 3b7e0c1..4784d53 100644
--- a/fs/cifs/dir.c
+++ b/fs/cifs/dir.c
@@ -202,6 +202,7 @@ cifs_do_create(struct inode *inode, struct dentry *direntry, unsigned int xid,
 	FILE_ALL_INFO *buf = NULL;
 	struct inode *newinode = NULL;
 	int disposition;
+	int share_access;
 	struct TCP_Server_Info *server = tcon->ses->server;
 
 	*oplock = 0;
@@ -292,6 +293,8 @@ cifs_do_create(struct inode *inode, struct dentry *direntry, unsigned int xid,
 	else
 		cFYI(1, "Create flag not set in create function");
 
+	share_access = cifs_get_share_flags(oflags);
+
 	/*
 	 * BB add processing to set equivalent of mode - e.g. via CreateX with
 	 * ACLs
@@ -319,8 +322,8 @@ cifs_do_create(struct inode *inode, struct dentry *direntry, unsigned int xid,
 		create_options |= CREATE_OPEN_BACKUP_INTENT;
 
 	rc = server->ops->open(xid, tcon, full_path, disposition,
-			       desired_access, create_options, fid, oplock,
-			       buf, cifs_sb);
+			       desired_access, share_access, create_options,
+			       fid, oplock, buf, cifs_sb);
 	if (rc) {
 		cFYI(1, "cifs_create returned 0x%x", rc);
 		goto out;
@@ -625,9 +628,9 @@ int cifs_mknod(struct inode *inode, struct dentry *direntry, umode_t mode,
 	if (backup_cred(cifs_sb))
 		create_options |= CREATE_OPEN_BACKUP_INTENT;
 
-	rc = CIFSSMBOpen(xid, pTcon, full_path, FILE_CREATE,
-			 GENERIC_WRITE, create_options,
-			 &fileHandle, &oplock, buf, cifs_sb->local_nls,
+	rc = CIFSSMBOpen(xid, pTcon, full_path, FILE_CREATE, GENERIC_WRITE,
+			 FILE_SHARE_ALL, create_options, &fileHandle, &oplock,
+			 buf, cifs_sb->local_nls,
 			 cifs_sb->mnt_cifs_flags & CIFS_MOUNT_MAP_SPECIAL_CHR);
 	if (rc)
 		goto mknod_out;
diff --git a/fs/cifs/file.c b/fs/cifs/file.c
index bceffa8..e2614b9 100644
--- a/fs/cifs/file.c
+++ b/fs/cifs/file.c
@@ -174,6 +174,7 @@ cifs_nt_open(char *full_path, struct inode *inode, struct cifs_sb_info *cifs_sb,
 {
 	int rc;
 	int desired_access;
+	int share_access;
 	int disposition;
 	int create_options = CREATE_NOT_DIR;
 	FILE_ALL_INFO *buf;
@@ -209,6 +210,7 @@ cifs_nt_open(char *full_path, struct inode *inode, struct cifs_sb_info *cifs_sb,
  *********************************************************************/
 
 	disposition = cifs_get_disposition(f_flags);
+	share_access = cifs_get_share_flags(f_flags);
 
 	/* BB pass O_SYNC flag through on file attributes .. BB */
 
@@ -220,8 +222,8 @@ cifs_nt_open(char *full_path, struct inode *inode, struct cifs_sb_info *cifs_sb,
 		create_options |= CREATE_OPEN_BACKUP_INTENT;
 
 	rc = server->ops->open(xid, tcon, full_path, disposition,
-			       desired_access, create_options, fid, oplock, buf,
-			       cifs_sb);
+			       desired_access, share_access, create_options,
+			       fid, oplock, buf, cifs_sb);
 
 	if (rc)
 		goto out;
@@ -551,6 +553,7 @@ cifs_reopen_file(struct cifsFileInfo *cfile, bool can_flush)
 	struct inode *inode;
 	char *full_path = NULL;
 	int desired_access;
+	int share_access;
 	int disposition = FILE_OPEN;
 	int create_options = CREATE_NOT_DIR;
 	struct cifs_fid fid;
@@ -615,6 +618,7 @@ cifs_reopen_file(struct cifsFileInfo *cfile, bool can_flush)
 	}
 
 	desired_access = cifs_convert_flags(cfile->f_flags);
+	share_access = cifs_get_share_flags(cfile->f_flags);
 
 	if (backup_cred(cifs_sb))
 		create_options |= CREATE_OPEN_BACKUP_INTENT;
@@ -630,8 +634,8 @@ cifs_reopen_file(struct cifsFileInfo *cfile, bool can_flush)
 	 * not dirty locally we could do this.
 	 */
 	rc = server->ops->open(xid, tcon, full_path, disposition,
-			       desired_access, create_options, &fid, &oplock,
-			       NULL, cifs_sb);
+			       desired_access, share_access, create_options,
+			       &fid, &oplock, NULL, cifs_sb);
 	if (rc) {
 		mutex_unlock(&cfile->fh_mutex);
 		cFYI(1, "cifs_reopen returned 0x%x", rc);
diff --git a/fs/cifs/inode.c b/fs/cifs/inode.c
index ed6208f..a497dfa 100644
--- a/fs/cifs/inode.c
+++ b/fs/cifs/inode.c
@@ -396,8 +396,8 @@ cifs_sfu_type(struct cifs_fattr *fattr, const unsigned char *path,
 	tcon = tlink_tcon(tlink);
 
 	rc = CIFSSMBOpen(xid, tcon, path, FILE_OPEN, GENERIC_READ,
-			 CREATE_NOT_DIR, &netfid, &oplock, NULL,
-			 cifs_sb->local_nls,
+			 FILE_SHARE_ALL, CREATE_NOT_DIR, &netfid, &oplock,
+			 NULL, cifs_sb->local_nls,
 			 cifs_sb->mnt_cifs_flags &
 				CIFS_MOUNT_MAP_SPECIAL_CHR);
 	if (rc == 0) {
@@ -987,8 +987,9 @@ cifs_rename_pending_delete(const char *full_path, struct dentry *dentry,
 	tcon = tlink_tcon(tlink);
 
 	rc = CIFSSMBOpen(xid, tcon, full_path, FILE_OPEN,
-			 DELETE|FILE_WRITE_ATTRIBUTES, CREATE_NOT_DIR,
-			 &netfid, &oplock, NULL, cifs_sb->local_nls,
+			 DELETE|FILE_WRITE_ATTRIBUTES, FILE_SHARE_ALL,
+			 CREATE_NOT_DIR, &netfid, &oplock, NULL,
+			 cifs_sb->local_nls,
 			 cifs_sb->mnt_cifs_flags & CIFS_MOUNT_MAP_SPECIAL_CHR);
 	if (rc != 0)
 		goto out;
@@ -1509,7 +1510,7 @@ cifs_do_rename(const unsigned int xid, struct dentry *from_dentry,
 
 	/* open the file to be renamed -- we need DELETE perms */
 	rc = CIFSSMBOpen(xid, tcon, from_path, FILE_OPEN, DELETE,
-			 CREATE_NOT_DIR, &srcfid, &oplock, NULL,
+			 FILE_SHARE_ALL, CREATE_NOT_DIR, &srcfid, &oplock, NULL,
 			 cifs_sb->local_nls, cifs_sb->mnt_cifs_flags &
 				CIFS_MOUNT_MAP_SPECIAL_CHR);
 	if (rc == 0) {
diff --git a/fs/cifs/link.c b/fs/cifs/link.c
index 51dc2fb..9b4f0db 100644
--- a/fs/cifs/link.c
+++ b/fs/cifs/link.c
@@ -212,7 +212,7 @@ CIFSCreateMFSymLink(const unsigned int xid, struct cifs_tcon *tcon,
 		create_options |= CREATE_OPEN_BACKUP_INTENT;
 
 	rc = CIFSSMBOpen(xid, tcon, fromName, FILE_CREATE, GENERIC_WRITE,
-			 create_options, &netfid, &oplock, NULL,
+			 FILE_SHARE_ALL, create_options, &netfid, &oplock, NULL,
 			 nls_codepage, remap);
 	if (rc != 0) {
 		kfree(buf);
@@ -254,8 +254,8 @@ CIFSQueryMFSymLink(const unsigned int xid, struct cifs_tcon *tcon,
 	FILE_ALL_INFO file_info;
 
 	rc = CIFSSMBOpen(xid, tcon, searchName, FILE_OPEN, GENERIC_READ,
-			 CREATE_NOT_DIR, &netfid, &oplock, &file_info,
-			 nls_codepage, remap);
+			 FILE_SHARE_ALL, CREATE_NOT_DIR, &netfid, &oplock,
+			 &file_info, nls_codepage, remap);
 	if (rc != 0)
 		return rc;
 
@@ -332,8 +332,8 @@ CIFSCheckMFSymlink(struct cifs_fattr *fattr,
 	pTcon = tlink_tcon(tlink);
 
 	rc = CIFSSMBOpen(xid, pTcon, path, FILE_OPEN, GENERIC_READ,
-			 CREATE_NOT_DIR, &netfid, &oplock, &file_info,
-			 cifs_sb->local_nls,
+			 FILE_SHARE_ALL, CREATE_NOT_DIR, &netfid, &oplock,
+			 &file_info, cifs_sb->local_nls,
 			 cifs_sb->mnt_cifs_flags &
 				CIFS_MOUNT_MAP_SPECIAL_CHR);
 	if (rc != 0)
diff --git a/fs/cifs/readdir.c b/fs/cifs/readdir.c
index 96fe44b..6a6a801 100644
--- a/fs/cifs/readdir.c
+++ b/fs/cifs/readdir.c
@@ -213,7 +213,7 @@ int get_symlink_reparse_path(char *full_path, struct cifs_sb_info *cifs_sb,
 	char *tmpbuffer;
 
 	rc = CIFSSMBOpen(xid, ptcon, full_path, FILE_OPEN, GENERIC_READ,
-			OPEN_REPARSE_POINT, &fid, &oplock, NULL,
+			FILE_SHARE_ALL, OPEN_REPARSE_POINT, &fid, &oplock, NULL,
 			cifs_sb->local_nls,
 			cifs_sb->mnt_cifs_flags & CIFS_MOUNT_MAP_SPECIAL_CHR);
 	if (!rc) {
diff --git a/fs/cifs/smb1ops.c b/fs/cifs/smb1ops.c
index a5d234c..561547f 100644
--- a/fs/cifs/smb1ops.c
+++ b/fs/cifs/smb1ops.c
@@ -664,9 +664,9 @@ cifs_mkdir_setinfo(struct inode *inode, const char *full_path,
 
 static int
 cifs_open_file(const unsigned int xid, struct cifs_tcon *tcon, const char *path,
-	       int disposition, int desired_access, int create_options,
-	       struct cifs_fid *fid, __u32 *oplock, FILE_ALL_INFO *buf,
-	       struct cifs_sb_info *cifs_sb)
+	       int disposition, int desired_access, int share_access,
+	       int create_options, struct cifs_fid *fid, __u32 *oplock,
+	       FILE_ALL_INFO *buf, struct cifs_sb_info *cifs_sb)
 {
 	if (!(tcon->ses->capabilities & CAP_NT_SMBS))
 		return SMBLegacyOpen(xid, tcon, path, disposition,
@@ -675,8 +675,8 @@ cifs_open_file(const unsigned int xid, struct cifs_tcon *tcon, const char *path,
 				     cifs_sb->local_nls, cifs_sb->mnt_cifs_flags
 						& CIFS_MOUNT_MAP_SPECIAL_CHR);
 	return CIFSSMBOpen(xid, tcon, path, disposition, desired_access,
-			   create_options, &fid->netfid, oplock, buf,
-			   cifs_sb->local_nls, cifs_sb->mnt_cifs_flags &
+			   share_access, create_options, &fid->netfid, oplock,
+			   buf, cifs_sb->local_nls, cifs_sb->mnt_cifs_flags &
 						CIFS_MOUNT_MAP_SPECIAL_CHR);
 }
 
@@ -772,8 +772,9 @@ smb_set_file_info(struct inode *inode, const char *full_path,
 	cFYI(1, "calling SetFileInfo since SetPathInfo for times not supported "
 		"by this server");
 	rc = CIFSSMBOpen(xid, tcon, full_path, FILE_OPEN,
-			 SYNCHRONIZE | FILE_WRITE_ATTRIBUTES, CREATE_NOT_DIR,
-			 &netfid, &oplock, NULL, cifs_sb->local_nls,
+			 SYNCHRONIZE | FILE_WRITE_ATTRIBUTES, FILE_SHARE_ALL,
+			 CREATE_NOT_DIR, &netfid, &oplock, NULL,
+			 cifs_sb->local_nls,
 			 cifs_sb->mnt_cifs_flags & CIFS_MOUNT_MAP_SPECIAL_CHR);
 
 	if (rc != 0) {
diff --git a/fs/cifs/smb2file.c b/fs/cifs/smb2file.c
index 71e6aed..7dfb50c 100644
--- a/fs/cifs/smb2file.c
+++ b/fs/cifs/smb2file.c
@@ -58,9 +58,9 @@ smb2_set_oplock_level(struct cifsInodeInfo *cinode, __u32 oplock)
 
 int
 smb2_open_file(const unsigned int xid, struct cifs_tcon *tcon, const char *path,
-	       int disposition, int desired_access, int create_options,
-	       struct cifs_fid *fid, __u32 *oplock, FILE_ALL_INFO *buf,
-	       struct cifs_sb_info *cifs_sb)
+	       int disposition, int desired_access, int share_access,
+	       int create_options, struct cifs_fid *fid, __u32 *oplock,
+	       FILE_ALL_INFO *buf, struct cifs_sb_info *cifs_sb)
 {
 	int rc;
 	__le16 *smb2_path;
@@ -87,8 +87,8 @@ smb2_open_file(const unsigned int xid, struct cifs_tcon *tcon, const char *path,
 		memcpy(smb2_oplock + 1, fid->lease_key, SMB2_LEASE_KEY_SIZE);
 
 	rc = SMB2_open(xid, tcon, smb2_path, &fid->persistent_fid,
-		       &fid->volatile_fid, desired_access, disposition,
-		       0, 0, smb2_oplock, smb2_data);
+		       &fid->volatile_fid, desired_access, share_access,
+		       disposition, 0, 0, smb2_oplock, smb2_data);
 	if (rc)
 		goto out;
 
diff --git a/fs/cifs/smb2inode.c b/fs/cifs/smb2inode.c
index 7064824..6af174a 100644
--- a/fs/cifs/smb2inode.c
+++ b/fs/cifs/smb2inode.c
@@ -54,8 +54,8 @@ smb2_open_op_close(const unsigned int xid, struct cifs_tcon *tcon,
 		return -ENOMEM;
 
 	rc = SMB2_open(xid, tcon, utf16_path, &persistent_fid, &volatile_fid,
-		       desired_access, create_disposition, file_attributes,
-		       create_options, &oplock, NULL);
+		       desired_access, FILE_SHARE_ALL, create_disposition,
+		       file_attributes, create_options, &oplock, NULL);
 	if (rc) {
 		kfree(utf16_path);
 		return rc;
diff --git a/fs/cifs/smb2ops.c b/fs/cifs/smb2ops.c
index ad4d96a..4056685 100644
--- a/fs/cifs/smb2ops.c
+++ b/fs/cifs/smb2ops.c
@@ -222,7 +222,8 @@ smb2_is_path_accessible(const unsigned int xid, struct cifs_tcon *tcon,
 		return -ENOMEM;
 
 	rc = SMB2_open(xid, tcon, utf16_path, &persistent_fid, &volatile_fid,
-		       FILE_READ_ATTRIBUTES, FILE_OPEN, 0, 0, &oplock, NULL);
+		       FILE_READ_ATTRIBUTES, FILE_SHARE_ALL, FILE_OPEN, 0, 0,
+		       &oplock, NULL);
 	if (rc) {
 		kfree(utf16_path);
 		return rc;
@@ -432,8 +433,8 @@ smb2_query_dir_first(const unsigned int xid, struct cifs_tcon *tcon,
 		return -ENOMEM;
 
 	rc = SMB2_open(xid, tcon, utf16_path, &persistent_fid, &volatile_fid,
-		       FILE_READ_ATTRIBUTES | FILE_READ_DATA, FILE_OPEN, 0, 0,
-		       &oplock, NULL);
+		       FILE_READ_ATTRIBUTES | FILE_READ_DATA, FILE_SHARE_ALL,
+		       FILE_OPEN, 0, 0, &oplock, NULL);
 	kfree(utf16_path);
 	if (rc) {
 		cERROR(1, "open dir failed");
@@ -515,7 +516,8 @@ smb2_queryfs(const unsigned int xid, struct cifs_tcon *tcon,
 	u8 oplock = SMB2_OPLOCK_LEVEL_NONE;
 
 	rc = SMB2_open(xid, tcon, &srch_path, &persistent_fid, &volatile_fid,
-		       FILE_READ_ATTRIBUTES, FILE_OPEN, 0, 0, &oplock, NULL);
+		       FILE_READ_ATTRIBUTES, FILE_SHARE_ALL, FILE_OPEN, 0, 0,
+		       &oplock, NULL);
 	if (rc)
 		return rc;
 	buf->f_type = SMB2_MAGIC_NUMBER;
diff --git a/fs/cifs/smb2pdu.c b/fs/cifs/smb2pdu.c
index cf33622..641eda3 100644
--- a/fs/cifs/smb2pdu.c
+++ b/fs/cifs/smb2pdu.c
@@ -909,8 +909,8 @@ parse_lease_state(struct smb2_create_rsp *rsp)
 int
 SMB2_open(const unsigned int xid, struct cifs_tcon *tcon, __le16 *path,
 	  u64 *persistent_fid, u64 *volatile_fid, __u32 desired_access,
-	  __u32 create_disposition, __u32 file_attributes, __u32 create_options,
-	  __u8 *oplock, struct smb2_file_all_info *buf)
+	  __u32 share_access, __u32 create_disposition, __u32 file_attributes,
+	  __u32 create_options,  __u8 *oplock, struct smb2_file_all_info *buf)
 {
 	struct smb2_create_req *req;
 	struct smb2_create_rsp *rsp;
@@ -939,7 +939,7 @@ SMB2_open(const unsigned int xid, struct cifs_tcon *tcon, __le16 *path,
 	req->DesiredAccess = cpu_to_le32(desired_access);
 	/* File attributes ignored on open (used in create though) */
 	req->FileAttributes = cpu_to_le32(file_attributes);
-	req->ShareAccess = FILE_SHARE_ALL_LE;
+	req->ShareAccess = cpu_to_le32(share_access);
 	req->CreateDisposition = cpu_to_le32(create_disposition);
 	req->CreateOptions = cpu_to_le32(create_options);
 	uni_path_len = (2 * UniStrnlen((wchar_t *)path, PATH_MAX)) + 2;
diff --git a/fs/cifs/smb2proto.h b/fs/cifs/smb2proto.h
index 7d25f8b..f836a0b 100644
--- a/fs/cifs/smb2proto.h
+++ b/fs/cifs/smb2proto.h
@@ -82,9 +82,10 @@ extern int smb2_create_hardlink(const unsigned int xid, struct cifs_tcon *tcon,
 
 extern int smb2_open_file(const unsigned int xid, struct cifs_tcon *tcon,
 			  const char *full_path, int disposition,
-			  int desired_access, int create_options,
-			  struct cifs_fid *fid, __u32 *oplock,
-			  FILE_ALL_INFO *buf, struct cifs_sb_info *cifs_sb);
+			  int desired_access, int share_access,
+			  int create_options,  struct cifs_fid *fid,
+			  __u32 *oplock, FILE_ALL_INFO *buf,
+			  struct cifs_sb_info *cifs_sb);
 extern void smb2_set_oplock_level(struct cifsInodeInfo *cinode, __u32 oplock);
 extern int smb2_unlock_range(struct cifsFileInfo *cfile,
 			     struct file_lock *flock, const unsigned int xid);
@@ -104,9 +105,10 @@ extern int SMB2_tcon(const unsigned int xid, struct cifs_ses *ses,
 extern int SMB2_tdis(const unsigned int xid, struct cifs_tcon *tcon);
 extern int SMB2_open(const unsigned int xid, struct cifs_tcon *tcon,
 		     __le16 *path, u64 *persistent_fid, u64 *volatile_fid,
-		     __u32 desired_access, __u32 create_disposition,
-		     __u32 file_attributes, __u32 create_options,
-		     __u8 *oplock, struct smb2_file_all_info *buf);
+		     __u32 desired_access, __u32 share_access,
+		     __u32 create_disposition, __u32 file_attributes,
+		     __u32 create_options, __u8 *oplock,
+		     struct smb2_file_all_info *buf);
 extern int SMB2_close(const unsigned int xid, struct cifs_tcon *tcon,
 		      u64 persistent_file_id, u64 volatile_file_id);
 extern int SMB2_flush(const unsigned int xid, struct cifs_tcon *tcon,
-- 
1.7.10.4

[PATCH 3/3] CIFS: Use NT_CREATE_ANDX command for forcemand mounts

[Pavel Shilovsky]

forcemand mount option now lets us use Windows mandatory style of
byte-range locks even if server supports posix ones - switches on
Windows locking mechanism. Share flags is another locking mehanism
provided by Windows semantic that can be used by NT_CREATE_ANDX
command. This patch combines all Windows locking mechanism in one
mount option by using NT_CREATE_ANDX to open files if forcemand is on.

Signed-off-by: Pavel Shilovsky <piastry@etersoft.ru>
---
 fs/cifs/dir.c  |    1 +
 fs/cifs/file.c |    6 ++++--
 2 files changed, 5 insertions(+), 2 deletions(-)

diff --git a/fs/cifs/dir.c b/fs/cifs/dir.c
index 4784d53..8edd950 100644
--- a/fs/cifs/dir.c
+++ b/fs/cifs/dir.c
@@ -216,6 +216,7 @@ cifs_do_create(struct inode *inode, struct dentry *direntry, unsigned int xid,
 	}
 
 	if (tcon->unix_ext && cap_unix(tcon->ses) && !tcon->broken_posix_open &&
+	    ((cifs_sb->mnt_cifs_flags & CIFS_MOUNT_NOPOSIXBRL) == 0) &&
 	    (CIFS_UNIX_POSIX_PATH_OPS_CAP &
 			le64_to_cpu(tcon->fsUnixInfo.Capability))) {
 		rc = cifs_posix_open(full_path, &newinode, inode->i_sb, mode,
diff --git a/fs/cifs/file.c b/fs/cifs/file.c
index e2614b9..8cfda53 100644
--- a/fs/cifs/file.c
+++ b/fs/cifs/file.c
@@ -426,8 +426,9 @@ int cifs_open(struct inode *inode, struct file *file)
 	else
 		oplock = 0;
 
-	if (!tcon->broken_posix_open && tcon->unix_ext &&
-	    cap_unix(tcon->ses) && (CIFS_UNIX_POSIX_PATH_OPS_CAP &
+	if (!tcon->broken_posix_open && tcon->unix_ext && cap_unix(tcon->ses)
+	    && ((cifs_sb->mnt_cifs_flags & CIFS_MOUNT_NOPOSIXBRL) == 0) &&
+	    (CIFS_UNIX_POSIX_PATH_OPS_CAP &
 				le64_to_cpu(tcon->fsUnixInfo.Capability))) {
 		/* can not refresh inode info since size could be stale */
 		rc = cifs_posix_open(full_path, &inode, inode->i_sb,
@@ -595,6 +596,7 @@ cifs_reopen_file(struct cifsFileInfo *cfile, bool can_flush)
 		oplock = 0;
 
 	if (tcon->unix_ext && cap_unix(tcon->ses) &&
+	    ((cifs_sb->mnt_cifs_flags & CIFS_MOUNT_NOPOSIXBRL) == 0) &&
 	    (CIFS_UNIX_POSIX_PATH_OPS_CAP &
 				le64_to_cpu(tcon->fsUnixInfo.Capability))) {
 		/*
-- 
1.7.10.4

Re: [PATCH 0/3] Add O_DENY* flags to fcntl and cifs

[Alan Cox]

On Thu,  6 Dec 2012 22:26:28 +0400
Pavel Shilovsky <piastry@etersoft.ru> wrote:

> Network filesystems CIFS, SMB2.0, SMB3.0 and NFSv4 have such flags - this change can benefit cifs and nfs modules. While this change is ok for network filesystems, itsn't not targeted for local filesystems due security problems (e.g. when a user process can deny root to delete a file).

If I have my root fs on NFS then the same applies does it not.

Your patches fail to describe the security semantics and what file rights
I must have to apply each option. How do I track down a lock user, what
tools are provided ? How do the new options interact with the security
layer?

I don't have a problem with the idea, but it needs a lot more clear
description of how it works so the model can be checked and if need be
things tweaked (eg needing write to denywrite etc)

Alan

Re: [PATCH 0/3] Add O_DENY* flags to fcntl and cifs

[Jeremy Allison]

On Thu, Dec 06, 2012 at 07:49:49PM +0000, Alan Cox wrote:
> On Thu,  6 Dec 2012 22:26:28 +0400
> Pavel Shilovsky <piastry@etersoft.ru> wrote:
> 
> > Network filesystems CIFS, SMB2.0, SMB3.0 and NFSv4 have such flags - this change can benefit cifs and nfs modules. While this change is ok for network filesystems, itsn't not targeted for local filesystems due security problems (e.g. when a user process can deny root to delete a file).
> 
> If I have my root fs on NFS then the same applies does it not.
> 
> Your patches fail to describe the security semantics and what file rights
> I must have to apply each option. How do I track down a lock user, what
> tools are provided ? How do the new options interact with the security
> layer?
> 
> I don't have a problem with the idea, but it needs a lot more clear
> description of how it works so the model can be checked and if need be
> things tweaked (eg needing write to denywrite etc)

And this is where things get really ugly of course :-).

For the CIFSFS client they're expecting to be able to
just ship them to a Windows server, where they'll
get the (insane) Windows semantics. These semantics
are not what would be wanted on a local filesystem.

So unless we just say "these things have Windows
semantics" (where openers of files can lock out others
under dubious circumstances) there'll be this horrible
difference between (I'm assuming) the sane semantics that
are defined for local filesystems and the insane ones
that you get when you're connecting remotely.

I don't know a good way to fix that, but I'm pretty
sure you don't want the Windows semantics defined
locally :-).

Jeremy.

Re: [PATCH 0/3] Add O_DENY* flags to fcntl and cifs

[Jeremy Allison]

On Thu, Dec 06, 2012 at 11:57:52AM -0800, Jeremy Allison wrote:
> On Thu, Dec 06, 2012 at 07:49:49PM +0000, Alan Cox wrote:
> > On Thu,  6 Dec 2012 22:26:28 +0400
> > Pavel Shilovsky <piastry@etersoft.ru> wrote:
> > 
> > > Network filesystems CIFS, SMB2.0, SMB3.0 and NFSv4 have such flags - this change can benefit cifs and nfs modules. While this change is ok for network filesystems, itsn't not targeted for local filesystems due security problems (e.g. when a user process can deny root to delete a file).
> > 
> > If I have my root fs on NFS then the same applies does it not.
> > 
> > Your patches fail to describe the security semantics and what file rights
> > I must have to apply each option. How do I track down a lock user, what
> > tools are provided ? How do the new options interact with the security
> > layer?
> > 
> > I don't have a problem with the idea, but it needs a lot more clear
> > description of how it works so the model can be checked and if need be
> > things tweaked (eg needing write to denywrite etc)
> 
> And this is where things get really ugly of course :-).
> 
> For the CIFSFS client they're expecting to be able to
> just ship them to a Windows server, where they'll
> get the (insane) Windows semantics. These semantics
> are not what would be wanted on a local filesystem.
> 
> So unless we just say "these things have Windows
> semantics" (where openers of files can lock out others
> under dubious circumstances) there'll be this horrible
> difference between (I'm assuming) the sane semantics that
> are defined for local filesystems and the insane ones
> that you get when you're connecting remotely.
> 
> I don't know a good way to fix that, but I'm pretty
> sure you don't want the Windows semantics defined
> locally :-).

You could just flags these as "ignored on local filesystems"
of course, exact semantics defined by the remote filesystem.

That's really what applications will get anyway. But it's not
condusive to writing documentation on what these things do :-).

Jeremy.

Re: [PATCH 0/3] Add O_DENY* flags to fcntl and cifs

[Theodore Ts'o]

On Thu, Dec 06, 2012 at 11:57:52AM -0800, Jeremy Allison wrote:
> 
> And this is where things get really ugly of course :-).
> 
> For the CIFSFS client they're expecting to be able to
> just ship them to a Windows server, where they'll
> get the (insane) Windows semantics. These semantics
> are not what would be wanted on a local filesystem.

I'm confused; why would a userspace application need to be able to
request this behavior?  I can understand why an SMB client might
depend on this so it can use Windows' insane cache coherency scheme.
Are you trying to let Samba act as a middle man, where a remote file
system is mounted on Linux, and then Samba will try to act as a SMB
server, so you want to be able to pass through these semantics, i.e.:

   Windows SMB Server <--->  Linux cifs remote file system <--->
      Linux Samba server <---> Windows SMB client

Is this somewhat contrivuewd example the intended use case?  Or
something else?

					- Ted

Re: [PATCH 0/3] Add O_DENY* flags to fcntl and cifs

[Jeremy Allison]

On Thu, Dec 06, 2012 at 04:31:33PM -0500, Theodore Ts'o wrote:
> On Thu, Dec 06, 2012 at 11:57:52AM -0800, Jeremy Allison wrote:
> > 
> > And this is where things get really ugly of course :-).
> > 
> > For the CIFSFS client they're expecting to be able to
> > just ship them to a Windows server, where they'll
> > get the (insane) Windows semantics. These semantics
> > are not what would be wanted on a local filesystem.
> 
> I'm confused; why would a userspace application need to be able to
> request this behavior?

This isn't my proposal Ted, I'm just commenting on it :-).

Jeremy.

Re: [PATCH 0/3] Add O_DENY* flags to fcntl and cifs

[Theodore Ts'o]

On Thu, Dec 06, 2012 at 01:33:29PM -0800, Jeremy Allison wrote:
> > I'm confused; why would a userspace application need to be able to
> > request this behavior?
> 
> This isn't my proposal Ted, I'm just commenting on it :-).

Ah, sorry, I thought was coming from the Samba team.  :-)

Hmm... I see wine-devel is cc'ed; is this coming from the Wine team,
wanting to do SMB paravirtualization?  It would be useful if the
commit description described how these flags are intended to be used....

       		   	     	       	     - Ted

Re: [PATCH 0/3] Add O_DENY* flags to fcntl and cifs

[Jeremy Allison]

On Thu, Dec 06, 2012 at 04:37:27PM -0500, Theodore Ts'o wrote:
> On Thu, Dec 06, 2012 at 01:33:29PM -0800, Jeremy Allison wrote:
> > > I'm confused; why would a userspace application need to be able to
> > > request this behavior?
> > 
> > This isn't my proposal Ted, I'm just commenting on it :-).
> 
> Ah, sorry, I thought was coming from the Samba team.  :-)

Well it sort of is, as the people working on cifsfs are also
Samba Team members, but this isn't an official "Samba" thing,
as I can't see exactly what apps would want this either (other
than Samba smbd running on top of smbd, re-exporting a cifsfs
share, pointing to a Samba server, running on a... ERROR STACK
OVERFLOW :-).

> Hmm... I see wine-devel is cc'ed; is this coming from the Wine team,
> wanting to do SMB paravirtualization?  It would be useful if the
> commit description described how these flags are intended to be used....

Indeed :-).

Jeremy.

Re: [PATCH 0/3] Add O_DENY* flags to fcntl and cifs

[Steve French]

On Thu, Dec 6, 2012 at 1:57 PM, Jeremy Allison <jra@samba.org> wrote:
> On Thu, Dec 06, 2012 at 07:49:49PM +0000, Alan Cox wrote:
>> On Thu,  6 Dec 2012 22:26:28 +0400
>> Pavel Shilovsky <piastry@etersoft.ru> wrote:
>>
>> > Network filesystems CIFS, SMB2.0, SMB3.0 and NFSv4 have such flags - this change can benefit cifs and nfs modules. While this change is ok for network filesystems, itsn't not targeted for local filesystems due security problems (e.g. when a user process can deny root to delete a file).
>>
>> If I have my root fs on NFS then the same applies does it not.
>>
>> Your patches fail to describe the security semantics and what file rights
>> I must have to apply each option. How do I track down a lock user, what
>> tools are provided ? How do the new options interact with the security
>> layer?
>>
>> I don't have a problem with the idea, but it needs a lot more clear
>> description of how it works so the model can be checked and if need be
>> things tweaked (eg needing write to denywrite etc)
>
> And this is where things get really ugly of course :-).
>
> For the CIFSFS client they're expecting to be able to
> just ship them to a Windows server, where they'll
> get the (insane) Windows semantics. These semantics
> are not what would be wanted on a local filesystem.
>
> So unless we just say "these things have Windows
> semantics" (where openers of files can lock out others

I suspect that WINE would have the same need
to ship them to an NFS server as to a Windows server,
and the NFS4 protocol specification also defines these,
although I could not find the same level of detail that MS-FSA
provides (e.g. see section 2.14.10 for the detailed
description of how lock conflicts are checked) but the
semantics are probably the same.

-- 
Thanks,

Steve

Re: [PATCH 0/3] Add O_DENY* flags to fcntl and cifs

[Steve French]

On Fri, Dec 7, 2012 at 8:29 AM, Steve French <smfrench@gmail.com> wrote:
> although I could not find the same level of detail that MS-FSA
> provides (e.g. see section 2.14.10 for the detailed

Typo It is section 2.1.4.10


-- 
Thanks,

Steve

Re: [PATCH 0/3] Add O_DENY* flags to fcntl and cifs

[Alan Cox]

> I suspect that WINE would have the same need

Tricky - Wine needs to enforce this behaviour solely between Wine and
the file server, Trying to muck up non emulated local behaviour would be
a bad mistake.

One way perhaps to look at this is you want some tasks to be able to *opt
in* to this behaviour.

First however I think we need a much better description of the setup that
uses them and why it matters.

Re: [PATCH 0/3] Add O_DENY* flags to fcntl and cifs

[Pavel Shilovsky]

2012/12/6 Pavel Shilovsky <piastry@etersoft.ru>:
> Network filesystems CIFS, SMB2.0, SMB3.0 and NFSv4 have such flags - this change can benefit cifs and nfs modules. While this change is ok for network filesystems, itsn't not targeted for local filesystems due security problems (e.g. when a user process can deny root to delete a file).
>
> Share flags are used by Windows applications and WINE have to deal with them too. While WINE can process open share flags itself on local filesystems, it can't do it if a file stored on a network share and is used by several clients. This patchset makes it possible for CIFS/SMB2.0/SMB3.0.
>
> Pavel Shilovsky (3):
>   fcntl: Introduce new O_DENY* open flags for network filesystems
>   CIFS: Add O_DENY* open flags support
>   CIFS: Use NT_CREATE_ANDX command for forcemand mounts
>
>  fs/cifs/cifsacl.c                |   10 ++++----
>  fs/cifs/cifsglob.h               |   11 ++++++++-
>  fs/cifs/cifsproto.h              |    9 ++++----
>  fs/cifs/cifssmb.c                |   47 ++++++++++++++++++++------------------
>  fs/cifs/dir.c                    |   14 ++++++++----
>  fs/cifs/file.c                   |   18 ++++++++++-----
>  fs/cifs/inode.c                  |   11 +++++----
>  fs/cifs/link.c                   |   10 ++++----
>  fs/cifs/readdir.c                |    2 +-
>  fs/cifs/smb1ops.c                |   15 ++++++------
>  fs/cifs/smb2file.c               |   10 ++++----
>  fs/cifs/smb2inode.c              |    4 ++--
>  fs/cifs/smb2ops.c                |   10 ++++----
>  fs/cifs/smb2pdu.c                |    6 ++---
>  fs/cifs/smb2proto.h              |   14 +++++++-----
>  fs/fcntl.c                       |    5 ++--
>  include/uapi/asm-generic/fcntl.h |   11 +++++++++
>  17 files changed, 125 insertions(+), 82 deletions(-)
>
> --
> 1.7.10.4
>

First of all, sorry for being unclear at this proposal.


I am not from Wine team but I am working on things related to
Wine+CIFS-client+Samba.

We (at Etersoft) need to organize the work of Wine applications
through cifs-client share mounted to Samba (or Windows server). They
are related to accounting (mostly Russian ones - e.g.
http://www.1c.ru/eng/title.htm). So, the problem is that such
applications use share flags to control the parallel access to the
same files of several clients on a remote share. Also, there can be a
situation where Windows (native) clients and Wine clients are working
together in the same time.

That's why we need such flags in the kernel (patch #1). With these
flags Wine can pass them to every open and they will be used by CIFS
(and probably NFS) file systems to pass to the Samba server. In the
same time if the file is on local filesystem - these flags will be
simply ignored (not implemented).

Now we have to make our own builds of cifs module for every kernel
that use these flags passed by our build of Wine - this scheme is
working but requires merging every time new kernel is released.
Getting things into mainline let Wine supports more applications.

As for the security layer, I don't think that we need any extra bits
to switch these flags on or off - it will be switched on/off by an
underlying filesystem. Since, this change is targeted for a network
purpose only, a tool, that shows us what process/user locks a file,
doesn't help a lot because the file can be locked remotely.

As was said above, this change let us give Wine application share
flags possibility for both Samba and Windows servers. Patch #2 enables
share flags support for cifs mounts of Windows servers or Samba
servers with nounix mount option. But we already have forcemand mount
option in cifs module that switches on mandatory byte-range locking
semantic for Samba without nounix. Since share flags capability is a
kind of 'mandatory' locking scheme, I suggest that this option should
switch on share flags for Samba without nounix too (by using
NTCreateAndX command for open rather than transaction2) - that is what
patch #3 does.

-- 
Best regards,
Pavel Shilovsky.

Re: [PATCH 0/3] Add O_DENY* flags to fcntl and cifs

[J. Bruce Fields]

On Fri, Dec 07, 2012 at 01:08:46PM +0400, Pavel Shilovsky wrote:
> 2012/12/6 Pavel Shilovsky <piastry@etersoft.ru>:
> > Network filesystems CIFS, SMB2.0, SMB3.0 and NFSv4 have such flags - this change can benefit cifs and nfs modules. While this change is ok for network filesystems, itsn't not targeted for local filesystems due security problems (e.g. when a user process can deny root to delete a file).
> >
> > Share flags are used by Windows applications and WINE have to deal with them too. While WINE can process open share flags itself on local filesystems, it can't do it if a file stored on a network share and is used by several clients. This patchset makes it possible for CIFS/SMB2.0/SMB3.0.
> >
> > Pavel Shilovsky (3):
> >   fcntl: Introduce new O_DENY* open flags for network filesystems
> >   CIFS: Add O_DENY* open flags support
> >   CIFS: Use NT_CREATE_ANDX command for forcemand mounts
> >
> >  fs/cifs/cifsacl.c                |   10 ++++----
> >  fs/cifs/cifsglob.h               |   11 ++++++++-
> >  fs/cifs/cifsproto.h              |    9 ++++----
> >  fs/cifs/cifssmb.c                |   47 ++++++++++++++++++++------------------
> >  fs/cifs/dir.c                    |   14 ++++++++----
> >  fs/cifs/file.c                   |   18 ++++++++++-----
> >  fs/cifs/inode.c                  |   11 +++++----
> >  fs/cifs/link.c                   |   10 ++++----
> >  fs/cifs/readdir.c                |    2 +-
> >  fs/cifs/smb1ops.c                |   15 ++++++------
> >  fs/cifs/smb2file.c               |   10 ++++----
> >  fs/cifs/smb2inode.c              |    4 ++--
> >  fs/cifs/smb2ops.c                |   10 ++++----
> >  fs/cifs/smb2pdu.c                |    6 ++---
> >  fs/cifs/smb2proto.h              |   14 +++++++-----
> >  fs/fcntl.c                       |    5 ++--
> >  include/uapi/asm-generic/fcntl.h |   11 +++++++++
> >  17 files changed, 125 insertions(+), 82 deletions(-)
> >
> > --
> > 1.7.10.4
> >
> 
> First of all, sorry for being unclear at this proposal.
> 
> 
> I am not from Wine team but I am working on things related to
> Wine+CIFS-client+Samba.
> 
> We (at Etersoft) need to organize the work of Wine applications
> through cifs-client share mounted to Samba (or Windows server). They
> are related to accounting (mostly Russian ones - e.g.
> http://www.1c.ru/eng/title.htm). So, the problem is that such
> applications use share flags to control the parallel access to the
> same files of several clients on a remote share. Also, there can be a
> situation where Windows (native) clients and Wine clients are working
> together in the same time.
> 
> That's why we need such flags in the kernel (patch #1). With these
> flags Wine can pass them to every open and they will be used by CIFS
> (and probably NFS) file systems to pass to the Samba server. In the
> same time if the file is on local filesystem - these flags will be
> simply ignored (not implemented).

NFS supports the deny-read and deny-write bits but not deny-delete.

If we could do such opens in-kernel on local and clustered filesystems,
that could also be useful for multi-protocol (Samba and NFS) and
clustered exports.

Currently knfsd tries to enforce deny bits in the nfsd code, which is a
bit ugly.

And knfsd currently requires write permissions for deny-read.  My
understanding is that Windows considers that wrong, but I'd be curious
to know whether that breaks Windows applications in practice.

--b.

Re: [PATCH 0/3] Add O_DENY* flags to fcntl and cifs

[simo]

On Fri, 2012-12-07 at 09:52 -0500, J. Bruce Fields wrote:
> On Fri, Dec 07, 2012 at 01:08:46PM +0400, Pavel Shilovsky wrote:
> > 2012/12/6 Pavel Shilovsky <piastry@etersoft.ru>:
> > > Network filesystems CIFS, SMB2.0, SMB3.0 and NFSv4 have such flags - this change can benefit cifs and nfs modules. While this change is ok for network filesystems, itsn't not targeted for local filesystems due security problems (e.g. when a user process can deny root to delete a file).
> > >
> > > Share flags are used by Windows applications and WINE have to deal with them too. While WINE can process open share flags itself on local filesystems, it can't do it if a file stored on a network share and is used by several clients. This patchset makes it possible for CIFS/SMB2.0/SMB3.0.
> > >
> > > Pavel Shilovsky (3):
> > >   fcntl: Introduce new O_DENY* open flags for network filesystems
> > >   CIFS: Add O_DENY* open flags support
> > >   CIFS: Use NT_CREATE_ANDX command for forcemand mounts
> > >
> > >  fs/cifs/cifsacl.c                |   10 ++++----
> > >  fs/cifs/cifsglob.h               |   11 ++++++++-
> > >  fs/cifs/cifsproto.h              |    9 ++++----
> > >  fs/cifs/cifssmb.c                |   47 ++++++++++++++++++++------------------
> > >  fs/cifs/dir.c                    |   14 ++++++++----
> > >  fs/cifs/file.c                   |   18 ++++++++++-----
> > >  fs/cifs/inode.c                  |   11 +++++----
> > >  fs/cifs/link.c                   |   10 ++++----
> > >  fs/cifs/readdir.c                |    2 +-
> > >  fs/cifs/smb1ops.c                |   15 ++++++------
> > >  fs/cifs/smb2file.c               |   10 ++++----
> > >  fs/cifs/smb2inode.c              |    4 ++--
> > >  fs/cifs/smb2ops.c                |   10 ++++----
> > >  fs/cifs/smb2pdu.c                |    6 ++---
> > >  fs/cifs/smb2proto.h              |   14 +++++++-----
> > >  fs/fcntl.c                       |    5 ++--
> > >  include/uapi/asm-generic/fcntl.h |   11 +++++++++
> > >  17 files changed, 125 insertions(+), 82 deletions(-)
> > >
> > > --
> > > 1.7.10.4
> > >
> > 
> > First of all, sorry for being unclear at this proposal.
> > 
> > 
> > I am not from Wine team but I am working on things related to
> > Wine+CIFS-client+Samba.
> > 
> > We (at Etersoft) need to organize the work of Wine applications
> > through cifs-client share mounted to Samba (or Windows server). They
> > are related to accounting (mostly Russian ones - e.g.
> > http://www.1c.ru/eng/title.htm). So, the problem is that such
> > applications use share flags to control the parallel access to the
> > same files of several clients on a remote share. Also, there can be a
> > situation where Windows (native) clients and Wine clients are working
> > together in the same time.
> > 
> > That's why we need such flags in the kernel (patch #1). With these
> > flags Wine can pass them to every open and they will be used by CIFS
> > (and probably NFS) file systems to pass to the Samba server. In the
> > same time if the file is on local filesystem - these flags will be
> > simply ignored (not implemented).
> 
> NFS supports the deny-read and deny-write bits but not deny-delete.
> 
> If we could do such opens in-kernel on local and clustered filesystems,
> that could also be useful for multi-protocol (Samba and NFS) and
> clustered exports.
> 
> Currently knfsd tries to enforce deny bits in the nfsd code, which is a
> bit ugly.
> 
> And knfsd currently requires write permissions for deny-read.  My
> understanding is that Windows considers that wrong, but I'd be curious
> to know whether that breaks Windows applications in practice.

It probably does if you look hard enough.
IIRC Deny-reads are very loosely like read locks, and you can take read
locks if you have read-only access.
Why does knfsd restrict deny-read to read-write access ?

Simo.

-- 
Simo Sorce
Samba Team GPL Compliance Officer <simo@samba.org>
Principal Software Engineer at Red Hat, Inc. <simo@redhat.com>

Re: [PATCH 0/3] Add O_DENY* flags to fcntl and cifs

[J. Bruce Fields]

On Fri, Dec 07, 2012 at 10:37:45AM -0500, simo wrote:
> On Fri, 2012-12-07 at 09:52 -0500, J. Bruce Fields wrote:
> > On Fri, Dec 07, 2012 at 01:08:46PM +0400, Pavel Shilovsky wrote:
> > > 2012/12/6 Pavel Shilovsky <piastry@etersoft.ru>:
> > > > Network filesystems CIFS, SMB2.0, SMB3.0 and NFSv4 have such flags - this change can benefit cifs and nfs modules. While this change is ok for network filesystems, itsn't not targeted for local filesystems due security problems (e.g. when a user process can deny root to delete a file).
> > > >
> > > > Share flags are used by Windows applications and WINE have to deal with them too. While WINE can process open share flags itself on local filesystems, it can't do it if a file stored on a network share and is used by several clients. This patchset makes it possible for CIFS/SMB2.0/SMB3.0.
> > > >
> > > > Pavel Shilovsky (3):
> > > >   fcntl: Introduce new O_DENY* open flags for network filesystems
> > > >   CIFS: Add O_DENY* open flags support
> > > >   CIFS: Use NT_CREATE_ANDX command for forcemand mounts
> > > >
> > > >  fs/cifs/cifsacl.c                |   10 ++++----
> > > >  fs/cifs/cifsglob.h               |   11 ++++++++-
> > > >  fs/cifs/cifsproto.h              |    9 ++++----
> > > >  fs/cifs/cifssmb.c                |   47 ++++++++++++++++++++------------------
> > > >  fs/cifs/dir.c                    |   14 ++++++++----
> > > >  fs/cifs/file.c                   |   18 ++++++++++-----
> > > >  fs/cifs/inode.c                  |   11 +++++----
> > > >  fs/cifs/link.c                   |   10 ++++----
> > > >  fs/cifs/readdir.c                |    2 +-
> > > >  fs/cifs/smb1ops.c                |   15 ++++++------
> > > >  fs/cifs/smb2file.c               |   10 ++++----
> > > >  fs/cifs/smb2inode.c              |    4 ++--
> > > >  fs/cifs/smb2ops.c                |   10 ++++----
> > > >  fs/cifs/smb2pdu.c                |    6 ++---
> > > >  fs/cifs/smb2proto.h              |   14 +++++++-----
> > > >  fs/fcntl.c                       |    5 ++--
> > > >  include/uapi/asm-generic/fcntl.h |   11 +++++++++
> > > >  17 files changed, 125 insertions(+), 82 deletions(-)
> > > >
> > > > --
> > > > 1.7.10.4
> > > >
> > > 
> > > First of all, sorry for being unclear at this proposal.
> > > 
> > > 
> > > I am not from Wine team but I am working on things related to
> > > Wine+CIFS-client+Samba.
> > > 
> > > We (at Etersoft) need to organize the work of Wine applications
> > > through cifs-client share mounted to Samba (or Windows server). They
> > > are related to accounting (mostly Russian ones - e.g.
> > > http://www.1c.ru/eng/title.htm). So, the problem is that such
> > > applications use share flags to control the parallel access to the
> > > same files of several clients on a remote share. Also, there can be a
> > > situation where Windows (native) clients and Wine clients are working
> > > together in the same time.
> > > 
> > > That's why we need such flags in the kernel (patch #1). With these
> > > flags Wine can pass them to every open and they will be used by CIFS
> > > (and probably NFS) file systems to pass to the Samba server. In the
> > > same time if the file is on local filesystem - these flags will be
> > > simply ignored (not implemented).
> > 
> > NFS supports the deny-read and deny-write bits but not deny-delete.
> > 
> > If we could do such opens in-kernel on local and clustered filesystems,
> > that could also be useful for multi-protocol (Samba and NFS) and
> > clustered exports.
> > 
> > Currently knfsd tries to enforce deny bits in the nfsd code, which is a
> > bit ugly.
> > 
> > And knfsd currently requires write permissions for deny-read.  My
> > understanding is that Windows considers that wrong, but I'd be curious
> > to know whether that breaks Windows applications in practice.
> 
> It probably does if you look hard enough.
> IIRC Deny-reads are very loosely like read locks, and you can take read
> locks if you have read-only access.

I had the impression they didn't care about read or write permissions at
all, but I don't know.

> Why does knfsd restrict deny-read to read-write access ?

Because I think it's normal to want files that everyone's able to read
without giving everyone the right to DOS readers indefinitely.

And because I'm having a hard time thinking why you'd care to keep
everyone from reading a file that you don't intend to modify.

--b.

Re: [PATCH 0/3] Add O_DENY* flags to fcntl and cifs

[Christoph Hellwig]

On Thu, Dec 06, 2012 at 10:26:28PM +0400, Pavel Shilovsky wrote:
> Network filesystems CIFS, SMB2.0, SMB3.0 and NFSv4 have such flags - this change can benefit cifs and nfs modules. While this change is ok for network filesystems, itsn't not targeted for local filesystems due security problems (e.g. when a user process can deny root to delete a file).
> 
> Share flags are used by Windows applications and WINE have to deal with them too. While WINE can process open share flags itself on local filesystems, it can't do it if a file stored on a network share and is used by several clients. This patchset makes it possible for CIFS/SMB2.0/SMB3.0.

I don't think introducing user visible flags that are only supported on
a single network filesystem is a good idea.

I'm not even sure adding these flags does make a lot of sense, but
assuming we'd actually want this (and I'd like some more detailed
explanation) I think we'd at least need to make sure that:

 a) opening files with the new modes gives a proper error message if not
    supported
 b) there needs to be local support for them as well
 c) we need to think really hard when they should be supported, and need
    a good rational for it.  I can't see how we could do it
    unconditionally for all users as that would introduce easy denial
    of services attacks the way I understand the semantics (correct me
    if I'm wrong).  So a mount option like you currently do probably is
    the least bad even if don't fell overly happy about that version.

What is the reason your special wine use case can't simply use a
userspace cifs client?  Given that wine uses windows filesystem
semantics and cifs does as well tunnelling it through a Posix-like API
inbetween is never going to be perfect.

Re: [PATCH 0/3] Add O_DENY* flags to fcntl and cifs

[Pavel Shilovsky]

Christoph Hellwig писал 07.12.2012 20:16:
> On Thu, Dec 06, 2012 at 10:26:28PM +0400, Pavel Shilovsky wrote:
>> Network filesystems CIFS, SMB2.0, SMB3.0 and NFSv4 have such flags - 
>> this change can benefit cifs and nfs modules. While this change is ok 
>> for network filesystems, itsn't not targeted for local filesystems due 
>> security problems (e.g. when a user process can deny root to delete a 
>> file).
>>
>> Share flags are used by Windows applications and WINE have to deal 
>> with them too. While WINE can process open share flags itself on local 
>> filesystems, it can't do it if a file stored on a network share and is 
>> used by several clients. This patchset makes it possible for 
>> CIFS/SMB2.0/SMB3.0.
>
> I don't think introducing user visible flags that are only supported 
> on
> a single network filesystem is a good idea.

It can bring benefits for both CIFS and NFS filesystems - so, at least 
two ones.

>
> I'm not even sure adding these flags does make a lot of sense, but
> assuming we'd actually want this (and I'd like some more detailed
> explanation) I think we'd at least need to make sure that:
>
>  a) opening files with the new modes gives a proper error message if 
> not
>     supported

It makes us add such checks for all other filesystems, if I understand 
right, - not a problem, I think.

>  b) there needs to be local support for them as well
>  c) we need to think really hard when they should be supported, and 
> need
>     a good rational for it.  I can't see how we could do it
>     unconditionally for all users as that would introduce easy denial
>     of services attacks the way I understand the semantics (correct 
> me
>     if I'm wrong).  So a mount option like you currently do probably 
> is
>     the least bad even if don't fell overly happy about that version.
>
> What is the reason your special wine use case can't simply use a
> userspace cifs client?  Given that wine uses windows filesystem
> semantics and cifs does as well tunnelling it through a Posix-like 
> API
> inbetween is never going to be perfect.

Ideally we should not make any difference between underlying 
filesystems in Wine: an application requests an open of the file and we 
issue this open with flags it passed. Since Wineserver can process share 
flags locally itself (for one linux user), we only need to add this 
support for CIFS (that is actively used by Wine applications because of 
it's Windows nature). Bringing these flags for local filesystems can 
benefit Wine too: it will help in cases when Wine applications of 
different users on the same machine use the same file and can make all 
those things easier, of course.

The problem is the possibility of denial-of-service attacks here. We 
can try to prevent them by:
1) specifying an extra security bit on the file that indicates that 
share flags are accepted (like we have for mandatory locks now) and 
setting it for neccessary files only, or
2) adding a special mount option (but it it probably makes sense if we 
decided to add this support for CIFS and NFS only).

--
Best regards,
Pavel Shilovsky.

Re: [PATCH 0/3] Add O_DENY* flags to fcntl and cifs

[Alan Cox]

> The problem is the possibility of denial-of-service attacks here. We 
> can try to prevent them by:
> 1) specifying an extra security bit on the file that indicates that 
> share flags are accepted (like we have for mandatory locks now) and 
> setting it for neccessary files only, or
> 2) adding a special mount option (but it it probably makes sense if we 
> decided to add this support for CIFS and NFS only).

3) making it a property that the process opts into - the same fs can have
Linux and wine users at once.

But for such a big set of changes and with the kind of potential fallout
you need to demonstrate a good practical use case IMHO, not just a "neat
idea I had".

Alan

RE: [PATCH 0/3] Add O_DENY* flags to fcntl and cifs

[Myklebust, Trond]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset="utf-8", Size: 3988 bytes --]

> -----Original Message-----
> From: linux-nfs-owner@vger.kernel.org [mailto:linux-nfs-
> owner@vger.kernel.org] On Behalf Of Pavel Shilovsky
> Sent: Friday, December 07, 2012 9:43 PM
> To: Christoph Hellwig
> Cc: linux-cifs@vger.kernel.org; linux-kernel@vger.kernel.org; linux-
> fsdevel@vger.kernel.org; wine-devel@winehq.org; linux-
> nfs@vger.kernel.org
> Subject: Re: [PATCH 0/3] Add O_DENY* flags to fcntl and cifs
> 
> Christoph Hellwig писал 07.12.2012 20:16:
> > On Thu, Dec 06, 2012 at 10:26:28PM +0400, Pavel Shilovsky wrote:
> >> Network filesystems CIFS, SMB2.0, SMB3.0 and NFSv4 have such flags -
> >> this change can benefit cifs and nfs modules. While this change is ok
> >> for network filesystems, itsn't not targeted for local filesystems
> >> due security problems (e.g. when a user process can deny root to
> >> delete a file).
> >>
> >> Share flags are used by Windows applications and WINE have to deal
> >> with them too. While WINE can process open share flags itself on
> >> local filesystems, it can't do it if a file stored on a network share
> >> and is used by several clients. This patchset makes it possible for
> >> CIFS/SMB2.0/SMB3.0.
> >
> > I don't think introducing user visible flags that are only supported
> > on a single network filesystem is a good idea.
> 
> It can bring benefits for both CIFS and NFS filesystems - so, at least two ones.
> 
> >
> > I'm not even sure adding these flags does make a lot of sense, but
> > assuming we'd actually want this (and I'd like some more detailed
> > explanation) I think we'd at least need to make sure that:
> >
> >  a) opening files with the new modes gives a proper error message if
> > not
> >     supported
> 
> It makes us add such checks for all other filesystems, if I understand right, -
> not a problem, I think.
> 
> >  b) there needs to be local support for them as well
> >  c) we need to think really hard when they should be supported, and
> > need
> >     a good rational for it.  I can't see how we could do it
> >     unconditionally for all users as that would introduce easy denial
> >     of services attacks the way I understand the semantics (correct me
> >     if I'm wrong).  So a mount option like you currently do probably
> > is
> >     the least bad even if don't fell overly happy about that version.
> >
> > What is the reason your special wine use case can't simply use a
> > userspace cifs client?  Given that wine uses windows filesystem
> > semantics and cifs does as well tunnelling it through a Posix-like API
> > inbetween is never going to be perfect.
> 
> Ideally we should not make any difference between underlying filesystems
> in Wine: an application requests an open of the file and we issue this open
> with flags it passed. Since Wineserver can process share flags locally itself (for
> one linux user), we only need to add this support for CIFS (that is actively
> used by Wine applications because of it's Windows nature). Bringing these
> flags for local filesystems can benefit Wine too: it will help in cases when
> Wine applications of different users on the same machine use the same file
> and can make all those things easier, of course.
> 
> The problem is the possibility of denial-of-service attacks here. We can try to
> prevent them by:
> 1) specifying an extra security bit on the file that indicates that share flags are
> accepted (like we have for mandatory locks now) and setting it for
> neccessary files only, or
> 2) adding a special mount option (but it it probably makes sense if we
> decided to add this support for CIFS and NFS only).

Why not just put it under the control of LSM? It seems to me that this doesn't so much want to be a per-mount switch but rather deserves to be a per-process MAC (i.e. is this running in a Wine sandbox or not)...

Cheers
   Trond
ÿôèº{.nÇ+‰·Ÿ®‰­†+%ŠËÿ±éݶ\x17¥Šwÿº{.nÇ+‰·¥Š{±þG«éÿŠ{ayº\x1dʇڙë,j\a­¢f£¢·hšïêÿ‘êçz_è®\x03(­éšŽŠÝ¢j"ú\x1a¶^[m§ÿÿ¾\a«þG«éÿ¢¸?™¨è­Ú&£ø§~á¶iO•æ¬z·švØ^\x14\x04\x1a¶^[m§ÿÿÃ\fÿ¶ìÿ¢¸?–I¥

Re: [PATCH 0/3] Add O_DENY* flags to fcntl and cifs

[J. Bruce Fields]

On Sat, Dec 08, 2012 at 12:43:14AM +0400, Pavel Shilovsky wrote:
> The problem is the possibility of denial-of-service attacks here. We
> can try to prevent them by:
> 1) specifying an extra security bit on the file that indicates that
> share flags are accepted (like we have for mandatory locks now) and
> setting it for neccessary files only, or
> 2) adding a special mount option (but it it probably makes sense if
> we decided to add this support for CIFS and NFS only).

In the case of knfsd and samba exporting a common filesystem, you'd also
want to be able to enforce it on the exported filesystem.

--b.

Re: [PATCH 0/3] Add O_DENY* flags to fcntl and cifs

[Jeff Layton]

On Mon, 10 Dec 2012 11:41:16 -0500
"J. Bruce Fields" <bfields@fieldses.org> wrote:

> On Sat, Dec 08, 2012 at 12:43:14AM +0400, Pavel Shilovsky wrote:
> > The problem is the possibility of denial-of-service attacks here. We
> > can try to prevent them by:
> > 1) specifying an extra security bit on the file that indicates that
> > share flags are accepted (like we have for mandatory locks now) and
> > setting it for neccessary files only, or
> > 2) adding a special mount option (but it it probably makes sense if
> > we decided to add this support for CIFS and NFS only).
> 
> In the case of knfsd and samba exporting a common filesystem, you'd also
> want to be able to enforce it on the exported filesystem.
> 

Sorry for chiming in late on this, but I've been looking at this
problem from the other end as well, from the POV of a fileserver. For
there, you absolutely do want to have some mechanism for enforcing this
on local filesystems. 

Currently, file servers generally enforce share reservations
internally. The upshot is that they're not aware when other tasks
outside the server modify a file. This is also problematic too in many
common fileserving situations -- when exporting files via both NFS and
SMB, for instance.

One thing that's important to note is that there is already some
support for this in the kernel. The LOCK_MAND flag and its associates
are intended for just this purpose. Samba even already calls into the
kernel to set LOCK_MAND locks, but the kernel just passes them through
to the fs. Rumor has it that GPFS does something with these flags, but
I can't confirm that.

Of course, LOCK_MAND is racy since you can't set it on open(), but it
might be nice to use that as a starting point for trying to add this
support.

At the very least, if we're going to do this, we need to consider what
to do with the LOCK_MAND interfaces. As a starting point for
discussion, here's a patch that I was playing with a few months ago. I
haven't had much time to really spend on this project, but it may be
worthwhile to consider. It works, but I'm not sure about the
semantics...

-----------------------------[snip]--------------------------------

locks: add enforcement of LOCK_MAND locks

The LOCK_MAND lock mechanism is currently a no-op for any in-tree
filesystem. The flags are passed to f_ops->flock, but the standard
flock routines basically ignore them.

Change this by adding enforcement against other LOCK_MAND locks. Also,
assume that LOCK_MAND also implies LOCK_NB.

Signed-off-by: Jeff Layton <jlayton@redhat.com>
---
 fs/locks.c | 45 ++++++++++++++++++++++++++++++++++++++++++---
 1 file changed, 42 insertions(+), 3 deletions(-)

diff --git a/fs/locks.c b/fs/locks.c
index 814c51d..736e38b 100644
--- a/fs/locks.c
+++ b/fs/locks.c
@@ -625,6 +625,43 @@ static int posix_locks_conflict(struct file_lock *caller_fl, struct file_lock *s
 	return (locks_conflict(caller_fl, sys_fl));
 }
 
+/*
+ * locks_mand_conflict - Determine if there's a share reservation conflict
+ * @caller_fl: lock we're attempting to acquire
+ * @sys_fl: lock already present on system that we're checking against
+ *
+ * Check to see if there's a share_reservation conflict. LOCK_READ/LOCK_WRITE
+ * tell us whether the reservation allows other readers and writers.
+ *
+ * We only check against other LOCK_MAND locks, so applications that want to
+ * use share mode locking will only conflict against one another. "normal"
+ * applications that open files won't be affected by and won't themselves
+ * affect the share reservations.
+ */
+static int locks_mand_conflict(struct file_lock *caller_fl,
+				struct file_lock *sys_fl)
+{
+	unsigned char caller_type = caller_fl->fl_type;
+	unsigned char sys_type = sys_fl->fl_type;
+	fmode_t	caller_fmode = caller_fl->fl_file->f_mode;
+	fmode_t sys_fmode = sys_fl->fl_file->f_mode;
+
+	/* they can only conflict if they're both LOCK_MAND */
+	if (!(caller_type & LOCK_MAND) || !(sys_type & LOCK_MAND))
+		return 0;
+
+	if (!(caller_type & LOCK_READ) && (sys_fmode & FMODE_READ))
+		return 1;
+	if (!(caller_type & LOCK_WRITE) && (sys_fmode & FMODE_WRITE))
+		return 1;
+	if (!(sys_type & LOCK_READ) && (caller_fmode & FMODE_READ))
+		return 1;
+	if (!(sys_type & LOCK_WRITE) && (caller_fmode & FMODE_WRITE))
+		return 1;
+
+	return 0;
+}
+
 /* Determine if lock sys_fl blocks lock caller_fl. FLOCK specific
  * checking before calling the locks_conflict().
  */
@@ -633,9 +670,11 @@ static int flock_locks_conflict(struct file_lock *caller_fl, struct file_lock *s
 	/* FLOCK locks referring to the same filp do not conflict with
 	 * each other.
 	 */
-	if (!IS_FLOCK(sys_fl) || (caller_fl->fl_file == sys_fl->fl_file))
-		return (0);
+	if (!IS_FLOCK(sys_fl))
+		return 0;
 	if ((caller_fl->fl_type & LOCK_MAND) || (sys_fl->fl_type & LOCK_MAND))
+		return locks_mand_conflict(caller_fl, sys_fl);
+	if (caller_fl->fl_file == sys_fl->fl_file)
 		return 0;
 
 	return (locks_conflict(caller_fl, sys_fl));
@@ -1646,7 +1685,7 @@ SYSCALL_DEFINE2(flock, unsigned int, fd, unsigned int, cmd)
 	if (!filp)
 		goto out;
 
-	can_sleep = !(cmd & LOCK_NB);
+	can_sleep = !(cmd & (LOCK_NB|LOCK_MAND));
 	cmd &= ~LOCK_NB;
 	unlock = (cmd == LOCK_UN);
 
-- 
1.7.11.7

Re: [PATCH 0/3] Add O_DENY* flags to fcntl and cifs

[David Laight]

On Sat, Dec 08, 2012 at 12:43:14AM +0400, Pavel Shilovsky wrote:
> 
> The problem is the possibility of denial-of-service attacks here. We 
> can try to prevent them by:

FWIW I already see a DoS 'attack'.
I have some filestore shared using NFS (to Linux and Solaris) and 
using samba (to Windows).

I use it for release builds of a product to ensure the versions
built for the different operating systems match, and because some
files have to be built on an 'alien' system (eg gcc targetted at
embedded card).

I can't run the windows build at the same time as the others
because the windows C compiler manages to obtain exclusive access
to the source files - stopping the other systems from reading them.

	David

-- 
David Laight: david@l8s.co.uk

Re: [PATCH 0/3] Add O_DENY* flags to fcntl and cifs

[Pavel Shilovsky]

2012/12/12 David Laight <david@l8s.co.uk>:
> On Sat, Dec 08, 2012 at 12:43:14AM +0400, Pavel Shilovsky wrote:
>>
>> The problem is the possibility of denial-of-service attacks here. We
>> can try to prevent them by:
>
> FWIW I already see a DoS 'attack'.
> I have some filestore shared using NFS (to Linux and Solaris) and
> using samba (to Windows).
>
> I use it for release builds of a product to ensure the versions
> built for the different operating systems match, and because some
> files have to be built on an 'alien' system (eg gcc targetted at
> embedded card).
>
> I can't run the windows build at the same time as the others
> because the windows C compiler manages to obtain exclusive access
> to the source files - stopping the other systems from reading them.

We can make this feature (passing O_DENY* flags received from clients
to filesystem) can be turned on/off on Samba/NFS server to let this
particular use case work. In general, I think we really need to be
sure that nobody has a read access for files that a Windows process
opened with O_DENYREAD (because there can be important reasons for the
Windows process to do so).

-- 
Best regards,
Pavel Shilovsky.

Re: [PATCH 0/3] Add O_DENY* flags to fcntl and cifs

[Alan Cox]

> We can make this feature (passing O_DENY* flags received from clients
> to filesystem) can be turned on/off on Samba/NFS server to let this
> particular use case work. In general, I think we really need to be
> sure that nobody has a read access for files that a Windows process
> opened with O_DENYREAD (because there can be important reasons for the
> Windows process to do so).

It should only affect windows emulated tasks, nothing else. If you want
to do locking friendly behaviour you could also take lockf locks so that
Linux domain code that *wishes* to be nice in this area behaves the way
you want.

Without that you break so much stuff its a horrible idea (eg backups,
ubuntu one sync etc).

Alan

Re: [PATCH 0/3] Add O_DENY* flags to fcntl and cifs

[Steve French]

On Fri, Dec 14, 2012 at 9:30 AM, Alan Cox <alan@lxorguk.ukuu.org.uk> wrote:
>> We can make this feature (passing O_DENY* flags received from clients
>> to filesystem) can be turned on/off on Samba/NFS server to let this
>> particular use case work. In general, I think we really need to be
>> sure that nobody has a read access for files that a Windows process
>> opened with O_DENYREAD (because there can be important reasons for the
>> Windows process to do so).
>
> It should only affect windows emulated tasks, nothing else

yes, but not just wine - there is probably a case for Samba server and
NFSv4 to optionally request such behafvior).   Also we are likely to
see more cases where users want to run Samba over an NFS mount and
vice versa.

-- 
Thanks,

Steve

Re: [PATCH 0/3] Add O_DENY* flags to fcntl and cifs

[J. Bruce Fields]

On Fri, Dec 14, 2012 at 01:19:18PM -0600, Steve French wrote:
> On Fri, Dec 14, 2012 at 9:30 AM, Alan Cox <alan@lxorguk.ukuu.org.uk> wrote:
> >> We can make this feature (passing O_DENY* flags received from clients
> >> to filesystem) can be turned on/off on Samba/NFS server to let this
> >> particular use case work. In general, I think we really need to be
> >> sure that nobody has a read access for files that a Windows process
> >> opened with O_DENYREAD (because there can be important reasons for the
> >> Windows process to do so).
> >
> > It should only affect windows emulated tasks, nothing else
> 
> yes, but not just wine - there is probably a case for Samba server and
> NFSv4 to optionally request such behafvior).

Agreed, but:

> Also we are likely to
> see more cases where users want to run Samba over an NFS mount and
> vice versa.

I don't personally see the interest in this case.

(And in fact I'd rather we removed the nfs export code for cifs; I seem
to recall from the last discussion that filehandle lookups get ESTALE
for inodes that have gone out of cache, and that that wasn't really
fixable.)

--b.