All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2 1/8] locks: make flock_lock_file take is_conflict callback parm
@ 2013-01-17 16:52 ` Pavel Shilovsky
  0 siblings, 0 replies; 32+ messages in thread
From: Pavel Shilovsky @ 2013-01-17 16:52 UTC (permalink / raw)
  To: linux-kernel-u79uwXL29TY76Z2rM5mHXA
  Cc: linux-fsdevel-u79uwXL29TY76Z2rM5mHXA,
	linux-cifs-u79uwXL29TY76Z2rM5mHXA,
	linux-nfs-u79uwXL29TY76Z2rM5mHXA,
	wine-devel-5vRYHf7vrtgdnm+yROfE0A

This parm demetermines how to check if locks have conflicts.
This let us use flock_lock_file funtions further to add
O_DENY* flags support through flocks.

Signed-off-by: Pavel Shilovsky <piastry-7qunaywFIewox3rIn2DAYQ@public.gmane.org>
---
 fs/locks.c | 14 ++++++++++----
 1 file changed, 10 insertions(+), 4 deletions(-)

diff --git a/fs/locks.c b/fs/locks.c
index a94e331..9edfec4 100644
--- a/fs/locks.c
+++ b/fs/locks.c
@@ -697,14 +697,20 @@ static int posix_locks_deadlock(struct file_lock *caller_fl,
 	return 0;
 }
 
-/* Try to create a FLOCK lock on filp. We always insert new FLOCK locks
+/*
+ * Try to create a FLOCK lock on filp. We always insert new FLOCK locks
  * after any leases, but before any posix locks.
  *
  * Note that if called with an FL_EXISTS argument, the caller may determine
  * whether or not a lock was successfully freed by testing the return
  * value for -ENOENT.
+ *
+ * Take @is_conflict callback that determines how to check if locks have
+ * conflicts or not.
  */
-static int flock_lock_file(struct file *filp, struct file_lock *request)
+static int
+flock_lock_file(struct file *filp, struct file_lock *request,
+		int (*is_conflict)(struct file_lock *, struct file_lock *))
 {
 	struct file_lock *new_fl = NULL;
 	struct file_lock **before;
@@ -760,7 +766,7 @@ find_conflict:
 			break;
 		if (IS_LEASE(fl))
 			continue;
-		if (!flock_locks_conflict(request, fl))
+		if (!is_conflict(request, fl))
 			continue;
 		error = -EAGAIN;
 		if (!(request->fl_flags & FL_SLEEP))
@@ -1589,7 +1595,7 @@ int flock_lock_file_wait(struct file *filp, struct file_lock *fl)
 	int error;
 	might_sleep();
 	for (;;) {
-		error = flock_lock_file(filp, fl);
+		error = flock_lock_file(filp, fl, flock_locks_conflict);
 		if (error != FILE_LOCK_DEFERRED)
 			break;
 		error = wait_event_interruptible(fl->fl_wait, !fl->fl_next);
-- 
1.8.1.1

^ permalink raw reply related	[flat|nested] 32+ messages in thread
* [PATCH v2 1/8] locks: make flock_lock_file take is_conflict callback parm
@ 2013-01-17 16:52 ` Pavel Shilovsky
  0 siblings, 0 replies; 32+ messages in thread
From: Pavel Shilovsky @ 2013-01-17 16:52 UTC (permalink / raw)
  To: linux-kernel; +Cc: linux-fsdevel, linux-cifs, linux-nfs, wine-devel

This parm demetermines how to check if locks have conflicts.
This let us use flock_lock_file funtions further to add
O_DENY* flags support through flocks.

Signed-off-by: Pavel Shilovsky <piastry@etersoft.ru>
---
 fs/locks.c | 14 ++++++++++----
 1 file changed, 10 insertions(+), 4 deletions(-)

diff --git a/fs/locks.c b/fs/locks.c
index a94e331..9edfec4 100644
--- a/fs/locks.c
+++ b/fs/locks.c
@@ -697,14 +697,20 @@ static int posix_locks_deadlock(struct file_lock *caller_fl,
 	return 0;
 }
 
-/* Try to create a FLOCK lock on filp. We always insert new FLOCK locks
+/*
+ * Try to create a FLOCK lock on filp. We always insert new FLOCK locks
  * after any leases, but before any posix locks.
  *
  * Note that if called with an FL_EXISTS argument, the caller may determine
  * whether or not a lock was successfully freed by testing the return
  * value for -ENOENT.
+ *
+ * Take @is_conflict callback that determines how to check if locks have
+ * conflicts or not.
  */
-static int flock_lock_file(struct file *filp, struct file_lock *request)
+static int
+flock_lock_file(struct file *filp, struct file_lock *request,
+		int (*is_conflict)(struct file_lock *, struct file_lock *))
 {
 	struct file_lock *new_fl = NULL;
 	struct file_lock **before;
@@ -760,7 +766,7 @@ find_conflict:
 			break;
 		if (IS_LEASE(fl))
 			continue;
-		if (!flock_locks_conflict(request, fl))
+		if (!is_conflict(request, fl))
 			continue;
 		error = -EAGAIN;
 		if (!(request->fl_flags & FL_SLEEP))
@@ -1589,7 +1595,7 @@ int flock_lock_file_wait(struct file *filp, struct file_lock *fl)
 	int error;
 	might_sleep();
 	for (;;) {
-		error = flock_lock_file(filp, fl);
+		error = flock_lock_file(filp, fl, flock_locks_conflict);
 		if (error != FILE_LOCK_DEFERRED)
 			break;
 		error = wait_event_interruptible(fl->fl_wait, !fl->fl_next);
-- 
1.8.1.1


^ permalink raw reply related	[flat|nested] 32+ messages in thread
* [PATCH v2 2/8] fcntl: Introduce new O_DENY* open flags
  2013-01-17 16:52 ` Pavel Shilovsky
  (?)
@ 2013-01-17 16:52 ` Pavel Shilovsky
  -1 siblings, 0 replies; 32+ messages in thread
From: Pavel Shilovsky @ 2013-01-17 16:52 UTC (permalink / raw)
  To: linux-kernel; +Cc: linux-fsdevel, linux-cifs, linux-nfs, wine-devel

This patch adds 4 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,
4) O_DENYMAND that enables O_DENY* flags checks.

Network filesystems CIFS, SMB2.0, SMB3.0 and NFSv4 have such flags -
this change can benefit cifs and nfs modules as well as Samba and
NFS file servers. These flags are only take affect for opens with
O_DENYMAND flags - there is no impact on native Linux opens.

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

diff --git a/fs/fcntl.c b/fs/fcntl.c
index 71a600a..d88a548 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(23 - 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	| O_DENYMAND
 		));
 
 	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..6e4e979 100644
--- a/include/uapi/asm-generic/fcntl.h
+++ b/include/uapi/asm-generic/fcntl.h
@@ -84,6 +84,20 @@
 #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_DENYMAND
+#define O_DENYMAND	0400000000	/* Enable O_DENY flags checks */
+#endif
+
 #ifndef O_NDELAY
 #define O_NDELAY	O_NONBLOCK
 #endif
-- 
1.8.1.1

^ permalink raw reply related	[flat|nested] 32+ messages in thread
* [PATCH v2 3/8] vfs: Add O_DENYREAD/WRITE flags support for open syscall
  2013-01-17 16:52 ` Pavel Shilovsky
  (?)
  (?)
@ 2013-01-17 16:52 ` Pavel Shilovsky
       [not found]   ` <1358441584-8783-3-git-send-email-piastry-7qunaywFIewox3rIn2DAYQ@public.gmane.org>
  -1 siblings, 1 reply; 32+ messages in thread
From: Pavel Shilovsky @ 2013-01-17 16:52 UTC (permalink / raw)
  To: linux-kernel; +Cc: linux-fsdevel, linux-cifs, linux-nfs, wine-devel

If O_DENYMAND flag is specified, O_DENYREAD/WRITE/MAND flags are
translated to flock's flags:

!O_DENYREAD  -> LOCK_READ
!O_DENYWRITE -> LOCK_WRITE
O_DENYMAND   -> LOCK_MAND

and set through flock_lock_file on a file.

This change only affects opens that use O_DENYMAND flag - all other
native Linux opens don't care about these flags. It allow us to
enable this feature for applications that need it (e.g. NFS and
Samba servers that export the same directory for Windows clients,
or Wine applications that access the same files simultaneously).

Signed-off-by: Pavel Shilovsky <piastry@etersoft.ru>
---
 fs/locks.c         | 106 +++++++++++++++++++++++++++++++++++++++++++++++++++--
 fs/namei.c         |  10 ++++-
 include/linux/fs.h |   6 +++
 3 files changed, 118 insertions(+), 4 deletions(-)

diff --git a/fs/locks.c b/fs/locks.c
index 9edfec4..ebe9a30 100644
--- a/fs/locks.c
+++ b/fs/locks.c
@@ -605,12 +605,86 @@ static int posix_locks_conflict(struct file_lock *caller_fl, struct file_lock *s
 	return (locks_conflict(caller_fl, sys_fl));
 }
 
-/* Determine if lock sys_fl blocks lock caller_fl. FLOCK specific
+static unsigned int
+deny_flags_to_cmd(unsigned int flags)
+{
+	unsigned int cmd = LOCK_MAND;
+
+	if (!(flags & O_DENYREAD))
+		cmd |= LOCK_READ;
+	if (!(flags & O_DENYWRITE))
+		cmd |= LOCK_WRITE;
+
+	return cmd;
+}
+
+/*
+ * 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. O_DENY* flags specific
+ * checking before calling the locks_conflict().
+ */
+static int
+deny_locks_conflict(struct file_lock *caller_fl, struct file_lock *sys_fl)
+{
+	/*
+	 * FLOCK locks referring to the same filp do not conflict with
+	 * each other.
+	 */
+	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);
+}
+
+/*
+ * Determine if lock sys_fl blocks lock caller_fl. FLOCK specific
  * checking before calling the locks_conflict().
  */
-static int flock_locks_conflict(struct file_lock *caller_fl, struct file_lock *sys_fl)
+static int
+flock_locks_conflict(struct file_lock *caller_fl, struct file_lock *sys_fl)
 {
-	/* FLOCK locks referring to the same filp do not conflict with
+	/*
+	 * 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))
@@ -789,6 +863,32 @@ out:
 	return error;
 }
 
+/*
+ * Determine if a file is allowed to be opened with specified access and deny
+ * modes. Lock the file and return 0 if checks passed, otherwise return a error
+ * code.
+ */
+int
+deny_lock_file(struct file *filp)
+{
+	struct file_lock *lock;
+	int error = 0;
+
+	if (!(filp->f_flags & O_DENYMAND))
+		return error;
+
+	error = flock_make_lock(filp, &lock, deny_flags_to_cmd(filp->f_flags));
+	if (error)
+		return error;
+
+	error = flock_lock_file(filp, lock, deny_locks_conflict);
+	if (error == -EAGAIN)
+		error = -ETXTBSY;
+
+	locks_free_lock(lock);
+	return error;
+}
+
 static int __posix_lock_file(struct inode *inode, struct file_lock *request, struct file_lock *conflock)
 {
 	struct file_lock *fl;
diff --git a/fs/namei.c b/fs/namei.c
index 5f4cdf3..bf3bb34 100644
--- a/fs/namei.c
+++ b/fs/namei.c
@@ -2569,9 +2569,14 @@ static int atomic_open(struct nameidata *nd, struct dentry *dentry,
 	 * here.
 	 */
 	error = may_open(&file->f_path, acc_mode, open_flag);
-	if (error)
+	if (error) {
 		fput(file);
+		goto out;
+	}
 
+	error = deny_lock_file(file);
+	if (error)
+		fput(file);
 out:
 	dput(dentry);
 	return error;
@@ -2908,6 +2913,9 @@ opened:
 		if (error)
 			goto exit_fput;
 	}
+	error = deny_lock_file(file);
+	if (error)
+		goto exit_fput;
 out:
 	if (got_write)
 		mnt_drop_write(nd->path.mnt);
diff --git a/include/linux/fs.h b/include/linux/fs.h
index 70a766ae..b8ed06e 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -1004,6 +1004,7 @@ extern int lease_modify(struct file_lock **, int);
 extern int lock_may_read(struct inode *, loff_t start, unsigned long count);
 extern int lock_may_write(struct inode *, loff_t start, unsigned long count);
 extern void locks_delete_block(struct file_lock *waiter);
+extern int deny_lock_file(struct file *);
 extern void lock_flocks(void);
 extern void unlock_flocks(void);
 #else /* !CONFIG_FILE_LOCKING */
@@ -1152,6 +1153,11 @@ static inline void locks_delete_block(struct file_lock *waiter)
 {
 }
 
+static inline int deny_lock_file(struct file *filp)
+{
+	return 0;
+}
+
 static inline void lock_flocks(void)
 {
 }
-- 
1.8.1.1

^ permalink raw reply related	[flat|nested] 32+ messages in thread
* [PATCH v2 4/8] CIFS: Add O_DENY* open flags support
  2013-01-17 16:52 ` Pavel Shilovsky
                   ` (2 preceding siblings ...)
  (?)
@ 2013-01-17 16:53 ` Pavel Shilovsky
       [not found]   ` <1358441584-8783-4-git-send-email-piastry-7qunaywFIewox3rIn2DAYQ@public.gmane.org>
  -1 siblings, 1 reply; 32+ messages in thread
From: Pavel Shilovsky @ 2013-01-17 16:53 UTC (permalink / raw)
  To: linux-kernel; +Cc: linux-fsdevel, linux-cifs, linux-nfs, wine-devel

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  | 12 +++++++++++-
 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, 107 insertions(+), 78 deletions(-)

diff --git a/fs/cifs/cifsacl.c b/fs/cifs/cifsacl.c
index 0fb15bb..a42c77f 100644
--- a/fs/cifs/cifsacl.c
+++ b/fs/cifs/cifsacl.c
@@ -1184,8 +1184,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);
@@ -1245,8 +1246,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 3763624..8c1a27f 100644
--- a/fs/cifs/cifsglob.h
+++ b/fs/cifs/cifsglob.h
@@ -313,7 +313,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);
@@ -948,6 +948,16 @@ 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 & O_DENYMAND) ?
+		(((~(flags >> CIFS_DENY_RW_FLAGS_SHIFT)) & 3) |
+		 ((~(flags >> CIFS_DENY_DEL_FLAG_SHIFT)) & 4)) : FILE_SHARE_ALL;
+}
+
 struct cifs_io_parms {
 	__u16 netfid;
 #ifdef CONFIG_CIFS_SMB2
diff --git a/fs/cifs/cifsproto.h b/fs/cifs/cifsproto.h
index 5144e9f..3f5a46a 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 d3671f2..12ee773 100644
--- a/fs/cifs/dir.c
+++ b/fs/cifs/dir.c
@@ -171,6 +171,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;
@@ -261,6 +262,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
@@ -288,8 +291,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;
@@ -594,9 +597,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 70b6f4c..4ddf450 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;
@@ -531,6 +533,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;
@@ -595,6 +598,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;
@@ -610,8 +614,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 afdff79..30c0a53 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 1c576e8..ad3f136 100644
--- a/fs/cifs/readdir.c
+++ b/fs/cifs/readdir.c
@@ -206,7 +206,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 34cea27..beaad4d 100644
--- a/fs/cifs/smb1ops.c
+++ b/fs/cifs/smb1ops.c
@@ -695,9 +695,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,
@@ -706,8 +706,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);
 }
 
@@ -803,8 +803,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 a93eec3..bfe22e8 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 381f9a5..169b9a8 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;
@@ -449,8 +450,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");
@@ -532,7 +533,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 a7e758a..9d8be7d 100644
--- a/fs/cifs/smb2pdu.c
+++ b/fs/cifs/smb2pdu.c
@@ -910,8 +910,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;
@@ -940,7 +940,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.8.1.1

^ permalink raw reply related	[flat|nested] 32+ messages in thread
* [PATCH v2 5/8] CIFS: Use NT_CREATE_ANDX command for forcemand mounts
  2013-01-17 16:52 ` Pavel Shilovsky
                   ` (3 preceding siblings ...)
  (?)
@ 2013-01-17 16:53 ` Pavel Shilovsky
  -1 siblings, 0 replies; 32+ messages in thread
From: Pavel Shilovsky @ 2013-01-17 16:53 UTC (permalink / raw)
  To: linux-kernel; +Cc: linux-fsdevel, linux-cifs, linux-nfs, wine-devel

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 12ee773..06be419 100644
--- a/fs/cifs/dir.c
+++ b/fs/cifs/dir.c
@@ -185,6 +185,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 4ddf450..bb10486 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,
@@ -575,6 +576,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.8.1.1

^ permalink raw reply related	[flat|nested] 32+ messages in thread
* [PATCH v2 6/8] CIFS: Translate SHARING_VIOLATION to -ETXTBSY error code for SMB2
  2013-01-17 16:52 ` Pavel Shilovsky
                   ` (4 preceding siblings ...)
  (?)
@ 2013-01-17 16:53 ` Pavel Shilovsky
  -1 siblings, 0 replies; 32+ messages in thread
From: Pavel Shilovsky @ 2013-01-17 16:53 UTC (permalink / raw)
  To: linux-kernel; +Cc: linux-fsdevel, linux-cifs, linux-nfs, wine-devel

to make it match CIFS and VFS variants.

Signed-off-by: Pavel Shilovsky <piastry@etersoft.ru>
---
 fs/cifs/smb2maperror.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fs/cifs/smb2maperror.c b/fs/cifs/smb2maperror.c
index 494c912..11e589e 100644
--- a/fs/cifs/smb2maperror.c
+++ b/fs/cifs/smb2maperror.c
@@ -356,7 +356,7 @@ static const struct status_to_posix_error smb2_error_map_table[] = {
 	{STATUS_PORT_CONNECTION_REFUSED, -ECONNREFUSED,
 	"STATUS_PORT_CONNECTION_REFUSED"},
 	{STATUS_INVALID_PORT_HANDLE, -EIO, "STATUS_INVALID_PORT_HANDLE"},
-	{STATUS_SHARING_VIOLATION, -EBUSY, "STATUS_SHARING_VIOLATION"},
+	{STATUS_SHARING_VIOLATION, -ETXTBSY, "STATUS_SHARING_VIOLATION"},
 	{STATUS_QUOTA_EXCEEDED, -EDQUOT, "STATUS_QUOTA_EXCEEDED"},
 	{STATUS_INVALID_PAGE_PROTECTION, -EIO,
 	"STATUS_INVALID_PAGE_PROTECTION"},
-- 
1.8.1.1

^ permalink raw reply related	[flat|nested] 32+ messages in thread
* [PATCH v2 7/8] NFSv4: Add O_DENY* open flags support
  2013-01-17 16:52 ` Pavel Shilovsky
                   ` (5 preceding siblings ...)
  (?)
@ 2013-01-17 16:53 ` Pavel Shilovsky
  -1 siblings, 0 replies; 32+ messages in thread
From: Pavel Shilovsky @ 2013-01-17 16:53 UTC (permalink / raw)
  To: linux-kernel; +Cc: linux-fsdevel, linux-cifs, linux-nfs, wine-devel

by passing these flags to NFSv4 open request.

Signed-off-by: Pavel Shilovsky <piastry@etersoft.ru>
---
 fs/nfs/nfs4xdr.c | 24 ++++++++++++++++++++----
 1 file changed, 20 insertions(+), 4 deletions(-)

diff --git a/fs/nfs/nfs4xdr.c b/fs/nfs/nfs4xdr.c
index 40836ee..0a0cd1e 100644
--- a/fs/nfs/nfs4xdr.c
+++ b/fs/nfs/nfs4xdr.c
@@ -1322,7 +1322,8 @@ static void encode_lookup(struct xdr_stream *xdr, const struct qstr *name, struc
 	encode_string(xdr, name->len, name->name);
 }
 
-static void encode_share_access(struct xdr_stream *xdr, fmode_t fmode)
+static void encode_share_access(struct xdr_stream *xdr, fmode_t fmode,
+				int open_flags)
 {
 	__be32 *p;
 
@@ -1340,7 +1341,22 @@ static void encode_share_access(struct xdr_stream *xdr, fmode_t fmode)
 	default:
 		*p++ = cpu_to_be32(0);
 	}
-	*p = cpu_to_be32(0);		/* for linux, share_deny = 0 always */
+	if (open_flags & O_DENYMAND) {
+		switch (open_flags & (O_DENYREAD|O_DENYWRITE)) {
+		case O_DENYREAD:
+			*p = cpu_to_be32(NFS4_SHARE_DENY_READ);
+			break;
+		case O_DENYWRITE:
+			*p = cpu_to_be32(NFS4_SHARE_DENY_WRITE);
+			break;
+		case O_DENYREAD|O_DENYWRITE:
+			*p = cpu_to_be32(NFS4_SHARE_DENY_BOTH);
+			break;
+		default:
+			*p = cpu_to_be32(0);
+		}
+	} else
+		*p = cpu_to_be32(0);
 }
 
 static inline void encode_openhdr(struct xdr_stream *xdr, const struct nfs_openargs *arg)
@@ -1351,7 +1367,7 @@ static inline void encode_openhdr(struct xdr_stream *xdr, const struct nfs_opena
  * owner 4 = 32
  */
 	encode_nfs4_seqid(xdr, arg->seqid);
-	encode_share_access(xdr, arg->fmode);
+	encode_share_access(xdr, arg->fmode, arg->open_flags);
 	p = reserve_space(xdr, 36);
 	p = xdr_encode_hyper(p, arg->clientid);
 	*p++ = cpu_to_be32(24);
@@ -1489,7 +1505,7 @@ static void encode_open_downgrade(struct xdr_stream *xdr, const struct nfs_close
 	encode_op_hdr(xdr, OP_OPEN_DOWNGRADE, decode_open_downgrade_maxsz, hdr);
 	encode_nfs4_stateid(xdr, arg->stateid);
 	encode_nfs4_seqid(xdr, arg->seqid);
-	encode_share_access(xdr, arg->fmode);
+	encode_share_access(xdr, arg->fmode, 0);
 }
 
 static void
-- 
1.8.1.1

^ permalink raw reply related	[flat|nested] 32+ messages in thread
* [PATCH v2 8/8] NFSD: Pass share reservations flags to VFS
  2013-01-17 16:52 ` Pavel Shilovsky
                   ` (6 preceding siblings ...)
  (?)
@ 2013-01-17 16:53 ` Pavel Shilovsky
  -1 siblings, 0 replies; 32+ messages in thread
From: Pavel Shilovsky @ 2013-01-17 16:53 UTC (permalink / raw)
  To: linux-kernel; +Cc: linux-fsdevel, linux-cifs, linux-nfs, wine-devel

that maps them into O_DENY flags and make them visible for
applications that use O_DENYMAND opens.

Signed-off-by: Pavel Shilovsky <piastry@etersoft.ru>
---
 fs/locks.c          |  1 +
 fs/nfsd/nfs4state.c | 46 +++++++++++++++++++++++++++++++++++++++++++++-
 2 files changed, 46 insertions(+), 1 deletion(-)

diff --git a/fs/locks.c b/fs/locks.c
index ebe9a30..ebdff1f 100644
--- a/fs/locks.c
+++ b/fs/locks.c
@@ -888,6 +888,7 @@ deny_lock_file(struct file *filp)
 	locks_free_lock(lock);
 	return error;
 }
+EXPORT_SYMBOL(deny_lock_file);
 
 static int __posix_lock_file(struct inode *inode, struct file_lock *request, struct file_lock *conflock)
 {
diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
index d0237f8..4aa2b8f 100644
--- a/fs/nfsd/nfs4state.c
+++ b/fs/nfsd/nfs4state.c
@@ -516,6 +516,19 @@ test_deny(u32 access, struct nfs4_ol_stateid *stp)
 	return test_bit(access, &stp->st_deny_bmap);
 }
 
+static int nfs4_deny_to_odeny(u32 access)
+{
+	switch (access & NFS4_SHARE_DENY_BOTH) {
+	case NFS4_SHARE_DENY_READ:
+		return O_DENYMAND | O_DENYREAD;
+	case NFS4_SHARE_DENY_WRITE:
+		return O_DENYWRITE | O_DENYMAND;
+	case NFS4_SHARE_DENY_BOTH:
+		return O_DENYREAD | O_DENYWRITE | O_DENYMAND;
+	}
+	return O_DENYMAND;
+}
+
 static int nfs4_access_to_omode(u32 access)
 {
 	switch (access & NFS4_SHARE_ACCESS_BOTH) {
@@ -2769,6 +2782,21 @@ nfsd4_truncate(struct svc_rqst *rqstp, struct svc_fh *fh,
 }
 
 static __be32
+nfs4_vfs_set_deny(struct nfs4_file *fp, unsigned long share_access,
+		  unsigned long deny_access)
+{
+	int oflag, rc;
+	__be32 status = nfs_ok;
+
+	oflag = nfs4_access_to_omode(share_access);
+	fp->fi_fds[oflag]->f_flags |= nfs4_deny_to_odeny(deny_access);
+	rc = deny_lock_file(fp->fi_fds[oflag]);
+	if (rc)
+		status = nfserrno(rc);
+	return status;
+}
+
+static __be32
 nfs4_upgrade_open(struct svc_rqst *rqstp, struct nfs4_file *fp, struct svc_fh *cur_fh, struct nfs4_ol_stateid *stp, struct nfsd4_open *open)
 {
 	u32 op_share_access = open->op_share_access;
@@ -2789,6 +2817,14 @@ nfs4_upgrade_open(struct svc_rqst *rqstp, struct nfs4_file *fp, struct svc_fh *c
 		}
 		return status;
 	}
+	status = nfs4_vfs_set_deny(fp, op_share_access, open->op_share_deny);
+	if (status) {
+		if (new_access) {
+			int oflag = nfs4_access_to_omode(op_share_access);
+			nfs4_file_put_access(fp, oflag);
+		}
+		return status;
+	}
 	/* remember the open */
 	set_access(op_share_access, stp);
 	set_deny(open->op_share_deny, stp);
@@ -3021,7 +3057,7 @@ nfsd4_process_open2(struct svc_rqst *rqstp, struct svc_fh *current_fh, struct nf
 
 	/*
 	 * OPEN the file, or upgrade an existing OPEN.
-	 * If truncate fails, the OPEN fails.
+	 * If truncate or setting deny fails, the OPEN fails.
 	 */
 	if (stp) {
 		/* Stateid was found, this is an OPEN upgrade */
@@ -3035,6 +3071,10 @@ nfsd4_process_open2(struct svc_rqst *rqstp, struct svc_fh *current_fh, struct nf
 		status = nfsd4_truncate(rqstp, current_fh, open);
 		if (status)
 			goto out;
+		status = nfs4_vfs_set_deny(fp, open->op_share_access,
+					   open->op_share_deny);
+		if (status)
+			goto out;
 		stp = open->op_stp;
 		open->op_stp = NULL;
 		init_open_stateid(stp, fp, open);
@@ -3713,6 +3753,10 @@ nfsd4_open_downgrade(struct svc_rqst *rqstp,
 	}
 	nfs4_stateid_downgrade(stp, od->od_share_access);
 
+	status = nfs4_vfs_set_deny(stp->st_file, od->od_share_access,
+				   od->od_share_deny);
+	if (status)
+		goto out;
 	reset_union_bmap_deny(od->od_share_deny, stp);
 
 	update_stateid(&stp->st_stid.sc_stateid);
-- 
1.8.1.1


^ permalink raw reply related	[flat|nested] 32+ messages in thread
* Re: [PATCH v2 3/8] vfs: Add O_DENYREAD/WRITE flags support for open syscall
  2013-01-17 16:52 ` [PATCH v2 3/8] vfs: Add O_DENYREAD/WRITE flags support for open syscall Pavel Shilovsky
@ 2013-01-30 22:16       ` J. Bruce Fields
  0 siblings, 0 replies; 32+ messages in thread
From: J. Bruce Fields @ 2013-01-30 22:16 UTC (permalink / raw)
  To: Pavel Shilovsky
  Cc: linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	linux-fsdevel-u79uwXL29TY76Z2rM5mHXA,
	linux-cifs-u79uwXL29TY76Z2rM5mHXA,
	linux-nfs-u79uwXL29TY76Z2rM5mHXA,
	wine-devel-5vRYHf7vrtgdnm+yROfE0A

On Thu, Jan 17, 2013 at 08:52:59PM +0400, Pavel Shilovsky wrote:
> If O_DENYMAND flag is specified, O_DENYREAD/WRITE/MAND flags are
> translated to flock's flags:
> 
> !O_DENYREAD  -> LOCK_READ
> !O_DENYWRITE -> LOCK_WRITE
> O_DENYMAND   -> LOCK_MAND
> 
> and set through flock_lock_file on a file.
> 
> This change only affects opens that use O_DENYMAND flag - all other
> native Linux opens don't care about these flags. It allow us to
> enable this feature for applications that need it (e.g. NFS and
> Samba servers that export the same directory for Windows clients,
> or Wine applications that access the same files simultaneously).

The use of an is_conflict callback seems unnecessarily convoluted.

If we need two different behaviors, let's just use another flag (or an
extra boolean argument if we need to, or something).

The only caller for this new deny_lock_file is in the nfs code--I'm a
little unclear why that is.

--b.

> 
> Signed-off-by: Pavel Shilovsky <piastry-7qunaywFIewox3rIn2DAYQ@public.gmane.org>
> ---
>  fs/locks.c         | 106 +++++++++++++++++++++++++++++++++++++++++++++++++++--
>  fs/namei.c         |  10 ++++-
>  include/linux/fs.h |   6 +++
>  3 files changed, 118 insertions(+), 4 deletions(-)
> 
> diff --git a/fs/locks.c b/fs/locks.c
> index 9edfec4..ebe9a30 100644
> --- a/fs/locks.c
> +++ b/fs/locks.c
> @@ -605,12 +605,86 @@ static int posix_locks_conflict(struct file_lock *caller_fl, struct file_lock *s
>  	return (locks_conflict(caller_fl, sys_fl));
>  }
>  
> -/* Determine if lock sys_fl blocks lock caller_fl. FLOCK specific
> +static unsigned int
> +deny_flags_to_cmd(unsigned int flags)
> +{
> +	unsigned int cmd = LOCK_MAND;
> +
> +	if (!(flags & O_DENYREAD))
> +		cmd |= LOCK_READ;
> +	if (!(flags & O_DENYWRITE))
> +		cmd |= LOCK_WRITE;
> +
> +	return cmd;
> +}
> +
> +/*
> + * 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. O_DENY* flags specific
> + * checking before calling the locks_conflict().
> + */
> +static int
> +deny_locks_conflict(struct file_lock *caller_fl, struct file_lock *sys_fl)
> +{
> +	/*
> +	 * FLOCK locks referring to the same filp do not conflict with
> +	 * each other.
> +	 */
> +	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);
> +}
> +
> +/*
> + * Determine if lock sys_fl blocks lock caller_fl. FLOCK specific
>   * checking before calling the locks_conflict().
>   */
> -static int flock_locks_conflict(struct file_lock *caller_fl, struct file_lock *sys_fl)
> +static int
> +flock_locks_conflict(struct file_lock *caller_fl, struct file_lock *sys_fl)
>  {
> -	/* FLOCK locks referring to the same filp do not conflict with
> +	/*
> +	 * 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))
> @@ -789,6 +863,32 @@ out:
>  	return error;
>  }
>  
> +/*
> + * Determine if a file is allowed to be opened with specified access and deny
> + * modes. Lock the file and return 0 if checks passed, otherwise return a error
> + * code.
> + */
> +int
> +deny_lock_file(struct file *filp)
> +{
> +	struct file_lock *lock;
> +	int error = 0;
> +
> +	if (!(filp->f_flags & O_DENYMAND))
> +		return error;
> +
> +	error = flock_make_lock(filp, &lock, deny_flags_to_cmd(filp->f_flags));
> +	if (error)
> +		return error;
> +
> +	error = flock_lock_file(filp, lock, deny_locks_conflict);
> +	if (error == -EAGAIN)
> +		error = -ETXTBSY;
> +
> +	locks_free_lock(lock);
> +	return error;
> +}
> +
>  static int __posix_lock_file(struct inode *inode, struct file_lock *request, struct file_lock *conflock)
>  {
>  	struct file_lock *fl;
> diff --git a/fs/namei.c b/fs/namei.c
> index 5f4cdf3..bf3bb34 100644
> --- a/fs/namei.c
> +++ b/fs/namei.c
> @@ -2569,9 +2569,14 @@ static int atomic_open(struct nameidata *nd, struct dentry *dentry,
>  	 * here.
>  	 */
>  	error = may_open(&file->f_path, acc_mode, open_flag);
> -	if (error)
> +	if (error) {
>  		fput(file);
> +		goto out;
> +	}
>  
> +	error = deny_lock_file(file);
> +	if (error)
> +		fput(file);
>  out:
>  	dput(dentry);
>  	return error;
> @@ -2908,6 +2913,9 @@ opened:
>  		if (error)
>  			goto exit_fput;
>  	}
> +	error = deny_lock_file(file);
> +	if (error)
> +		goto exit_fput;
>  out:
>  	if (got_write)
>  		mnt_drop_write(nd->path.mnt);
> diff --git a/include/linux/fs.h b/include/linux/fs.h
> index 70a766ae..b8ed06e 100644
> --- a/include/linux/fs.h
> +++ b/include/linux/fs.h
> @@ -1004,6 +1004,7 @@ extern int lease_modify(struct file_lock **, int);
>  extern int lock_may_read(struct inode *, loff_t start, unsigned long count);
>  extern int lock_may_write(struct inode *, loff_t start, unsigned long count);
>  extern void locks_delete_block(struct file_lock *waiter);
> +extern int deny_lock_file(struct file *);
>  extern void lock_flocks(void);
>  extern void unlock_flocks(void);
>  #else /* !CONFIG_FILE_LOCKING */
> @@ -1152,6 +1153,11 @@ static inline void locks_delete_block(struct file_lock *waiter)
>  {
>  }
>  
> +static inline int deny_lock_file(struct file *filp)
> +{
> +	return 0;
> +}
> +
>  static inline void lock_flocks(void)
>  {
>  }
> -- 
> 1.8.1.1
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
> the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 32+ messages in thread
* Re: [PATCH v2 3/8] vfs: Add O_DENYREAD/WRITE flags support for open syscall
@ 2013-01-30 22:16       ` J. Bruce Fields
  0 siblings, 0 replies; 32+ messages in thread
From: J. Bruce Fields @ 2013-01-30 22:16 UTC (permalink / raw)
  To: Pavel Shilovsky
  Cc: linux-kernel, linux-fsdevel, linux-cifs, linux-nfs, wine-devel

On Thu, Jan 17, 2013 at 08:52:59PM +0400, Pavel Shilovsky wrote:
> If O_DENYMAND flag is specified, O_DENYREAD/WRITE/MAND flags are
> translated to flock's flags:
> 
> !O_DENYREAD  -> LOCK_READ
> !O_DENYWRITE -> LOCK_WRITE
> O_DENYMAND   -> LOCK_MAND
> 
> and set through flock_lock_file on a file.
> 
> This change only affects opens that use O_DENYMAND flag - all other
> native Linux opens don't care about these flags. It allow us to
> enable this feature for applications that need it (e.g. NFS and
> Samba servers that export the same directory for Windows clients,
> or Wine applications that access the same files simultaneously).

The use of an is_conflict callback seems unnecessarily convoluted.

If we need two different behaviors, let's just use another flag (or an
extra boolean argument if we need to, or something).

The only caller for this new deny_lock_file is in the nfs code--I'm a
little unclear why that is.

--b.

> 
> Signed-off-by: Pavel Shilovsky <piastry@etersoft.ru>
> ---
>  fs/locks.c         | 106 +++++++++++++++++++++++++++++++++++++++++++++++++++--
>  fs/namei.c         |  10 ++++-
>  include/linux/fs.h |   6 +++
>  3 files changed, 118 insertions(+), 4 deletions(-)
> 
> diff --git a/fs/locks.c b/fs/locks.c
> index 9edfec4..ebe9a30 100644
> --- a/fs/locks.c
> +++ b/fs/locks.c
> @@ -605,12 +605,86 @@ static int posix_locks_conflict(struct file_lock *caller_fl, struct file_lock *s
>  	return (locks_conflict(caller_fl, sys_fl));
>  }
>  
> -/* Determine if lock sys_fl blocks lock caller_fl. FLOCK specific
> +static unsigned int
> +deny_flags_to_cmd(unsigned int flags)
> +{
> +	unsigned int cmd = LOCK_MAND;
> +
> +	if (!(flags & O_DENYREAD))
> +		cmd |= LOCK_READ;
> +	if (!(flags & O_DENYWRITE))
> +		cmd |= LOCK_WRITE;
> +
> +	return cmd;
> +}
> +
> +/*
> + * 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. O_DENY* flags specific
> + * checking before calling the locks_conflict().
> + */
> +static int
> +deny_locks_conflict(struct file_lock *caller_fl, struct file_lock *sys_fl)
> +{
> +	/*
> +	 * FLOCK locks referring to the same filp do not conflict with
> +	 * each other.
> +	 */
> +	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);
> +}
> +
> +/*
> + * Determine if lock sys_fl blocks lock caller_fl. FLOCK specific
>   * checking before calling the locks_conflict().
>   */
> -static int flock_locks_conflict(struct file_lock *caller_fl, struct file_lock *sys_fl)
> +static int
> +flock_locks_conflict(struct file_lock *caller_fl, struct file_lock *sys_fl)
>  {
> -	/* FLOCK locks referring to the same filp do not conflict with
> +	/*
> +	 * 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))
> @@ -789,6 +863,32 @@ out:
>  	return error;
>  }
>  
> +/*
> + * Determine if a file is allowed to be opened with specified access and deny
> + * modes. Lock the file and return 0 if checks passed, otherwise return a error
> + * code.
> + */
> +int
> +deny_lock_file(struct file *filp)
> +{
> +	struct file_lock *lock;
> +	int error = 0;
> +
> +	if (!(filp->f_flags & O_DENYMAND))
> +		return error;
> +
> +	error = flock_make_lock(filp, &lock, deny_flags_to_cmd(filp->f_flags));
> +	if (error)
> +		return error;
> +
> +	error = flock_lock_file(filp, lock, deny_locks_conflict);
> +	if (error == -EAGAIN)
> +		error = -ETXTBSY;
> +
> +	locks_free_lock(lock);
> +	return error;
> +}
> +
>  static int __posix_lock_file(struct inode *inode, struct file_lock *request, struct file_lock *conflock)
>  {
>  	struct file_lock *fl;
> diff --git a/fs/namei.c b/fs/namei.c
> index 5f4cdf3..bf3bb34 100644
> --- a/fs/namei.c
> +++ b/fs/namei.c
> @@ -2569,9 +2569,14 @@ static int atomic_open(struct nameidata *nd, struct dentry *dentry,
>  	 * here.
>  	 */
>  	error = may_open(&file->f_path, acc_mode, open_flag);
> -	if (error)
> +	if (error) {
>  		fput(file);
> +		goto out;
> +	}
>  
> +	error = deny_lock_file(file);
> +	if (error)
> +		fput(file);
>  out:
>  	dput(dentry);
>  	return error;
> @@ -2908,6 +2913,9 @@ opened:
>  		if (error)
>  			goto exit_fput;
>  	}
> +	error = deny_lock_file(file);
> +	if (error)
> +		goto exit_fput;
>  out:
>  	if (got_write)
>  		mnt_drop_write(nd->path.mnt);
> diff --git a/include/linux/fs.h b/include/linux/fs.h
> index 70a766ae..b8ed06e 100644
> --- a/include/linux/fs.h
> +++ b/include/linux/fs.h
> @@ -1004,6 +1004,7 @@ extern int lease_modify(struct file_lock **, int);
>  extern int lock_may_read(struct inode *, loff_t start, unsigned long count);
>  extern int lock_may_write(struct inode *, loff_t start, unsigned long count);
>  extern void locks_delete_block(struct file_lock *waiter);
> +extern int deny_lock_file(struct file *);
>  extern void lock_flocks(void);
>  extern void unlock_flocks(void);
>  #else /* !CONFIG_FILE_LOCKING */
> @@ -1152,6 +1153,11 @@ static inline void locks_delete_block(struct file_lock *waiter)
>  {
>  }
>  
> +static inline int deny_lock_file(struct file *filp)
> +{
> +	return 0;
> +}
> +
>  static inline void lock_flocks(void)
>  {
>  }
> -- 
> 1.8.1.1
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 32+ messages in thread
* Re: [PATCH v2 4/8] CIFS: Add O_DENY* open flags support
  2013-01-17 16:53 ` [PATCH v2 4/8] CIFS: Add O_DENY* open flags support Pavel Shilovsky
@ 2013-01-30 22:16       ` J. Bruce Fields
  0 siblings, 0 replies; 32+ messages in thread
From: J. Bruce Fields @ 2013-01-30 22:16 UTC (permalink / raw)
  To: Pavel Shilovsky
  Cc: linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	linux-fsdevel-u79uwXL29TY76Z2rM5mHXA,
	linux-cifs-u79uwXL29TY76Z2rM5mHXA,
	linux-nfs-u79uwXL29TY76Z2rM5mHXA,
	wine-devel-5vRYHf7vrtgdnm+yROfE0A

On Thu, Jan 17, 2013 at 08:53:00PM +0400, Pavel Shilovsky wrote:
> 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-7qunaywFIewox3rIn2DAYQ@public.gmane.org>
> ---
>  fs/cifs/cifsacl.c   | 10 ++++++----
>  fs/cifs/cifsglob.h  | 12 +++++++++++-
>  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, 107 insertions(+), 78 deletions(-)
> 
> diff --git a/fs/cifs/cifsacl.c b/fs/cifs/cifsacl.c
> index 0fb15bb..a42c77f 100644
> --- a/fs/cifs/cifsacl.c
> +++ b/fs/cifs/cifsacl.c
> @@ -1184,8 +1184,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);
> @@ -1245,8 +1246,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);

It would be easier to tell what you changed in the above two chunks if
you didn't change the whitespace at the same time.

--b.

>  	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 3763624..8c1a27f 100644
> --- a/fs/cifs/cifsglob.h
> +++ b/fs/cifs/cifsglob.h
> @@ -313,7 +313,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);
> @@ -948,6 +948,16 @@ 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 & O_DENYMAND) ?
> +		(((~(flags >> CIFS_DENY_RW_FLAGS_SHIFT)) & 3) |
> +		 ((~(flags >> CIFS_DENY_DEL_FLAG_SHIFT)) & 4)) : FILE_SHARE_ALL;
> +}
> +
>  struct cifs_io_parms {
>  	__u16 netfid;
>  #ifdef CONFIG_CIFS_SMB2
> diff --git a/fs/cifs/cifsproto.h b/fs/cifs/cifsproto.h
> index 5144e9f..3f5a46a 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 d3671f2..12ee773 100644
> --- a/fs/cifs/dir.c
> +++ b/fs/cifs/dir.c
> @@ -171,6 +171,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;
> @@ -261,6 +262,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
> @@ -288,8 +291,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;
> @@ -594,9 +597,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 70b6f4c..4ddf450 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;
> @@ -531,6 +533,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;
> @@ -595,6 +598,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;
> @@ -610,8 +614,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 afdff79..30c0a53 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 1c576e8..ad3f136 100644
> --- a/fs/cifs/readdir.c
> +++ b/fs/cifs/readdir.c
> @@ -206,7 +206,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 34cea27..beaad4d 100644
> --- a/fs/cifs/smb1ops.c
> +++ b/fs/cifs/smb1ops.c
> @@ -695,9 +695,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,
> @@ -706,8 +706,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);
>  }
>  
> @@ -803,8 +803,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 a93eec3..bfe22e8 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 381f9a5..169b9a8 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;
> @@ -449,8 +450,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");
> @@ -532,7 +533,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 a7e758a..9d8be7d 100644
> --- a/fs/cifs/smb2pdu.c
> +++ b/fs/cifs/smb2pdu.c
> @@ -910,8 +910,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;
> @@ -940,7 +940,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.8.1.1
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
> the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 32+ messages in thread
* Re: [PATCH v2 4/8] CIFS: Add O_DENY* open flags support
@ 2013-01-30 22:16       ` J. Bruce Fields
  0 siblings, 0 replies; 32+ messages in thread
From: J. Bruce Fields @ 2013-01-30 22:16 UTC (permalink / raw)
  To: Pavel Shilovsky
  Cc: linux-kernel, linux-fsdevel, linux-cifs, linux-nfs, wine-devel

On Thu, Jan 17, 2013 at 08:53:00PM +0400, Pavel Shilovsky wrote:
> 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  | 12 +++++++++++-
>  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, 107 insertions(+), 78 deletions(-)
> 
> diff --git a/fs/cifs/cifsacl.c b/fs/cifs/cifsacl.c
> index 0fb15bb..a42c77f 100644
> --- a/fs/cifs/cifsacl.c
> +++ b/fs/cifs/cifsacl.c
> @@ -1184,8 +1184,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);
> @@ -1245,8 +1246,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);

It would be easier to tell what you changed in the above two chunks if
you didn't change the whitespace at the same time.

--b.

>  	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 3763624..8c1a27f 100644
> --- a/fs/cifs/cifsglob.h
> +++ b/fs/cifs/cifsglob.h
> @@ -313,7 +313,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);
> @@ -948,6 +948,16 @@ 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 & O_DENYMAND) ?
> +		(((~(flags >> CIFS_DENY_RW_FLAGS_SHIFT)) & 3) |
> +		 ((~(flags >> CIFS_DENY_DEL_FLAG_SHIFT)) & 4)) : FILE_SHARE_ALL;
> +}
> +
>  struct cifs_io_parms {
>  	__u16 netfid;
>  #ifdef CONFIG_CIFS_SMB2
> diff --git a/fs/cifs/cifsproto.h b/fs/cifs/cifsproto.h
> index 5144e9f..3f5a46a 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 d3671f2..12ee773 100644
> --- a/fs/cifs/dir.c
> +++ b/fs/cifs/dir.c
> @@ -171,6 +171,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;
> @@ -261,6 +262,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
> @@ -288,8 +291,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;
> @@ -594,9 +597,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 70b6f4c..4ddf450 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;
> @@ -531,6 +533,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;
> @@ -595,6 +598,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;
> @@ -610,8 +614,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 afdff79..30c0a53 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 1c576e8..ad3f136 100644
> --- a/fs/cifs/readdir.c
> +++ b/fs/cifs/readdir.c
> @@ -206,7 +206,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 34cea27..beaad4d 100644
> --- a/fs/cifs/smb1ops.c
> +++ b/fs/cifs/smb1ops.c
> @@ -695,9 +695,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,
> @@ -706,8 +706,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);
>  }
>  
> @@ -803,8 +803,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 a93eec3..bfe22e8 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 381f9a5..169b9a8 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;
> @@ -449,8 +450,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");
> @@ -532,7 +533,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 a7e758a..9d8be7d 100644
> --- a/fs/cifs/smb2pdu.c
> +++ b/fs/cifs/smb2pdu.c
> @@ -910,8 +910,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;
> @@ -940,7 +940,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.8.1.1
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 32+ messages in thread
* Re: [PATCH v2 3/8] vfs: Add O_DENYREAD/WRITE flags support for open syscall
  2013-01-30 22:16       ` J. Bruce Fields
@ 2013-02-05 11:45           ` Pavel Shilovsky
  -1 siblings, 0 replies; 32+ messages in thread
From: Pavel Shilovsky @ 2013-02-05 11:45 UTC (permalink / raw)
  To: J. Bruce Fields
  Cc: linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	linux-fsdevel-u79uwXL29TY76Z2rM5mHXA,
	linux-cifs-u79uwXL29TY76Z2rM5mHXA,
	linux-nfs-u79uwXL29TY76Z2rM5mHXA,
	wine-devel-5vRYHf7vrtgdnm+yROfE0A

2013/1/31 J. Bruce Fields <bfields-uC3wQj2KruNg9hUCZPvPmw@public.gmane.org>:
> On Thu, Jan 17, 2013 at 08:52:59PM +0400, Pavel Shilovsky wrote:
>> If O_DENYMAND flag is specified, O_DENYREAD/WRITE/MAND flags are
>> translated to flock's flags:
>>
>> !O_DENYREAD  -> LOCK_READ
>> !O_DENYWRITE -> LOCK_WRITE
>> O_DENYMAND   -> LOCK_MAND
>>
>> and set through flock_lock_file on a file.
>>
>> This change only affects opens that use O_DENYMAND flag - all other
>> native Linux opens don't care about these flags. It allow us to
>> enable this feature for applications that need it (e.g. NFS and
>> Samba servers that export the same directory for Windows clients,
>> or Wine applications that access the same files simultaneously).
>
> The use of an is_conflict callback seems unnecessarily convoluted.
>
> If we need two different behaviors, let's just use another flag (or an
> extra boolean argument if we need to, or something).

Ok, we can pass "bool is_mand" to flock_lock_file that will pass it
further to flock_locks_conflict.

>
> The only caller for this new deny_lock_file is in the nfs code--I'm a
> little unclear why that is.

deny_lock_file is called not only in the nfs code but also in 2 places
of fs/namei.c -- that enable this logic for VFS.

-- 
Best regards,
Pavel Shilovsky.

^ permalink raw reply	[flat|nested] 32+ messages in thread
* Re: [PATCH v2 3/8] vfs: Add O_DENYREAD/WRITE flags support for open syscall
@ 2013-02-05 11:45           ` Pavel Shilovsky
  0 siblings, 0 replies; 32+ messages in thread
From: Pavel Shilovsky @ 2013-02-05 11:45 UTC (permalink / raw)
  To: J. Bruce Fields
  Cc: linux-kernel, linux-fsdevel, linux-cifs, linux-nfs, wine-devel

2013/1/31 J. Bruce Fields <bfields@fieldses.org>:
> On Thu, Jan 17, 2013 at 08:52:59PM +0400, Pavel Shilovsky wrote:
>> If O_DENYMAND flag is specified, O_DENYREAD/WRITE/MAND flags are
>> translated to flock's flags:
>>
>> !O_DENYREAD  -> LOCK_READ
>> !O_DENYWRITE -> LOCK_WRITE
>> O_DENYMAND   -> LOCK_MAND
>>
>> and set through flock_lock_file on a file.
>>
>> This change only affects opens that use O_DENYMAND flag - all other
>> native Linux opens don't care about these flags. It allow us to
>> enable this feature for applications that need it (e.g. NFS and
>> Samba servers that export the same directory for Windows clients,
>> or Wine applications that access the same files simultaneously).
>
> The use of an is_conflict callback seems unnecessarily convoluted.
>
> If we need two different behaviors, let's just use another flag (or an
> extra boolean argument if we need to, or something).

Ok, we can pass "bool is_mand" to flock_lock_file that will pass it
further to flock_locks_conflict.

>
> The only caller for this new deny_lock_file is in the nfs code--I'm a
> little unclear why that is.

deny_lock_file is called not only in the nfs code but also in 2 places
of fs/namei.c -- that enable this logic for VFS.

-- 
Best regards,
Pavel Shilovsky.

^ permalink raw reply	[flat|nested] 32+ messages in thread
* Re: [PATCH v2 4/8] CIFS: Add O_DENY* open flags support
  2013-01-30 22:16       ` J. Bruce Fields
@ 2013-02-05 11:54           ` Pavel Shilovsky
  -1 siblings, 0 replies; 32+ messages in thread
From: Pavel Shilovsky @ 2013-02-05 11:54 UTC (permalink / raw)
  To: J. Bruce Fields
  Cc: linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	linux-fsdevel-u79uwXL29TY76Z2rM5mHXA,
	linux-cifs-u79uwXL29TY76Z2rM5mHXA,
	linux-nfs-u79uwXL29TY76Z2rM5mHXA,
	wine-devel-5vRYHf7vrtgdnm+yROfE0A

2013/1/31 J. Bruce Fields <bfields-uC3wQj2KruNg9hUCZPvPmw@public.gmane.org>:
> On Thu, Jan 17, 2013 at 08:53:00PM +0400, Pavel Shilovsky wrote:
>> 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-7qunaywFIewox3rIn2DAYQ@public.gmane.org>
>> ---
>>  fs/cifs/cifsacl.c   | 10 ++++++----
>>  fs/cifs/cifsglob.h  | 12 +++++++++++-
>>  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, 107 insertions(+), 78 deletions(-)
>>
>> diff --git a/fs/cifs/cifsacl.c b/fs/cifs/cifsacl.c
>> index 0fb15bb..a42c77f 100644
>> --- a/fs/cifs/cifsacl.c
>> +++ b/fs/cifs/cifsacl.c
>> @@ -1184,8 +1184,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);
>> @@ -1245,8 +1246,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);
>
> It would be easier to tell what you changed in the above two chunks if
> you didn't change the whitespace at the same time.
>
> --b.

Ok, thank - will fix it in the further version of the patch.

-- 
Best regards,
Pavel Shilovsky.
--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 32+ messages in thread
* Re: [PATCH v2 4/8] CIFS: Add O_DENY* open flags support
@ 2013-02-05 11:54           ` Pavel Shilovsky
  0 siblings, 0 replies; 32+ messages in thread
From: Pavel Shilovsky @ 2013-02-05 11:54 UTC (permalink / raw)
  To: J. Bruce Fields
  Cc: linux-kernel, linux-fsdevel, linux-cifs, linux-nfs, wine-devel

2013/1/31 J. Bruce Fields <bfields@fieldses.org>:
> On Thu, Jan 17, 2013 at 08:53:00PM +0400, Pavel Shilovsky wrote:
>> 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  | 12 +++++++++++-
>>  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, 107 insertions(+), 78 deletions(-)
>>
>> diff --git a/fs/cifs/cifsacl.c b/fs/cifs/cifsacl.c
>> index 0fb15bb..a42c77f 100644
>> --- a/fs/cifs/cifsacl.c
>> +++ b/fs/cifs/cifsacl.c
>> @@ -1184,8 +1184,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);
>> @@ -1245,8 +1246,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);
>
> It would be easier to tell what you changed in the above two chunks if
> you didn't change the whitespace at the same time.
>
> --b.

Ok, thank - will fix it in the further version of the patch.

-- 
Best regards,
Pavel Shilovsky.

^ permalink raw reply	[flat|nested] 32+ messages in thread
* Re: [PATCH v2 3/8] vfs: Add O_DENYREAD/WRITE flags support for open syscall
  2013-02-05 11:45           ` Pavel Shilovsky
  (?)
@ 2013-02-05 14:35           ` J. Bruce Fields
  2013-02-07  9:53             ` Pavel Shilovsky
  -1 siblings, 1 reply; 32+ messages in thread
From: J. Bruce Fields @ 2013-02-05 14:35 UTC (permalink / raw)
  To: Pavel Shilovsky
  Cc: linux-kernel, linux-fsdevel, linux-cifs, linux-nfs, wine-devel

On Tue, Feb 05, 2013 at 03:45:31PM +0400, Pavel Shilovsky wrote:
> 2013/1/31 J. Bruce Fields <bfields@fieldses.org>:
> > On Thu, Jan 17, 2013 at 08:52:59PM +0400, Pavel Shilovsky wrote:
> >> If O_DENYMAND flag is specified, O_DENYREAD/WRITE/MAND flags are
> >> translated to flock's flags:
> >>
> >> !O_DENYREAD  -> LOCK_READ
> >> !O_DENYWRITE -> LOCK_WRITE
> >> O_DENYMAND   -> LOCK_MAND
> >>
> >> and set through flock_lock_file on a file.
> >>
> >> This change only affects opens that use O_DENYMAND flag - all other
> >> native Linux opens don't care about these flags. It allow us to
> >> enable this feature for applications that need it (e.g. NFS and
> >> Samba servers that export the same directory for Windows clients,
> >> or Wine applications that access the same files simultaneously).
> >
> > The use of an is_conflict callback seems unnecessarily convoluted.
> >
> > If we need two different behaviors, let's just use another flag (or an
> > extra boolean argument if we need to, or something).
> 
> Ok, we can pass "bool is_mand" to flock_lock_file that will pass it
> further to flock_locks_conflict.
> 
> >
> > The only caller for this new deny_lock_file is in the nfs code--I'm a
> > little unclear why that is.
> 
> deny_lock_file is called not only in the nfs code but also in 2 places
> of fs/namei.c -- that enable this logic for VFS.

Oops, apologies, I overlooked those somehow.

What prevents somebody else from grabbing a lock on a newly-created file
before we grab our own lock?

I couldn't tell on a quick look whether we hold some lock that prevents
that.

--b.

^ permalink raw reply	[flat|nested] 32+ messages in thread
* Re: [PATCH v2 3/8] vfs: Add O_DENYREAD/WRITE flags support for open syscall
  2013-02-05 14:35           ` J. Bruce Fields
@ 2013-02-07  9:53             ` Pavel Shilovsky
       [not found]               ` <CAKywueS6oGNu9r_bypu32A89pOoiGb47kjXqw2ZcZQhZK+JV3g-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
  0 siblings, 1 reply; 32+ messages in thread
From: Pavel Shilovsky @ 2013-02-07  9:53 UTC (permalink / raw)
  To: J. Bruce Fields
  Cc: linux-kernel, linux-fsdevel, linux-cifs, linux-nfs, wine-devel

2013/2/5 J. Bruce Fields <bfields@fieldses.org>:
> On Tue, Feb 05, 2013 at 03:45:31PM +0400, Pavel Shilovsky wrote:
>> 2013/1/31 J. Bruce Fields <bfields@fieldses.org>:
>> > On Thu, Jan 17, 2013 at 08:52:59PM +0400, Pavel Shilovsky wrote:
>> >> If O_DENYMAND flag is specified, O_DENYREAD/WRITE/MAND flags are
>> >> translated to flock's flags:
>> >>
>> >> !O_DENYREAD  -> LOCK_READ
>> >> !O_DENYWRITE -> LOCK_WRITE
>> >> O_DENYMAND   -> LOCK_MAND
>> >>
>> >> and set through flock_lock_file on a file.
>> >>
>> >> This change only affects opens that use O_DENYMAND flag - all other
>> >> native Linux opens don't care about these flags. It allow us to
>> >> enable this feature for applications that need it (e.g. NFS and
>> >> Samba servers that export the same directory for Windows clients,
>> >> or Wine applications that access the same files simultaneously).
>> >
>> > The use of an is_conflict callback seems unnecessarily convoluted.
>> >
>> > If we need two different behaviors, let's just use another flag (or an
>> > extra boolean argument if we need to, or something).
>>
>> Ok, we can pass "bool is_mand" to flock_lock_file that will pass it
>> further to flock_locks_conflict.
>>
>> >
>> > The only caller for this new deny_lock_file is in the nfs code--I'm a
>> > little unclear why that is.
>>
>> deny_lock_file is called not only in the nfs code but also in 2 places
>> of fs/namei.c -- that enable this logic for VFS.
>
> Oops, apologies, I overlooked those somehow.
>
> What prevents somebody else from grabbing a lock on a newly-created file
> before we grab our own lock?
>
> I couldn't tell on a quick look whether we hold some lock that prevents
> that.

Nothing prevents it. If somebody grabbed a share mode lock on a file
before we call deny_lock_file, we simply close this file and return
-ETXTBSY. We can't grab it before atomic_open because we don't have an
inode there. Anyway, we can't make it atomic for VFS without big code
changes, but for CIFS and NFS it is already atomic with the discussed
patch.

-- 
Best regards,
Pavel Shilovsky.

^ permalink raw reply	[flat|nested] 32+ messages in thread
* Re: [PATCH v2 3/8] vfs: Add O_DENYREAD/WRITE flags support for open syscall
  2013-02-07  9:53             ` Pavel Shilovsky
@ 2013-02-07 14:18                   ` J. Bruce Fields
  0 siblings, 0 replies; 32+ messages in thread
From: J. Bruce Fields @ 2013-02-07 14:18 UTC (permalink / raw)
  To: Pavel Shilovsky
  Cc: linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	linux-fsdevel-u79uwXL29TY76Z2rM5mHXA,
	linux-cifs-u79uwXL29TY76Z2rM5mHXA,
	linux-nfs-u79uwXL29TY76Z2rM5mHXA,
	wine-devel-5vRYHf7vrtgdnm+yROfE0A

On Thu, Feb 07, 2013 at 01:53:46PM +0400, Pavel Shilovsky wrote:
> 2013/2/5 J. Bruce Fields <bfields-uC3wQj2KruNg9hUCZPvPmw@public.gmane.org>:
> > On Tue, Feb 05, 2013 at 03:45:31PM +0400, Pavel Shilovsky wrote:
> >> 2013/1/31 J. Bruce Fields <bfields-uC3wQj2KruNg9hUCZPvPmw@public.gmane.org>:
> >> > On Thu, Jan 17, 2013 at 08:52:59PM +0400, Pavel Shilovsky wrote:
> >> >> If O_DENYMAND flag is specified, O_DENYREAD/WRITE/MAND flags are
> >> >> translated to flock's flags:
> >> >>
> >> >> !O_DENYREAD  -> LOCK_READ
> >> >> !O_DENYWRITE -> LOCK_WRITE
> >> >> O_DENYMAND   -> LOCK_MAND
> >> >>
> >> >> and set through flock_lock_file on a file.
> >> >>
> >> >> This change only affects opens that use O_DENYMAND flag - all other
> >> >> native Linux opens don't care about these flags. It allow us to
> >> >> enable this feature for applications that need it (e.g. NFS and
> >> >> Samba servers that export the same directory for Windows clients,
> >> >> or Wine applications that access the same files simultaneously).
> >> >
> >> > The use of an is_conflict callback seems unnecessarily convoluted.
> >> >
> >> > If we need two different behaviors, let's just use another flag (or an
> >> > extra boolean argument if we need to, or something).
> >>
> >> Ok, we can pass "bool is_mand" to flock_lock_file that will pass it
> >> further to flock_locks_conflict.
> >>
> >> >
> >> > The only caller for this new deny_lock_file is in the nfs code--I'm a
> >> > little unclear why that is.
> >>
> >> deny_lock_file is called not only in the nfs code but also in 2 places
> >> of fs/namei.c -- that enable this logic for VFS.
> >
> > Oops, apologies, I overlooked those somehow.
> >
> > What prevents somebody else from grabbing a lock on a newly-created file
> > before we grab our own lock?
> >
> > I couldn't tell on a quick look whether we hold some lock that prevents
> > that.
> 
> Nothing prevents it. If somebody grabbed a share mode lock on a file
> before we call deny_lock_file, we simply close this file and return
> -ETXTBSY.

But leave the newly-created file there--ugh.

> We can't grab it before atomic_open because we don't have an
> inode there.

If you can get the lock while still holding the directory i_mutex can't
you prevent anyone else from looking up the new file until you've gotten
the lock?

--b.

> Anyway, we can't make it atomic for VFS without big code
> changes, but for CIFS and NFS it is already atomic with the discussed
> patch.

^ permalink raw reply	[flat|nested] 32+ messages in thread
* Re: [PATCH v2 3/8] vfs: Add O_DENYREAD/WRITE flags support for open syscall
@ 2013-02-07 14:18                   ` J. Bruce Fields
  0 siblings, 0 replies; 32+ messages in thread
From: J. Bruce Fields @ 2013-02-07 14:18 UTC (permalink / raw)
  To: Pavel Shilovsky
  Cc: linux-kernel, linux-fsdevel, linux-cifs, linux-nfs, wine-devel

On Thu, Feb 07, 2013 at 01:53:46PM +0400, Pavel Shilovsky wrote:
> 2013/2/5 J. Bruce Fields <bfields@fieldses.org>:
> > On Tue, Feb 05, 2013 at 03:45:31PM +0400, Pavel Shilovsky wrote:
> >> 2013/1/31 J. Bruce Fields <bfields@fieldses.org>:
> >> > On Thu, Jan 17, 2013 at 08:52:59PM +0400, Pavel Shilovsky wrote:
> >> >> If O_DENYMAND flag is specified, O_DENYREAD/WRITE/MAND flags are
> >> >> translated to flock's flags:
> >> >>
> >> >> !O_DENYREAD  -> LOCK_READ
> >> >> !O_DENYWRITE -> LOCK_WRITE
> >> >> O_DENYMAND   -> LOCK_MAND
> >> >>
> >> >> and set through flock_lock_file on a file.
> >> >>
> >> >> This change only affects opens that use O_DENYMAND flag - all other
> >> >> native Linux opens don't care about these flags. It allow us to
> >> >> enable this feature for applications that need it (e.g. NFS and
> >> >> Samba servers that export the same directory for Windows clients,
> >> >> or Wine applications that access the same files simultaneously).
> >> >
> >> > The use of an is_conflict callback seems unnecessarily convoluted.
> >> >
> >> > If we need two different behaviors, let's just use another flag (or an
> >> > extra boolean argument if we need to, or something).
> >>
> >> Ok, we can pass "bool is_mand" to flock_lock_file that will pass it
> >> further to flock_locks_conflict.
> >>
> >> >
> >> > The only caller for this new deny_lock_file is in the nfs code--I'm a
> >> > little unclear why that is.
> >>
> >> deny_lock_file is called not only in the nfs code but also in 2 places
> >> of fs/namei.c -- that enable this logic for VFS.
> >
> > Oops, apologies, I overlooked those somehow.
> >
> > What prevents somebody else from grabbing a lock on a newly-created file
> > before we grab our own lock?
> >
> > I couldn't tell on a quick look whether we hold some lock that prevents
> > that.
> 
> Nothing prevents it. If somebody grabbed a share mode lock on a file
> before we call deny_lock_file, we simply close this file and return
> -ETXTBSY.

But leave the newly-created file there--ugh.

> We can't grab it before atomic_open because we don't have an
> inode there.

If you can get the lock while still holding the directory i_mutex can't
you prevent anyone else from looking up the new file until you've gotten
the lock?

--b.

> Anyway, we can't make it atomic for VFS without big code
> changes, but for CIFS and NFS it is already atomic with the discussed
> patch.

^ permalink raw reply	[flat|nested] 32+ messages in thread
* Re: [PATCH v2 3/8] vfs: Add O_DENYREAD/WRITE flags support for open syscall
  2013-02-07 14:18                   ` J. Bruce Fields
@ 2013-02-07 14:32                       ` Pavel Shilovsky
  -1 siblings, 0 replies; 32+ messages in thread
From: Pavel Shilovsky @ 2013-02-07 14:32 UTC (permalink / raw)
  To: J. Bruce Fields
  Cc: linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	linux-fsdevel-u79uwXL29TY76Z2rM5mHXA,
	linux-cifs-u79uwXL29TY76Z2rM5mHXA,
	linux-nfs-u79uwXL29TY76Z2rM5mHXA,
	wine-devel-5vRYHf7vrtgdnm+yROfE0A

2013/2/7 J. Bruce Fields <bfields-uC3wQj2KruNg9hUCZPvPmw@public.gmane.org>:
> On Thu, Feb 07, 2013 at 01:53:46PM +0400, Pavel Shilovsky wrote:
>> Nothing prevents it. If somebody grabbed a share mode lock on a file
>> before we call deny_lock_file, we simply close this file and return
>> -ETXTBSY.
>
> But leave the newly-created file there--ugh.
>
>> We can't grab it before atomic_open because we don't have an
>> inode there.
>
> If you can get the lock while still holding the directory i_mutex can't
> you prevent anyone else from looking up the new file until you've gotten
> the lock?
>

Hm..., seems you are right, I missed this part:
mutex_lock
lookup_open -> atomic_open -> deny_lock_file
mutex_unlock

that means that nobody can open and of course set flock on the newly
created file (because flock is done through file descriptor). So, it
should be fine to call flock after f_ops->atomic_open in atomic_open
function. Thanks.

-- 
Best regards,
Pavel Shilovsky.
--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 32+ messages in thread
* Re: [PATCH v2 3/8] vfs: Add O_DENYREAD/WRITE flags support for open syscall
@ 2013-02-07 14:32                       ` Pavel Shilovsky
  0 siblings, 0 replies; 32+ messages in thread
From: Pavel Shilovsky @ 2013-02-07 14:32 UTC (permalink / raw)
  To: J. Bruce Fields
  Cc: linux-kernel, linux-fsdevel, linux-cifs, linux-nfs, wine-devel

2013/2/7 J. Bruce Fields <bfields@fieldses.org>:
> On Thu, Feb 07, 2013 at 01:53:46PM +0400, Pavel Shilovsky wrote:
>> Nothing prevents it. If somebody grabbed a share mode lock on a file
>> before we call deny_lock_file, we simply close this file and return
>> -ETXTBSY.
>
> But leave the newly-created file there--ugh.
>
>> We can't grab it before atomic_open because we don't have an
>> inode there.
>
> If you can get the lock while still holding the directory i_mutex can't
> you prevent anyone else from looking up the new file until you've gotten
> the lock?
>

Hm..., seems you are right, I missed this part:
mutex_lock
lookup_open -> atomic_open -> deny_lock_file
mutex_unlock

that means that nobody can open and of course set flock on the newly
created file (because flock is done through file descriptor). So, it
should be fine to call flock after f_ops->atomic_open in atomic_open
function. Thanks.

-- 
Best regards,
Pavel Shilovsky.

^ permalink raw reply	[flat|nested] 32+ messages in thread
* Re: [PATCH v2 3/8] vfs: Add O_DENYREAD/WRITE flags support for open syscall
  2013-02-07 14:32                       ` Pavel Shilovsky
@ 2013-02-07 14:41                           ` J. Bruce Fields
  -1 siblings, 0 replies; 32+ messages in thread
From: J. Bruce Fields @ 2013-02-07 14:41 UTC (permalink / raw)
  To: Pavel Shilovsky
  Cc: linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	linux-fsdevel-u79uwXL29TY76Z2rM5mHXA,
	linux-cifs-u79uwXL29TY76Z2rM5mHXA,
	linux-nfs-u79uwXL29TY76Z2rM5mHXA,
	wine-devel-5vRYHf7vrtgdnm+yROfE0A

On Thu, Feb 07, 2013 at 06:32:38PM +0400, Pavel Shilovsky wrote:
> 2013/2/7 J. Bruce Fields <bfields-uC3wQj2KruNg9hUCZPvPmw@public.gmane.org>:
> > On Thu, Feb 07, 2013 at 01:53:46PM +0400, Pavel Shilovsky wrote:
> >> Nothing prevents it. If somebody grabbed a share mode lock on a file
> >> before we call deny_lock_file, we simply close this file and return
> >> -ETXTBSY.
> >
> > But leave the newly-created file there--ugh.
> >
> >> We can't grab it before atomic_open because we don't have an
> >> inode there.
> >
> > If you can get the lock while still holding the directory i_mutex can't
> > you prevent anyone else from looking up the new file until you've gotten
> > the lock?
> >
> 
> Hm..., seems you are right, I missed this part:
> mutex_lock
> lookup_open -> atomic_open -> deny_lock_file
> mutex_unlock
> 
> that means that nobody can open and of course set flock on the newly
> created file (because flock is done through file descriptor). So, it
> should be fine to call flock after f_ops->atomic_open in atomic_open
> function. Thanks.

Whether that works may also depend on how the new dentry is set up?  If
it's hashed before you call flock then I suppose it's already visible to
others.

Not knowing that code as well as I should, I might test by introducing
an artificial delay there and trying to reproduce the race.

--b.

^ permalink raw reply	[flat|nested] 32+ messages in thread
* Re: [PATCH v2 3/8] vfs: Add O_DENYREAD/WRITE flags support for open syscall
@ 2013-02-07 14:41                           ` J. Bruce Fields
  0 siblings, 0 replies; 32+ messages in thread
From: J. Bruce Fields @ 2013-02-07 14:41 UTC (permalink / raw)
  To: Pavel Shilovsky
  Cc: linux-kernel, linux-fsdevel, linux-cifs, linux-nfs, wine-devel

On Thu, Feb 07, 2013 at 06:32:38PM +0400, Pavel Shilovsky wrote:
> 2013/2/7 J. Bruce Fields <bfields@fieldses.org>:
> > On Thu, Feb 07, 2013 at 01:53:46PM +0400, Pavel Shilovsky wrote:
> >> Nothing prevents it. If somebody grabbed a share mode lock on a file
> >> before we call deny_lock_file, we simply close this file and return
> >> -ETXTBSY.
> >
> > But leave the newly-created file there--ugh.
> >
> >> We can't grab it before atomic_open because we don't have an
> >> inode there.
> >
> > If you can get the lock while still holding the directory i_mutex can't
> > you prevent anyone else from looking up the new file until you've gotten
> > the lock?
> >
> 
> Hm..., seems you are right, I missed this part:
> mutex_lock
> lookup_open -> atomic_open -> deny_lock_file
> mutex_unlock
> 
> that means that nobody can open and of course set flock on the newly
> created file (because flock is done through file descriptor). So, it
> should be fine to call flock after f_ops->atomic_open in atomic_open
> function. Thanks.

Whether that works may also depend on how the new dentry is set up?  If
it's hashed before you call flock then I suppose it's already visible to
others.

Not knowing that code as well as I should, I might test by introducing
an artificial delay there and trying to reproduce the race.

--b.

^ permalink raw reply	[flat|nested] 32+ messages in thread
* Re: [PATCH v2 3/8] vfs: Add O_DENYREAD/WRITE flags support for open syscall
  2013-02-07 14:41                           ` J. Bruce Fields
@ 2013-02-07 16:00                               ` Pavel Shilovsky
  -1 siblings, 0 replies; 32+ messages in thread
From: Pavel Shilovsky @ 2013-02-07 16:00 UTC (permalink / raw)
  To: J. Bruce Fields
  Cc: linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	linux-fsdevel-u79uwXL29TY76Z2rM5mHXA,
	linux-cifs-u79uwXL29TY76Z2rM5mHXA,
	linux-nfs-u79uwXL29TY76Z2rM5mHXA,
	wine-devel-5vRYHf7vrtgdnm+yROfE0A

2013/2/7 J. Bruce Fields <bfields-uC3wQj2KruNg9hUCZPvPmw@public.gmane.org>:
> On Thu, Feb 07, 2013 at 06:32:38PM +0400, Pavel Shilovsky wrote:
>> 2013/2/7 J. Bruce Fields <bfields-uC3wQj2KruNg9hUCZPvPmw@public.gmane.org>:
>> > On Thu, Feb 07, 2013 at 01:53:46PM +0400, Pavel Shilovsky wrote:
>> >> Nothing prevents it. If somebody grabbed a share mode lock on a file
>> >> before we call deny_lock_file, we simply close this file and return
>> >> -ETXTBSY.
>> >
>> > But leave the newly-created file there--ugh.
>> >
>> >> We can't grab it before atomic_open because we don't have an
>> >> inode there.
>> >
>> > If you can get the lock while still holding the directory i_mutex can't
>> > you prevent anyone else from looking up the new file until you've gotten
>> > the lock?
>> >
>>
>> Hm..., seems you are right, I missed this part:
>> mutex_lock
>> lookup_open -> atomic_open -> deny_lock_file
>> mutex_unlock
>>
>> that means that nobody can open and of course set flock on the newly
>> created file (because flock is done through file descriptor). So, it
>> should be fine to call flock after f_ops->atomic_open in atomic_open
>> function. Thanks.
>
> Whether that works may also depend on how the new dentry is set up?  If
> it's hashed before you call flock then I suppose it's already visible to
> others.

It seems it should be hashed in f_ops->atomic_open() (at least cifs
and nfs do it this way). In do_last when we do an ordinary open, we
don't hit parent i_mutex if lookup is succeeded through lookup_fast.
lookup_fast can catch newly created dentry and set it's share mode
before atomic_open codepath hits deny_lock_file.

Also, I noted that: atomic open does f_ops->atomic_open and then it
processes may_open check; if may_open fails, the file is closed and
open returns with a error code (but file is created anyway) . I think
there is no difference between this case and the situation with
deny_lock_file there.

-- 
Best regards,
Pavel Shilovsky.

^ permalink raw reply	[flat|nested] 32+ messages in thread
* Re: [PATCH v2 3/8] vfs: Add O_DENYREAD/WRITE flags support for open syscall
@ 2013-02-07 16:00                               ` Pavel Shilovsky
  0 siblings, 0 replies; 32+ messages in thread
From: Pavel Shilovsky @ 2013-02-07 16:00 UTC (permalink / raw)
  To: J. Bruce Fields
  Cc: linux-kernel, linux-fsdevel, linux-cifs, linux-nfs, wine-devel

2013/2/7 J. Bruce Fields <bfields@fieldses.org>:
> On Thu, Feb 07, 2013 at 06:32:38PM +0400, Pavel Shilovsky wrote:
>> 2013/2/7 J. Bruce Fields <bfields@fieldses.org>:
>> > On Thu, Feb 07, 2013 at 01:53:46PM +0400, Pavel Shilovsky wrote:
>> >> Nothing prevents it. If somebody grabbed a share mode lock on a file
>> >> before we call deny_lock_file, we simply close this file and return
>> >> -ETXTBSY.
>> >
>> > But leave the newly-created file there--ugh.
>> >
>> >> We can't grab it before atomic_open because we don't have an
>> >> inode there.
>> >
>> > If you can get the lock while still holding the directory i_mutex can't
>> > you prevent anyone else from looking up the new file until you've gotten
>> > the lock?
>> >
>>
>> Hm..., seems you are right, I missed this part:
>> mutex_lock
>> lookup_open -> atomic_open -> deny_lock_file
>> mutex_unlock
>>
>> that means that nobody can open and of course set flock on the newly
>> created file (because flock is done through file descriptor). So, it
>> should be fine to call flock after f_ops->atomic_open in atomic_open
>> function. Thanks.
>
> Whether that works may also depend on how the new dentry is set up?  If
> it's hashed before you call flock then I suppose it's already visible to
> others.

It seems it should be hashed in f_ops->atomic_open() (at least cifs
and nfs do it this way). In do_last when we do an ordinary open, we
don't hit parent i_mutex if lookup is succeeded through lookup_fast.
lookup_fast can catch newly created dentry and set it's share mode
before atomic_open codepath hits deny_lock_file.

Also, I noted that: atomic open does f_ops->atomic_open and then it
processes may_open check; if may_open fails, the file is closed and
open returns with a error code (but file is created anyway) . I think
there is no difference between this case and the situation with
deny_lock_file there.

-- 
Best regards,
Pavel Shilovsky.

^ permalink raw reply	[flat|nested] 32+ messages in thread
* Re: [PATCH v2 3/8] vfs: Add O_DENYREAD/WRITE flags support for open syscall
  2013-02-07 16:00                               ` Pavel Shilovsky
@ 2013-02-07 16:19                                   ` J. Bruce Fields
  -1 siblings, 0 replies; 32+ messages in thread
From: J. Bruce Fields @ 2013-02-07 16:19 UTC (permalink / raw)
  To: Pavel Shilovsky
  Cc: linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	linux-fsdevel-u79uwXL29TY76Z2rM5mHXA,
	linux-cifs-u79uwXL29TY76Z2rM5mHXA,
	linux-nfs-u79uwXL29TY76Z2rM5mHXA,
	wine-devel-5vRYHf7vrtgdnm+yROfE0A

On Thu, Feb 07, 2013 at 08:00:13PM +0400, Pavel Shilovsky wrote:
> 2013/2/7 J. Bruce Fields <bfields-uC3wQj2KruNg9hUCZPvPmw@public.gmane.org>:
> > On Thu, Feb 07, 2013 at 06:32:38PM +0400, Pavel Shilovsky wrote:
> >> 2013/2/7 J. Bruce Fields <bfields-uC3wQj2KruNg9hUCZPvPmw@public.gmane.org>:
> >> > On Thu, Feb 07, 2013 at 01:53:46PM +0400, Pavel Shilovsky wrote:
> >> >> Nothing prevents it. If somebody grabbed a share mode lock on a file
> >> >> before we call deny_lock_file, we simply close this file and return
> >> >> -ETXTBSY.
> >> >
> >> > But leave the newly-created file there--ugh.
> >> >
> >> >> We can't grab it before atomic_open because we don't have an
> >> >> inode there.
> >> >
> >> > If you can get the lock while still holding the directory i_mutex can't
> >> > you prevent anyone else from looking up the new file until you've gotten
> >> > the lock?
> >> >
> >>
> >> Hm..., seems you are right, I missed this part:
> >> mutex_lock
> >> lookup_open -> atomic_open -> deny_lock_file
> >> mutex_unlock
> >>
> >> that means that nobody can open and of course set flock on the newly
> >> created file (because flock is done through file descriptor). So, it
> >> should be fine to call flock after f_ops->atomic_open in atomic_open
> >> function. Thanks.
> >
> > Whether that works may also depend on how the new dentry is set up?  If
> > it's hashed before you call flock then I suppose it's already visible to
> > others.
> 
> It seems it should be hashed in f_ops->atomic_open() (at least cifs
> and nfs do it this way). In do_last when we do an ordinary open, we
> don't hit parent i_mutex if lookup is succeeded through lookup_fast.
> lookup_fast can catch newly created dentry and set it's share mode
> before atomic_open codepath hits deny_lock_file.
> 
> Also, I noted that: atomic open does f_ops->atomic_open and then it
> processes may_open check; if may_open fails, the file is closed and
> open returns with a error code (but file is created anyway).

That would be a bug, I think.  E.g. "man 3posix open":

	No  files  shall  be created or modified if the function returns
	-1.

Looking at the code...  See the references to FILE_CREATED in
atomic_open--looks like that's trying to prevent may_open from failing
in this case.

> I think
> there is no difference between this case and the situation with
> deny_lock_file there.

Looks to me like it would be a bug in either case.

--b.

^ permalink raw reply	[flat|nested] 32+ messages in thread
* Re: [PATCH v2 3/8] vfs: Add O_DENYREAD/WRITE flags support for open syscall
@ 2013-02-07 16:19                                   ` J. Bruce Fields
  0 siblings, 0 replies; 32+ messages in thread
From: J. Bruce Fields @ 2013-02-07 16:19 UTC (permalink / raw)
  To: Pavel Shilovsky
  Cc: linux-kernel, linux-fsdevel, linux-cifs, linux-nfs, wine-devel

On Thu, Feb 07, 2013 at 08:00:13PM +0400, Pavel Shilovsky wrote:
> 2013/2/7 J. Bruce Fields <bfields@fieldses.org>:
> > On Thu, Feb 07, 2013 at 06:32:38PM +0400, Pavel Shilovsky wrote:
> >> 2013/2/7 J. Bruce Fields <bfields@fieldses.org>:
> >> > On Thu, Feb 07, 2013 at 01:53:46PM +0400, Pavel Shilovsky wrote:
> >> >> Nothing prevents it. If somebody grabbed a share mode lock on a file
> >> >> before we call deny_lock_file, we simply close this file and return
> >> >> -ETXTBSY.
> >> >
> >> > But leave the newly-created file there--ugh.
> >> >
> >> >> We can't grab it before atomic_open because we don't have an
> >> >> inode there.
> >> >
> >> > If you can get the lock while still holding the directory i_mutex can't
> >> > you prevent anyone else from looking up the new file until you've gotten
> >> > the lock?
> >> >
> >>
> >> Hm..., seems you are right, I missed this part:
> >> mutex_lock
> >> lookup_open -> atomic_open -> deny_lock_file
> >> mutex_unlock
> >>
> >> that means that nobody can open and of course set flock on the newly
> >> created file (because flock is done through file descriptor). So, it
> >> should be fine to call flock after f_ops->atomic_open in atomic_open
> >> function. Thanks.
> >
> > Whether that works may also depend on how the new dentry is set up?  If
> > it's hashed before you call flock then I suppose it's already visible to
> > others.
> 
> It seems it should be hashed in f_ops->atomic_open() (at least cifs
> and nfs do it this way). In do_last when we do an ordinary open, we
> don't hit parent i_mutex if lookup is succeeded through lookup_fast.
> lookup_fast can catch newly created dentry and set it's share mode
> before atomic_open codepath hits deny_lock_file.
> 
> Also, I noted that: atomic open does f_ops->atomic_open and then it
> processes may_open check; if may_open fails, the file is closed and
> open returns with a error code (but file is created anyway).

That would be a bug, I think.  E.g. "man 3posix open":

	No  files  shall  be created or modified if the function returns
	-1.

Looking at the code...  See the references to FILE_CREATED in
atomic_open--looks like that's trying to prevent may_open from failing
in this case.

> I think
> there is no difference between this case and the situation with
> deny_lock_file there.

Looks to me like it would be a bug in either case.

--b.

^ permalink raw reply	[flat|nested] 32+ messages in thread
* Re: [PATCH v2 3/8] vfs: Add O_DENYREAD/WRITE flags support for open syscall
  2013-02-07 16:19                                   ` J. Bruce Fields
@ 2013-02-07 16:50                                       ` Pavel Shilovsky
  -1 siblings, 0 replies; 32+ messages in thread
From: Pavel Shilovsky @ 2013-02-07 16:50 UTC (permalink / raw)
  To: J. Bruce Fields
  Cc: linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	linux-fsdevel-u79uwXL29TY76Z2rM5mHXA,
	linux-cifs-u79uwXL29TY76Z2rM5mHXA,
	linux-nfs-u79uwXL29TY76Z2rM5mHXA,
	wine-devel-5vRYHf7vrtgdnm+yROfE0A

2013/2/7 J. Bruce Fields <bfields-uC3wQj2KruNg9hUCZPvPmw@public.gmane.org>:
> That would be a bug, I think.  E.g. "man 3posix open":
>
>         No  files  shall  be created or modified if the function returns
>         -1.
>
> Looking at the code...  See the references to FILE_CREATED in
> atomic_open--looks like that's trying to prevent may_open from failing
> in this case.
>
>> I think
>> there is no difference between this case and the situation with
>> deny_lock_file there.
>
> Looks to me like it would be a bug in either case.

Then we returned from lookup_open in do_last we go to 'opened' lable.
Then we have a 3(!) chances to return -1 while a file is created
(open_check_o_direct, ima_file_check, handle_truncate). In this case
these places are bugs too.

We can call vfs_unlink if we failed after a file was created, but
possible affects need to be investigated.

-- 
Best regards,
Pavel Shilovsky.

^ permalink raw reply	[flat|nested] 32+ messages in thread
* Re: [PATCH v2 3/8] vfs: Add O_DENYREAD/WRITE flags support for open syscall
@ 2013-02-07 16:50                                       ` Pavel Shilovsky
  0 siblings, 0 replies; 32+ messages in thread
From: Pavel Shilovsky @ 2013-02-07 16:50 UTC (permalink / raw)
  To: J. Bruce Fields
  Cc: linux-kernel, linux-fsdevel, linux-cifs, linux-nfs, wine-devel

2013/2/7 J. Bruce Fields <bfields@fieldses.org>:
> That would be a bug, I think.  E.g. "man 3posix open":
>
>         No  files  shall  be created or modified if the function returns
>         -1.
>
> Looking at the code...  See the references to FILE_CREATED in
> atomic_open--looks like that's trying to prevent may_open from failing
> in this case.
>
>> I think
>> there is no difference between this case and the situation with
>> deny_lock_file there.
>
> Looks to me like it would be a bug in either case.

Then we returned from lookup_open in do_last we go to 'opened' lable.
Then we have a 3(!) chances to return -1 while a file is created
(open_check_o_direct, ima_file_check, handle_truncate). In this case
these places are bugs too.

We can call vfs_unlink if we failed after a file was created, but
possible affects need to be investigated.

-- 
Best regards,
Pavel Shilovsky.

^ permalink raw reply	[flat|nested] 32+ messages in thread
* Re: [PATCH v2 3/8] vfs: Add O_DENYREAD/WRITE flags support for open syscall
  2013-02-07 16:50                                       ` Pavel Shilovsky
  (?)
@ 2013-02-07 17:03                                       ` J. Bruce Fields
  -1 siblings, 0 replies; 32+ messages in thread
From: J. Bruce Fields @ 2013-02-07 17:03 UTC (permalink / raw)
  To: Pavel Shilovsky
  Cc: linux-kernel, linux-fsdevel, linux-cifs, linux-nfs, wine-devel

On Thu, Feb 07, 2013 at 08:50:16PM +0400, Pavel Shilovsky wrote:
> 2013/2/7 J. Bruce Fields <bfields@fieldses.org>:
> > That would be a bug, I think.  E.g. "man 3posix open":
> >
> >         No  files  shall  be created or modified if the function returns
> >         -1.
> >
> > Looking at the code...  See the references to FILE_CREATED in
> > atomic_open--looks like that's trying to prevent may_open from failing
> > in this case.
> >
> >> I think
> >> there is no difference between this case and the situation with
> >> deny_lock_file there.
> >
> > Looks to me like it would be a bug in either case.
> 
> Then we returned from lookup_open in do_last we go to 'opened' lable.
> Then we have a 3(!) chances to return -1 while a file is created
> (open_check_o_direct, ima_file_check, handle_truncate

I don't know about the first two, but handle_truncate won't be hit since
will_truncate is false.

> ). In this case
> these places are bugs too.
> 
> We can call vfs_unlink if we failed after a file was created, but
> possible affects need to be investigated.

We definitely don't want to try to undo the create with an unlink.

--b.

^ permalink raw reply	[flat|nested] 32+ messages in thread
end of thread, other threads:[~2013-02-07 17:03 UTC | newest]

Thread overview: 32+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-01-17 16:52 [PATCH v2 1/8] locks: make flock_lock_file take is_conflict callback parm Pavel Shilovsky
2013-01-17 16:52 ` Pavel Shilovsky
2013-01-17 16:52 ` [PATCH v2 2/8] fcntl: Introduce new O_DENY* open flags Pavel Shilovsky
2013-01-17 16:52 ` [PATCH v2 3/8] vfs: Add O_DENYREAD/WRITE flags support for open syscall Pavel Shilovsky
     [not found]   ` <1358441584-8783-3-git-send-email-piastry-7qunaywFIewox3rIn2DAYQ@public.gmane.org>
2013-01-30 22:16     ` J. Bruce Fields
2013-01-30 22:16       ` J. Bruce Fields
     [not found]       ` <20130130221602.GC15584-uC3wQj2KruNg9hUCZPvPmw@public.gmane.org>
2013-02-05 11:45         ` Pavel Shilovsky
2013-02-05 11:45           ` Pavel Shilovsky
2013-02-05 14:35           ` J. Bruce Fields
2013-02-07  9:53             ` Pavel Shilovsky
     [not found]               ` <CAKywueS6oGNu9r_bypu32A89pOoiGb47kjXqw2ZcZQhZK+JV3g-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2013-02-07 14:18                 ` J. Bruce Fields
2013-02-07 14:18                   ` J. Bruce Fields
     [not found]                   ` <20130207141832.GA3222-uC3wQj2KruNg9hUCZPvPmw@public.gmane.org>
2013-02-07 14:32                     ` Pavel Shilovsky
2013-02-07 14:32                       ` Pavel Shilovsky
     [not found]                       ` <CAKywueTmU_L5cwwJSoUY6753eHf_38VtHadZGt9tcLS59sqmOA-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2013-02-07 14:41                         ` J. Bruce Fields
2013-02-07 14:41                           ` J. Bruce Fields
     [not found]                           ` <20130207144156.GB3222-uC3wQj2KruNg9hUCZPvPmw@public.gmane.org>
2013-02-07 16:00                             ` Pavel Shilovsky
2013-02-07 16:00                               ` Pavel Shilovsky
     [not found]                               ` <CAKywueShbPd9b+WmJwnfwPR_vzk_atBVRBZNTf-HpS7N2CK+AA-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2013-02-07 16:19                                 ` J. Bruce Fields
2013-02-07 16:19                                   ` J. Bruce Fields
     [not found]                                   ` <20130207161948.GG3222-uC3wQj2KruNg9hUCZPvPmw@public.gmane.org>
2013-02-07 16:50                                     ` Pavel Shilovsky
2013-02-07 16:50                                       ` Pavel Shilovsky
2013-02-07 17:03                                       ` J. Bruce Fields
2013-01-17 16:53 ` [PATCH v2 4/8] CIFS: Add O_DENY* open flags support Pavel Shilovsky
     [not found]   ` <1358441584-8783-4-git-send-email-piastry-7qunaywFIewox3rIn2DAYQ@public.gmane.org>
2013-01-30 22:16     ` J. Bruce Fields
2013-01-30 22:16       ` J. Bruce Fields
     [not found]       ` <20130130221657.GD15584-uC3wQj2KruNg9hUCZPvPmw@public.gmane.org>
2013-02-05 11:54         ` Pavel Shilovsky
2013-02-05 11:54           ` Pavel Shilovsky
2013-01-17 16:53 ` [PATCH v2 5/8] CIFS: Use NT_CREATE_ANDX command for forcemand mounts Pavel Shilovsky
2013-01-17 16:53 ` [PATCH v2 6/8] CIFS: Translate SHARING_VIOLATION to -ETXTBSY error code for SMB2 Pavel Shilovsky
2013-01-17 16:53 ` [PATCH v2 7/8] NFSv4: Add O_DENY* open flags support Pavel Shilovsky
2013-01-17 16:53 ` [PATCH v2 8/8] NFSD: Pass share reservations flags to VFS Pavel Shilovsky

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.