2013-04-09 12:36:33

by Pavel Shilovsky

[permalink] [raw]
Subject: [PATCH v5 0/7] Add O_DENY* support for VFS and CIFS/NFS

Main changes fom v4:
1) deny_lock_file uses FS_DOES_SHARELOCK flag from fs_flags to determine whether to use VFS locks or not.
2) Make nfs code return -EBUSY for share conflicts (was -EACCESS).

Main changes from v3
1) O_DENYMAND is removed, sharelock mount option is introduced.
2) Patch fcntl.h and VFS patches are united into one.
3) flock/LOCK_MAND is disabled for sharelock mounts.

This patchset adds support of O_DENY* flags for Linux fs layer. These flags can be used by any application that needs share reservations to organize a file access. VFS already has some sort of this capability - now it's done through flock/LOCK_MAND mechanis, but that approach is non-atomic. This patchset build new capabilities on top of the existing one but doesn't bring any changes into the flock call semantic.

These flags can be used by NFS (built-in-kernel) and CIFS (Samba) servers and Wine applications through VFS (for local filesystems) or CIFS/NFS modules. This will help when e.g. Samba and NFS server share the same directory for Windows and Linux users or Wine applications use Samba/NFS share to access the same data from different clients.

According to the previous discussions the most problematic question is how to prevent situations like DoS attacks where e.g /lib/liba.so file can be open with DENYREAD, or smth like this. That's why extra mount option 'sharelock' is added for switching on/off O_DENY* flags processing. It allows us to avoid use of these flags on critical places (like /, /lib) and turn them on if we really want it proccessed that way.

So, we have 3 new flags:
O_DENYREAD - to prevent other opens with read access,
O_DENYWRITE - to prevent other opens with write access,
O_DENYDELETE - to prevent delete operations (this flag is not implemented in VFS and NFS part and only suitable for CIFS module),

The patchset avoid data races problem on newely created files: open with O_CREAT can return the -ETXTBSY error for a successfully created file if this files was locked with a deny lock by another task. Also, it turns flock/LOCK_MAND capability off for mounts with 'sharelock' option. This let us not mix one share reservation approach with another.

The #1 patch adds flags to fcntl and implements VFS part. The patches #3, #4, #5 are related to CIFS-specific changes, #6 and #7 describe NFS and NFSD parts.

The preliminary patch for Samba that replaces the existing use of flock/LOCK_MAND mechanism with O_DENY* flags:
http://git.etersoft.ru/people/piastry/packages/?p=samba.git;a=commitdiff;h=173070262b7f29134ad6b2e49a760017c11aec4a

The future part of open man page patch:

-----
O_DENYREAD, O_DENYWRIYE, O_DENYDELETE - used to inforce a mandatory share reservation scheme of the file access. If these flag is passed, the open fails with -EBUSY in following cases:
1) if O_DENYREAD flag is specified and there is another open with READ access to the file;
2) if O_DENYWRITE flag is specified and there is another open with WRITE access to the file;
3) if READ access is requested and there is another open with O_DENYREAD flags;
4) if WRITE access is requested and there is another open with O_DENYWRITE flags.

If O_DENYDELETE flag is specified and the open succeded, any further unlink operation will fail with -EBUSY untill this open is closed. Now this flag is processed by CIFS filesystems only.

This mechanism is done through flocks. If O_DENY* flags are specified, flock syscall is processed after the open. The share modes are translated into flock according the following rules:
1) !O_DENYREAD -> LOCK_READ | LOCK_MAND
2) !O_DENYWRITE -> LOCK_WRITE | LOCK_MAND
-----

Pavel Shilovsky (7):
fcntl: Introduce new O_DENY* open flags
CIFS: Add share_access parm to open request
CIFS: Add O_DENY* open flags support
CIFS: Use NT_CREATE_ANDX command for forcemand mounts
NFSv4: Add O_DENY* open flags support
NFSD: Pass share reservations flags to VFS
locks: Disable LOCK_MAND support for MS_SHARELOCK mounts

fs/cifs/cifsacl.c | 8 +--
fs/cifs/cifsfs.c | 2 +-
fs/cifs/cifsglob.h | 18 ++++++-
fs/cifs/cifsproto.h | 8 +--
fs/cifs/cifssmb.c | 50 ++++++++++---------
fs/cifs/dir.c | 16 +++---
fs/cifs/file.c | 20 +++++---
fs/cifs/inode.c | 16 +++---
fs/cifs/link.c | 25 +++-------
fs/cifs/readdir.c | 5 +-
fs/cifs/smb1ops.c | 16 +++---
fs/cifs/smb2file.c | 10 ++--
fs/cifs/smb2inode.c | 4 +-
fs/cifs/smb2ops.c | 10 ++--
fs/cifs/smb2pdu.c | 6 +--
fs/cifs/smb2proto.h | 14 +++---
fs/fcntl.c | 5 +-
fs/locks.c | 105 ++++++++++++++++++++++++++++++++++++---
fs/namei.c | 45 +++++++++++++++--
fs/nfs/internal.h | 3 +-
fs/nfs/nfs4proc.c | 4 +-
fs/nfs/nfs4super.c | 9 ++--
fs/nfs/nfs4xdr.c | 21 ++++++--
fs/nfs/super.c | 7 ++-
fs/nfsd/nfs4state.c | 46 ++++++++++++++++-
fs/proc_namespace.c | 1 +
include/linux/fs.h | 8 +++
include/uapi/asm-generic/fcntl.h | 11 ++++
include/uapi/linux/fs.h | 1 +
29 files changed, 364 insertions(+), 130 deletions(-)

--
1.8.1.5


2013-04-09 12:36:36

by Pavel Shilovsky

[permalink] [raw]
Subject: [PATCH v5 1/7] fcntl: Introduce new O_DENY* open flags

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

Network filesystems CIFS, SMB2.0, SMB3.0 and NFSv4 have such flags -
this change can benefit cifs and nfs modules as well as Samba and
NFS file servers that export the same directory for Windows clients,
or Wine applications that access the same files simultaneously.

These flags are only take affect for opens on mounts with 'sharelock'
mount option. They are translated to flock's flags:

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

and set through flock_lock_file on a file. If the file can't be locked
due conflicts with another open with O_DENY* flags, the -EBUSY error
code is returned.

Create codepath is slightly changed to prevent data races on
newely created files: when open with O_CREAT can return with -EBUSY
error for successfully created files due to a deny lock set by
another task.

Signed-off-by: Pavel Shilovsky <[email protected]>
---
fs/fcntl.c | 5 ++-
fs/locks.c | 93 +++++++++++++++++++++++++++++++++++++---
fs/namei.c | 45 +++++++++++++++++--
fs/proc_namespace.c | 1 +
include/linux/fs.h | 7 +++
include/uapi/asm-generic/fcntl.h | 11 +++++
include/uapi/linux/fs.h | 1 +
7 files changed, 150 insertions(+), 13 deletions(-)

diff --git a/fs/fcntl.c b/fs/fcntl.c
index 71a600a..7abce5a 100644
--- a/fs/fcntl.c
+++ b/fs/fcntl.c
@@ -730,14 +730,15 @@ static int __init fcntl_init(void)
* Exceptions: O_NONBLOCK is a two bit define on parisc; O_NDELAY
* is defined as O_NONBLOCK on some platforms and not on others.
*/
- BUILD_BUG_ON(19 - 1 /* for O_RDONLY being 0 */ != HWEIGHT32(
+ BUILD_BUG_ON(22 - 1 /* for O_RDONLY being 0 */ != HWEIGHT32(
O_RDONLY | O_WRONLY | O_RDWR |
O_CREAT | O_EXCL | O_NOCTTY |
O_TRUNC | O_APPEND | /* O_NONBLOCK | */
__O_SYNC | O_DSYNC | FASYNC |
O_DIRECT | O_LARGEFILE | O_DIRECTORY |
O_NOFOLLOW | O_NOATIME | O_CLOEXEC |
- __FMODE_EXEC | O_PATH
+ __FMODE_EXEC | O_PATH | O_DENYREAD |
+ O_DENYWRITE | O_DENYDELETE
));

fasync_cache = kmem_cache_create("fasync_cache",
diff --git a/fs/locks.c b/fs/locks.c
index a94e331..1eccc75 100644
--- a/fs/locks.c
+++ b/fs/locks.c
@@ -605,20 +605,73 @@ 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
- * checking before calling the locks_conflict().
+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.
+ */
+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 FS is mounted with MS_SHARELOCK */
+ if (!IS_SHARELOCK(caller_fl->fl_file->f_path.dentry->d_inode))
+ return 0;
+
+ /* they can only conflict if they're both LOCK_MAND */
+ if (!(caller_type & LOCK_MAND) || !(sys_type & LOCK_MAND))
+ return 0;
+
+ if (!(caller_type & LOCK_READ) && (sys_fmode & FMODE_READ))
+ return 1;
+ if (!(caller_type & LOCK_WRITE) && (sys_fmode & FMODE_WRITE))
+ return 1;
+ if (!(sys_type & LOCK_READ) && (caller_fmode & FMODE_READ))
+ return 1;
+ if (!(sys_type & LOCK_WRITE) && (caller_fmode & FMODE_WRITE))
+ return 1;
+
+ return 0;
+}
+
+/*
+ * Determine if lock sys_fl blocks lock caller_fl. FLOCK specific checking
+ * before calling the locks_conflict().
*/
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
+ 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);
+ /*
+ * FLOCK locks referring to the same filp do not conflict with
* each other.
*/
- if (!IS_FLOCK(sys_fl) || (caller_fl->fl_file == sys_fl->fl_file))
- return (0);
- if ((caller_fl->fl_type & LOCK_MAND) || (sys_fl->fl_type & LOCK_MAND))
+ if (caller_fl->fl_file == sys_fl->fl_file)
return 0;

- return (locks_conflict(caller_fl, sys_fl));
+ return locks_conflict(caller_fl, sys_fl);
}

void
@@ -783,6 +836,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 (!IS_SHARELOCK(filp->f_path.dentry->d_inode))
+ 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);
+ if (error == -EAGAIN)
+ error = -EBUSY;
+
+ 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 ec97aef..416c74f 100644
--- a/fs/namei.c
+++ b/fs/namei.c
@@ -2557,9 +2557,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;
@@ -2769,9 +2774,9 @@ retry_lookup:
}
mutex_lock(&dir->d_inode->i_mutex);
error = lookup_open(nd, path, file, op, got_write, opened);
- mutex_unlock(&dir->d_inode->i_mutex);

if (error <= 0) {
+ mutex_unlock(&dir->d_inode->i_mutex);
if (error)
goto out;

@@ -2789,8 +2794,32 @@ retry_lookup:
will_truncate = false;
acc_mode = MAY_OPEN;
path_to_nameidata(path, nd);
- goto finish_open_created;
+
+ /*
+ * Unlock parent i_mutex later when the open finishes - prevent
+ * races when a file can be locked with a deny lock by another
+ * task that opens the file.
+ */
+ error = may_open(&nd->path, acc_mode, open_flag);
+ if (error) {
+ mutex_unlock(&dir->d_inode->i_mutex);
+ goto out;
+ }
+ file->f_path.mnt = nd->path.mnt;
+ error = finish_open(file, nd->path.dentry, NULL, opened);
+ if (error) {
+ mutex_unlock(&dir->d_inode->i_mutex);
+ if (error == -EOPENSTALE)
+ goto stale_open;
+ goto out;
+ }
+ error = deny_lock_file(file);
+ mutex_unlock(&dir->d_inode->i_mutex);
+ if (error)
+ goto exit_fput;
+ goto opened;
}
+ mutex_unlock(&dir->d_inode->i_mutex);

/*
* create/update audit record if it already exists.
@@ -2872,7 +2901,6 @@ finish_open:
goto out;
got_write = true;
}
-finish_open_created:
error = may_open(&nd->path, acc_mode, open_flag);
if (error)
goto out;
@@ -2883,6 +2911,15 @@ finish_open_created:
goto stale_open;
goto out;
}
+ /*
+ * Lock parent i_mutex to prevent races with deny locks on newely
+ * created files.
+ */
+ mutex_lock(&dir->d_inode->i_mutex);
+ error = deny_lock_file(file);
+ mutex_unlock(&dir->d_inode->i_mutex);
+ if (error)
+ goto exit_fput;
opened:
error = open_check_o_direct(file);
if (error)
diff --git a/fs/proc_namespace.c b/fs/proc_namespace.c
index 2033f74..6edbadc 100644
--- a/fs/proc_namespace.c
+++ b/fs/proc_namespace.c
@@ -44,6 +44,7 @@ static int show_sb_opts(struct seq_file *m, struct super_block *sb)
{ MS_SYNCHRONOUS, ",sync" },
{ MS_DIRSYNC, ",dirsync" },
{ MS_MANDLOCK, ",mand" },
+ { MS_SHARELOCK, ",sharelock" },
{ 0, NULL }
};
const struct proc_fs_info *fs_infop;
diff --git a/include/linux/fs.h b/include/linux/fs.h
index 1a39c33..073e31a 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -1005,6 +1005,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 */
@@ -1153,6 +1154,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)
{
}
@@ -1668,6 +1674,7 @@ struct super_operations {
#define IS_PRIVATE(inode) ((inode)->i_flags & S_PRIVATE)
#define IS_IMA(inode) ((inode)->i_flags & S_IMA)
#define IS_AUTOMOUNT(inode) ((inode)->i_flags & S_AUTOMOUNT)
+#define IS_SHARELOCK(inode) __IS_FLG(inode, MS_SHARELOCK)
#define IS_NOSEC(inode) ((inode)->i_flags & S_NOSEC)

/*
diff --git a/include/uapi/asm-generic/fcntl.h b/include/uapi/asm-generic/fcntl.h
index a48937d..5ac0d49 100644
--- a/include/uapi/asm-generic/fcntl.h
+++ b/include/uapi/asm-generic/fcntl.h
@@ -84,6 +84,17 @@
#define O_PATH 010000000
#endif

+#ifndef O_DENYREAD
+#define O_DENYREAD 020000000 /* Do not permit read access */
+#endif
+#ifndef O_DENYWRITE
+#define O_DENYWRITE 040000000 /* Do not permit write access */
+#endif
+/* FMODE_NONOTIFY 0100000000 */
+#ifndef O_DENYDELETE
+#define O_DENYDELETE 0200000000 /* Do not permit delete or rename */
+#endif
+
#ifndef O_NDELAY
#define O_NDELAY O_NONBLOCK
#endif
diff --git a/include/uapi/linux/fs.h b/include/uapi/linux/fs.h
index 780d4c6..f1925f7 100644
--- a/include/uapi/linux/fs.h
+++ b/include/uapi/linux/fs.h
@@ -86,6 +86,7 @@ struct inodes_stat_t {
#define MS_KERNMOUNT (1<<22) /* this is a kern_mount call */
#define MS_I_VERSION (1<<23) /* Update inode I_version field */
#define MS_STRICTATIME (1<<24) /* Always perform atime updates */
+#define MS_SHARELOCK (1<<25) /* Allow share locks on an FS */
#define MS_NOSEC (1<<28)
#define MS_BORN (1<<29)
#define MS_ACTIVE (1<<30)
--
1.8.1.5

2013-04-09 12:36:42

by Pavel Shilovsky

[permalink] [raw]
Subject: [PATCH v5 3/7] CIFS: Add O_DENY* open flags support

Construct share_access value from O_DENY* flags and send it to
the server.

Signed-off-by: Pavel Shilovsky <[email protected]>
---
fs/cifs/cifsfs.c | 2 +-
fs/cifs/cifsglob.h | 16 +++++++++++++++-
fs/cifs/dir.c | 3 +++
fs/cifs/file.c | 4 ++++
fs/locks.c | 7 ++++++-
include/linux/fs.h | 1 +
6 files changed, 30 insertions(+), 3 deletions(-)

diff --git a/fs/cifs/cifsfs.c b/fs/cifs/cifsfs.c
index e328339..469dffb 100644
--- a/fs/cifs/cifsfs.c
+++ b/fs/cifs/cifsfs.c
@@ -769,7 +769,7 @@ struct file_system_type cifs_fs_type = {
.name = "cifs",
.mount = cifs_do_mount,
.kill_sb = cifs_kill_sb,
- /* .fs_flags */
+ .fs_flags = FS_DOES_SHARELOCK,
};
const struct inode_operations cifs_dir_inode_ops = {
.create = cifs_create,
diff --git a/fs/cifs/cifsglob.h b/fs/cifs/cifsglob.h
index cd4bbf3..7e876b9 100644
--- a/fs/cifs/cifsglob.h
+++ b/fs/cifs/cifsglob.h
@@ -465,7 +465,7 @@ struct smb_vol {
CIFS_MOUNT_CIFS_BACKUPUID | CIFS_MOUNT_CIFS_BACKUPGID)

#define CIFS_MS_MASK (MS_RDONLY | MS_MANDLOCK | MS_NOEXEC | MS_NOSUID | \
- MS_NODEV | MS_SYNCHRONOUS)
+ MS_NODEV | MS_SYNCHRONOUS | MS_SHARELOCK)

struct cifs_mnt_data {
struct cifs_sb_info *cifs_sb;
@@ -947,6 +947,20 @@ struct cifsFileInfo {
struct work_struct oplock_break; /* work for oplock breaks */
};

+static inline int
+cifs_get_share_flags(unsigned int flags)
+{
+ int share_access = 0;
+
+ if (!(flags & O_DENYREAD))
+ share_access |= FILE_SHARE_READ;
+ if (!(flags & O_DENYWRITE))
+ share_access |= FILE_SHARE_WRITE;
+ if (!(flags & O_DENYDELETE))
+ share_access |= FILE_SHARE_DELETE;
+ return share_access;
+}
+
struct cifs_io_parms {
__u16 netfid;
#ifdef CONFIG_CIFS_SMB2
diff --git a/fs/cifs/dir.c b/fs/cifs/dir.c
index 267b608..d4331de 100644
--- a/fs/cifs/dir.c
+++ b/fs/cifs/dir.c
@@ -294,6 +294,9 @@ cifs_do_create(struct inode *inode, struct dentry *direntry, unsigned int xid,
else
cFYI(1, "Create flag not set in create function");

+ if (IS_SHARELOCK(inode))
+ share_access = cifs_get_share_flags(oflags);
+
/*
* BB add processing to set equivalent of mode - e.g. via CreateX with
* ACLs
diff --git a/fs/cifs/file.c b/fs/cifs/file.c
index 0d524a7..9394b2b 100644
--- a/fs/cifs/file.c
+++ b/fs/cifs/file.c
@@ -210,6 +210,8 @@ cifs_nt_open(char *full_path, struct inode *inode, struct cifs_sb_info *cifs_sb,
*********************************************************************/

disposition = cifs_get_disposition(f_flags);
+ if (IS_SHARELOCK(inode))
+ share_access = cifs_get_share_flags(f_flags);

/* BB pass O_SYNC flag through on file attributes .. BB */

@@ -645,6 +647,8 @@ cifs_reopen_file(struct cifsFileInfo *cfile, bool can_flush)
}

desired_access = cifs_convert_flags(cfile->f_flags);
+ if (IS_SHARELOCK(inode))
+ share_access = cifs_get_share_flags(cfile->f_flags);

if (backup_cred(cifs_sb))
create_options |= CREATE_OPEN_BACKUP_INTENT;
diff --git a/fs/locks.c b/fs/locks.c
index 1eccc75..52c9ea1 100644
--- a/fs/locks.c
+++ b/fs/locks.c
@@ -846,8 +846,13 @@ deny_lock_file(struct file *filp)
{
struct file_lock *lock;
int error = 0;
+ struct inode *inode = filp->f_path.dentry->d_inode;

- if (!IS_SHARELOCK(filp->f_path.dentry->d_inode))
+ if (!IS_SHARELOCK(inode))
+ return error;
+
+ /* Don't set a lock on file systems that do it internally */
+ if (inode->i_sb->s_type->fs_flags & FS_DOES_SHARELOCK)
return error;

error = flock_make_lock(filp, &lock, deny_flags_to_cmd(filp->f_flags));
diff --git a/include/linux/fs.h b/include/linux/fs.h
index 073e31a..8a4232e 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -1816,6 +1816,7 @@ struct file_system_type {
#define FS_USERNS_DEV_MOUNT 16 /* A userns mount does not imply MNT_NODEV */
#define FS_REVAL_DOT 16384 /* Check the paths ".", ".." for staleness */
#define FS_RENAME_DOES_D_MOVE 32768 /* FS will handle d_move() during rename() internally. */
+#define FS_DOES_SHARELOCK 65536 /* FS does sharelocks internally */
struct dentry *(*mount) (struct file_system_type *, int,
const char *, void *);
void (*kill_sb) (struct super_block *);
--
1.8.1.5

2013-04-09 12:36:49

by Pavel Shilovsky

[permalink] [raw]
Subject: [PATCH v5 7/7] locks: Disable LOCK_MAND support for MS_SHARELOCK mounts

Signed-off-by: Pavel Shilovsky <[email protected]>
---
fs/locks.c | 6 ++++++
1 file changed, 6 insertions(+)

diff --git a/fs/locks.c b/fs/locks.c
index 1402a43..a67857c 100644
--- a/fs/locks.c
+++ b/fs/locks.c
@@ -1719,6 +1719,12 @@ SYSCALL_DEFINE2(flock, unsigned int, fd, unsigned int, cmd)
if (!f.file)
goto out;

+ /* Disable LOCK_MAND support for FS mounted with MS_SHARELOCK */
+ if (IS_SHARELOCK(f.file->f_path.dentry->d_inode)) {
+ error = -EPERM;
+ goto out;
+ }
+
can_sleep = !(cmd & LOCK_NB);
cmd &= ~LOCK_NB;
unlock = (cmd == LOCK_UN);
--
1.8.1.5

2013-04-09 12:36:46

by Pavel Shilovsky

[permalink] [raw]
Subject: [PATCH v5 5/7] NFSv4: Add O_DENY* open flags support

by passing these flags to NFSv4 open request. Also make it return
-EBUSY on share conflicts with other opens.

Signed-off-by: Pavel Shilovsky <[email protected]>
---
fs/nfs/internal.h | 3 ++-
fs/nfs/nfs4proc.c | 4 +++-
fs/nfs/nfs4super.c | 9 ++++++---
fs/nfs/nfs4xdr.c | 21 +++++++++++++++++----
fs/nfs/super.c | 7 +++++--
5 files changed, 33 insertions(+), 11 deletions(-)

diff --git a/fs/nfs/internal.h b/fs/nfs/internal.h
index f0e6c7d..c770830 100644
--- a/fs/nfs/internal.h
+++ b/fs/nfs/internal.h
@@ -6,7 +6,8 @@
#include <linux/mount.h>
#include <linux/security.h>

-#define NFS_MS_MASK (MS_RDONLY|MS_NOSUID|MS_NODEV|MS_NOEXEC|MS_SYNCHRONOUS)
+#define NFS_MS_MASK (MS_RDONLY | MS_NOSUID | MS_NODEV | MS_NOEXEC | \
+ MS_SYNCHRONOUS | MS_SHARELOCK)

struct nfs_string;

diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
index 3cb5e77..5d44763 100644
--- a/fs/nfs/nfs4proc.c
+++ b/fs/nfs/nfs4proc.c
@@ -100,7 +100,7 @@ static int nfs4_map_errors(int err)
case -NFS4ERR_BADNAME:
return -EINVAL;
case -NFS4ERR_SHARE_DENIED:
- return -EACCES;
+ return -EBUSY;
case -NFS4ERR_MINOR_VERS_MISMATCH:
return -EPROTONOSUPPORT;
case -NFS4ERR_ACCESS:
@@ -793,6 +793,8 @@ static struct nfs4_opendata *nfs4_opendata_alloc(struct dentry *dentry,
atomic_inc(&sp->so_count);
p->o_arg.fh = NFS_FH(dir);
p->o_arg.open_flags = flags;
+ if (!IS_SHARELOCK(dir))
+ p->o_arg.open_flags &= ~(O_DENYREAD | O_DENYWRITE);
p->o_arg.fmode = fmode & (FMODE_READ|FMODE_WRITE);
/* don't put an ACCESS op in OPEN compound if O_EXCL, because ACCESS
* will return permission denied for all bits until close */
diff --git a/fs/nfs/nfs4super.c b/fs/nfs/nfs4super.c
index 84d2e9e..ca791cd 100644
--- a/fs/nfs/nfs4super.c
+++ b/fs/nfs/nfs4super.c
@@ -28,7 +28,8 @@ static struct file_system_type nfs4_remote_fs_type = {
.name = "nfs4",
.mount = nfs4_remote_mount,
.kill_sb = nfs_kill_super,
- .fs_flags = FS_RENAME_DOES_D_MOVE|FS_REVAL_DOT|FS_BINARY_MOUNTDATA,
+ .fs_flags = FS_RENAME_DOES_D_MOVE | FS_REVAL_DOT |
+ FS_BINARY_MOUNTDATA | FS_DOES_SHARELOCK,
};

static struct file_system_type nfs4_remote_referral_fs_type = {
@@ -36,7 +37,8 @@ static struct file_system_type nfs4_remote_referral_fs_type = {
.name = "nfs4",
.mount = nfs4_remote_referral_mount,
.kill_sb = nfs_kill_super,
- .fs_flags = FS_RENAME_DOES_D_MOVE|FS_REVAL_DOT|FS_BINARY_MOUNTDATA,
+ .fs_flags = FS_RENAME_DOES_D_MOVE | FS_REVAL_DOT |
+ FS_BINARY_MOUNTDATA | FS_DOES_SHARELOCK,
};

struct file_system_type nfs4_referral_fs_type = {
@@ -44,7 +46,8 @@ struct file_system_type nfs4_referral_fs_type = {
.name = "nfs4",
.mount = nfs4_referral_mount,
.kill_sb = nfs_kill_super,
- .fs_flags = FS_RENAME_DOES_D_MOVE|FS_REVAL_DOT|FS_BINARY_MOUNTDATA,
+ .fs_flags = FS_RENAME_DOES_D_MOVE | FS_REVAL_DOT |
+ FS_BINARY_MOUNTDATA | FS_DOES_SHARELOCK,
};

static const struct super_operations nfs4_sops = {
diff --git a/fs/nfs/nfs4xdr.c b/fs/nfs/nfs4xdr.c
index 26b1439..8fe0221 100644
--- a/fs/nfs/nfs4xdr.c
+++ b/fs/nfs/nfs4xdr.c
@@ -1325,7 +1325,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;

@@ -1343,7 +1344,19 @@ 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 */
+ 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);
+ }
}

static inline void encode_openhdr(struct xdr_stream *xdr, const struct nfs_openargs *arg)
@@ -1354,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);
@@ -1491,7 +1504,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
diff --git a/fs/nfs/super.c b/fs/nfs/super.c
index b056b16..cf9db61 100644
--- a/fs/nfs/super.c
+++ b/fs/nfs/super.c
@@ -331,7 +331,8 @@ struct file_system_type nfs4_fs_type = {
.name = "nfs4",
.mount = nfs_fs_mount,
.kill_sb = nfs_kill_super,
- .fs_flags = FS_RENAME_DOES_D_MOVE|FS_REVAL_DOT|FS_BINARY_MOUNTDATA,
+ .fs_flags = FS_RENAME_DOES_D_MOVE | FS_REVAL_DOT |
+ FS_BINARY_MOUNTDATA | FS_DOES_SHARELOCK,
};
EXPORT_SYMBOL_GPL(nfs4_fs_type);

@@ -2589,6 +2590,8 @@ nfs_xdev_mount(struct file_system_type *fs_type, int flags,
struct nfs_server *server;
struct dentry *mntroot = ERR_PTR(-ENOMEM);
struct nfs_subversion *nfs_mod = NFS_SB(data->sb)->nfs_client->cl_nfs_mod;
+ /* save sharelock option */
+ int sharelock = data->sb->s_flags & MS_SHARELOCK;

dprintk("--> nfs_xdev_mount()\n");

@@ -2600,7 +2603,7 @@ nfs_xdev_mount(struct file_system_type *fs_type, int flags,
if (IS_ERR(server))
mntroot = ERR_CAST(server);
else
- mntroot = nfs_fs_mount_common(server, flags,
+ mntroot = nfs_fs_mount_common(server, flags | sharelock,
dev_name, &mount_info, nfs_mod);

dprintk("<-- nfs_xdev_mount() = %ld\n",
--
1.8.1.5

2013-04-09 12:38:06

by Pavel Shilovsky

[permalink] [raw]
Subject: [PATCH v5 6/7] NFSD: Pass share reservations flags to VFS

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

Signed-off-by: Pavel Shilovsky <[email protected]>
---
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 52c9ea1..1402a43 100644
--- a/fs/locks.c
+++ b/fs/locks.c
@@ -866,6 +866,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 a8309c6..eb40c4f 100644
--- a/fs/nfsd/nfs4state.c
+++ b/fs/nfsd/nfs4state.c
@@ -476,6 +476,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_DENYREAD;
+ case NFS4_SHARE_DENY_WRITE:
+ return O_DENYWRITE;
+ case NFS4_SHARE_DENY_BOTH:
+ return O_DENYREAD | O_DENYWRITE;
+ }
+ return 0;
+}
+
static int nfs4_access_to_omode(u32 access)
{
switch (access & NFS4_SHARE_ACCESS_BOTH) {
@@ -2795,6 +2808,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;
@@ -2815,6 +2843,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);
@@ -3048,7 +3084,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 */
@@ -3062,6 +3098,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);
@@ -3760,6 +3800,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.5

2013-04-09 12:38:37

by Pavel Shilovsky

[permalink] [raw]
Subject: [PATCH v5 4/7] CIFS: Use NT_CREATE_ANDX command for forcemand mounts

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 <[email protected]>
---
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 d4331de..8587021 100644
--- a/fs/cifs/dir.c
+++ b/fs/cifs/dir.c
@@ -217,6 +217,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 9394b2b..19038a4 100644
--- a/fs/cifs/file.c
+++ b/fs/cifs/file.c
@@ -455,8 +455,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,
@@ -624,6 +625,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.5

2013-04-09 12:39:20

by Pavel Shilovsky

[permalink] [raw]
Subject: [PATCH v5 2/7] CIFS: Add share_access parm to open request

and simplify CIFSSMBOpen params.

Signed-off-by: Pavel Shilovsky <[email protected]>
---
fs/cifs/cifsacl.c | 8 ++++----
fs/cifs/cifsglob.h | 2 +-
fs/cifs/cifsproto.h | 8 ++++----
fs/cifs/cifssmb.c | 50 +++++++++++++++++++++++++++-----------------------
fs/cifs/dir.c | 12 ++++++------
fs/cifs/file.c | 10 ++++++----
fs/cifs/inode.c | 16 ++++++----------
fs/cifs/link.c | 25 ++++++++-----------------
fs/cifs/readdir.c | 5 ++---
fs/cifs/smb1ops.c | 16 +++++++---------
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, 95 insertions(+), 101 deletions(-)

diff --git a/fs/cifs/cifsacl.c b/fs/cifs/cifsacl.c
index 5cbd00e..3820e61 100644
--- a/fs/cifs/cifsacl.c
+++ b/fs/cifs/cifsacl.c
@@ -891,8 +891,8 @@ 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);
if (!rc) {
rc = CIFSSMBGetCIFSACL(xid, tcon, fid, &pntsd, pacllen);
CIFSSMBClose(xid, tcon, fid);
@@ -952,8 +952,8 @@ 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);
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 e6899ce..cd4bbf3 100644
--- a/fs/cifs/cifsglob.h
+++ b/fs/cifs/cifsglob.h
@@ -310,7 +310,7 @@ struct smb_version_operations {
struct cifs_sb_info *);
/* open a file for non-posix mounts */
int (*open)(const unsigned int, struct cifs_tcon *, const char *, int,
- int, int, struct cifs_fid *, __u32 *, FILE_ALL_INFO *,
+ int, int, int, struct cifs_fid *, __u32 *, FILE_ALL_INFO *,
struct cifs_sb_info *);
/* set fid protocol-specific info */
void (*set_fid)(struct cifsFileInfo *, struct cifs_fid *, __u32);
diff --git a/fs/cifs/cifsproto.h b/fs/cifs/cifsproto.h
index 1988c1b..89f7dac 100644
--- a/fs/cifs/cifsproto.h
+++ b/fs/cifs/cifsproto.h
@@ -361,10 +361,10 @@ 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 desired_access, const int share_access,
+ const int omode, __u16 *netfid, int *oplock,
+ FILE_ALL_INFO *, struct cifs_sb_info *);
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..d619646 100644
--- a/fs/cifs/cifssmb.c
+++ b/fs/cifs/cifssmb.c
@@ -1289,10 +1289,10 @@ 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 desired_access, const int share_access,
+ const int create_options, __u16 *netfid, int *oplock,
+ FILE_ALL_INFO *file_info, struct cifs_sb_info *cifs_sb)
{
int rc = -EACCES;
OPEN_REQ *pSMB = NULL;
@@ -1300,6 +1300,8 @@ CIFSSMBOpen(const unsigned int xid, struct cifs_tcon *tcon,
int bytes_returned;
int name_len;
__u16 count;
+ struct nls_table *nls_codepage = cifs_sb->local_nls;
+ int remap = cifs_sb->mnt_cifs_flags & CIFS_MOUNT_MAP_SPECIAL_CHR;

openRetry:
rc = smb_init(SMB_COM_NT_CREATE_ANDX, 24, tcon, (void **) &pSMB,
@@ -1313,26 +1315,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->DesiredAccess = cpu_to_le32(desired_access);
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 +1351,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_access);
+ 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 +1370,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 8719bbe..267b608 100644
--- a/fs/cifs/dir.c
+++ b/fs/cifs/dir.c
@@ -203,6 +203,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 = FILE_SHARE_ALL;
struct TCP_Server_Info *server = tcon->ses->server;

*oplock = 0;
@@ -320,8 +321,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;
@@ -626,10 +627,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,
- cifs_sb->mnt_cifs_flags & CIFS_MOUNT_MAP_SPECIAL_CHR);
+ rc = CIFSSMBOpen(xid, pTcon, full_path, FILE_CREATE, GENERIC_WRITE,
+ FILE_SHARE_ALL, create_options, &fileHandle, &oplock,
+ buf, cifs_sb);
if (rc)
goto mknod_out;

diff --git a/fs/cifs/file.c b/fs/cifs/file.c
index 8ea6ca5..0d524a7 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 = FILE_SHARE_ALL;
int disposition;
int create_options = CREATE_NOT_DIR;
FILE_ALL_INFO *buf;
@@ -220,8 +221,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;
@@ -579,6 +580,7 @@ cifs_reopen_file(struct cifsFileInfo *cfile, bool can_flush)
struct inode *inode;
char *full_path = NULL;
int desired_access;
+ int share_access = FILE_SHARE_ALL;
int disposition = FILE_OPEN;
int create_options = CREATE_NOT_DIR;
struct cifs_fid fid;
@@ -658,8 +660,8 @@ cifs_reopen_file(struct cifsFileInfo *cfile, bool can_flush)
* not dirty locally we could do this.
*/
rc = server->ops->open(xid, tcon, full_path, disposition,
- desired_access, create_options, &fid, &oplock,
- NULL, cifs_sb);
+ desired_access, share_access, create_options,
+ &fid, &oplock, NULL, cifs_sb);
if (rc) {
mutex_unlock(&cfile->fh_mutex);
cFYI(1, "cifs_reopen returned 0x%x", rc);
diff --git a/fs/cifs/inode.c b/fs/cifs/inode.c
index ed6208f..233b2cc 100644
--- a/fs/cifs/inode.c
+++ b/fs/cifs/inode.c
@@ -396,10 +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,
- cifs_sb->mnt_cifs_flags &
- CIFS_MOUNT_MAP_SPECIAL_CHR);
+ FILE_SHARE_ALL, CREATE_NOT_DIR, &netfid, &oplock,
+ NULL, cifs_sb);
if (rc == 0) {
int buf_type = CIFS_NO_BUFFER;
/* Read header */
@@ -987,9 +985,8 @@ 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,
- cifs_sb->mnt_cifs_flags & CIFS_MOUNT_MAP_SPECIAL_CHR);
+ DELETE|FILE_WRITE_ATTRIBUTES, FILE_SHARE_ALL,
+ CREATE_NOT_DIR, &netfid, &oplock, NULL, cifs_sb);
if (rc != 0)
goto out;

@@ -1509,9 +1506,8 @@ 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,
- cifs_sb->local_nls, cifs_sb->mnt_cifs_flags &
- CIFS_MOUNT_MAP_SPECIAL_CHR);
+ FILE_SHARE_ALL, CREATE_NOT_DIR, &srcfid, &oplock, NULL,
+ cifs_sb);
if (rc == 0) {
rc = CIFSSMBRenameOpenFile(xid, tcon, srcfid,
(const char *) to_dentry->d_name.name,
diff --git a/fs/cifs/link.c b/fs/cifs/link.c
index 51dc2fb..d467881 100644
--- a/fs/cifs/link.c
+++ b/fs/cifs/link.c
@@ -187,16 +187,11 @@ CIFSCreateMFSymLink(const unsigned int xid, struct cifs_tcon *tcon,
{
int rc;
int oplock = 0;
- int remap;
int create_options = CREATE_NOT_DIR;
__u16 netfid = 0;
u8 *buf;
unsigned int bytes_written = 0;
struct cifs_io_parms io_parms;
- struct nls_table *nls_codepage;
-
- nls_codepage = cifs_sb->local_nls;
- remap = cifs_sb->mnt_cifs_flags & CIFS_MOUNT_MAP_SPECIAL_CHR;

buf = kmalloc(CIFS_MF_SYMLINK_FILE_SIZE, GFP_KERNEL);
if (!buf)
@@ -212,8 +207,8 @@ 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,
- nls_codepage, remap);
+ FILE_SHARE_ALL, create_options, &netfid, &oplock, NULL,
+ cifs_sb);
if (rc != 0) {
kfree(buf);
return rc;
@@ -240,7 +235,7 @@ CIFSCreateMFSymLink(const unsigned int xid, struct cifs_tcon *tcon,
static int
CIFSQueryMFSymLink(const unsigned int xid, struct cifs_tcon *tcon,
const unsigned char *searchName, char **symlinkinfo,
- const struct nls_table *nls_codepage, int remap)
+ struct cifs_sb_info *cifs_sb)
{
int rc;
int oplock = 0;
@@ -254,8 +249,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, cifs_sb);
if (rc != 0)
return rc;

@@ -332,10 +327,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,
- cifs_sb->mnt_cifs_flags &
- CIFS_MOUNT_MAP_SPECIAL_CHR);
+ FILE_SHARE_ALL, CREATE_NOT_DIR, &netfid, &oplock,
+ &file_info, cifs_sb);
if (rc != 0)
goto out;

@@ -530,9 +523,7 @@ cifs_follow_link(struct dentry *direntry, struct nameidata *nd)
*/
if (cifs_sb->mnt_cifs_flags & CIFS_MOUNT_MF_SYMLINKS)
rc = CIFSQueryMFSymLink(xid, tcon, full_path, &target_path,
- cifs_sb->local_nls,
- cifs_sb->mnt_cifs_flags &
- CIFS_MOUNT_MAP_SPECIAL_CHR);
+ cifs_sb);

if ((rc != 0) && cap_unix(tcon->ses))
rc = CIFSSMBUnixQuerySymLink(xid, tcon, full_path, &target_path,
diff --git a/fs/cifs/readdir.c b/fs/cifs/readdir.c
index cdd6ff4..d51927b 100644
--- a/fs/cifs/readdir.c
+++ b/fs/cifs/readdir.c
@@ -224,9 +224,8 @@ 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,
- cifs_sb->local_nls,
- cifs_sb->mnt_cifs_flags & CIFS_MOUNT_MAP_SPECIAL_CHR);
+ FILE_SHARE_ALL, OPEN_REPARSE_POINT, &fid, &oplock, NULL,
+ cifs_sb);
if (!rc) {
tmpbuffer = kmalloc(maxpath);
rc = CIFSSMBQueryReparseLinkInfo(xid, ptcon, full_path,
diff --git a/fs/cifs/smb1ops.c b/fs/cifs/smb1ops.c
index 47bc5a8..a76950a 100644
--- a/fs/cifs/smb1ops.c
+++ b/fs/cifs/smb1ops.c
@@ -671,9 +671,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,
@@ -682,9 +682,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 &
- CIFS_MOUNT_MAP_SPECIAL_CHR);
+ share_access, create_options, &fid->netfid, oplock,
+ buf, cifs_sb);
}

static void
@@ -779,9 +778,8 @@ 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,
- cifs_sb->mnt_cifs_flags & CIFS_MOUNT_MAP_SPECIAL_CHR);
+ SYNCHRONIZE | FILE_WRITE_ATTRIBUTES, FILE_SHARE_ALL,
+ CREATE_NOT_DIR, &netfid, &oplock, NULL, cifs_sb);

if (rc != 0) {
if (rc == -EIO)
diff --git a/fs/cifs/smb2file.c b/fs/cifs/smb2file.c
index 71e6aed..7dfb50c 100644
--- a/fs/cifs/smb2file.c
+++ b/fs/cifs/smb2file.c
@@ -58,9 +58,9 @@ smb2_set_oplock_level(struct cifsInodeInfo *cinode, __u32 oplock)

int
smb2_open_file(const unsigned int xid, struct cifs_tcon *tcon, const char *path,
- int disposition, int desired_access, int create_options,
- struct cifs_fid *fid, __u32 *oplock, FILE_ALL_INFO *buf,
- struct cifs_sb_info *cifs_sb)
+ int disposition, int desired_access, int share_access,
+ int create_options, struct cifs_fid *fid, __u32 *oplock,
+ FILE_ALL_INFO *buf, struct cifs_sb_info *cifs_sb)
{
int rc;
__le16 *smb2_path;
@@ -87,8 +87,8 @@ smb2_open_file(const unsigned int xid, struct cifs_tcon *tcon, const char *path,
memcpy(smb2_oplock + 1, fid->lease_key, SMB2_LEASE_KEY_SIZE);

rc = SMB2_open(xid, tcon, smb2_path, &fid->persistent_fid,
- &fid->volatile_fid, desired_access, disposition,
- 0, 0, smb2_oplock, smb2_data);
+ &fid->volatile_fid, desired_access, share_access,
+ disposition, 0, 0, smb2_oplock, smb2_data);
if (rc)
goto out;

diff --git a/fs/cifs/smb2inode.c b/fs/cifs/smb2inode.c
index 7064824..6af174a 100644
--- a/fs/cifs/smb2inode.c
+++ b/fs/cifs/smb2inode.c
@@ -54,8 +54,8 @@ smb2_open_op_close(const unsigned int xid, struct cifs_tcon *tcon,
return -ENOMEM;

rc = SMB2_open(xid, tcon, utf16_path, &persistent_fid, &volatile_fid,
- desired_access, create_disposition, file_attributes,
- create_options, &oplock, NULL);
+ desired_access, FILE_SHARE_ALL, create_disposition,
+ file_attributes, create_options, &oplock, NULL);
if (rc) {
kfree(utf16_path);
return rc;
diff --git a/fs/cifs/smb2ops.c b/fs/cifs/smb2ops.c
index bceffe7..22aef1d 100644
--- a/fs/cifs/smb2ops.c
+++ b/fs/cifs/smb2ops.c
@@ -222,7 +222,8 @@ smb2_is_path_accessible(const unsigned int xid, struct cifs_tcon *tcon,
return -ENOMEM;

rc = SMB2_open(xid, tcon, utf16_path, &persistent_fid, &volatile_fid,
- FILE_READ_ATTRIBUTES, FILE_OPEN, 0, 0, &oplock, NULL);
+ FILE_READ_ATTRIBUTES, FILE_SHARE_ALL, FILE_OPEN, 0, 0,
+ &oplock, NULL);
if (rc) {
kfree(utf16_path);
return rc;
@@ -432,8 +433,8 @@ smb2_query_dir_first(const unsigned int xid, struct cifs_tcon *tcon,
return -ENOMEM;

rc = SMB2_open(xid, tcon, utf16_path, &persistent_fid, &volatile_fid,
- FILE_READ_ATTRIBUTES | FILE_READ_DATA, FILE_OPEN, 0, 0,
- &oplock, NULL);
+ FILE_READ_ATTRIBUTES | FILE_READ_DATA, FILE_SHARE_ALL,
+ FILE_OPEN, 0, 0, &oplock, NULL);
kfree(utf16_path);
if (rc) {
cERROR(1, "open dir failed");
@@ -515,7 +516,8 @@ smb2_queryfs(const unsigned int xid, struct cifs_tcon *tcon,
u8 oplock = SMB2_OPLOCK_LEVEL_NONE;

rc = SMB2_open(xid, tcon, &srch_path, &persistent_fid, &volatile_fid,
- FILE_READ_ATTRIBUTES, FILE_OPEN, 0, 0, &oplock, NULL);
+ FILE_READ_ATTRIBUTES, FILE_SHARE_ALL, FILE_OPEN, 0, 0,
+ &oplock, NULL);
if (rc)
return rc;
buf->f_type = SMB2_MAGIC_NUMBER;
diff --git a/fs/cifs/smb2pdu.c b/fs/cifs/smb2pdu.c
index 41d9d07..45fa2dc 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 2aa3535..edff8f6 100644
--- a/fs/cifs/smb2proto.h
+++ b/fs/cifs/smb2proto.h
@@ -86,9 +86,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);
@@ -108,9 +109,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.5

2013-04-10 11:18:31

by Jeff Layton

[permalink] [raw]
Subject: Re: [PATCH v5 1/7] fcntl: Introduce new O_DENY* open flags

On Tue, 9 Apr 2013 16:40:21 +0400
Pavel Shilovsky <[email protected]> wrote:

> This patch adds 3 flags:
> 1) O_DENYREAD that doesn't permit read access,
> 2) O_DENYWRITE that doesn't permit write access,
> 3) O_DENYDELETE that doesn't permit delete or rename,
>
> Network filesystems CIFS, SMB2.0, SMB3.0 and NFSv4 have such flags -
> this change can benefit cifs and nfs modules as well as Samba and
> NFS file servers that export the same directory for Windows clients,
> or Wine applications that access the same files simultaneously.
>
> These flags are only take affect for opens on mounts with 'sharelock'
> mount option. They are translated to flock's flags:
>
> !O_DENYREAD -> LOCK_READ | LOCK_MAND
> !O_DENYWRITE -> LOCK_WRITE | LOCK_MAND
>
> and set through flock_lock_file on a file. If the file can't be locked
> due conflicts with another open with O_DENY* flags, the -EBUSY error
> code is returned.
>
> Create codepath is slightly changed to prevent data races on
> newely created files: when open with O_CREAT can return with -EBUSY
> error for successfully created files due to a deny lock set by
> another task.
>

It's good that this is consistent with the new patchset. I'm still not
100% sure that -EBUSY is the right error return here, but it should at
least help distinguish between "mode bits don't allow me to open" and
"file has share reservation set".

Heck, since we're departing from POSIX here, maybe we should consider a
new error code altogether? ESHAREDENIED or something?

> Signed-off-by: Pavel Shilovsky <[email protected]>
> ---
> fs/fcntl.c | 5 ++-
> fs/locks.c | 93 +++++++++++++++++++++++++++++++++++++---
> fs/namei.c | 45 +++++++++++++++++--
> fs/proc_namespace.c | 1 +
> include/linux/fs.h | 7 +++
> include/uapi/asm-generic/fcntl.h | 11 +++++
> include/uapi/linux/fs.h | 1 +
> 7 files changed, 150 insertions(+), 13 deletions(-)
>
> diff --git a/fs/fcntl.c b/fs/fcntl.c
> index 71a600a..7abce5a 100644
> --- a/fs/fcntl.c
> +++ b/fs/fcntl.c
> @@ -730,14 +730,15 @@ static int __init fcntl_init(void)
> * Exceptions: O_NONBLOCK is a two bit define on parisc; O_NDELAY
> * is defined as O_NONBLOCK on some platforms and not on others.
> */
> - BUILD_BUG_ON(19 - 1 /* for O_RDONLY being 0 */ != HWEIGHT32(
> + BUILD_BUG_ON(22 - 1 /* for O_RDONLY being 0 */ != HWEIGHT32(
> O_RDONLY | O_WRONLY | O_RDWR |
> O_CREAT | O_EXCL | O_NOCTTY |
> O_TRUNC | O_APPEND | /* O_NONBLOCK | */
> __O_SYNC | O_DSYNC | FASYNC |
> O_DIRECT | O_LARGEFILE | O_DIRECTORY |
> O_NOFOLLOW | O_NOATIME | O_CLOEXEC |
> - __FMODE_EXEC | O_PATH
> + __FMODE_EXEC | O_PATH | O_DENYREAD |
> + O_DENYWRITE | O_DENYDELETE
> ));
>
> fasync_cache = kmem_cache_create("fasync_cache",
> diff --git a/fs/locks.c b/fs/locks.c
> index a94e331..1eccc75 100644
> --- a/fs/locks.c
> +++ b/fs/locks.c
> @@ -605,20 +605,73 @@ 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
> - * checking before calling the locks_conflict().
> +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.
> + */
> +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 FS is mounted with MS_SHARELOCK */
> + if (!IS_SHARELOCK(caller_fl->fl_file->f_path.dentry->d_inode))
> + return 0;
> +
> + /* they can only conflict if they're both LOCK_MAND */
> + if (!(caller_type & LOCK_MAND) || !(sys_type & LOCK_MAND))
> + return 0;
> +
> + if (!(caller_type & LOCK_READ) && (sys_fmode & FMODE_READ))
> + return 1;
> + if (!(caller_type & LOCK_WRITE) && (sys_fmode & FMODE_WRITE))
> + return 1;
> + if (!(sys_type & LOCK_READ) && (caller_fmode & FMODE_READ))
> + return 1;
> + if (!(sys_type & LOCK_WRITE) && (caller_fmode & FMODE_WRITE))
> + return 1;
> +
> + return 0;
> +}
> +
> +/*
> + * Determine if lock sys_fl blocks lock caller_fl. FLOCK specific checking
> + * before calling the locks_conflict().
> */
> 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
> + 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);
> + /*
> + * FLOCK locks referring to the same filp do not conflict with
> * each other.
> */
> - if (!IS_FLOCK(sys_fl) || (caller_fl->fl_file == sys_fl->fl_file))
> - return (0);
> - if ((caller_fl->fl_type & LOCK_MAND) || (sys_fl->fl_type & LOCK_MAND))
> + if (caller_fl->fl_file == sys_fl->fl_file)
> return 0;
>
> - return (locks_conflict(caller_fl, sys_fl));
> + return locks_conflict(caller_fl, sys_fl);
> }
>
> void
> @@ -783,6 +836,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 (!IS_SHARELOCK(filp->f_path.dentry->d_inode))
> + 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);
> + if (error == -EAGAIN)
> + error = -EBUSY;
> +
> + 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 ec97aef..416c74f 100644
> --- a/fs/namei.c
> +++ b/fs/namei.c
> @@ -2557,9 +2557,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;
> @@ -2769,9 +2774,9 @@ retry_lookup:
> }
> mutex_lock(&dir->d_inode->i_mutex);
> error = lookup_open(nd, path, file, op, got_write, opened);
> - mutex_unlock(&dir->d_inode->i_mutex);
>
> if (error <= 0) {
> + mutex_unlock(&dir->d_inode->i_mutex);
> if (error)
> goto out;
>
> @@ -2789,8 +2794,32 @@ retry_lookup:
> will_truncate = false;
> acc_mode = MAY_OPEN;
> path_to_nameidata(path, nd);
> - goto finish_open_created;
> +
> + /*
> + * Unlock parent i_mutex later when the open finishes - prevent
> + * races when a file can be locked with a deny lock by another
> + * task that opens the file.
> + */
> + error = may_open(&nd->path, acc_mode, open_flag);
> + if (error) {
> + mutex_unlock(&dir->d_inode->i_mutex);
> + goto out;
> + }
> + file->f_path.mnt = nd->path.mnt;
> + error = finish_open(file, nd->path.dentry, NULL, opened);
> + if (error) {
> + mutex_unlock(&dir->d_inode->i_mutex);
> + if (error == -EOPENSTALE)
> + goto stale_open;
> + goto out;
> + }
> + error = deny_lock_file(file);
> + mutex_unlock(&dir->d_inode->i_mutex);
> + if (error)
> + goto exit_fput;
> + goto opened;
> }
> + mutex_unlock(&dir->d_inode->i_mutex);
>
> /*
> * create/update audit record if it already exists.
> @@ -2872,7 +2901,6 @@ finish_open:
> goto out;
> got_write = true;
> }
> -finish_open_created:
> error = may_open(&nd->path, acc_mode, open_flag);
> if (error)
> goto out;
> @@ -2883,6 +2911,15 @@ finish_open_created:
> goto stale_open;
> goto out;
> }
> + /*
> + * Lock parent i_mutex to prevent races with deny locks on newely
> + * created files.
> + */
> + mutex_lock(&dir->d_inode->i_mutex);
> + error = deny_lock_file(file);
> + mutex_unlock(&dir->d_inode->i_mutex);
> + if (error)
> + goto exit_fput;
> opened:
> error = open_check_o_direct(file);
> if (error)
> diff --git a/fs/proc_namespace.c b/fs/proc_namespace.c
> index 2033f74..6edbadc 100644
> --- a/fs/proc_namespace.c
> +++ b/fs/proc_namespace.c
> @@ -44,6 +44,7 @@ static int show_sb_opts(struct seq_file *m, struct super_block *sb)
> { MS_SYNCHRONOUS, ",sync" },
> { MS_DIRSYNC, ",dirsync" },
> { MS_MANDLOCK, ",mand" },
> + { MS_SHARELOCK, ",sharelock" },
> { 0, NULL }
> };
> const struct proc_fs_info *fs_infop;
> diff --git a/include/linux/fs.h b/include/linux/fs.h
> index 1a39c33..073e31a 100644
> --- a/include/linux/fs.h
> +++ b/include/linux/fs.h
> @@ -1005,6 +1005,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 */
> @@ -1153,6 +1154,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)
> {
> }
> @@ -1668,6 +1674,7 @@ struct super_operations {
> #define IS_PRIVATE(inode) ((inode)->i_flags & S_PRIVATE)
> #define IS_IMA(inode) ((inode)->i_flags & S_IMA)
> #define IS_AUTOMOUNT(inode) ((inode)->i_flags & S_AUTOMOUNT)
> +#define IS_SHARELOCK(inode) __IS_FLG(inode, MS_SHARELOCK)
> #define IS_NOSEC(inode) ((inode)->i_flags & S_NOSEC)
>
> /*
> diff --git a/include/uapi/asm-generic/fcntl.h b/include/uapi/asm-generic/fcntl.h
> index a48937d..5ac0d49 100644
> --- a/include/uapi/asm-generic/fcntl.h
> +++ b/include/uapi/asm-generic/fcntl.h
> @@ -84,6 +84,17 @@
> #define O_PATH 010000000
> #endif
>
> +#ifndef O_DENYREAD
> +#define O_DENYREAD 020000000 /* Do not permit read access */
> +#endif
> +#ifndef O_DENYWRITE
> +#define O_DENYWRITE 040000000 /* Do not permit write access */
> +#endif
> +/* FMODE_NONOTIFY 0100000000 */
> +#ifndef O_DENYDELETE
> +#define O_DENYDELETE 0200000000 /* Do not permit delete or rename */
> +#endif
> +

You're adding O_DENYDELETE here, but there's no support for it in the
patchset aside from the passthrough in the cifs code. Is that
intentional? What happens if I specify O_DENYDELETE on a non-cifs fs
that was mounted with "sharelock"? I assume it's just ignored?

> #ifndef O_NDELAY
> #define O_NDELAY O_NONBLOCK
> #endif
> diff --git a/include/uapi/linux/fs.h b/include/uapi/linux/fs.h
> index 780d4c6..f1925f7 100644
> --- a/include/uapi/linux/fs.h
> +++ b/include/uapi/linux/fs.h
> @@ -86,6 +86,7 @@ struct inodes_stat_t {
> #define MS_KERNMOUNT (1<<22) /* this is a kern_mount call */
> #define MS_I_VERSION (1<<23) /* Update inode I_version field */
> #define MS_STRICTATIME (1<<24) /* Always perform atime updates */
> +#define MS_SHARELOCK (1<<25) /* Allow share locks on an FS */
> #define MS_NOSEC (1<<28)
> #define MS_BORN (1<<29)
> #define MS_ACTIVE (1<<30)


--
Jeff Layton <[email protected]>

2013-04-10 11:19:44

by Jeffrey Layton

[permalink] [raw]
Subject: Re: [PATCH v5 4/7] CIFS: Use NT_CREATE_ANDX command for forcemand mounts

On Tue, 9 Apr 2013 16:40:24 +0400
Pavel Shilovsky <[email protected]> wrote:

> 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 <[email protected]>
> ---
> 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 d4331de..8587021 100644
> --- a/fs/cifs/dir.c
> +++ b/fs/cifs/dir.c
> @@ -217,6 +217,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 9394b2b..19038a4 100644
> --- a/fs/cifs/file.c
> +++ b/fs/cifs/file.c
> @@ -455,8 +455,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,
> @@ -624,6 +625,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))) {
> /*

I'm trying to understand why "forcemand" would matter here. Wouldn't
you just want to switch to using NT_CREATE_ANDX if O_DENY* is set
instead? What happens if I didn't mount with forcemand and then try to
use O_DENY*?

--
Jeff Layton <[email protected]>

2013-04-10 11:45:37

by Pavel Shilovsky

[permalink] [raw]
Subject: Re: [PATCH v5 4/7] CIFS: Use NT_CREATE_ANDX command for forcemand mounts

2013/4/10 Jeff Layton <[email protected]>:
> On Tue, 9 Apr 2013 16:40:24 +0400
> Pavel Shilovsky <[email protected]> wrote:
>
>> 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 <[email protected]>
>> ---
>> 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 d4331de..8587021 100644
>> --- a/fs/cifs/dir.c
>> +++ b/fs/cifs/dir.c
>> @@ -217,6 +217,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 9394b2b..19038a4 100644
>> --- a/fs/cifs/file.c
>> +++ b/fs/cifs/file.c
>> @@ -455,8 +455,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,
>> @@ -624,6 +625,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))) {
>> /*
>
> I'm trying to understand why "forcemand" would matter here. Wouldn't
> you just want to switch to using NT_CREATE_ANDX if O_DENY* is set
> instead? What happens if I didn't mount with forcemand and then try to
> use O_DENY*?

If cifs client mounts Samba share and negotiates posix extensions, it
uses trans2 command to open files. In this case O_DENY* flags that are
passed to open syscall won't be sent to the server and the file won't
be locked. This patch gives us an opportunity to make the client
always use NT_CREATE_ANDX command to open file - in this case O_DENY*
flags won't be missed.

You are right, we can leave forcemand option without changes and use
an appropriate smb command depending on openflags we have. Another
possibility is to make the client use NT_CREATE_ANDX command if new
'sharelock' VFS mount options is specified. If we mount a share with
sharelock mount option, we need O_DENY* flags sent to the server, but
the only one way to do it is to use NT_CREATE_ANDX command all the
time we need to open a file - so, using trans2 open command doesn't
make any sense in the case of 'sharelock' mounts.

Thoughts?

--
Best regards,
Pavel Shilovsky.

2013-04-10 12:12:29

by Jeffrey Layton

[permalink] [raw]
Subject: Re: [PATCH v5 4/7] CIFS: Use NT_CREATE_ANDX command for forcemand mounts

On Wed, 10 Apr 2013 15:45:33 +0400
Pavel Shilovsky <[email protected]> wrote:

> 2013/4/10 Jeff Layton <[email protected]>:
> > On Tue, 9 Apr 2013 16:40:24 +0400
> > Pavel Shilovsky <[email protected]> wrote:
> >
> >> 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 <[email protected]>
> >> ---
> >> 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 d4331de..8587021 100644
> >> --- a/fs/cifs/dir.c
> >> +++ b/fs/cifs/dir.c
> >> @@ -217,6 +217,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 9394b2b..19038a4 100644
> >> --- a/fs/cifs/file.c
> >> +++ b/fs/cifs/file.c
> >> @@ -455,8 +455,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,
> >> @@ -624,6 +625,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))) {
> >> /*
> >
> > I'm trying to understand why "forcemand" would matter here. Wouldn't
> > you just want to switch to using NT_CREATE_ANDX if O_DENY* is set
> > instead? What happens if I didn't mount with forcemand and then try to
> > use O_DENY*?
>
> If cifs client mounts Samba share and negotiates posix extensions, it
> uses trans2 command to open files. In this case O_DENY* flags that are
> passed to open syscall won't be sent to the server and the file won't
> be locked. This patch gives us an opportunity to make the client
> always use NT_CREATE_ANDX command to open file - in this case O_DENY*
> flags won't be missed.
>
> You are right, we can leave forcemand option without changes and use
> an appropriate smb command depending on openflags we have. Another
> possibility is to make the client use NT_CREATE_ANDX command if new
> 'sharelock' VFS mount options is specified. If we mount a share with
> sharelock mount option, we need O_DENY* flags sent to the server, but
> the only one way to do it is to use NT_CREATE_ANDX command all the
> time we need to open a file - so, using trans2 open command doesn't
> make any sense in the case of 'sharelock' mounts.
>

(cc'ing samba-technical)

I don't understand. Why would we need to use NT_CREATE_ANDX in lieu of
the POSIX trans2 open if the client isn't setting a share reservation
on that particular open? The server should still enforce share
reservations that are already set on the file regardless of which
method is used.

Perhaps too we should add these flags to the POSIX open call as well.
It would be nice not to have to fall back to NT_CREATE_ANDX.

--
Jeff Layton <[email protected]>

2013-04-10 13:24:42

by Pavel Shilovsky

[permalink] [raw]
Subject: Re: [PATCH v5 1/7] fcntl: Introduce new O_DENY* open flags

2013/4/10 Jeff Layton <[email protected]>:
> On Tue, 9 Apr 2013 16:40:21 +0400
> Pavel Shilovsky <[email protected]> wrote:
>
>> This patch adds 3 flags:
>> 1) O_DENYREAD that doesn't permit read access,
>> 2) O_DENYWRITE that doesn't permit write access,
>> 3) O_DENYDELETE that doesn't permit delete or rename,
>>
>> Network filesystems CIFS, SMB2.0, SMB3.0 and NFSv4 have such flags -
>> this change can benefit cifs and nfs modules as well as Samba and
>> NFS file servers that export the same directory for Windows clients,
>> or Wine applications that access the same files simultaneously.
>>
>> These flags are only take affect for opens on mounts with 'sharelock'
>> mount option. They are translated to flock's flags:
>>
>> !O_DENYREAD -> LOCK_READ | LOCK_MAND
>> !O_DENYWRITE -> LOCK_WRITE | LOCK_MAND
>>
>> and set through flock_lock_file on a file. If the file can't be locked
>> due conflicts with another open with O_DENY* flags, the -EBUSY error
>> code is returned.
>>
>> Create codepath is slightly changed to prevent data races on
>> newely created files: when open with O_CREAT can return with -EBUSY
>> error for successfully created files due to a deny lock set by
>> another task.
>>
>
> It's good that this is consistent with the new patchset. I'm still not
> 100% sure that -EBUSY is the right error return here, but it should at
> least help distinguish between "mode bits don't allow me to open" and
> "file has share reservation set".
>
> Heck, since we're departing from POSIX here, maybe we should consider a
> new error code altogether? ESHAREDENIED or something?

That can make sense. I don't mind to change this part.

>
>> Signed-off-by: Pavel Shilovsky <[email protected]>
>> ---
>> fs/fcntl.c | 5 ++-
>> fs/locks.c | 93 +++++++++++++++++++++++++++++++++++++---
>> fs/namei.c | 45 +++++++++++++++++--
>> fs/proc_namespace.c | 1 +
>> include/linux/fs.h | 7 +++
>> include/uapi/asm-generic/fcntl.h | 11 +++++
>> include/uapi/linux/fs.h | 1 +
>> 7 files changed, 150 insertions(+), 13 deletions(-)
>>
>> diff --git a/fs/fcntl.c b/fs/fcntl.c
>> index 71a600a..7abce5a 100644
>> --- a/fs/fcntl.c
>> +++ b/fs/fcntl.c
>> @@ -730,14 +730,15 @@ static int __init fcntl_init(void)
>> * Exceptions: O_NONBLOCK is a two bit define on parisc; O_NDELAY
>> * is defined as O_NONBLOCK on some platforms and not on others.
>> */
>> - BUILD_BUG_ON(19 - 1 /* for O_RDONLY being 0 */ != HWEIGHT32(
>> + BUILD_BUG_ON(22 - 1 /* for O_RDONLY being 0 */ != HWEIGHT32(
>> O_RDONLY | O_WRONLY | O_RDWR |
>> O_CREAT | O_EXCL | O_NOCTTY |
>> O_TRUNC | O_APPEND | /* O_NONBLOCK | */
>> __O_SYNC | O_DSYNC | FASYNC |
>> O_DIRECT | O_LARGEFILE | O_DIRECTORY |
>> O_NOFOLLOW | O_NOATIME | O_CLOEXEC |
>> - __FMODE_EXEC | O_PATH
>> + __FMODE_EXEC | O_PATH | O_DENYREAD |
>> + O_DENYWRITE | O_DENYDELETE
>> ));
>>
>> fasync_cache = kmem_cache_create("fasync_cache",
>> diff --git a/fs/locks.c b/fs/locks.c
>> index a94e331..1eccc75 100644
>> --- a/fs/locks.c
>> +++ b/fs/locks.c
>> @@ -605,20 +605,73 @@ 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
>> - * checking before calling the locks_conflict().
>> +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.
>> + */
>> +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 FS is mounted with MS_SHARELOCK */
>> + if (!IS_SHARELOCK(caller_fl->fl_file->f_path.dentry->d_inode))
>> + return 0;
>> +
>> + /* they can only conflict if they're both LOCK_MAND */
>> + if (!(caller_type & LOCK_MAND) || !(sys_type & LOCK_MAND))
>> + return 0;
>> +
>> + if (!(caller_type & LOCK_READ) && (sys_fmode & FMODE_READ))
>> + return 1;
>> + if (!(caller_type & LOCK_WRITE) && (sys_fmode & FMODE_WRITE))
>> + return 1;
>> + if (!(sys_type & LOCK_READ) && (caller_fmode & FMODE_READ))
>> + return 1;
>> + if (!(sys_type & LOCK_WRITE) && (caller_fmode & FMODE_WRITE))
>> + return 1;
>> +
>> + return 0;
>> +}
>> +
>> +/*
>> + * Determine if lock sys_fl blocks lock caller_fl. FLOCK specific checking
>> + * before calling the locks_conflict().
>> */
>> 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
>> + 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);
>> + /*
>> + * FLOCK locks referring to the same filp do not conflict with
>> * each other.
>> */
>> - if (!IS_FLOCK(sys_fl) || (caller_fl->fl_file == sys_fl->fl_file))
>> - return (0);
>> - if ((caller_fl->fl_type & LOCK_MAND) || (sys_fl->fl_type & LOCK_MAND))
>> + if (caller_fl->fl_file == sys_fl->fl_file)
>> return 0;
>>
>> - return (locks_conflict(caller_fl, sys_fl));
>> + return locks_conflict(caller_fl, sys_fl);
>> }
>>
>> void
>> @@ -783,6 +836,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 (!IS_SHARELOCK(filp->f_path.dentry->d_inode))
>> + 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);
>> + if (error == -EAGAIN)
>> + error = -EBUSY;
>> +
>> + 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 ec97aef..416c74f 100644
>> --- a/fs/namei.c
>> +++ b/fs/namei.c
>> @@ -2557,9 +2557,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;
>> @@ -2769,9 +2774,9 @@ retry_lookup:
>> }
>> mutex_lock(&dir->d_inode->i_mutex);
>> error = lookup_open(nd, path, file, op, got_write, opened);
>> - mutex_unlock(&dir->d_inode->i_mutex);
>>
>> if (error <= 0) {
>> + mutex_unlock(&dir->d_inode->i_mutex);
>> if (error)
>> goto out;
>>
>> @@ -2789,8 +2794,32 @@ retry_lookup:
>> will_truncate = false;
>> acc_mode = MAY_OPEN;
>> path_to_nameidata(path, nd);
>> - goto finish_open_created;
>> +
>> + /*
>> + * Unlock parent i_mutex later when the open finishes - prevent
>> + * races when a file can be locked with a deny lock by another
>> + * task that opens the file.
>> + */
>> + error = may_open(&nd->path, acc_mode, open_flag);
>> + if (error) {
>> + mutex_unlock(&dir->d_inode->i_mutex);
>> + goto out;
>> + }
>> + file->f_path.mnt = nd->path.mnt;
>> + error = finish_open(file, nd->path.dentry, NULL, opened);
>> + if (error) {
>> + mutex_unlock(&dir->d_inode->i_mutex);
>> + if (error == -EOPENSTALE)
>> + goto stale_open;
>> + goto out;
>> + }
>> + error = deny_lock_file(file);
>> + mutex_unlock(&dir->d_inode->i_mutex);
>> + if (error)
>> + goto exit_fput;
>> + goto opened;
>> }
>> + mutex_unlock(&dir->d_inode->i_mutex);
>>
>> /*
>> * create/update audit record if it already exists.
>> @@ -2872,7 +2901,6 @@ finish_open:
>> goto out;
>> got_write = true;
>> }
>> -finish_open_created:
>> error = may_open(&nd->path, acc_mode, open_flag);
>> if (error)
>> goto out;
>> @@ -2883,6 +2911,15 @@ finish_open_created:
>> goto stale_open;
>> goto out;
>> }
>> + /*
>> + * Lock parent i_mutex to prevent races with deny locks on newely
>> + * created files.
>> + */
>> + mutex_lock(&dir->d_inode->i_mutex);
>> + error = deny_lock_file(file);
>> + mutex_unlock(&dir->d_inode->i_mutex);
>> + if (error)
>> + goto exit_fput;
>> opened:
>> error = open_check_o_direct(file);
>> if (error)
>> diff --git a/fs/proc_namespace.c b/fs/proc_namespace.c
>> index 2033f74..6edbadc 100644
>> --- a/fs/proc_namespace.c
>> +++ b/fs/proc_namespace.c
>> @@ -44,6 +44,7 @@ static int show_sb_opts(struct seq_file *m, struct super_block *sb)
>> { MS_SYNCHRONOUS, ",sync" },
>> { MS_DIRSYNC, ",dirsync" },
>> { MS_MANDLOCK, ",mand" },
>> + { MS_SHARELOCK, ",sharelock" },
>> { 0, NULL }
>> };
>> const struct proc_fs_info *fs_infop;
>> diff --git a/include/linux/fs.h b/include/linux/fs.h
>> index 1a39c33..073e31a 100644
>> --- a/include/linux/fs.h
>> +++ b/include/linux/fs.h
>> @@ -1005,6 +1005,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 */
>> @@ -1153,6 +1154,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)
>> {
>> }
>> @@ -1668,6 +1674,7 @@ struct super_operations {
>> #define IS_PRIVATE(inode) ((inode)->i_flags & S_PRIVATE)
>> #define IS_IMA(inode) ((inode)->i_flags & S_IMA)
>> #define IS_AUTOMOUNT(inode) ((inode)->i_flags & S_AUTOMOUNT)
>> +#define IS_SHARELOCK(inode) __IS_FLG(inode, MS_SHARELOCK)
>> #define IS_NOSEC(inode) ((inode)->i_flags & S_NOSEC)
>>
>> /*
>> diff --git a/include/uapi/asm-generic/fcntl.h b/include/uapi/asm-generic/fcntl.h
>> index a48937d..5ac0d49 100644
>> --- a/include/uapi/asm-generic/fcntl.h
>> +++ b/include/uapi/asm-generic/fcntl.h
>> @@ -84,6 +84,17 @@
>> #define O_PATH 010000000
>> #endif
>>
>> +#ifndef O_DENYREAD
>> +#define O_DENYREAD 020000000 /* Do not permit read access */
>> +#endif
>> +#ifndef O_DENYWRITE
>> +#define O_DENYWRITE 040000000 /* Do not permit write access */
>> +#endif
>> +/* FMODE_NONOTIFY 0100000000 */
>> +#ifndef O_DENYDELETE
>> +#define O_DENYDELETE 0200000000 /* Do not permit delete or rename */
>> +#endif
>> +
>
> You're adding O_DENYDELETE here, but there's no support for it in the
> patchset aside from the passthrough in the cifs code. Is that
> intentional? What happens if I specify O_DENYDELETE on a non-cifs fs
> that was mounted with "sharelock"? I assume it's just ignored?

I am trying to keep changes as small as possible and did it
intentional because this flag is supported by CIFS only and flock has
only LOCK_READ and LOCK_WRITE type now. I am not sure what to do with
NFSv4 client when we decide to support this flag for VFS: we pass
O_DENYREAD and O_DENYWRITE flags to NFS clients, but do O_DENYDELETE
in VFS scope... or we can drop O_DENYDELETE possibility for NFS at all
(just do nothing in this case).

>From fcntl.h I found out that there is one free bit (16) in l_type
short integer - between LOCK_UN and LOCK_MAND - we can try to use it
for new LOCK_DELETE flag that can be set for opens with O_DENYDELETE.

>> #ifndef O_NDELAY
>> #define O_NDELAY O_NONBLOCK
>> #endif
>> diff --git a/include/uapi/linux/fs.h b/include/uapi/linux/fs.h
>> index 780d4c6..f1925f7 100644
>> --- a/include/uapi/linux/fs.h
>> +++ b/include/uapi/linux/fs.h
>> @@ -86,6 +86,7 @@ struct inodes_stat_t {
>> #define MS_KERNMOUNT (1<<22) /* this is a kern_mount call */
>> #define MS_I_VERSION (1<<23) /* Update inode I_version field */
>> #define MS_STRICTATIME (1<<24) /* Always perform atime updates */
>> +#define MS_SHARELOCK (1<<25) /* Allow share locks on an FS */
>> #define MS_NOSEC (1<<28)
>> #define MS_BORN (1<<29)
>> #define MS_ACTIVE (1<<30)
>
>
> --
> Jeff Layton <[email protected]>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-cifs" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html



--
Best regards,
Pavel Shilovsky.

2013-04-10 13:59:32

by Pavel Shilovsky

[permalink] [raw]
Subject: Re: [PATCH v5 4/7] CIFS: Use NT_CREATE_ANDX command for forcemand mounts

2013/4/10 Jeff Layton <[email protected]>:
> (cc'ing samba-technical)
>
> I don't understand. Why would we need to use NT_CREATE_ANDX in lieu of
> the POSIX trans2 open if the client isn't setting a share reservation
> on that particular open? The server should still enforce share
> reservations that are already set on the file regardless of which
> method is used.

Ok, I checked it out: Samba still enforce share reservations for
trans2 opens too. In this case we need a check like

if (IS_SHARELOCK(inode) && cifs_get_share_flags(openflags) != FILE_SHARE_ALL)
/* use NT_CREATE_ANDX */
else
/* use TRANS2 OPEN */

--
Best regards,
Pavel Shilovsky.

2013-04-10 16:35:27

by Frank Filz

[permalink] [raw]
Subject: Re: [PATCH v5 1/7] fcntl: Introduce new O_DENY* open flags

[email protected] wrote on 04/10/2013 06:24:39 AM:
> 2013/4/10 Jeff Layton <[email protected]>:
> > On Tue, 9 Apr 2013 16:40:21 +0400
> > Pavel Shilovsky <[email protected]> wrote:
> >
> >> This patch adds 3 flags:
> >> 1) O_DENYREAD that doesn't permit read access,
> >> 2) O_DENYWRITE that doesn't permit write access,
> >> 3) O_DENYDELETE that doesn't permit delete or rename,
> >>
> >> Network filesystems CIFS, SMB2.0, SMB3.0 and NFSv4 have such flags -
> >> this change can benefit cifs and nfs modules as well as Samba and
> >> NFS file servers that export the same directory for Windows clients,
> >> or Wine applications that access the same files simultaneously.
> >>
> >> These flags are only take affect for opens on mounts with 'sharelock'
> >> mount option. They are translated to flock's flags:
> >>
> >> !O_DENYREAD -> LOCK_READ | LOCK_MAND
> >> !O_DENYWRITE -> LOCK_WRITE | LOCK_MAND
> >>
> >> and set through flock_lock_file on a file. If the file can't be locked
> >> due conflicts with another open with O_DENY* flags, the -EBUSY error
> >> code is returned.
> >>
> >> Create codepath is slightly changed to prevent data races on
> >> newely created files: when open with O_CREAT can return with -EBUSY
> >> error for successfully created files due to a deny lock set by
> >> another task.
> >>
> >
> > It's good that this is consistent with the new patchset. I'm still not
> > 100% sure that -EBUSY is the right error return here, but it should at
> > least help distinguish between "mode bits don't allow me to open" and
> > "file has share reservation set".
> >
> > Heck, since we're departing from POSIX here, maybe we should consider a
> > new error code altogether? ESHAREDENIED or something?
>
> That can make sense. I don't mind to change this part.

I like this return code, it will help user space file servers return the
correct error to their clients when they depend on the kernel to resolve
share reservations. Internal to the kernel, having ESHAREDENIED will
also be helpful to the NFS v4 and NLM code.

Frank