linux-fsdevel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Christian Brauner <brauner@kernel.org>
To: linux-fsdevel@vger.kernel.org
Cc: Christian Brauner <brauner@kernel.org>,
	Seth Forshee <sforshee@kernel.org>,
	Christoph Hellwig <hch@lst.de>, Al Viro <viro@zeniv.linux.org.uk>,
	linux-security-module@vger.kernel.org
Subject: [PATCH v4 01/30] orangefs: rework posix acl handling when creating new filesystem objects
Date: Thu, 29 Sep 2022 17:30:11 +0200	[thread overview]
Message-ID: <20220929153041.500115-2-brauner@kernel.org> (raw)
In-Reply-To: <20220929153041.500115-1-brauner@kernel.org>

When creating new filesytem objects orangefs used to create posix acls
after it had created and inserted a new inode. This made it necessary to
all posix_acl_chmod() on the newly created inode in case the mode of the
inode would be changed by the posix acls.

Instead of doing it this way calculate the correct mode directly before
actually creating the inode. So we first create posix acls, then pass
the mode that posix acls mandate into the orangefs getattr helper and
calculate the correct mode. This is needed so we can simply change
posix_acl_chmod() to take a dentry instead of an inode argument in the
next patch.

Reviewed-by: Christoph Hellwig <hch@lst.de>
Signed-off-by: Christian Brauner (Microsoft) <brauner@kernel.org>
---

Notes:
    /* v2 */
    Christoph Hellwig <hch@lst.de>:
    - Add separate patch for orangefs rework.
    
    /* v3 */
    unchanged
    
    /* v4 */
    Christoph Hellwig <hch@lst.de>:
    - drop extern from function declaration

 fs/orangefs/acl.c             | 44 ++---------------------------------
 fs/orangefs/inode.c           | 44 ++++++++++++++++++++++++++++-------
 fs/orangefs/orangefs-kernel.h |  6 +++--
 fs/orangefs/orangefs-utils.c  | 12 +++++++---
 4 files changed, 51 insertions(+), 55 deletions(-)

diff --git a/fs/orangefs/acl.c b/fs/orangefs/acl.c
index 605e5a3506ec..0e2db840c217 100644
--- a/fs/orangefs/acl.c
+++ b/fs/orangefs/acl.c
@@ -64,8 +64,7 @@ struct posix_acl *orangefs_get_acl(struct inode *inode, int type, bool rcu)
 	return acl;
 }
 
-static int __orangefs_set_acl(struct inode *inode, struct posix_acl *acl,
-			      int type)
+int __orangefs_set_acl(struct inode *inode, struct posix_acl *acl, int type)
 {
 	int error = 0;
 	void *value = NULL;
@@ -153,46 +152,7 @@ int orangefs_set_acl(struct user_namespace *mnt_userns, struct inode *inode,
 	rc = __orangefs_set_acl(inode, acl, type);
 
 	if (!rc && (iattr.ia_valid == ATTR_MODE))
-		rc = __orangefs_setattr(inode, &iattr);
+		rc = __orangefs_setattr_mode(inode, &iattr);
 
 	return rc;
 }
-
-int orangefs_init_acl(struct inode *inode, struct inode *dir)
-{
-	struct posix_acl *default_acl, *acl;
-	umode_t mode = inode->i_mode;
-	struct iattr iattr;
-	int error = 0;
-
-	error = posix_acl_create(dir, &mode, &default_acl, &acl);
-	if (error)
-		return error;
-
-	if (default_acl) {
-		error = __orangefs_set_acl(inode, default_acl,
-					   ACL_TYPE_DEFAULT);
-		posix_acl_release(default_acl);
-	} else {
-		inode->i_default_acl = NULL;
-	}
-
-	if (acl) {
-		if (!error)
-			error = __orangefs_set_acl(inode, acl, ACL_TYPE_ACCESS);
-		posix_acl_release(acl);
-	} else {
-		inode->i_acl = NULL;
-	}
-
-	/* If mode of the inode was changed, then do a forcible ->setattr */
-	if (mode != inode->i_mode) {
-		memset(&iattr, 0, sizeof iattr);
-		inode->i_mode = mode;
-		iattr.ia_mode = mode;
-		iattr.ia_valid |= ATTR_MODE;
-		__orangefs_setattr(inode, &iattr);
-	}
-
-	return error;
-}
diff --git a/fs/orangefs/inode.c b/fs/orangefs/inode.c
index 7a8c0c6e698d..35788cde6d24 100644
--- a/fs/orangefs/inode.c
+++ b/fs/orangefs/inode.c
@@ -828,15 +828,22 @@ int __orangefs_setattr(struct inode *inode, struct iattr *iattr)
 	spin_unlock(&inode->i_lock);
 	mark_inode_dirty(inode);
 
-	if (iattr->ia_valid & ATTR_MODE)
-		/* change mod on a file that has ACLs */
-		ret = posix_acl_chmod(&init_user_ns, inode, inode->i_mode);
-
 	ret = 0;
 out:
 	return ret;
 }
 
+int __orangefs_setattr_mode(struct inode *inode, struct iattr *iattr)
+{
+	int ret;
+
+	ret = __orangefs_setattr(inode, iattr);
+	/* change mode on a file that has ACLs */
+	if (!ret && (iattr->ia_valid & ATTR_MODE))
+		ret = posix_acl_chmod(&init_user_ns, inode, inode->i_mode);
+	return ret;
+}
+
 /*
  * Change attributes of an object referenced by dentry.
  */
@@ -849,7 +856,7 @@ int orangefs_setattr(struct user_namespace *mnt_userns, struct dentry *dentry,
 	ret = setattr_prepare(&init_user_ns, dentry, iattr);
 	if (ret)
 	        goto out;
-	ret = __orangefs_setattr(d_inode(dentry), iattr);
+	ret = __orangefs_setattr_mode(d_inode(dentry), iattr);
 	sync_inode_metadata(d_inode(dentry), 1);
 out:
 	gossip_debug(GOSSIP_INODE_DEBUG, "orangefs_setattr: returning %d\n",
@@ -1097,8 +1104,9 @@ struct inode *orangefs_iget(struct super_block *sb,
  * Allocate an inode for a newly created file and insert it into the inode hash.
  */
 struct inode *orangefs_new_inode(struct super_block *sb, struct inode *dir,
-		int mode, dev_t dev, struct orangefs_object_kref *ref)
+		umode_t mode, dev_t dev, struct orangefs_object_kref *ref)
 {
+	struct posix_acl *acl = NULL, *default_acl = NULL;
 	unsigned long hash = orangefs_handle_hash(ref);
 	struct inode *inode;
 	int error;
@@ -1115,16 +1123,33 @@ struct inode *orangefs_new_inode(struct super_block *sb, struct inode *dir,
 	if (!inode)
 		return ERR_PTR(-ENOMEM);
 
+	error = posix_acl_create(dir, &mode, &default_acl, &acl);
+	if (error)
+		goto out_iput;
+
 	orangefs_set_inode(inode, ref);
 	inode->i_ino = hash;	/* needed for stat etc */
 
-	error = orangefs_inode_getattr(inode, ORANGEFS_GETATTR_NEW);
+	error = __orangefs_inode_getattr(inode, mode, ORANGEFS_GETATTR_NEW);
 	if (error)
 		goto out_iput;
 
 	orangefs_init_iops(inode);
 	inode->i_rdev = dev;
 
+	if (default_acl) {
+		error = __orangefs_set_acl(inode, default_acl,
+					   ACL_TYPE_DEFAULT);
+		if (error)
+			goto out_iput;
+	}
+
+	if (acl) {
+		error = __orangefs_set_acl(inode, acl, ACL_TYPE_ACCESS);
+		if (error)
+			goto out_iput;
+	}
+
 	error = insert_inode_locked4(inode, hash, orangefs_test_inode, ref);
 	if (error < 0)
 		goto out_iput;
@@ -1132,10 +1157,13 @@ struct inode *orangefs_new_inode(struct super_block *sb, struct inode *dir,
 	gossip_debug(GOSSIP_INODE_DEBUG,
 		     "Initializing ACL's for inode %pU\n",
 		     get_khandle_from_ino(inode));
-	orangefs_init_acl(inode, dir);
+	posix_acl_release(acl);
+	posix_acl_release(default_acl);
 	return inode;
 
 out_iput:
 	iput(inode);
+	posix_acl_release(acl);
+	posix_acl_release(default_acl);
 	return ERR_PTR(error);
 }
diff --git a/fs/orangefs/orangefs-kernel.h b/fs/orangefs/orangefs-kernel.h
index b5940ec1836a..3298b15684b7 100644
--- a/fs/orangefs/orangefs-kernel.h
+++ b/fs/orangefs/orangefs-kernel.h
@@ -103,13 +103,13 @@ enum orangefs_vfs_op_states {
 #define ORANGEFS_CACHE_CREATE_FLAGS 0
 #endif
 
-extern int orangefs_init_acl(struct inode *inode, struct inode *dir);
 extern const struct xattr_handler *orangefs_xattr_handlers[];
 
 extern struct posix_acl *orangefs_get_acl(struct inode *inode, int type, bool rcu);
 extern int orangefs_set_acl(struct user_namespace *mnt_userns,
 			    struct inode *inode, struct posix_acl *acl,
 			    int type);
+int __orangefs_set_acl(struct inode *inode, struct posix_acl *acl, int type);
 
 /*
  * orangefs data structures
@@ -356,11 +356,12 @@ void fsid_key_table_finalize(void);
 vm_fault_t orangefs_page_mkwrite(struct vm_fault *);
 struct inode *orangefs_new_inode(struct super_block *sb,
 			      struct inode *dir,
-			      int mode,
+			      umode_t mode,
 			      dev_t dev,
 			      struct orangefs_object_kref *ref);
 
 int __orangefs_setattr(struct inode *, struct iattr *);
+int __orangefs_setattr_mode(struct inode *inode, struct iattr *iattr);
 int orangefs_setattr(struct user_namespace *, struct dentry *, struct iattr *);
 
 int orangefs_getattr(struct user_namespace *mnt_userns, const struct path *path,
@@ -422,6 +423,7 @@ int orangefs_inode_setxattr(struct inode *inode,
 #define ORANGEFS_GETATTR_SIZE 2
 
 int orangefs_inode_getattr(struct inode *, int);
+int __orangefs_inode_getattr(struct inode *inode, umode_t mode, int flags);
 
 int orangefs_inode_check_changed(struct inode *inode);
 
diff --git a/fs/orangefs/orangefs-utils.c b/fs/orangefs/orangefs-utils.c
index 46b7dcff18ac..2351a62a7b37 100644
--- a/fs/orangefs/orangefs-utils.c
+++ b/fs/orangefs/orangefs-utils.c
@@ -233,7 +233,7 @@ static int orangefs_inode_is_stale(struct inode *inode,
 	return 0;
 }
 
-int orangefs_inode_getattr(struct inode *inode, int flags)
+int __orangefs_inode_getattr(struct inode *inode, umode_t mode, int flags)
 {
 	struct orangefs_inode_s *orangefs_inode = ORANGEFS_I(inode);
 	struct orangefs_kernel_op_s *new_op;
@@ -368,8 +368,9 @@ int orangefs_inode_getattr(struct inode *inode, int flags)
 	inode->i_ctime.tv_nsec = 0;
 
 	/* special case: mark the root inode as sticky */
-	inode->i_mode = type | (is_root_handle(inode) ? S_ISVTX : 0) |
-	    orangefs_inode_perms(&new_op->downcall.resp.getattr.attributes);
+	inode->i_mode = type | (is_root_handle(inode) ? S_ISVTX : 0) | mode |
+	    orangefs_inode_perms(&new_op->downcall.resp.getattr.attributes) |
+	    mode;
 
 	orangefs_inode->getattr_time = jiffies +
 	    orangefs_getattr_timeout_msecs*HZ/1000;
@@ -381,6 +382,11 @@ int orangefs_inode_getattr(struct inode *inode, int flags)
 	return ret;
 }
 
+int orangefs_inode_getattr(struct inode *inode, int flags)
+{
+	return __orangefs_inode_getattr(inode, 0, flags);
+}
+
 int orangefs_inode_check_changed(struct inode *inode)
 {
 	struct orangefs_inode_s *orangefs_inode = ORANGEFS_I(inode);
-- 
2.34.1


  reply	other threads:[~2022-09-29 15:31 UTC|newest]

Thread overview: 60+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-09-29 15:30 [PATCH v4 00/30] acl: add vfs posix acl api Christian Brauner
2022-09-29 15:30 ` Christian Brauner [this message]
2022-09-29 15:30 ` [PATCH v4 02/30] fs: pass dentry to set acl method Christian Brauner
2022-09-29 15:30 ` [PATCH v4 03/30] fs: rename current get " Christian Brauner
2022-09-29 15:30 ` [PATCH v4 04/30] fs: add new " Christian Brauner
2022-09-30  8:53   ` Miklos Szeredi
2022-09-30  9:09     ` Christian Brauner
2022-09-30  9:43       ` Miklos Szeredi
2022-09-30 10:05         ` Christian Brauner
2022-09-30 12:24           ` Miklos Szeredi
2022-09-30 12:49             ` Christian Brauner
2022-09-30 13:01               ` Miklos Szeredi
2022-09-30 13:51                 ` Christian Brauner
2022-10-04 19:53         ` Steve French
2022-10-05  7:15           ` Christian Brauner
2022-10-06  6:31             ` Miklos Szeredi
2022-10-06  7:40               ` Christian Brauner
2022-10-06  9:07                 ` Miklos Szeredi
2022-09-29 15:30 ` [PATCH v4 05/30] cifs: implement " Christian Brauner
2022-09-29 15:30 ` [PATCH v4 06/30] cifs: implement set " Christian Brauner
2022-09-29 15:30 ` [PATCH v4 07/30] 9p: implement get " Christian Brauner
2022-09-29 15:30 ` [PATCH v4 08/30] 9p: implement set " Christian Brauner
2022-09-29 15:30 ` [PATCH v4 09/30] security: add get, remove and set acl hook Christian Brauner
2022-09-29 19:15   ` Paul Moore
2022-09-29 15:30 ` [PATCH v4 10/30] selinux: implement get, set and remove " Christian Brauner
2022-09-29 19:15   ` Paul Moore
2022-09-30  8:38     ` Christian Brauner
2022-09-29 15:30 ` [PATCH v4 11/30] smack: " Christian Brauner
2022-09-29 19:15   ` Paul Moore
2022-09-30  8:40     ` Christian Brauner
2022-09-29 15:30 ` [PATCH v4 12/30] integrity: implement get and set " Christian Brauner
2022-09-29 19:14   ` Paul Moore
2022-09-30  3:19     ` Mimi Zohar
2022-09-30 14:11       ` Paul Moore
2022-09-30  8:11     ` Christian Brauner
2022-09-29 15:30 ` [PATCH v4 13/30] evm: add post " Christian Brauner
2022-09-30  1:44   ` Mimi Zohar
2022-09-30  2:51     ` Mimi Zohar
2022-09-30  8:44     ` Christian Brauner
2022-09-30 11:48       ` Mimi Zohar
2022-10-04  7:04         ` Christian Brauner
2022-09-29 15:30 ` [PATCH v4 14/30] internal: add may_write_xattr() Christian Brauner
2022-09-29 15:30 ` [PATCH v4 15/30] acl: add vfs_set_acl() Christian Brauner
2022-09-29 15:30 ` [PATCH v4 16/30] acl: add vfs_get_acl() Christian Brauner
2022-09-29 15:30 ` [PATCH v4 17/30] acl: add vfs_remove_acl() Christian Brauner
2022-09-29 15:30 ` [PATCH v4 18/30] ksmbd: use vfs_remove_acl() Christian Brauner
2022-09-29 15:30 ` [PATCH v4 19/30] ecryptfs: implement get acl method Christian Brauner
2022-09-29 15:30 ` [PATCH v4 20/30] ecryptfs: implement set " Christian Brauner
2022-09-29 15:30 ` [PATCH v4 21/30] ovl: implement get " Christian Brauner
2022-09-29 15:30 ` [PATCH v4 22/30] ovl: implement set " Christian Brauner
2022-10-06 12:39   ` Miklos Szeredi
2022-09-29 15:30 ` [PATCH v4 23/30] ovl: use posix acl api Christian Brauner
2022-10-06 12:50   ` Miklos Szeredi
2022-09-29 15:30 ` [PATCH v4 24/30] xattr: " Christian Brauner
2022-09-29 15:30 ` [PATCH v4 25/30] evm: remove evm_xattr_acl_change() Christian Brauner
2022-09-29 15:30 ` [PATCH v4 26/30] ecryptfs: use stub posix acl handlers Christian Brauner
2022-09-29 15:30 ` [PATCH v4 27/30] ovl: " Christian Brauner
2022-09-29 15:30 ` [PATCH v4 28/30] cifs: " Christian Brauner
2022-09-29 15:30 ` [PATCH v4 29/30] 9p: " Christian Brauner
2022-09-29 15:30 ` [PATCH v4 30/30] acl: remove a slew of now unused helpers Christian Brauner

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20220929153041.500115-2-brauner@kernel.org \
    --to=brauner@kernel.org \
    --cc=hch@lst.de \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-security-module@vger.kernel.org \
    --cc=sforshee@kernel.org \
    --cc=viro@zeniv.linux.org.uk \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).