Network filesystems CIFS, SMB2.0, SMB3.0 and NFSv4 have such flags - this change can benefit cifs and nfs modules. While this change is ok for network filesystems, itsn't not targeted for local filesystems due security problems (e.g. when a user process can deny root to delete a file).
Share flags are used by Windows applications and WINE have to deal with them too. While WINE can process open share flags itself on local filesystems, it can't do it if a file stored on a network share and is used by several clients. This patchset makes it possible for CIFS/SMB2.0/SMB3.0.
Pavel Shilovsky (3):
fcntl: Introduce new O_DENY* open flags for network filesystems
CIFS: Add O_DENY* open flags support
CIFS: Use NT_CREATE_ANDX command for forcemand mounts
fs/cifs/cifsacl.c | 10 ++++----
fs/cifs/cifsglob.h | 11 ++++++++-
fs/cifs/cifsproto.h | 9 ++++----
fs/cifs/cifssmb.c | 47 ++++++++++++++++++++------------------
fs/cifs/dir.c | 14 ++++++++----
fs/cifs/file.c | 18 ++++++++++-----
fs/cifs/inode.c | 11 +++++----
fs/cifs/link.c | 10 ++++----
fs/cifs/readdir.c | 2 +-
fs/cifs/smb1ops.c | 15 ++++++------
fs/cifs/smb2file.c | 10 ++++----
fs/cifs/smb2inode.c | 4 ++--
fs/cifs/smb2ops.c | 10 ++++----
fs/cifs/smb2pdu.c | 6 ++---
fs/cifs/smb2proto.h | 14 +++++++-----
fs/fcntl.c | 5 ++--
include/uapi/asm-generic/fcntl.h | 11 +++++++++
17 files changed, 125 insertions(+), 82 deletions(-)
--
1.7.10.4
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 4784d53..8edd950 100644
--- a/fs/cifs/dir.c
+++ b/fs/cifs/dir.c
@@ -216,6 +216,7 @@ cifs_do_create(struct inode *inode, struct dentry *direntry, unsigned int xid,
}
if (tcon->unix_ext && cap_unix(tcon->ses) && !tcon->broken_posix_open &&
+ ((cifs_sb->mnt_cifs_flags & CIFS_MOUNT_NOPOSIXBRL) == 0) &&
(CIFS_UNIX_POSIX_PATH_OPS_CAP &
le64_to_cpu(tcon->fsUnixInfo.Capability))) {
rc = cifs_posix_open(full_path, &newinode, inode->i_sb, mode,
diff --git a/fs/cifs/file.c b/fs/cifs/file.c
index e2614b9..8cfda53 100644
--- a/fs/cifs/file.c
+++ b/fs/cifs/file.c
@@ -426,8 +426,9 @@ int cifs_open(struct inode *inode, struct file *file)
else
oplock = 0;
- if (!tcon->broken_posix_open && tcon->unix_ext &&
- cap_unix(tcon->ses) && (CIFS_UNIX_POSIX_PATH_OPS_CAP &
+ if (!tcon->broken_posix_open && tcon->unix_ext && cap_unix(tcon->ses)
+ && ((cifs_sb->mnt_cifs_flags & CIFS_MOUNT_NOPOSIXBRL) == 0) &&
+ (CIFS_UNIX_POSIX_PATH_OPS_CAP &
le64_to_cpu(tcon->fsUnixInfo.Capability))) {
/* can not refresh inode info since size could be stale */
rc = cifs_posix_open(full_path, &inode, inode->i_sb,
@@ -595,6 +596,7 @@ cifs_reopen_file(struct cifsFileInfo *cfile, bool can_flush)
oplock = 0;
if (tcon->unix_ext && cap_unix(tcon->ses) &&
+ ((cifs_sb->mnt_cifs_flags & CIFS_MOUNT_NOPOSIXBRL) == 0) &&
(CIFS_UNIX_POSIX_PATH_OPS_CAP &
le64_to_cpu(tcon->fsUnixInfo.Capability))) {
/*
--
1.7.10.4
This patch adds 3 flags:
1) O_DENYREAD that doesn't permit read access
2) O_DENYWRITE that doesn't permit write access
3) O_DENYDELETE that doesn't permit delete or rename
Network filesystems CIFS, SMB2.0, SMB3.0 and NFSv4 have such flags -
this change can benefit cifs and nfs modules. While this change is ok
for network filesystems, itsn't not targeted for local filesystems due
to security problems (e.g. when a user process can deny root to delete
a file).
Signed-off-by: Pavel Shilovsky <[email protected]>
---
fs/fcntl.c | 5 +++--
include/uapi/asm-generic/fcntl.h | 11 +++++++++++
2 files changed, 14 insertions(+), 2 deletions(-)
diff --git a/fs/fcntl.c b/fs/fcntl.c
index 71a600a..7abce5a 100644
--- a/fs/fcntl.c
+++ b/fs/fcntl.c
@@ -730,14 +730,15 @@ static int __init fcntl_init(void)
* Exceptions: O_NONBLOCK is a two bit define on parisc; O_NDELAY
* is defined as O_NONBLOCK on some platforms and not on others.
*/
- BUILD_BUG_ON(19 - 1 /* for O_RDONLY being 0 */ != HWEIGHT32(
+ BUILD_BUG_ON(22 - 1 /* for O_RDONLY being 0 */ != HWEIGHT32(
O_RDONLY | O_WRONLY | O_RDWR |
O_CREAT | O_EXCL | O_NOCTTY |
O_TRUNC | O_APPEND | /* O_NONBLOCK | */
__O_SYNC | O_DSYNC | FASYNC |
O_DIRECT | O_LARGEFILE | O_DIRECTORY |
O_NOFOLLOW | O_NOATIME | O_CLOEXEC |
- __FMODE_EXEC | O_PATH
+ __FMODE_EXEC | O_PATH | O_DENYREAD |
+ O_DENYWRITE | O_DENYDELETE
));
fasync_cache = kmem_cache_create("fasync_cache",
diff --git a/include/uapi/asm-generic/fcntl.h b/include/uapi/asm-generic/fcntl.h
index a48937d..5ac0d49 100644
--- a/include/uapi/asm-generic/fcntl.h
+++ b/include/uapi/asm-generic/fcntl.h
@@ -84,6 +84,17 @@
#define O_PATH 010000000
#endif
+#ifndef O_DENYREAD
+#define O_DENYREAD 020000000 /* Do not permit read access */
+#endif
+#ifndef O_DENYWRITE
+#define O_DENYWRITE 040000000 /* Do not permit write access */
+#endif
+/* FMODE_NONOTIFY 0100000000 */
+#ifndef O_DENYDELETE
+#define O_DENYDELETE 0200000000 /* Do not permit delete or rename */
+#endif
+
#ifndef O_NDELAY
#define O_NDELAY O_NONBLOCK
#endif
--
1.7.10.4
2012/11/30 Pavel Shilovsky <[email protected]>:
> Network filesystems CIFS, SMB2.0, SMB3.0 and NFSv4 have such flags - this change can benefit cifs and nfs modules. While this change is ok for network filesystems, itsn't not targeted for local filesystems due security problems (e.g. when a user process can deny root to delete a file).
>
> Share flags are used by Windows applications and WINE have to deal with them too. While WINE can process open share flags itself on local filesystems, it can't do it if a file stored on a network share and is used by several clients. This patchset makes it possible for CIFS/SMB2.0/SMB3.0.
>
> Pavel Shilovsky (3):
> fcntl: Introduce new O_DENY* open flags for network filesystems
> CIFS: Add O_DENY* open flags support
> CIFS: Use NT_CREATE_ANDX command for forcemand mounts
>
> fs/cifs/cifsacl.c | 10 ++++----
> fs/cifs/cifsglob.h | 11 ++++++++-
> fs/cifs/cifsproto.h | 9 ++++----
> fs/cifs/cifssmb.c | 47 ++++++++++++++++++++------------------
> fs/cifs/dir.c | 14 ++++++++----
> fs/cifs/file.c | 18 ++++++++++-----
> fs/cifs/inode.c | 11 +++++----
> fs/cifs/link.c | 10 ++++----
> fs/cifs/readdir.c | 2 +-
> fs/cifs/smb1ops.c | 15 ++++++------
> fs/cifs/smb2file.c | 10 ++++----
> fs/cifs/smb2inode.c | 4 ++--
> fs/cifs/smb2ops.c | 10 ++++----
> fs/cifs/smb2pdu.c | 6 ++---
> fs/cifs/smb2proto.h | 14 +++++++-----
> fs/fcntl.c | 5 ++--
> include/uapi/asm-generic/fcntl.h | 11 +++++++++
> 17 files changed, 125 insertions(+), 82 deletions(-)
>
> --
> 1.7.10.4
>
CC'ing wine-devel@.
--
Best regards,
Pavel Shilovsky.
Make CIFSSMBOpen take share_flags as a parm that allows us
to pass new O_DENY* flags to the server.
Signed-off-by: Pavel Shilovsky <[email protected]>
---
fs/cifs/cifsacl.c | 10 ++++++----
fs/cifs/cifsglob.h | 11 ++++++++++-
fs/cifs/cifsproto.h | 9 +++++----
fs/cifs/cifssmb.c | 47 +++++++++++++++++++++++++----------------------
fs/cifs/dir.c | 13 ++++++++-----
fs/cifs/file.c | 12 ++++++++----
fs/cifs/inode.c | 11 ++++++-----
fs/cifs/link.c | 10 +++++-----
fs/cifs/readdir.c | 2 +-
fs/cifs/smb1ops.c | 15 ++++++++-------
fs/cifs/smb2file.c | 10 +++++-----
fs/cifs/smb2inode.c | 4 ++--
fs/cifs/smb2ops.c | 10 ++++++----
fs/cifs/smb2pdu.c | 6 +++---
fs/cifs/smb2proto.h | 14 ++++++++------
15 files changed, 106 insertions(+), 78 deletions(-)
diff --git a/fs/cifs/cifsacl.c b/fs/cifs/cifsacl.c
index 9c3ebbd..86981df 100644
--- a/fs/cifs/cifsacl.c
+++ b/fs/cifs/cifsacl.c
@@ -895,8 +895,9 @@ static struct cifs_ntsd *get_cifs_acl_by_path(struct cifs_sb_info *cifs_sb,
create_options |= CREATE_OPEN_BACKUP_INTENT;
rc = CIFSSMBOpen(xid, tcon, path, FILE_OPEN, READ_CONTROL,
- create_options, &fid, &oplock, NULL, cifs_sb->local_nls,
- cifs_sb->mnt_cifs_flags & CIFS_MOUNT_MAP_SPECIAL_CHR);
+ FILE_SHARE_ALL, create_options, &fid, &oplock,
+ NULL, cifs_sb->local_nls,
+ cifs_sb->mnt_cifs_flags & CIFS_MOUNT_MAP_SPECIAL_CHR);
if (!rc) {
rc = CIFSSMBGetCIFSACL(xid, tcon, fid, &pntsd, pacllen);
CIFSSMBClose(xid, tcon, fid);
@@ -956,8 +957,9 @@ int set_cifs_acl(struct cifs_ntsd *pnntsd, __u32 acllen,
access_flags = WRITE_DAC;
rc = CIFSSMBOpen(xid, tcon, path, FILE_OPEN, access_flags,
- create_options, &fid, &oplock, NULL, cifs_sb->local_nls,
- cifs_sb->mnt_cifs_flags & CIFS_MOUNT_MAP_SPECIAL_CHR);
+ FILE_SHARE_ALL, create_options, &fid, &oplock,
+ NULL, cifs_sb->local_nls,
+ cifs_sb->mnt_cifs_flags & CIFS_MOUNT_MAP_SPECIAL_CHR);
if (rc) {
cERROR(1, "Unable to open file to set ACL");
goto out;
diff --git a/fs/cifs/cifsglob.h b/fs/cifs/cifsglob.h
index ac66409..e3b2490 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);
@@ -945,6 +945,15 @@ struct cifsFileInfo {
struct work_struct oplock_break; /* work for oplock breaks */
};
+#define CIFS_DENY_RW_FLAGS_SHIFT 22
+#define CIFS_DENY_DEL_FLAG_SHIFT 23
+
+static inline int cifs_get_share_flags(unsigned int flags)
+{
+ return ((~(flags >> CIFS_DENY_RW_FLAGS_SHIFT)) & 3) |
+ ((~(flags >> CIFS_DENY_DEL_FLAG_SHIFT)) & 4);
+}
+
struct cifs_io_parms {
__u16 netfid;
#ifdef CONFIG_CIFS_SMB2
diff --git a/fs/cifs/cifsproto.h b/fs/cifs/cifsproto.h
index 17cadf2..480541c 100644
--- a/fs/cifs/cifsproto.h
+++ b/fs/cifs/cifsproto.h
@@ -363,10 +363,11 @@ extern int CIFSSMBQueryReparseLinkInfo(const unsigned int xid,
const struct nls_table *nls_codepage);
#endif /* temporarily unused until cifs_symlink fixed */
extern int CIFSSMBOpen(const unsigned int xid, struct cifs_tcon *tcon,
- const char *fileName, const int disposition,
- const int access_flags, const int omode,
- __u16 *netfid, int *pOplock, FILE_ALL_INFO *,
- const struct nls_table *nls_codepage, int remap);
+ const char *file_name, const int disposition,
+ const int access_flags, const int share_flags,
+ const int omode, __u16 *netfid, int *oplock,
+ FILE_ALL_INFO *, const struct nls_table *nls_codepage,
+ int remap);
extern int SMBLegacyOpen(const unsigned int xid, struct cifs_tcon *tcon,
const char *fileName, const int disposition,
const int access_flags, const int omode,
diff --git a/fs/cifs/cifssmb.c b/fs/cifs/cifssmb.c
index 76d0d29..9c4632f 100644
--- a/fs/cifs/cifssmb.c
+++ b/fs/cifs/cifssmb.c
@@ -1289,10 +1289,11 @@ OldOpenRetry:
int
CIFSSMBOpen(const unsigned int xid, struct cifs_tcon *tcon,
- const char *fileName, const int openDisposition,
- const int access_flags, const int create_options, __u16 *netfid,
- int *pOplock, FILE_ALL_INFO *pfile_info,
- const struct nls_table *nls_codepage, int remap)
+ const char *file_name, const int disposition,
+ const int access_flags, const int share_flags,
+ const int create_options, __u16 *netfid, int *oplock,
+ FILE_ALL_INFO *file_info, const struct nls_table *nls_codepage,
+ int remap)
{
int rc = -EACCES;
OPEN_REQ *pSMB = NULL;
@@ -1313,26 +1314,28 @@ openRetry:
count = 1; /* account for one byte pad to word boundary */
name_len =
cifsConvertToUTF16((__le16 *) (pSMB->fileName + 1),
- fileName, PATH_MAX, nls_codepage, remap);
+ file_name, PATH_MAX, nls_codepage,
+ remap);
name_len++; /* trailing null */
name_len *= 2;
pSMB->NameLength = cpu_to_le16(name_len);
} else { /* BB improve check for buffer overruns BB */
count = 0; /* no pad */
- name_len = strnlen(fileName, PATH_MAX);
+ name_len = strnlen(file_name, PATH_MAX);
name_len++; /* trailing null */
pSMB->NameLength = cpu_to_le16(name_len);
- strncpy(pSMB->fileName, fileName, name_len);
+ strncpy(pSMB->fileName, file_name, name_len);
}
- if (*pOplock & REQ_OPLOCK)
+ if (*oplock & REQ_OPLOCK)
pSMB->OpenFlags = cpu_to_le32(REQ_OPLOCK);
- else if (*pOplock & REQ_BATCHOPLOCK)
+ else if (*oplock & REQ_BATCHOPLOCK)
pSMB->OpenFlags = cpu_to_le32(REQ_BATCHOPLOCK);
pSMB->DesiredAccess = cpu_to_le32(access_flags);
pSMB->AllocationSize = 0;
- /* set file as system file if special file such
- as fifo and server expecting SFU style and
- no Unix extensions */
+ /*
+ * set file as system file if special file such as fifo and server
+ * expecting SFU style and no Unix extensions
+ */
if (create_options & CREATE_OPTION_SPECIAL)
pSMB->FileAttributes = cpu_to_le32(ATTR_SYSTEM);
else
@@ -1347,8 +1350,8 @@ openRetry:
if (create_options & CREATE_OPTION_READONLY)
pSMB->FileAttributes |= cpu_to_le32(ATTR_READONLY);
- pSMB->ShareAccess = cpu_to_le32(FILE_SHARE_ALL);
- pSMB->CreateDisposition = cpu_to_le32(openDisposition);
+ pSMB->ShareAccess = cpu_to_le32(share_flags);
+ pSMB->CreateDisposition = cpu_to_le32(disposition);
pSMB->CreateOptions = cpu_to_le32(create_options & CREATE_OPTIONS_MASK);
/* BB Expirement with various impersonation levels and verify */
pSMB->ImpersonationLevel = cpu_to_le32(SECURITY_IMPERSONATION);
@@ -1366,20 +1369,20 @@ openRetry:
if (rc) {
cFYI(1, "Error in Open = %d", rc);
} else {
- *pOplock = pSMBr->OplockLevel; /* 1 byte no need to le_to_cpu */
+ *oplock = pSMBr->OplockLevel; /* 1 byte no need to le_to_cpu */
*netfid = pSMBr->Fid; /* cifs fid stays in le */
/* Let caller know file was created so we can set the mode. */
/* Do we care about the CreateAction in any other cases? */
if (cpu_to_le32(FILE_CREATE) == pSMBr->CreateAction)
- *pOplock |= CIFS_CREATE_ACTION;
- if (pfile_info) {
- memcpy((char *)pfile_info, (char *)&pSMBr->CreationTime,
+ *oplock |= CIFS_CREATE_ACTION;
+ if (file_info) {
+ memcpy((char *)file_info, (char *)&pSMBr->CreationTime,
36 /* CreationTime to Attributes */);
/* the file_info buf is endian converted by caller */
- pfile_info->AllocationSize = pSMBr->AllocationSize;
- pfile_info->EndOfFile = pSMBr->EndOfFile;
- pfile_info->NumberOfLinks = cpu_to_le32(1);
- pfile_info->DeletePending = 0;
+ file_info->AllocationSize = pSMBr->AllocationSize;
+ file_info->EndOfFile = pSMBr->EndOfFile;
+ file_info->NumberOfLinks = cpu_to_le32(1);
+ file_info->DeletePending = 0;
}
}
diff --git a/fs/cifs/dir.c b/fs/cifs/dir.c
index 3b7e0c1..4784d53 100644
--- a/fs/cifs/dir.c
+++ b/fs/cifs/dir.c
@@ -202,6 +202,7 @@ cifs_do_create(struct inode *inode, struct dentry *direntry, unsigned int xid,
FILE_ALL_INFO *buf = NULL;
struct inode *newinode = NULL;
int disposition;
+ int share_access;
struct TCP_Server_Info *server = tcon->ses->server;
*oplock = 0;
@@ -292,6 +293,8 @@ cifs_do_create(struct inode *inode, struct dentry *direntry, unsigned int xid,
else
cFYI(1, "Create flag not set in create function");
+ share_access = cifs_get_share_flags(oflags);
+
/*
* BB add processing to set equivalent of mode - e.g. via CreateX with
* ACLs
@@ -319,8 +322,8 @@ cifs_do_create(struct inode *inode, struct dentry *direntry, unsigned int xid,
create_options |= CREATE_OPEN_BACKUP_INTENT;
rc = server->ops->open(xid, tcon, full_path, disposition,
- desired_access, create_options, fid, oplock,
- buf, cifs_sb);
+ desired_access, share_access, create_options,
+ fid, oplock, buf, cifs_sb);
if (rc) {
cFYI(1, "cifs_create returned 0x%x", rc);
goto out;
@@ -625,9 +628,9 @@ int cifs_mknod(struct inode *inode, struct dentry *direntry, umode_t mode,
if (backup_cred(cifs_sb))
create_options |= CREATE_OPEN_BACKUP_INTENT;
- rc = CIFSSMBOpen(xid, pTcon, full_path, FILE_CREATE,
- GENERIC_WRITE, create_options,
- &fileHandle, &oplock, buf, cifs_sb->local_nls,
+ rc = CIFSSMBOpen(xid, pTcon, full_path, FILE_CREATE, GENERIC_WRITE,
+ FILE_SHARE_ALL, create_options, &fileHandle, &oplock,
+ buf, cifs_sb->local_nls,
cifs_sb->mnt_cifs_flags & CIFS_MOUNT_MAP_SPECIAL_CHR);
if (rc)
goto mknod_out;
diff --git a/fs/cifs/file.c b/fs/cifs/file.c
index bceffa8..e2614b9 100644
--- a/fs/cifs/file.c
+++ b/fs/cifs/file.c
@@ -174,6 +174,7 @@ cifs_nt_open(char *full_path, struct inode *inode, struct cifs_sb_info *cifs_sb,
{
int rc;
int desired_access;
+ int share_access;
int disposition;
int create_options = CREATE_NOT_DIR;
FILE_ALL_INFO *buf;
@@ -209,6 +210,7 @@ cifs_nt_open(char *full_path, struct inode *inode, struct cifs_sb_info *cifs_sb,
*********************************************************************/
disposition = cifs_get_disposition(f_flags);
+ share_access = cifs_get_share_flags(f_flags);
/* BB pass O_SYNC flag through on file attributes .. BB */
@@ -220,8 +222,8 @@ cifs_nt_open(char *full_path, struct inode *inode, struct cifs_sb_info *cifs_sb,
create_options |= CREATE_OPEN_BACKUP_INTENT;
rc = server->ops->open(xid, tcon, full_path, disposition,
- desired_access, create_options, fid, oplock, buf,
- cifs_sb);
+ desired_access, share_access, create_options,
+ fid, oplock, buf, cifs_sb);
if (rc)
goto out;
@@ -551,6 +553,7 @@ cifs_reopen_file(struct cifsFileInfo *cfile, bool can_flush)
struct inode *inode;
char *full_path = NULL;
int desired_access;
+ int share_access;
int disposition = FILE_OPEN;
int create_options = CREATE_NOT_DIR;
struct cifs_fid fid;
@@ -615,6 +618,7 @@ cifs_reopen_file(struct cifsFileInfo *cfile, bool can_flush)
}
desired_access = cifs_convert_flags(cfile->f_flags);
+ share_access = cifs_get_share_flags(cfile->f_flags);
if (backup_cred(cifs_sb))
create_options |= CREATE_OPEN_BACKUP_INTENT;
@@ -630,8 +634,8 @@ cifs_reopen_file(struct cifsFileInfo *cfile, bool can_flush)
* not dirty locally we could do this.
*/
rc = server->ops->open(xid, tcon, full_path, disposition,
- desired_access, create_options, &fid, &oplock,
- NULL, cifs_sb);
+ desired_access, share_access, create_options,
+ &fid, &oplock, NULL, cifs_sb);
if (rc) {
mutex_unlock(&cfile->fh_mutex);
cFYI(1, "cifs_reopen returned 0x%x", rc);
diff --git a/fs/cifs/inode.c b/fs/cifs/inode.c
index ed6208f..a497dfa 100644
--- a/fs/cifs/inode.c
+++ b/fs/cifs/inode.c
@@ -396,8 +396,8 @@ cifs_sfu_type(struct cifs_fattr *fattr, const unsigned char *path,
tcon = tlink_tcon(tlink);
rc = CIFSSMBOpen(xid, tcon, path, FILE_OPEN, GENERIC_READ,
- CREATE_NOT_DIR, &netfid, &oplock, NULL,
- cifs_sb->local_nls,
+ FILE_SHARE_ALL, CREATE_NOT_DIR, &netfid, &oplock,
+ NULL, cifs_sb->local_nls,
cifs_sb->mnt_cifs_flags &
CIFS_MOUNT_MAP_SPECIAL_CHR);
if (rc == 0) {
@@ -987,8 +987,9 @@ cifs_rename_pending_delete(const char *full_path, struct dentry *dentry,
tcon = tlink_tcon(tlink);
rc = CIFSSMBOpen(xid, tcon, full_path, FILE_OPEN,
- DELETE|FILE_WRITE_ATTRIBUTES, CREATE_NOT_DIR,
- &netfid, &oplock, NULL, cifs_sb->local_nls,
+ DELETE|FILE_WRITE_ATTRIBUTES, FILE_SHARE_ALL,
+ CREATE_NOT_DIR, &netfid, &oplock, NULL,
+ cifs_sb->local_nls,
cifs_sb->mnt_cifs_flags & CIFS_MOUNT_MAP_SPECIAL_CHR);
if (rc != 0)
goto out;
@@ -1509,7 +1510,7 @@ cifs_do_rename(const unsigned int xid, struct dentry *from_dentry,
/* open the file to be renamed -- we need DELETE perms */
rc = CIFSSMBOpen(xid, tcon, from_path, FILE_OPEN, DELETE,
- CREATE_NOT_DIR, &srcfid, &oplock, NULL,
+ FILE_SHARE_ALL, CREATE_NOT_DIR, &srcfid, &oplock, NULL,
cifs_sb->local_nls, cifs_sb->mnt_cifs_flags &
CIFS_MOUNT_MAP_SPECIAL_CHR);
if (rc == 0) {
diff --git a/fs/cifs/link.c b/fs/cifs/link.c
index 51dc2fb..9b4f0db 100644
--- a/fs/cifs/link.c
+++ b/fs/cifs/link.c
@@ -212,7 +212,7 @@ CIFSCreateMFSymLink(const unsigned int xid, struct cifs_tcon *tcon,
create_options |= CREATE_OPEN_BACKUP_INTENT;
rc = CIFSSMBOpen(xid, tcon, fromName, FILE_CREATE, GENERIC_WRITE,
- create_options, &netfid, &oplock, NULL,
+ FILE_SHARE_ALL, create_options, &netfid, &oplock, NULL,
nls_codepage, remap);
if (rc != 0) {
kfree(buf);
@@ -254,8 +254,8 @@ CIFSQueryMFSymLink(const unsigned int xid, struct cifs_tcon *tcon,
FILE_ALL_INFO file_info;
rc = CIFSSMBOpen(xid, tcon, searchName, FILE_OPEN, GENERIC_READ,
- CREATE_NOT_DIR, &netfid, &oplock, &file_info,
- nls_codepage, remap);
+ FILE_SHARE_ALL, CREATE_NOT_DIR, &netfid, &oplock,
+ &file_info, nls_codepage, remap);
if (rc != 0)
return rc;
@@ -332,8 +332,8 @@ CIFSCheckMFSymlink(struct cifs_fattr *fattr,
pTcon = tlink_tcon(tlink);
rc = CIFSSMBOpen(xid, pTcon, path, FILE_OPEN, GENERIC_READ,
- CREATE_NOT_DIR, &netfid, &oplock, &file_info,
- cifs_sb->local_nls,
+ FILE_SHARE_ALL, CREATE_NOT_DIR, &netfid, &oplock,
+ &file_info, cifs_sb->local_nls,
cifs_sb->mnt_cifs_flags &
CIFS_MOUNT_MAP_SPECIAL_CHR);
if (rc != 0)
diff --git a/fs/cifs/readdir.c b/fs/cifs/readdir.c
index 96fe44b..6a6a801 100644
--- a/fs/cifs/readdir.c
+++ b/fs/cifs/readdir.c
@@ -213,7 +213,7 @@ int get_symlink_reparse_path(char *full_path, struct cifs_sb_info *cifs_sb,
char *tmpbuffer;
rc = CIFSSMBOpen(xid, ptcon, full_path, FILE_OPEN, GENERIC_READ,
- OPEN_REPARSE_POINT, &fid, &oplock, NULL,
+ FILE_SHARE_ALL, OPEN_REPARSE_POINT, &fid, &oplock, NULL,
cifs_sb->local_nls,
cifs_sb->mnt_cifs_flags & CIFS_MOUNT_MAP_SPECIAL_CHR);
if (!rc) {
diff --git a/fs/cifs/smb1ops.c b/fs/cifs/smb1ops.c
index a5d234c..561547f 100644
--- a/fs/cifs/smb1ops.c
+++ b/fs/cifs/smb1ops.c
@@ -664,9 +664,9 @@ cifs_mkdir_setinfo(struct inode *inode, const char *full_path,
static int
cifs_open_file(const unsigned int xid, struct cifs_tcon *tcon, const char *path,
- int disposition, int desired_access, int create_options,
- struct cifs_fid *fid, __u32 *oplock, FILE_ALL_INFO *buf,
- struct cifs_sb_info *cifs_sb)
+ int disposition, int desired_access, int share_access,
+ int create_options, struct cifs_fid *fid, __u32 *oplock,
+ FILE_ALL_INFO *buf, struct cifs_sb_info *cifs_sb)
{
if (!(tcon->ses->capabilities & CAP_NT_SMBS))
return SMBLegacyOpen(xid, tcon, path, disposition,
@@ -675,8 +675,8 @@ cifs_open_file(const unsigned int xid, struct cifs_tcon *tcon, const char *path,
cifs_sb->local_nls, cifs_sb->mnt_cifs_flags
& CIFS_MOUNT_MAP_SPECIAL_CHR);
return CIFSSMBOpen(xid, tcon, path, disposition, desired_access,
- create_options, &fid->netfid, oplock, buf,
- cifs_sb->local_nls, cifs_sb->mnt_cifs_flags &
+ share_access, create_options, &fid->netfid, oplock,
+ buf, cifs_sb->local_nls, cifs_sb->mnt_cifs_flags &
CIFS_MOUNT_MAP_SPECIAL_CHR);
}
@@ -772,8 +772,9 @@ smb_set_file_info(struct inode *inode, const char *full_path,
cFYI(1, "calling SetFileInfo since SetPathInfo for times not supported "
"by this server");
rc = CIFSSMBOpen(xid, tcon, full_path, FILE_OPEN,
- SYNCHRONIZE | FILE_WRITE_ATTRIBUTES, CREATE_NOT_DIR,
- &netfid, &oplock, NULL, cifs_sb->local_nls,
+ SYNCHRONIZE | FILE_WRITE_ATTRIBUTES, FILE_SHARE_ALL,
+ CREATE_NOT_DIR, &netfid, &oplock, NULL,
+ cifs_sb->local_nls,
cifs_sb->mnt_cifs_flags & CIFS_MOUNT_MAP_SPECIAL_CHR);
if (rc != 0) {
diff --git a/fs/cifs/smb2file.c b/fs/cifs/smb2file.c
index 71e6aed..7dfb50c 100644
--- a/fs/cifs/smb2file.c
+++ b/fs/cifs/smb2file.c
@@ -58,9 +58,9 @@ smb2_set_oplock_level(struct cifsInodeInfo *cinode, __u32 oplock)
int
smb2_open_file(const unsigned int xid, struct cifs_tcon *tcon, const char *path,
- int disposition, int desired_access, int create_options,
- struct cifs_fid *fid, __u32 *oplock, FILE_ALL_INFO *buf,
- struct cifs_sb_info *cifs_sb)
+ int disposition, int desired_access, int share_access,
+ int create_options, struct cifs_fid *fid, __u32 *oplock,
+ FILE_ALL_INFO *buf, struct cifs_sb_info *cifs_sb)
{
int rc;
__le16 *smb2_path;
@@ -87,8 +87,8 @@ smb2_open_file(const unsigned int xid, struct cifs_tcon *tcon, const char *path,
memcpy(smb2_oplock + 1, fid->lease_key, SMB2_LEASE_KEY_SIZE);
rc = SMB2_open(xid, tcon, smb2_path, &fid->persistent_fid,
- &fid->volatile_fid, desired_access, disposition,
- 0, 0, smb2_oplock, smb2_data);
+ &fid->volatile_fid, desired_access, share_access,
+ disposition, 0, 0, smb2_oplock, smb2_data);
if (rc)
goto out;
diff --git a/fs/cifs/smb2inode.c b/fs/cifs/smb2inode.c
index 7064824..6af174a 100644
--- a/fs/cifs/smb2inode.c
+++ b/fs/cifs/smb2inode.c
@@ -54,8 +54,8 @@ smb2_open_op_close(const unsigned int xid, struct cifs_tcon *tcon,
return -ENOMEM;
rc = SMB2_open(xid, tcon, utf16_path, &persistent_fid, &volatile_fid,
- desired_access, create_disposition, file_attributes,
- create_options, &oplock, NULL);
+ desired_access, FILE_SHARE_ALL, create_disposition,
+ file_attributes, create_options, &oplock, NULL);
if (rc) {
kfree(utf16_path);
return rc;
diff --git a/fs/cifs/smb2ops.c b/fs/cifs/smb2ops.c
index ad4d96a..4056685 100644
--- a/fs/cifs/smb2ops.c
+++ b/fs/cifs/smb2ops.c
@@ -222,7 +222,8 @@ smb2_is_path_accessible(const unsigned int xid, struct cifs_tcon *tcon,
return -ENOMEM;
rc = SMB2_open(xid, tcon, utf16_path, &persistent_fid, &volatile_fid,
- FILE_READ_ATTRIBUTES, FILE_OPEN, 0, 0, &oplock, NULL);
+ FILE_READ_ATTRIBUTES, FILE_SHARE_ALL, FILE_OPEN, 0, 0,
+ &oplock, NULL);
if (rc) {
kfree(utf16_path);
return rc;
@@ -432,8 +433,8 @@ smb2_query_dir_first(const unsigned int xid, struct cifs_tcon *tcon,
return -ENOMEM;
rc = SMB2_open(xid, tcon, utf16_path, &persistent_fid, &volatile_fid,
- FILE_READ_ATTRIBUTES | FILE_READ_DATA, FILE_OPEN, 0, 0,
- &oplock, NULL);
+ FILE_READ_ATTRIBUTES | FILE_READ_DATA, FILE_SHARE_ALL,
+ FILE_OPEN, 0, 0, &oplock, NULL);
kfree(utf16_path);
if (rc) {
cERROR(1, "open dir failed");
@@ -515,7 +516,8 @@ smb2_queryfs(const unsigned int xid, struct cifs_tcon *tcon,
u8 oplock = SMB2_OPLOCK_LEVEL_NONE;
rc = SMB2_open(xid, tcon, &srch_path, &persistent_fid, &volatile_fid,
- FILE_READ_ATTRIBUTES, FILE_OPEN, 0, 0, &oplock, NULL);
+ FILE_READ_ATTRIBUTES, FILE_SHARE_ALL, FILE_OPEN, 0, 0,
+ &oplock, NULL);
if (rc)
return rc;
buf->f_type = SMB2_MAGIC_NUMBER;
diff --git a/fs/cifs/smb2pdu.c b/fs/cifs/smb2pdu.c
index cf33622..641eda3 100644
--- a/fs/cifs/smb2pdu.c
+++ b/fs/cifs/smb2pdu.c
@@ -909,8 +909,8 @@ parse_lease_state(struct smb2_create_rsp *rsp)
int
SMB2_open(const unsigned int xid, struct cifs_tcon *tcon, __le16 *path,
u64 *persistent_fid, u64 *volatile_fid, __u32 desired_access,
- __u32 create_disposition, __u32 file_attributes, __u32 create_options,
- __u8 *oplock, struct smb2_file_all_info *buf)
+ __u32 share_access, __u32 create_disposition, __u32 file_attributes,
+ __u32 create_options, __u8 *oplock, struct smb2_file_all_info *buf)
{
struct smb2_create_req *req;
struct smb2_create_rsp *rsp;
@@ -939,7 +939,7 @@ SMB2_open(const unsigned int xid, struct cifs_tcon *tcon, __le16 *path,
req->DesiredAccess = cpu_to_le32(desired_access);
/* File attributes ignored on open (used in create though) */
req->FileAttributes = cpu_to_le32(file_attributes);
- req->ShareAccess = FILE_SHARE_ALL_LE;
+ req->ShareAccess = cpu_to_le32(share_access);
req->CreateDisposition = cpu_to_le32(create_disposition);
req->CreateOptions = cpu_to_le32(create_options);
uni_path_len = (2 * UniStrnlen((wchar_t *)path, PATH_MAX)) + 2;
diff --git a/fs/cifs/smb2proto.h b/fs/cifs/smb2proto.h
index 7d25f8b..f836a0b 100644
--- a/fs/cifs/smb2proto.h
+++ b/fs/cifs/smb2proto.h
@@ -82,9 +82,10 @@ extern int smb2_create_hardlink(const unsigned int xid, struct cifs_tcon *tcon,
extern int smb2_open_file(const unsigned int xid, struct cifs_tcon *tcon,
const char *full_path, int disposition,
- int desired_access, int create_options,
- struct cifs_fid *fid, __u32 *oplock,
- FILE_ALL_INFO *buf, struct cifs_sb_info *cifs_sb);
+ int desired_access, int share_access,
+ int create_options, struct cifs_fid *fid,
+ __u32 *oplock, FILE_ALL_INFO *buf,
+ struct cifs_sb_info *cifs_sb);
extern void smb2_set_oplock_level(struct cifsInodeInfo *cinode, __u32 oplock);
extern int smb2_unlock_range(struct cifsFileInfo *cfile,
struct file_lock *flock, const unsigned int xid);
@@ -104,9 +105,10 @@ extern int SMB2_tcon(const unsigned int xid, struct cifs_ses *ses,
extern int SMB2_tdis(const unsigned int xid, struct cifs_tcon *tcon);
extern int SMB2_open(const unsigned int xid, struct cifs_tcon *tcon,
__le16 *path, u64 *persistent_fid, u64 *volatile_fid,
- __u32 desired_access, __u32 create_disposition,
- __u32 file_attributes, __u32 create_options,
- __u8 *oplock, struct smb2_file_all_info *buf);
+ __u32 desired_access, __u32 share_access,
+ __u32 create_disposition, __u32 file_attributes,
+ __u32 create_options, __u8 *oplock,
+ struct smb2_file_all_info *buf);
extern int SMB2_close(const unsigned int xid, struct cifs_tcon *tcon,
u64 persistent_file_id, u64 volatile_file_id);
extern int SMB2_flush(const unsigned int xid, struct cifs_tcon *tcon,
--
1.7.10.4
2012/12/6 Pavel Shilovsky <[email protected]>:
> Network filesystems CIFS, SMB2.0, SMB3.0 and NFSv4 have such flags - this change can benefit cifs and nfs modules. While this change is ok for network filesystems, itsn't not targeted for local filesystems due security problems (e.g. when a user process can deny root to delete a file).
>
> Share flags are used by Windows applications and WINE have to deal with them too. While WINE can process open share flags itself on local filesystems, it can't do it if a file stored on a network share and is used by several clients. This patchset makes it possible for CIFS/SMB2.0/SMB3.0.
>
> Pavel Shilovsky (3):
> fcntl: Introduce new O_DENY* open flags for network filesystems
> CIFS: Add O_DENY* open flags support
> CIFS: Use NT_CREATE_ANDX command for forcemand mounts
>
> fs/cifs/cifsacl.c | 10 ++++----
> fs/cifs/cifsglob.h | 11 ++++++++-
> fs/cifs/cifsproto.h | 9 ++++----
> fs/cifs/cifssmb.c | 47 ++++++++++++++++++++------------------
> fs/cifs/dir.c | 14 ++++++++----
> fs/cifs/file.c | 18 ++++++++++-----
> fs/cifs/inode.c | 11 +++++----
> fs/cifs/link.c | 10 ++++----
> fs/cifs/readdir.c | 2 +-
> fs/cifs/smb1ops.c | 15 ++++++------
> fs/cifs/smb2file.c | 10 ++++----
> fs/cifs/smb2inode.c | 4 ++--
> fs/cifs/smb2ops.c | 10 ++++----
> fs/cifs/smb2pdu.c | 6 ++---
> fs/cifs/smb2proto.h | 14 +++++++-----
> fs/fcntl.c | 5 ++--
> include/uapi/asm-generic/fcntl.h | 11 +++++++++
> 17 files changed, 125 insertions(+), 82 deletions(-)
>
> --
> 1.7.10.4
>
First of all, sorry for being unclear at this proposal.
I am not from Wine team but I am working on things related to
Wine+CIFS-client+Samba.
We (at Etersoft) need to organize the work of Wine applications
through cifs-client share mounted to Samba (or Windows server). They
are related to accounting (mostly Russian ones - e.g.
http://www.1c.ru/eng/title.htm). So, the problem is that such
applications use share flags to control the parallel access to the
same files of several clients on a remote share. Also, there can be a
situation where Windows (native) clients and Wine clients are working
together in the same time.
That's why we need such flags in the kernel (patch #1). With these
flags Wine can pass them to every open and they will be used by CIFS
(and probably NFS) file systems to pass to the Samba server. In the
same time if the file is on local filesystem - these flags will be
simply ignored (not implemented).
Now we have to make our own builds of cifs module for every kernel
that use these flags passed by our build of Wine - this scheme is
working but requires merging every time new kernel is released.
Getting things into mainline let Wine supports more applications.
As for the security layer, I don't think that we need any extra bits
to switch these flags on or off - it will be switched on/off by an
underlying filesystem. Since, this change is targeted for a network
purpose only, a tool, that shows us what process/user locks a file,
doesn't help a lot because the file can be locked remotely.
As was said above, this change let us give Wine application share
flags possibility for both Samba and Windows servers. Patch #2 enables
share flags support for cifs mounts of Windows servers or Samba
servers with nounix mount option. But we already have forcemand mount
option in cifs module that switches on mandatory byte-range locking
semantic for Samba without nounix. Since share flags capability is a
kind of 'mandatory' locking scheme, I suggest that this option should
switch on share flags for Samba without nounix too (by using
NTCreateAndX command for open rather than transaction2) - that is what
patch #3 does.
--
Best regards,
Pavel Shilovsky.
On Thu, Dec 6, 2012 at 1:57 PM, Jeremy Allison <[email protected]> wrote:
> On Thu, Dec 06, 2012 at 07:49:49PM +0000, Alan Cox wrote:
>> On Thu, 6 Dec 2012 22:26:28 +0400
>> Pavel Shilovsky <[email protected]> wrote:
>>
>> > Network filesystems CIFS, SMB2.0, SMB3.0 and NFSv4 have such flags - this change can benefit cifs and nfs modules. While this change is ok for network filesystems, itsn't not targeted for local filesystems due security problems (e.g. when a user process can deny root to delete a file).
>>
>> If I have my root fs on NFS then the same applies does it not.
>>
>> Your patches fail to describe the security semantics and what file rights
>> I must have to apply each option. How do I track down a lock user, what
>> tools are provided ? How do the new options interact with the security
>> layer?
>>
>> I don't have a problem with the idea, but it needs a lot more clear
>> description of how it works so the model can be checked and if need be
>> things tweaked (eg needing write to denywrite etc)
>
> And this is where things get really ugly of course :-).
>
> For the CIFSFS client they're expecting to be able to
> just ship them to a Windows server, where they'll
> get the (insane) Windows semantics. These semantics
> are not what would be wanted on a local filesystem.
>
> So unless we just say "these things have Windows
> semantics" (where openers of files can lock out others
I suspect that WINE would have the same need
to ship them to an NFS server as to a Windows server,
and the NFS4 protocol specification also defines these,
although I could not find the same level of detail that MS-FSA
provides (e.g. see section 2.14.10 for the detailed
description of how lock conflicts are checked) but the
semantics are probably the same.
--
Thanks,
Steve
Christoph Hellwig писал 07.12.2012 20:16:
> On Thu, Dec 06, 2012 at 10:26:28PM +0400, Pavel Shilovsky wrote:
>> Network filesystems CIFS, SMB2.0, SMB3.0 and NFSv4 have such flags -
>> this change can benefit cifs and nfs modules. While this change is ok
>> for network filesystems, itsn't not targeted for local filesystems due
>> security problems (e.g. when a user process can deny root to delete a
>> file).
>>
>> Share flags are used by Windows applications and WINE have to deal
>> with them too. While WINE can process open share flags itself on local
>> filesystems, it can't do it if a file stored on a network share and is
>> used by several clients. This patchset makes it possible for
>> CIFS/SMB2.0/SMB3.0.
>
> I don't think introducing user visible flags that are only supported
> on
> a single network filesystem is a good idea.
It can bring benefits for both CIFS and NFS filesystems - so, at least
two ones.
>
> I'm not even sure adding these flags does make a lot of sense, but
> assuming we'd actually want this (and I'd like some more detailed
> explanation) I think we'd at least need to make sure that:
>
> a) opening files with the new modes gives a proper error message if
> not
> supported
It makes us add such checks for all other filesystems, if I understand
right, - not a problem, I think.
> b) there needs to be local support for them as well
> c) we need to think really hard when they should be supported, and
> need
> a good rational for it. I can't see how we could do it
> unconditionally for all users as that would introduce easy denial
> of services attacks the way I understand the semantics (correct
> me
> if I'm wrong). So a mount option like you currently do probably
> is
> the least bad even if don't fell overly happy about that version.
>
> What is the reason your special wine use case can't simply use a
> userspace cifs client? Given that wine uses windows filesystem
> semantics and cifs does as well tunnelling it through a Posix-like
> API
> inbetween is never going to be perfect.
Ideally we should not make any difference between underlying
filesystems in Wine: an application requests an open of the file and we
issue this open with flags it passed. Since Wineserver can process share
flags locally itself (for one linux user), we only need to add this
support for CIFS (that is actively used by Wine applications because of
it's Windows nature). Bringing these flags for local filesystems can
benefit Wine too: it will help in cases when Wine applications of
different users on the same machine use the same file and can make all
those things easier, of course.
The problem is the possibility of denial-of-service attacks here. We
can try to prevent them by:
1) specifying an extra security bit on the file that indicates that
share flags are accepted (like we have for mandatory locks now) and
setting it for neccessary files only, or
2) adding a special mount option (but it it probably makes sense if we
decided to add this support for CIFS and NFS only).
--
Best regards,
Pavel Shilovsky.
On Fri, Dec 07, 2012 at 01:08:46PM +0400, Pavel Shilovsky wrote:
> 2012/12/6 Pavel Shilovsky <[email protected]>:
> > Network filesystems CIFS, SMB2.0, SMB3.0 and NFSv4 have such flags - this change can benefit cifs and nfs modules. While this change is ok for network filesystems, itsn't not targeted for local filesystems due security problems (e.g. when a user process can deny root to delete a file).
> >
> > Share flags are used by Windows applications and WINE have to deal with them too. While WINE can process open share flags itself on local filesystems, it can't do it if a file stored on a network share and is used by several clients. This patchset makes it possible for CIFS/SMB2.0/SMB3.0.
> >
> > Pavel Shilovsky (3):
> > fcntl: Introduce new O_DENY* open flags for network filesystems
> > CIFS: Add O_DENY* open flags support
> > CIFS: Use NT_CREATE_ANDX command for forcemand mounts
> >
> > fs/cifs/cifsacl.c | 10 ++++----
> > fs/cifs/cifsglob.h | 11 ++++++++-
> > fs/cifs/cifsproto.h | 9 ++++----
> > fs/cifs/cifssmb.c | 47 ++++++++++++++++++++------------------
> > fs/cifs/dir.c | 14 ++++++++----
> > fs/cifs/file.c | 18 ++++++++++-----
> > fs/cifs/inode.c | 11 +++++----
> > fs/cifs/link.c | 10 ++++----
> > fs/cifs/readdir.c | 2 +-
> > fs/cifs/smb1ops.c | 15 ++++++------
> > fs/cifs/smb2file.c | 10 ++++----
> > fs/cifs/smb2inode.c | 4 ++--
> > fs/cifs/smb2ops.c | 10 ++++----
> > fs/cifs/smb2pdu.c | 6 ++---
> > fs/cifs/smb2proto.h | 14 +++++++-----
> > fs/fcntl.c | 5 ++--
> > include/uapi/asm-generic/fcntl.h | 11 +++++++++
> > 17 files changed, 125 insertions(+), 82 deletions(-)
> >
> > --
> > 1.7.10.4
> >
>
> First of all, sorry for being unclear at this proposal.
>
>
> I am not from Wine team but I am working on things related to
> Wine+CIFS-client+Samba.
>
> We (at Etersoft) need to organize the work of Wine applications
> through cifs-client share mounted to Samba (or Windows server). They
> are related to accounting (mostly Russian ones - e.g.
> http://www.1c.ru/eng/title.htm). So, the problem is that such
> applications use share flags to control the parallel access to the
> same files of several clients on a remote share. Also, there can be a
> situation where Windows (native) clients and Wine clients are working
> together in the same time.
>
> That's why we need such flags in the kernel (patch #1). With these
> flags Wine can pass them to every open and they will be used by CIFS
> (and probably NFS) file systems to pass to the Samba server. In the
> same time if the file is on local filesystem - these flags will be
> simply ignored (not implemented).
NFS supports the deny-read and deny-write bits but not deny-delete.
If we could do such opens in-kernel on local and clustered filesystems,
that could also be useful for multi-protocol (Samba and NFS) and
clustered exports.
Currently knfsd tries to enforce deny bits in the nfsd code, which is a
bit ugly.
And knfsd currently requires write permissions for deny-read. My
understanding is that Windows considers that wrong, but I'd be curious
to know whether that breaks Windows applications in practice.
--b.
> The problem is the possibility of denial-of-service attacks here. We
> can try to prevent them by:
> 1) specifying an extra security bit on the file that indicates that
> share flags are accepted (like we have for mandatory locks now) and
> setting it for neccessary files only, or
> 2) adding a special mount option (but it it probably makes sense if we
> decided to add this support for CIFS and NFS only).
3) making it a property that the process opts into - the same fs can have
Linux and wine users at once.
But for such a big set of changes and with the kind of potential fallout
you need to demonstrate a good practical use case IMHO, not just a "neat
idea I had".
Alan
On Thu, Dec 06, 2012 at 11:57:52AM -0800, Jeremy Allison wrote:
>
> And this is where things get really ugly of course :-).
>
> For the CIFSFS client they're expecting to be able to
> just ship them to a Windows server, where they'll
> get the (insane) Windows semantics. These semantics
> are not what would be wanted on a local filesystem.
I'm confused; why would a userspace application need to be able to
request this behavior? I can understand why an SMB client might
depend on this so it can use Windows' insane cache coherency scheme.
Are you trying to let Samba act as a middle man, where a remote file
system is mounted on Linux, and then Samba will try to act as a SMB
server, so you want to be able to pass through these semantics, i.e.:
Windows SMB Server <---> Linux cifs remote file system <--->
Linux Samba server <---> Windows SMB client
Is this somewhat contrivuewd example the intended use case? Or
something else?
- Ted
2012/12/12 David Laight <[email protected]>:
> On Sat, Dec 08, 2012 at 12:43:14AM +0400, Pavel Shilovsky wrote:
>>
>> The problem is the possibility of denial-of-service attacks here. We
>> can try to prevent them by:
>
> FWIW I already see a DoS 'attack'.
> I have some filestore shared using NFS (to Linux and Solaris) and
> using samba (to Windows).
>
> I use it for release builds of a product to ensure the versions
> built for the different operating systems match, and because some
> files have to be built on an 'alien' system (eg gcc targetted at
> embedded card).
>
> I can't run the windows build at the same time as the others
> because the windows C compiler manages to obtain exclusive access
> to the source files - stopping the other systems from reading them.
We can make this feature (passing O_DENY* flags received from clients
to filesystem) can be turned on/off on Samba/NFS server to let this
particular use case work. In general, I think we really need to be
sure that nobody has a read access for files that a Windows process
opened with O_DENYREAD (because there can be important reasons for the
Windows process to do so).
--
Best regards,
Pavel Shilovsky.
On Mon, 10 Dec 2012 11:41:16 -0500
"J. Bruce Fields" <[email protected]> wrote:
> On Sat, Dec 08, 2012 at 12:43:14AM +0400, Pavel Shilovsky wrote:
> > The problem is the possibility of denial-of-service attacks here. We
> > can try to prevent them by:
> > 1) specifying an extra security bit on the file that indicates that
> > share flags are accepted (like we have for mandatory locks now) and
> > setting it for neccessary files only, or
> > 2) adding a special mount option (but it it probably makes sense if
> > we decided to add this support for CIFS and NFS only).
>
> In the case of knfsd and samba exporting a common filesystem, you'd also
> want to be able to enforce it on the exported filesystem.
>
Sorry for chiming in late on this, but I've been looking at this
problem from the other end as well, from the POV of a fileserver. For
there, you absolutely do want to have some mechanism for enforcing this
on local filesystems.
Currently, file servers generally enforce share reservations
internally. The upshot is that they're not aware when other tasks
outside the server modify a file. This is also problematic too in many
common fileserving situations -- when exporting files via both NFS and
SMB, for instance.
One thing that's important to note is that there is already some
support for this in the kernel. The LOCK_MAND flag and its associates
are intended for just this purpose. Samba even already calls into the
kernel to set LOCK_MAND locks, but the kernel just passes them through
to the fs. Rumor has it that GPFS does something with these flags, but
I can't confirm that.
Of course, LOCK_MAND is racy since you can't set it on open(), but it
might be nice to use that as a starting point for trying to add this
support.
At the very least, if we're going to do this, we need to consider what
to do with the LOCK_MAND interfaces. As a starting point for
discussion, here's a patch that I was playing with a few months ago. I
haven't had much time to really spend on this project, but it may be
worthwhile to consider. It works, but I'm not sure about the
semantics...
-----------------------------[snip]--------------------------------
locks: add enforcement of LOCK_MAND locks
The LOCK_MAND lock mechanism is currently a no-op for any in-tree
filesystem. The flags are passed to f_ops->flock, but the standard
flock routines basically ignore them.
Change this by adding enforcement against other LOCK_MAND locks. Also,
assume that LOCK_MAND also implies LOCK_NB.
Signed-off-by: Jeff Layton <[email protected]>
---
fs/locks.c | 45 ++++++++++++++++++++++++++++++++++++++++++---
1 file changed, 42 insertions(+), 3 deletions(-)
diff --git a/fs/locks.c b/fs/locks.c
index 814c51d..736e38b 100644
--- a/fs/locks.c
+++ b/fs/locks.c
@@ -625,6 +625,43 @@ static int posix_locks_conflict(struct file_lock *caller_fl, struct file_lock *s
return (locks_conflict(caller_fl, sys_fl));
}
+/*
+ * locks_mand_conflict - Determine if there's a share reservation conflict
+ * @caller_fl: lock we're attempting to acquire
+ * @sys_fl: lock already present on system that we're checking against
+ *
+ * Check to see if there's a share_reservation conflict. LOCK_READ/LOCK_WRITE
+ * tell us whether the reservation allows other readers and writers.
+ *
+ * We only check against other LOCK_MAND locks, so applications that want to
+ * use share mode locking will only conflict against one another. "normal"
+ * applications that open files won't be affected by and won't themselves
+ * affect the share reservations.
+ */
+static int locks_mand_conflict(struct file_lock *caller_fl,
+ struct file_lock *sys_fl)
+{
+ unsigned char caller_type = caller_fl->fl_type;
+ unsigned char sys_type = sys_fl->fl_type;
+ fmode_t caller_fmode = caller_fl->fl_file->f_mode;
+ fmode_t sys_fmode = sys_fl->fl_file->f_mode;
+
+ /* they can only conflict if they're both LOCK_MAND */
+ if (!(caller_type & LOCK_MAND) || !(sys_type & LOCK_MAND))
+ return 0;
+
+ if (!(caller_type & LOCK_READ) && (sys_fmode & FMODE_READ))
+ return 1;
+ if (!(caller_type & LOCK_WRITE) && (sys_fmode & FMODE_WRITE))
+ return 1;
+ if (!(sys_type & LOCK_READ) && (caller_fmode & FMODE_READ))
+ return 1;
+ if (!(sys_type & LOCK_WRITE) && (caller_fmode & FMODE_WRITE))
+ return 1;
+
+ return 0;
+}
+
/* Determine if lock sys_fl blocks lock caller_fl. FLOCK specific
* checking before calling the locks_conflict().
*/
@@ -633,9 +670,11 @@ static int flock_locks_conflict(struct file_lock *caller_fl, struct file_lock *s
/* FLOCK locks referring to the same filp do not conflict with
* each other.
*/
- if (!IS_FLOCK(sys_fl) || (caller_fl->fl_file == sys_fl->fl_file))
- return (0);
+ if (!IS_FLOCK(sys_fl))
+ return 0;
if ((caller_fl->fl_type & LOCK_MAND) || (sys_fl->fl_type & LOCK_MAND))
+ return locks_mand_conflict(caller_fl, sys_fl);
+ if (caller_fl->fl_file == sys_fl->fl_file)
return 0;
return (locks_conflict(caller_fl, sys_fl));
@@ -1646,7 +1685,7 @@ SYSCALL_DEFINE2(flock, unsigned int, fd, unsigned int, cmd)
if (!filp)
goto out;
- can_sleep = !(cmd & LOCK_NB);
+ can_sleep = !(cmd & (LOCK_NB|LOCK_MAND));
cmd &= ~LOCK_NB;
unlock = (cmd == LOCK_UN);
--
1.7.11.7
On Thu, 6 Dec 2012 22:26:28 +0400
Pavel Shilovsky <[email protected]> wrote:
> Network filesystems CIFS, SMB2.0, SMB3.0 and NFSv4 have such flags - this change can benefit cifs and nfs modules. While this change is ok for network filesystems, itsn't not targeted for local filesystems due security problems (e.g. when a user process can deny root to delete a file).
If I have my root fs on NFS then the same applies does it not.
Your patches fail to describe the security semantics and what file rights
I must have to apply each option. How do I track down a lock user, what
tools are provided ? How do the new options interact with the security
layer?
I don't have a problem with the idea, but it needs a lot more clear
description of how it works so the model can be checked and if need be
things tweaked (eg needing write to denywrite etc)
Alan
On Thu, Dec 06, 2012 at 04:31:33PM -0500, Theodore Ts'o wrote:
> On Thu, Dec 06, 2012 at 11:57:52AM -0800, Jeremy Allison wrote:
> >
> > And this is where things get really ugly of course :-).
> >
> > For the CIFSFS client they're expecting to be able to
> > just ship them to a Windows server, where they'll
> > get the (insane) Windows semantics. These semantics
> > are not what would be wanted on a local filesystem.
>
> I'm confused; why would a userspace application need to be able to
> request this behavior?
This isn't my proposal Ted, I'm just commenting on it :-).
Jeremy.
PiAtLS0tLU9yaWdpbmFsIE1lc3NhZ2UtLS0tLQ0KPiBGcm9tOiBsaW51eC1uZnMtb3duZXJAdmdl
ci5rZXJuZWwub3JnIFttYWlsdG86bGludXgtbmZzLQ0KPiBvd25lckB2Z2VyLmtlcm5lbC5vcmdd
IE9uIEJlaGFsZiBPZiBQYXZlbCBTaGlsb3Zza3kNCj4gU2VudDogRnJpZGF5LCBEZWNlbWJlciAw
NywgMjAxMiA5OjQzIFBNDQo+IFRvOiBDaHJpc3RvcGggSGVsbHdpZw0KPiBDYzogbGludXgtY2lm
c0B2Z2VyLmtlcm5lbC5vcmc7IGxpbnV4LWtlcm5lbEB2Z2VyLmtlcm5lbC5vcmc7IGxpbnV4LQ0K
PiBmc2RldmVsQHZnZXIua2VybmVsLm9yZzsgd2luZS1kZXZlbEB3aW5laHEub3JnOyBsaW51eC0N
Cj4gbmZzQHZnZXIua2VybmVsLm9yZw0KPiBTdWJqZWN0OiBSZTogW1BBVENIIDAvM10gQWRkIE9f
REVOWSogZmxhZ3MgdG8gZmNudGwgYW5kIGNpZnMNCj4gDQo+IENocmlzdG9waCBIZWxsd2lnINC/
0LjRgdCw0LsgMDcuMTIuMjAxMiAyMDoxNjoNCj4gPiBPbiBUaHUsIERlYyAwNiwgMjAxMiBhdCAx
MDoyNjoyOFBNICswNDAwLCBQYXZlbCBTaGlsb3Zza3kgd3JvdGU6DQo+ID4+IE5ldHdvcmsgZmls
ZXN5c3RlbXMgQ0lGUywgU01CMi4wLCBTTUIzLjAgYW5kIE5GU3Y0IGhhdmUgc3VjaCBmbGFncyAt
DQo+ID4+IHRoaXMgY2hhbmdlIGNhbiBiZW5lZml0IGNpZnMgYW5kIG5mcyBtb2R1bGVzLiBXaGls
ZSB0aGlzIGNoYW5nZSBpcyBvaw0KPiA+PiBmb3IgbmV0d29yayBmaWxlc3lzdGVtcywgaXRzbid0
IG5vdCB0YXJnZXRlZCBmb3IgbG9jYWwgZmlsZXN5c3RlbXMNCj4gPj4gZHVlIHNlY3VyaXR5IHBy
b2JsZW1zIChlLmcuIHdoZW4gYSB1c2VyIHByb2Nlc3MgY2FuIGRlbnkgcm9vdCB0bw0KPiA+PiBk
ZWxldGUgYSBmaWxlKS4NCj4gPj4NCj4gPj4gU2hhcmUgZmxhZ3MgYXJlIHVzZWQgYnkgV2luZG93
cyBhcHBsaWNhdGlvbnMgYW5kIFdJTkUgaGF2ZSB0byBkZWFsDQo+ID4+IHdpdGggdGhlbSB0b28u
IFdoaWxlIFdJTkUgY2FuIHByb2Nlc3Mgb3BlbiBzaGFyZSBmbGFncyBpdHNlbGYgb24NCj4gPj4g
bG9jYWwgZmlsZXN5c3RlbXMsIGl0IGNhbid0IGRvIGl0IGlmIGEgZmlsZSBzdG9yZWQgb24gYSBu
ZXR3b3JrIHNoYXJlDQo+ID4+IGFuZCBpcyB1c2VkIGJ5IHNldmVyYWwgY2xpZW50cy4gVGhpcyBw
YXRjaHNldCBtYWtlcyBpdCBwb3NzaWJsZSBmb3INCj4gPj4gQ0lGUy9TTUIyLjAvU01CMy4wLg0K
PiA+DQo+ID4gSSBkb24ndCB0aGluayBpbnRyb2R1Y2luZyB1c2VyIHZpc2libGUgZmxhZ3MgdGhh
dCBhcmUgb25seSBzdXBwb3J0ZWQNCj4gPiBvbiBhIHNpbmdsZSBuZXR3b3JrIGZpbGVzeXN0ZW0g
aXMgYSBnb29kIGlkZWEuDQo+IA0KPiBJdCBjYW4gYnJpbmcgYmVuZWZpdHMgZm9yIGJvdGggQ0lG
UyBhbmQgTkZTIGZpbGVzeXN0ZW1zIC0gc28sIGF0IGxlYXN0IHR3byBvbmVzLg0KPiANCj4gPg0K
PiA+IEknbSBub3QgZXZlbiBzdXJlIGFkZGluZyB0aGVzZSBmbGFncyBkb2VzIG1ha2UgYSBsb3Qg
b2Ygc2Vuc2UsIGJ1dA0KPiA+IGFzc3VtaW5nIHdlJ2QgYWN0dWFsbHkgd2FudCB0aGlzIChhbmQg
SSdkIGxpa2Ugc29tZSBtb3JlIGRldGFpbGVkDQo+ID4gZXhwbGFuYXRpb24pIEkgdGhpbmsgd2Un
ZCBhdCBsZWFzdCBuZWVkIHRvIG1ha2Ugc3VyZSB0aGF0Og0KPiA+DQo+ID4gIGEpIG9wZW5pbmcg
ZmlsZXMgd2l0aCB0aGUgbmV3IG1vZGVzIGdpdmVzIGEgcHJvcGVyIGVycm9yIG1lc3NhZ2UgaWYN
Cj4gPiBub3QNCj4gPiAgICAgc3VwcG9ydGVkDQo+IA0KPiBJdCBtYWtlcyB1cyBhZGQgc3VjaCBj
aGVja3MgZm9yIGFsbCBvdGhlciBmaWxlc3lzdGVtcywgaWYgSSB1bmRlcnN0YW5kIHJpZ2h0LCAt
DQo+IG5vdCBhIHByb2JsZW0sIEkgdGhpbmsuDQo+IA0KPiA+ICBiKSB0aGVyZSBuZWVkcyB0byBi
ZSBsb2NhbCBzdXBwb3J0IGZvciB0aGVtIGFzIHdlbGwNCj4gPiAgYykgd2UgbmVlZCB0byB0aGlu
ayByZWFsbHkgaGFyZCB3aGVuIHRoZXkgc2hvdWxkIGJlIHN1cHBvcnRlZCwgYW5kDQo+ID4gbmVl
ZA0KPiA+ICAgICBhIGdvb2QgcmF0aW9uYWwgZm9yIGl0LiAgSSBjYW4ndCBzZWUgaG93IHdlIGNv
dWxkIGRvIGl0DQo+ID4gICAgIHVuY29uZGl0aW9uYWxseSBmb3IgYWxsIHVzZXJzIGFzIHRoYXQg
d291bGQgaW50cm9kdWNlIGVhc3kgZGVuaWFsDQo+ID4gICAgIG9mIHNlcnZpY2VzIGF0dGFja3Mg
dGhlIHdheSBJIHVuZGVyc3RhbmQgdGhlIHNlbWFudGljcyAoY29ycmVjdCBtZQ0KPiA+ICAgICBp
ZiBJJ20gd3JvbmcpLiAgU28gYSBtb3VudCBvcHRpb24gbGlrZSB5b3UgY3VycmVudGx5IGRvIHBy
b2JhYmx5DQo+ID4gaXMNCj4gPiAgICAgdGhlIGxlYXN0IGJhZCBldmVuIGlmIGRvbid0IGZlbGwg
b3Zlcmx5IGhhcHB5IGFib3V0IHRoYXQgdmVyc2lvbi4NCj4gPg0KPiA+IFdoYXQgaXMgdGhlIHJl
YXNvbiB5b3VyIHNwZWNpYWwgd2luZSB1c2UgY2FzZSBjYW4ndCBzaW1wbHkgdXNlIGENCj4gPiB1
c2Vyc3BhY2UgY2lmcyBjbGllbnQ/ICBHaXZlbiB0aGF0IHdpbmUgdXNlcyB3aW5kb3dzIGZpbGVz
eXN0ZW0NCj4gPiBzZW1hbnRpY3MgYW5kIGNpZnMgZG9lcyBhcyB3ZWxsIHR1bm5lbGxpbmcgaXQg
dGhyb3VnaCBhIFBvc2l4LWxpa2UgQVBJDQo+ID4gaW5iZXR3ZWVuIGlzIG5ldmVyIGdvaW5nIHRv
IGJlIHBlcmZlY3QuDQo+IA0KPiBJZGVhbGx5IHdlIHNob3VsZCBub3QgbWFrZSBhbnkgZGlmZmVy
ZW5jZSBiZXR3ZWVuIHVuZGVybHlpbmcgZmlsZXN5c3RlbXMNCj4gaW4gV2luZTogYW4gYXBwbGlj
YXRpb24gcmVxdWVzdHMgYW4gb3BlbiBvZiB0aGUgZmlsZSBhbmQgd2UgaXNzdWUgdGhpcyBvcGVu
DQo+IHdpdGggZmxhZ3MgaXQgcGFzc2VkLiBTaW5jZSBXaW5lc2VydmVyIGNhbiBwcm9jZXNzIHNo
YXJlIGZsYWdzIGxvY2FsbHkgaXRzZWxmIChmb3INCj4gb25lIGxpbnV4IHVzZXIpLCB3ZSBvbmx5
IG5lZWQgdG8gYWRkIHRoaXMgc3VwcG9ydCBmb3IgQ0lGUyAodGhhdCBpcyBhY3RpdmVseQ0KPiB1
c2VkIGJ5IFdpbmUgYXBwbGljYXRpb25zIGJlY2F1c2Ugb2YgaXQncyBXaW5kb3dzIG5hdHVyZSku
IEJyaW5naW5nIHRoZXNlDQo+IGZsYWdzIGZvciBsb2NhbCBmaWxlc3lzdGVtcyBjYW4gYmVuZWZp
dCBXaW5lIHRvbzogaXQgd2lsbCBoZWxwIGluIGNhc2VzIHdoZW4NCj4gV2luZSBhcHBsaWNhdGlv
bnMgb2YgZGlmZmVyZW50IHVzZXJzIG9uIHRoZSBzYW1lIG1hY2hpbmUgdXNlIHRoZSBzYW1lIGZp
bGUNCj4gYW5kIGNhbiBtYWtlIGFsbCB0aG9zZSB0aGluZ3MgZWFzaWVyLCBvZiBjb3Vyc2UuDQo+
IA0KPiBUaGUgcHJvYmxlbSBpcyB0aGUgcG9zc2liaWxpdHkgb2YgZGVuaWFsLW9mLXNlcnZpY2Ug
YXR0YWNrcyBoZXJlLiBXZSBjYW4gdHJ5IHRvDQo+IHByZXZlbnQgdGhlbSBieToNCj4gMSkgc3Bl
Y2lmeWluZyBhbiBleHRyYSBzZWN1cml0eSBiaXQgb24gdGhlIGZpbGUgdGhhdCBpbmRpY2F0ZXMg
dGhhdCBzaGFyZSBmbGFncyBhcmUNCj4gYWNjZXB0ZWQgKGxpa2Ugd2UgaGF2ZSBmb3IgbWFuZGF0
b3J5IGxvY2tzIG5vdykgYW5kIHNldHRpbmcgaXQgZm9yDQo+IG5lY2Nlc3NhcnkgZmlsZXMgb25s
eSwgb3INCj4gMikgYWRkaW5nIGEgc3BlY2lhbCBtb3VudCBvcHRpb24gKGJ1dCBpdCBpdCBwcm9i
YWJseSBtYWtlcyBzZW5zZSBpZiB3ZQ0KPiBkZWNpZGVkIHRvIGFkZCB0aGlzIHN1cHBvcnQgZm9y
IENJRlMgYW5kIE5GUyBvbmx5KS4NCg0KV2h5IG5vdCBqdXN0IHB1dCBpdCB1bmRlciB0aGUgY29u
dHJvbCBvZiBMU00/IEl0IHNlZW1zIHRvIG1lIHRoYXQgdGhpcyBkb2Vzbid0IHNvIG11Y2ggd2Fu
dCB0byBiZSBhIHBlci1tb3VudCBzd2l0Y2ggYnV0IHJhdGhlciBkZXNlcnZlcyB0byBiZSBhIHBl
ci1wcm9jZXNzIE1BQyAoaS5lLiBpcyB0aGlzIHJ1bm5pbmcgaW4gYSBXaW5lIHNhbmRib3ggb3Ig
bm90KS4uLg0KDQpDaGVlcnMNCiAgIFRyb25kDQo=
On Sat, Dec 08, 2012 at 12:43:14AM +0400, Pavel Shilovsky wrote:
>
> The problem is the possibility of denial-of-service attacks here. We
> can try to prevent them by:
FWIW I already see a DoS 'attack'.
I have some filestore shared using NFS (to Linux and Solaris) and
using samba (to Windows).
I use it for release builds of a product to ensure the versions
built for the different operating systems match, and because some
files have to be built on an 'alien' system (eg gcc targetted at
embedded card).
I can't run the windows build at the same time as the others
because the windows C compiler manages to obtain exclusive access
to the source files - stopping the other systems from reading them.
David
--
David Laight: [email protected]
On Thu, Dec 06, 2012 at 07:49:49PM +0000, Alan Cox wrote:
> On Thu, 6 Dec 2012 22:26:28 +0400
> Pavel Shilovsky <[email protected]> wrote:
>
> > Network filesystems CIFS, SMB2.0, SMB3.0 and NFSv4 have such flags - this change can benefit cifs and nfs modules. While this change is ok for network filesystems, itsn't not targeted for local filesystems due security problems (e.g. when a user process can deny root to delete a file).
>
> If I have my root fs on NFS then the same applies does it not.
>
> Your patches fail to describe the security semantics and what file rights
> I must have to apply each option. How do I track down a lock user, what
> tools are provided ? How do the new options interact with the security
> layer?
>
> I don't have a problem with the idea, but it needs a lot more clear
> description of how it works so the model can be checked and if need be
> things tweaked (eg needing write to denywrite etc)
And this is where things get really ugly of course :-).
For the CIFSFS client they're expecting to be able to
just ship them to a Windows server, where they'll
get the (insane) Windows semantics. These semantics
are not what would be wanted on a local filesystem.
So unless we just say "these things have Windows
semantics" (where openers of files can lock out others
under dubious circumstances) there'll be this horrible
difference between (I'm assuming) the sane semantics that
are defined for local filesystems and the insane ones
that you get when you're connecting remotely.
I don't know a good way to fix that, but I'm pretty
sure you don't want the Windows semantics defined
locally :-).
Jeremy.
On Thu, Dec 06, 2012 at 04:37:27PM -0500, Theodore Ts'o wrote:
> On Thu, Dec 06, 2012 at 01:33:29PM -0800, Jeremy Allison wrote:
> > > I'm confused; why would a userspace application need to be able to
> > > request this behavior?
> >
> > This isn't my proposal Ted, I'm just commenting on it :-).
>
> Ah, sorry, I thought was coming from the Samba team. :-)
Well it sort of is, as the people working on cifsfs are also
Samba Team members, but this isn't an official "Samba" thing,
as I can't see exactly what apps would want this either (other
than Samba smbd running on top of smbd, re-exporting a cifsfs
share, pointing to a Samba server, running on a... ERROR STACK
OVERFLOW :-).
> Hmm... I see wine-devel is cc'ed; is this coming from the Wine team,
> wanting to do SMB paravirtualization? It would be useful if the
> commit description described how these flags are intended to be used....
Indeed :-).
Jeremy.
On Sat, Dec 08, 2012 at 12:43:14AM +0400, Pavel Shilovsky wrote:
> The problem is the possibility of denial-of-service attacks here. We
> can try to prevent them by:
> 1) specifying an extra security bit on the file that indicates that
> share flags are accepted (like we have for mandatory locks now) and
> setting it for neccessary files only, or
> 2) adding a special mount option (but it it probably makes sense if
> we decided to add this support for CIFS and NFS only).
In the case of knfsd and samba exporting a common filesystem, you'd also
want to be able to enforce it on the exported filesystem.
--b.
On Thu, Dec 06, 2012 at 11:57:52AM -0800, Jeremy Allison wrote:
> On Thu, Dec 06, 2012 at 07:49:49PM +0000, Alan Cox wrote:
> > On Thu, 6 Dec 2012 22:26:28 +0400
> > Pavel Shilovsky <[email protected]> wrote:
> >
> > > Network filesystems CIFS, SMB2.0, SMB3.0 and NFSv4 have such flags - this change can benefit cifs and nfs modules. While this change is ok for network filesystems, itsn't not targeted for local filesystems due security problems (e.g. when a user process can deny root to delete a file).
> >
> > If I have my root fs on NFS then the same applies does it not.
> >
> > Your patches fail to describe the security semantics and what file rights
> > I must have to apply each option. How do I track down a lock user, what
> > tools are provided ? How do the new options interact with the security
> > layer?
> >
> > I don't have a problem with the idea, but it needs a lot more clear
> > description of how it works so the model can be checked and if need be
> > things tweaked (eg needing write to denywrite etc)
>
> And this is where things get really ugly of course :-).
>
> For the CIFSFS client they're expecting to be able to
> just ship them to a Windows server, where they'll
> get the (insane) Windows semantics. These semantics
> are not what would be wanted on a local filesystem.
>
> So unless we just say "these things have Windows
> semantics" (where openers of files can lock out others
> under dubious circumstances) there'll be this horrible
> difference between (I'm assuming) the sane semantics that
> are defined for local filesystems and the insane ones
> that you get when you're connecting remotely.
>
> I don't know a good way to fix that, but I'm pretty
> sure you don't want the Windows semantics defined
> locally :-).
You could just flags these as "ignored on local filesystems"
of course, exact semantics defined by the remote filesystem.
That's really what applications will get anyway. But it's not
condusive to writing documentation on what these things do :-).
Jeremy.
On Fri, 2012-12-07 at 09:52 -0500, J. Bruce Fields wrote:
> On Fri, Dec 07, 2012 at 01:08:46PM +0400, Pavel Shilovsky wrote:
> > 2012/12/6 Pavel Shilovsky <[email protected]>:
> > > Network filesystems CIFS, SMB2.0, SMB3.0 and NFSv4 have such flags - this change can benefit cifs and nfs modules. While this change is ok for network filesystems, itsn't not targeted for local filesystems due security problems (e.g. when a user process can deny root to delete a file).
> > >
> > > Share flags are used by Windows applications and WINE have to deal with them too. While WINE can process open share flags itself on local filesystems, it can't do it if a file stored on a network share and is used by several clients. This patchset makes it possible for CIFS/SMB2.0/SMB3.0.
> > >
> > > Pavel Shilovsky (3):
> > > fcntl: Introduce new O_DENY* open flags for network filesystems
> > > CIFS: Add O_DENY* open flags support
> > > CIFS: Use NT_CREATE_ANDX command for forcemand mounts
> > >
> > > fs/cifs/cifsacl.c | 10 ++++----
> > > fs/cifs/cifsglob.h | 11 ++++++++-
> > > fs/cifs/cifsproto.h | 9 ++++----
> > > fs/cifs/cifssmb.c | 47 ++++++++++++++++++++------------------
> > > fs/cifs/dir.c | 14 ++++++++----
> > > fs/cifs/file.c | 18 ++++++++++-----
> > > fs/cifs/inode.c | 11 +++++----
> > > fs/cifs/link.c | 10 ++++----
> > > fs/cifs/readdir.c | 2 +-
> > > fs/cifs/smb1ops.c | 15 ++++++------
> > > fs/cifs/smb2file.c | 10 ++++----
> > > fs/cifs/smb2inode.c | 4 ++--
> > > fs/cifs/smb2ops.c | 10 ++++----
> > > fs/cifs/smb2pdu.c | 6 ++---
> > > fs/cifs/smb2proto.h | 14 +++++++-----
> > > fs/fcntl.c | 5 ++--
> > > include/uapi/asm-generic/fcntl.h | 11 +++++++++
> > > 17 files changed, 125 insertions(+), 82 deletions(-)
> > >
> > > --
> > > 1.7.10.4
> > >
> >
> > First of all, sorry for being unclear at this proposal.
> >
> >
> > I am not from Wine team but I am working on things related to
> > Wine+CIFS-client+Samba.
> >
> > We (at Etersoft) need to organize the work of Wine applications
> > through cifs-client share mounted to Samba (or Windows server). They
> > are related to accounting (mostly Russian ones - e.g.
> > http://www.1c.ru/eng/title.htm). So, the problem is that such
> > applications use share flags to control the parallel access to the
> > same files of several clients on a remote share. Also, there can be a
> > situation where Windows (native) clients and Wine clients are working
> > together in the same time.
> >
> > That's why we need such flags in the kernel (patch #1). With these
> > flags Wine can pass them to every open and they will be used by CIFS
> > (and probably NFS) file systems to pass to the Samba server. In the
> > same time if the file is on local filesystem - these flags will be
> > simply ignored (not implemented).
>
> NFS supports the deny-read and deny-write bits but not deny-delete.
>
> If we could do such opens in-kernel on local and clustered filesystems,
> that could also be useful for multi-protocol (Samba and NFS) and
> clustered exports.
>
> Currently knfsd tries to enforce deny bits in the nfsd code, which is a
> bit ugly.
>
> And knfsd currently requires write permissions for deny-read. My
> understanding is that Windows considers that wrong, but I'd be curious
> to know whether that breaks Windows applications in practice.
It probably does if you look hard enough.
IIRC Deny-reads are very loosely like read locks, and you can take read
locks if you have read-only access.
Why does knfsd restrict deny-read to read-write access ?
Simo.
--
Simo Sorce
Samba Team GPL Compliance Officer <[email protected]>
Principal Software Engineer at Red Hat, Inc. <[email protected]>
> We can make this feature (passing O_DENY* flags received from clients
> to filesystem) can be turned on/off on Samba/NFS server to let this
> particular use case work. In general, I think we really need to be
> sure that nobody has a read access for files that a Windows process
> opened with O_DENYREAD (because there can be important reasons for the
> Windows process to do so).
It should only affect windows emulated tasks, nothing else. If you want
to do locking friendly behaviour you could also take lockf locks so that
Linux domain code that *wishes* to be nice in this area behaves the way
you want.
Without that you break so much stuff its a horrible idea (eg backups,
ubuntu one sync etc).
Alan
On Thu, Dec 06, 2012 at 01:33:29PM -0800, Jeremy Allison wrote:
> > I'm confused; why would a userspace application need to be able to
> > request this behavior?
>
> This isn't my proposal Ted, I'm just commenting on it :-).
Ah, sorry, I thought was coming from the Samba team. :-)
Hmm... I see wine-devel is cc'ed; is this coming from the Wine team,
wanting to do SMB paravirtualization? It would be useful if the
commit description described how these flags are intended to be used....
- Ted
> I suspect that WINE would have the same need
Tricky - Wine needs to enforce this behaviour solely between Wine and
the file server, Trying to muck up non emulated local behaviour would be
a bad mistake.
One way perhaps to look at this is you want some tasks to be able to *opt
in* to this behaviour.
First however I think we need a much better description of the setup that
uses them and why it matters.
On Fri, Dec 7, 2012 at 8:29 AM, Steve French <[email protected]> wrote:
> although I could not find the same level of detail that MS-FSA
> provides (e.g. see section 2.14.10 for the detailed
Typo It is section 2.1.4.10
--
Thanks,
Steve