From b5accbb0dfae36d8d36cd882096943c98d5ede15 Mon Sep 17 00:00:00 2001 From: Jan Kara Date: Thu, 22 Jun 2017 15:31:13 +0200 Subject: [PATCH 1/9] orangefs: Don't clear SGID when inheriting ACLs When new directory 'DIR1' is created in a directory 'DIR0' with SGID bit set, DIR1 is expected to have SGID bit set (and owning group equal to the owning group of 'DIR0'). However when 'DIR0' also has some default ACLs that 'DIR1' inherits, setting these ACLs will result in SGID bit on 'DIR1' to get cleared if user is not member of the owning group. Fix the problem by creating __orangefs_set_acl() function that does not call posix_acl_update_mode() and use it when inheriting ACLs. That prevents SGID bit clearing and the mode has been properly set by posix_acl_create() anyway. Fixes: 073931017b49d9458aa351605b43a7e34598caef CC: stable@vger.kernel.org CC: Mike Marshall CC: pvfs2-developers@beowulf-underground.org Signed-off-by: Jan Kara Signed-off-by: Mike Marshall --- fs/orangefs/acl.c | 48 +++++++++++++++++++++++++++-------------------- 1 file changed, 28 insertions(+), 20 deletions(-) diff --git a/fs/orangefs/acl.c b/fs/orangefs/acl.c index 7a3754488312..9409aac232f7 100644 --- a/fs/orangefs/acl.c +++ b/fs/orangefs/acl.c @@ -61,9 +61,9 @@ struct posix_acl *orangefs_get_acl(struct inode *inode, int type) return acl; } -int orangefs_set_acl(struct inode *inode, struct posix_acl *acl, int type) +static int __orangefs_set_acl(struct inode *inode, struct posix_acl *acl, + int type) { - struct orangefs_inode_s *orangefs_inode = ORANGEFS_I(inode); int error = 0; void *value = NULL; size_t size = 0; @@ -72,22 +72,6 @@ int orangefs_set_acl(struct inode *inode, struct posix_acl *acl, int type) switch (type) { case ACL_TYPE_ACCESS: name = XATTR_NAME_POSIX_ACL_ACCESS; - if (acl) { - umode_t mode; - - error = posix_acl_update_mode(inode, &mode, &acl); - if (error) { - gossip_err("%s: posix_acl_update_mode err: %d\n", - __func__, - error); - return error; - } - - if (inode->i_mode != mode) - SetModeFlag(orangefs_inode); - inode->i_mode = mode; - mark_inode_dirty_sync(inode); - } break; case ACL_TYPE_DEFAULT: name = XATTR_NAME_POSIX_ACL_DEFAULT; @@ -132,6 +116,29 @@ int orangefs_set_acl(struct inode *inode, struct posix_acl *acl, int type) return error; } +int orangefs_set_acl(struct inode *inode, struct posix_acl *acl, int type) +{ + int error; + + if (type == ACL_TYPE_ACCESS && acl) { + umode_t mode; + + error = posix_acl_update_mode(inode, &mode, &acl); + if (error) { + gossip_err("%s: posix_acl_update_mode err: %d\n", + __func__, + error); + return error; + } + + if (inode->i_mode != mode) + SetModeFlag(ORANGEFS_I(inode)); + inode->i_mode = mode; + mark_inode_dirty_sync(inode); + } + return __orangefs_set_acl(inode, acl, type); +} + int orangefs_init_acl(struct inode *inode, struct inode *dir) { struct orangefs_inode_s *orangefs_inode = ORANGEFS_I(inode); @@ -146,13 +153,14 @@ int orangefs_init_acl(struct inode *inode, struct inode *dir) return error; if (default_acl) { - error = orangefs_set_acl(inode, default_acl, ACL_TYPE_DEFAULT); + error = __orangefs_set_acl(inode, default_acl, + ACL_TYPE_DEFAULT); posix_acl_release(default_acl); } if (acl) { if (!error) - error = orangefs_set_acl(inode, acl, ACL_TYPE_ACCESS); + error = __orangefs_set_acl(inode, acl, ACL_TYPE_ACCESS); posix_acl_release(acl); } From 4bef69000d93799b6b3983c45c81b3cb2cceaa99 Mon Sep 17 00:00:00 2001 From: Mike Marshall Date: Thu, 10 Aug 2017 13:50:50 -0400 Subject: [PATCH 2/9] orangefs: react properly to posix_acl_update_mode's aftermath. posix_acl_update_mode checks to see if the permissions described by the ACL can be encoded into the object's mode. If so, it sets "acl" to NULL and "mode" to the new desired value. Prior to this patch we failed to actually propagate the new mode back to the server. Signed-off-by: Mike Marshall --- fs/orangefs/acl.c | 29 +++++++++++++++++++++-------- 1 file changed, 21 insertions(+), 8 deletions(-) diff --git a/fs/orangefs/acl.c b/fs/orangefs/acl.c index 9409aac232f7..45f27cf51fd8 100644 --- a/fs/orangefs/acl.c +++ b/fs/orangefs/acl.c @@ -119,11 +119,18 @@ static int __orangefs_set_acl(struct inode *inode, struct posix_acl *acl, int orangefs_set_acl(struct inode *inode, struct posix_acl *acl, int type) { int error; + struct iattr iattr; + int rc; if (type == ACL_TYPE_ACCESS && acl) { - umode_t mode; - - error = posix_acl_update_mode(inode, &mode, &acl); + /* + * posix_acl_update_mode checks to see if the permissions + * described by the ACL can be encoded into the + * object's mode. If so, it sets "acl" to NULL + * and "mode" to the new desired value. It is up to + * us to propagate the new mode back to the server... + */ + error = posix_acl_update_mode(inode, &iattr.ia_mode, &acl); if (error) { gossip_err("%s: posix_acl_update_mode err: %d\n", __func__, @@ -131,12 +138,18 @@ int orangefs_set_acl(struct inode *inode, struct posix_acl *acl, int type) return error; } - if (inode->i_mode != mode) - SetModeFlag(ORANGEFS_I(inode)); - inode->i_mode = mode; - mark_inode_dirty_sync(inode); + if (acl) { + rc = __orangefs_set_acl(inode, acl, type); + } else { + iattr.ia_valid = ATTR_MODE; + rc = orangefs_inode_setattr(inode, &iattr); + } + + return rc; + + } else { + return -EINVAL; } - return __orangefs_set_acl(inode, acl, type); } int orangefs_init_acl(struct inode *inode, struct inode *dir) From ba5e79ea183a2b809a9bbd2f8d76dc69ca32a92b Mon Sep 17 00:00:00 2001 From: Mike Marshall Date: Thu, 10 Aug 2017 13:56:45 -0400 Subject: [PATCH 3/9] orangefs: documentation clean up Signed-off-by: Mike Marshall --- Documentation/filesystems/orangefs.txt | 14 ++++---------- 1 file changed, 4 insertions(+), 10 deletions(-) diff --git a/Documentation/filesystems/orangefs.txt b/Documentation/filesystems/orangefs.txt index 1dfdec790946..e2818b60a5c2 100644 --- a/Documentation/filesystems/orangefs.txt +++ b/Documentation/filesystems/orangefs.txt @@ -45,14 +45,11 @@ upstream version of the kernel client. BUILDING THE USERSPACE FILESYSTEM ON A SINGLE SERVER ==================================================== -When Orangefs is upstream, "--with-kernel" shouldn't be needed, but -until then the path to where the kernel with the Orangefs kernel client -patch was built is needed to ensure that pvfs2-client-core (the bridge -between kernel space and user space) will build properly. You can omit ---prefix if you don't care that things are sprinkled around in -/usr/local. +You can omit --prefix if you don't care that things are sprinkled around in +/usr/local. As of version 2.9.6, Orangefs uses Berkeley DB by default, we +will probably be changing the default to lmdb soon. -./configure --prefix=/opt/ofs --with-kernel=/path/to/orangefs/kernel +./configure --prefix=/opt/ofs --with-db-backend=lmdb make @@ -82,9 +79,6 @@ prove things are working with: /opt/osf/bin/pvfs2-ls /mymountpoint -You might not want to enforce selinux, it doesn't seem to matter by -linux 3.11... - If stuff seems to be working, turn on the client core: /opt/osf/sbin/pvfs2-client -p /opt/osf/sbin/pvfs2-client-core From 5f13e58767a53ebb54265e03c0c4a67650286263 Mon Sep 17 00:00:00 2001 From: Dan Carpenter Date: Mon, 22 May 2017 15:08:31 +0300 Subject: [PATCH 4/9] orangefs: off by ones in xattr size checks A previous patch which claimed to remove off by ones actually introduced them. strlen() returns the length of the string not including the NUL character. We are using strcpy() to copy "name" into a buffer which is ORANGEFS_MAX_XATTR_NAMELEN characters long. We should make sure to leave space for the NUL, otherwise we're writing one character beyond the end of the buffer. Fixes: e675c5ec51fe ("orangefs: clean up oversize xattr validation") Signed-off-by: Dan Carpenter Signed-off-by: Mike Marshall --- fs/orangefs/xattr.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/fs/orangefs/xattr.c b/fs/orangefs/xattr.c index 237c9c04dc3b..a34b25be39c5 100644 --- a/fs/orangefs/xattr.c +++ b/fs/orangefs/xattr.c @@ -76,7 +76,7 @@ ssize_t orangefs_inode_getxattr(struct inode *inode, const char *name, if (S_ISLNK(inode->i_mode)) return -EOPNOTSUPP; - if (strlen(name) > ORANGEFS_MAX_XATTR_NAMELEN) + if (strlen(name) >= ORANGEFS_MAX_XATTR_NAMELEN) return -EINVAL; fsuid = from_kuid(&init_user_ns, current_fsuid()); @@ -169,7 +169,7 @@ static int orangefs_inode_removexattr(struct inode *inode, const char *name, struct orangefs_kernel_op_s *new_op = NULL; int ret = -ENOMEM; - if (strlen(name) > ORANGEFS_MAX_XATTR_NAMELEN) + if (strlen(name) >= ORANGEFS_MAX_XATTR_NAMELEN) return -EINVAL; down_write(&orangefs_inode->xattr_sem); @@ -233,7 +233,7 @@ int orangefs_inode_setxattr(struct inode *inode, const char *name, if (size > ORANGEFS_MAX_XATTR_VALUELEN) return -EINVAL; - if (strlen(name) > ORANGEFS_MAX_XATTR_NAMELEN) + if (strlen(name) >= ORANGEFS_MAX_XATTR_NAMELEN) return -EINVAL; internal_flag = convert_to_internal_xattr_flags(flags); From 49e5571324a81ddbb0d06ff38f698534a2a47e33 Mon Sep 17 00:00:00 2001 From: Jeff Layton Date: Wed, 12 Apr 2017 08:06:02 -0400 Subject: [PATCH 5/9] orangefs: don't call filemap_write_and_wait from fsync Orangefs doesn't do buffered writes yet, so there's no point in initiating and waiting for writeback. Signed-off-by: Jeff Layton Signed-off-by: Mike Marshall --- fs/orangefs/file.c | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/fs/orangefs/file.c b/fs/orangefs/file.c index 28f38d813ad2..336ecbf8c268 100644 --- a/fs/orangefs/file.c +++ b/fs/orangefs/file.c @@ -646,14 +646,11 @@ static int orangefs_fsync(struct file *file, loff_t end, int datasync) { - int ret = -EINVAL; + int ret; struct orangefs_inode_s *orangefs_inode = ORANGEFS_I(file_inode(file)); struct orangefs_kernel_op_s *new_op = NULL; - /* required call */ - filemap_write_and_wait_range(file->f_mapping, start, end); - new_op = op_alloc(ORANGEFS_VFS_OP_FSYNC); if (!new_op) return -ENOMEM; From 121744440582004a1751c688a2622e5dfeaae09d Mon Sep 17 00:00:00 2001 From: Julia Lawall Date: Wed, 2 Aug 2017 10:35:14 +0200 Subject: [PATCH 6/9] orangefs: constify xattr_handler structure The xattr_handler structure is only stored in an array of const structures. Thus the xattr_handler structure itself can be const. Signed-off-by: Julia Lawall Signed-off-by: Mike Marshall --- fs/orangefs/xattr.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/fs/orangefs/xattr.c b/fs/orangefs/xattr.c index a34b25be39c5..fed0e9aae23b 100644 --- a/fs/orangefs/xattr.c +++ b/fs/orangefs/xattr.c @@ -442,7 +442,7 @@ static int orangefs_xattr_get_default(const struct xattr_handler *handler, } -static struct xattr_handler orangefs_xattr_default_handler = { +static const struct xattr_handler orangefs_xattr_default_handler = { .prefix = "", /* match any name => handlers called with full name */ .get = orangefs_xattr_get_default, .set = orangefs_xattr_set_default, From 07a258531c7550f8bb481dfe2ec12bb876224487 Mon Sep 17 00:00:00 2001 From: Markus Elfring Date: Thu, 17 Aug 2017 21:00:07 +0200 Subject: [PATCH 7/9] orangefs: Delete error messages for a failed memory allocation in five functions Omit an extra message for a memory allocation failure in these functions. This issue was detected by using the Coccinelle software. Signed-off-by: Markus Elfring Signed-off-by: Mike Marshall --- fs/orangefs/devorangefs-req.c | 9 +++------ fs/orangefs/orangefs-bufmap.c | 10 ++-------- fs/orangefs/orangefs-debugfs.c | 1 - fs/orangefs/orangefs-mod.c | 1 - fs/orangefs/super.c | 4 +--- 5 files changed, 6 insertions(+), 19 deletions(-) diff --git a/fs/orangefs/devorangefs-req.c b/fs/orangefs/devorangefs-req.c index c19f0787c9c6..2826859bdc2c 100644 --- a/fs/orangefs/devorangefs-req.c +++ b/fs/orangefs/devorangefs-req.c @@ -461,13 +461,10 @@ static ssize_t orangefs_devreq_write_iter(struct kiocb *iocb, if (op->downcall.type != ORANGEFS_VFS_OP_READDIR) goto wakeup; - op->downcall.trailer_buf = - vmalloc(op->downcall.trailer_size); - if (op->downcall.trailer_buf == NULL) { - gossip_err("%s: failed trailer vmalloc.\n", - __func__); + op->downcall.trailer_buf = vmalloc(op->downcall.trailer_size); + if (!op->downcall.trailer_buf) goto Enomem; - } + memset(op->downcall.trailer_buf, 0, op->downcall.trailer_size); if (!copy_from_iter_full(op->downcall.trailer_buf, op->downcall.trailer_size, iter)) { diff --git a/fs/orangefs/orangefs-bufmap.c b/fs/orangefs/orangefs-bufmap.c index 038d67545d9f..7ef473f3d642 100644 --- a/fs/orangefs/orangefs-bufmap.c +++ b/fs/orangefs/orangefs-bufmap.c @@ -244,20 +244,14 @@ orangefs_bufmap_alloc(struct ORANGEFS_dev_map_desc *user_desc) bufmap->buffer_index_array = kzalloc(DIV_ROUND_UP(bufmap->desc_count, BITS_PER_LONG), GFP_KERNEL); - if (!bufmap->buffer_index_array) { - gossip_err("orangefs: could not allocate %d buffer indices\n", - bufmap->desc_count); + if (!bufmap->buffer_index_array) goto out_free_bufmap; - } bufmap->desc_array = kcalloc(bufmap->desc_count, sizeof(struct orangefs_bufmap_desc), GFP_KERNEL); - if (!bufmap->desc_array) { - gossip_err("orangefs: could not allocate %d descriptors\n", - bufmap->desc_count); + if (!bufmap->desc_array) goto out_free_index_array; - } bufmap->page_count = bufmap->total_size / PAGE_SIZE; diff --git a/fs/orangefs/orangefs-debugfs.c b/fs/orangefs/orangefs-debugfs.c index 716ed337f166..93fe8f8e60f1 100644 --- a/fs/orangefs/orangefs-debugfs.c +++ b/fs/orangefs/orangefs-debugfs.c @@ -575,7 +575,6 @@ static int orangefs_prepare_cdm_array(char *debug_array_string) kzalloc(cdm_element_count * sizeof(struct client_debug_mask), GFP_KERNEL); if (!cdm_array) { - pr_info("malloc failed for cdm_array!\n"); rc = -ENOMEM; goto out; } diff --git a/fs/orangefs/orangefs-mod.c b/fs/orangefs/orangefs-mod.c index c1b5174cb5a9..85ef87245a87 100644 --- a/fs/orangefs/orangefs-mod.c +++ b/fs/orangefs/orangefs-mod.c @@ -98,7 +98,6 @@ static int __init orangefs_init(void) orangefs_htable_ops_in_progress = kcalloc(hash_table_size, sizeof(struct list_head), GFP_KERNEL); if (!orangefs_htable_ops_in_progress) { - gossip_err("Failed to initialize op hashtable"); ret = -ENOMEM; goto cleanup_inode; } diff --git a/fs/orangefs/super.c b/fs/orangefs/super.c index 5a1bed6c8c6a..47f3fb9cbec4 100644 --- a/fs/orangefs/super.c +++ b/fs/orangefs/super.c @@ -107,10 +107,8 @@ static struct inode *orangefs_alloc_inode(struct super_block *sb) struct orangefs_inode_s *orangefs_inode; orangefs_inode = kmem_cache_alloc(orangefs_inode_cache, GFP_KERNEL); - if (orangefs_inode == NULL) { - gossip_err("Failed to allocate orangefs_inode\n"); + if (!orangefs_inode) return NULL; - } /* * We want to clear everything except for rw_semaphore and the From 5e273a0e06ee5a50bb9cd40026177feca060c101 Mon Sep 17 00:00:00 2001 From: Markus Elfring Date: Thu, 17 Aug 2017 21:18:01 +0200 Subject: [PATCH 8/9] orangefs: Use kcalloc() in orangefs_prepare_cdm_array() * A multiplication for the size determination of a memory allocation indicated that an array data structure should be processed. Thus use the corresponding function "kcalloc". This issue was detected by using the Coccinelle software. * Replace the specification of a data structure by a pointer dereference to make the corresponding size determination a bit safer according to the Linux coding style convention. Signed-off-by: Markus Elfring Signed-off-by: Mike Marshall --- fs/orangefs/orangefs-debugfs.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/fs/orangefs/orangefs-debugfs.c b/fs/orangefs/orangefs-debugfs.c index 93fe8f8e60f1..5f59917fd631 100644 --- a/fs/orangefs/orangefs-debugfs.c +++ b/fs/orangefs/orangefs-debugfs.c @@ -571,9 +571,7 @@ static int orangefs_prepare_cdm_array(char *debug_array_string) goto out; } - cdm_array = - kzalloc(cdm_element_count * sizeof(struct client_debug_mask), - GFP_KERNEL); + cdm_array = kcalloc(cdm_element_count, sizeof(*cdm_array), GFP_KERNEL); if (!cdm_array) { rc = -ENOMEM; goto out; From 0b08273c8ab7e688832120c4817b1adfdc56e231 Mon Sep 17 00:00:00 2001 From: Markus Elfring Date: Thu, 17 Aug 2017 21:35:16 +0200 Subject: [PATCH 9/9] orangefs: Adjust three checks for null pointers MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The script “checkpatch.pl” pointed information out like the following. Comparison to NULL could be written !… Thus fix affected source code places. Signed-off-by: Markus Elfring Signed-off-by: Mike Marshall --- fs/orangefs/acl.c | 2 +- fs/orangefs/xattr.c | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/fs/orangefs/acl.c b/fs/orangefs/acl.c index 45f27cf51fd8..9108ef433e6d 100644 --- a/fs/orangefs/acl.c +++ b/fs/orangefs/acl.c @@ -35,7 +35,7 @@ struct posix_acl *orangefs_get_acl(struct inode *inode, int type) * I don't do that for now. */ value = kmalloc(ORANGEFS_MAX_XATTR_VALUELEN, GFP_KERNEL); - if (value == NULL) + if (!value) return ERR_PTR(-ENOMEM); gossip_debug(GOSSIP_ACL_DEBUG, diff --git a/fs/orangefs/xattr.c b/fs/orangefs/xattr.c index fed0e9aae23b..81ac88bb91ff 100644 --- a/fs/orangefs/xattr.c +++ b/fs/orangefs/xattr.c @@ -239,7 +239,7 @@ int orangefs_inode_setxattr(struct inode *inode, const char *name, internal_flag = convert_to_internal_xattr_flags(flags); /* This is equivalent to a removexattr */ - if (size == 0 && value == NULL) { + if (size == 0 && !value) { gossip_debug(GOSSIP_XATTR_DEBUG, "removing xattr (%s)\n", name); @@ -311,7 +311,7 @@ ssize_t orangefs_listxattr(struct dentry *dentry, char *buffer, size_t size) int i = 0; int returned_count = 0; - if (size > 0 && buffer == NULL) { + if (size > 0 && !buffer) { gossip_err("%s: bogus NULL pointers\n", __func__); return -EINVAL; }