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