From: Christian Brauner <brauner@kernel.org>
To: Mike Marshall <hubcap@omnibond.com>
Cc: Christian Brauner <brauner@kernel.org>,
	linux-fsdevel <linux-fsdevel@vger.kernel.org>
Subject: [PATCH] orangefs: fix mode handling
Date: Sun, 13 Nov 2022 12:50:02 +0100	[thread overview]
Message-ID: <20221113115002.2006026-1-brauner@kernel.org> (raw)
In-Reply-To: <20221112100945.uig2jef74oxbg6gd@wittgenstein>
In 4053d2500beb ("orangefs: rework posix acl handling when creating new
filesystem objects") we tried to precalculate the correct mode when
creating a new inode. However, this leads to regressions when creating new
filesystem objects.
Even if we precalculate the mode we still need to call __orangefs_setattr()
to perform additional checks and we also need to update the mode of
ACL_TYPE_ACCESS acls set on the inode. The patch referenced above regressed
that. Restore that part of the old behavior and remove the mode
precalculation as it doesn't get us anything anymore.
Fixes: 4053d2500beb ("orangefs: rework posix acl handling when creating new filesystem objects")
Reported-by: Mike Marshall <hubcap@omnibond.com>
Signed-off-by: Christian Brauner (Microsoft) <brauner@kernel.org>
---
Notes:
    Link: https://lore.kernel.org/linux-fsdevel/CAOg9mSSq_yV=q5J6sEh8qHWwLe_wYwwsb1rTEh11k52D2nm11g@mail.gmail.com
    
    Hey Mike,
    
    The fix is pretty straightforward. I forgot to update the mode through
    __orangefs_setattr() and in the ACL_TYPE_ACCESS acl. This patch restores
    this and fixes the reported xfstest failure. I've pushed this out now.
    
    Thanks!
    Christian
 fs/orangefs/inode.c           | 11 ++++++++++-
 fs/orangefs/orangefs-kernel.h |  1 -
 fs/orangefs/orangefs-utils.c  | 10 ++--------
 3 files changed, 12 insertions(+), 10 deletions(-)
diff --git a/fs/orangefs/inode.c b/fs/orangefs/inode.c
index 8974b0fbf00d..3d65accfae17 100644
--- a/fs/orangefs/inode.c
+++ b/fs/orangefs/inode.c
@@ -1131,7 +1131,7 @@ struct inode *orangefs_new_inode(struct super_block *sb, struct inode *dir,
 	orangefs_set_inode(inode, ref);
 	inode->i_ino = hash;	/* needed for stat etc */
 
-	error = __orangefs_inode_getattr(inode, mode, ORANGEFS_GETATTR_NEW);
+	error = orangefs_inode_getattr(inode, ORANGEFS_GETATTR_NEW);
 	if (error)
 		goto out_iput;
 
@@ -1158,6 +1158,15 @@ 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));
+	if (mode != inode->i_mode) {
+		struct iattr iattr = {
+			.ia_mode = mode,
+			.ia_valid = ATTR_MODE,
+		};
+		inode->i_mode = mode;
+		__orangefs_setattr(inode, &iattr);
+		__posix_acl_chmod(&acl, GFP_KERNEL, inode->i_mode);
+	}
 	posix_acl_release(acl);
 	posix_acl_release(default_acl);
 	return inode;
diff --git a/fs/orangefs/orangefs-kernel.h b/fs/orangefs/orangefs-kernel.h
index 55cd6d50eea1..6e0cc01b3a14 100644
--- a/fs/orangefs/orangefs-kernel.h
+++ b/fs/orangefs/orangefs-kernel.h
@@ -423,7 +423,6 @@ 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 334a2fd98c37..46b7dcff18ac 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, umode_t mode, int flags)
+int orangefs_inode_getattr(struct inode *inode, int flags)
 {
 	struct orangefs_inode_s *orangefs_inode = ORANGEFS_I(inode);
 	struct orangefs_kernel_op_s *new_op;
@@ -369,8 +369,7 @@ int __orangefs_inode_getattr(struct inode *inode, umode_t mode, int flags)
 
 	/* 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) |
-	    mode;
+	    orangefs_inode_perms(&new_op->downcall.resp.getattr.attributes);
 
 	orangefs_inode->getattr_time = jiffies +
 	    orangefs_getattr_timeout_msecs*HZ/1000;
@@ -382,11 +381,6 @@ int __orangefs_inode_getattr(struct inode *inode, umode_t mode, 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);
base-commit: 5b52aebef8954cadff29918bb61d7fdc7be07837
-- 
2.34.1
next prev parent reply	other threads:[~2022-11-13 11:50 UTC|newest]
Thread overview: 38+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-10-18 11:56 [PATCH v5 00/30] acl: add vfs posix acl api Christian Brauner
2022-10-18 11:56 ` [PATCH v5 01/30] orangefs: rework posix acl handling when creating new filesystem objects Christian Brauner
2022-11-11 21:38   ` Mike Marshall
2022-11-12 10:09     ` Christian Brauner
2022-11-13 11:50       ` Christian Brauner [this message]
2022-10-18 11:56 ` [PATCH v5 02/30] fs: pass dentry to set acl method Christian Brauner
2022-11-18 10:09   ` Mickaël Salaün
2022-11-18 10:33     ` Christian Brauner
2022-11-18 12:32       ` Mickaël Salaün
2022-10-18 11:56 ` [PATCH v5 03/30] fs: rename current get " Christian Brauner
2022-10-18 11:56 ` [PATCH v5 04/30] fs: add new " Christian Brauner
2022-10-18 11:56 ` [PATCH v5 05/30] cifs: implement " Christian Brauner
2022-10-18 11:56 ` [PATCH v5 06/30] cifs: implement set " Christian Brauner
2022-10-18 11:56 ` [PATCH v5 07/30] 9p: implement get " Christian Brauner
2022-10-18 11:56 ` [PATCH v5 08/30] 9p: implement set " Christian Brauner
2022-10-18 11:56 ` [PATCH v5 09/30] security: add get, remove and set acl hook Christian Brauner
2022-10-18 11:56 ` [PATCH v5 10/30] selinux: implement get, set and remove " Christian Brauner
2022-10-18 11:56 ` [PATCH v5 11/30] smack: " Christian Brauner
2022-10-18 11:56 ` [PATCH v5 12/30] integrity: implement get and set " Christian Brauner
2022-10-18 19:17   ` Paul Moore
2022-10-18 11:56 ` [PATCH v5 13/30] evm: add post " Christian Brauner
2022-10-18 11:56 ` [PATCH v5 14/30] internal: add may_write_xattr() Christian Brauner
2022-10-18 11:56 ` [PATCH v5 15/30] acl: add vfs_set_acl() Christian Brauner
2022-10-18 11:56 ` [PATCH v5 16/30] acl: add vfs_get_acl() Christian Brauner
2022-10-18 11:56 ` [PATCH v5 17/30] acl: add vfs_remove_acl() Christian Brauner
2022-10-18 11:56 ` [PATCH v5 18/30] ksmbd: use vfs_remove_acl() Christian Brauner
2022-10-18 11:56 ` [PATCH v5 19/30] ecryptfs: implement get acl method Christian Brauner
2022-10-18 11:56 ` [PATCH v5 20/30] ecryptfs: implement set " Christian Brauner
2022-10-18 11:56 ` [PATCH v5 21/30] ovl: implement get " Christian Brauner
2022-10-18 11:56 ` [PATCH v5 22/30] ovl: implement set " Christian Brauner
2022-10-18 11:56 ` [PATCH v5 23/30] ovl: use posix acl api Christian Brauner
2022-10-18 11:56 ` [PATCH v5 24/30] xattr: " Christian Brauner
2022-10-18 11:56 ` [PATCH v5 25/30] evm: remove evm_xattr_acl_change() Christian Brauner
2022-10-18 11:56 ` [PATCH v5 26/30] ecryptfs: use stub posix acl handlers Christian Brauner
2022-10-18 11:56 ` [PATCH v5 27/30] ovl: " Christian Brauner
2022-10-18 11:56 ` [PATCH v5 28/30] cifs: " Christian Brauner
2022-10-18 11:56 ` [PATCH v5 29/30] 9p: " Christian Brauner
2022-10-18 11:57 ` [PATCH v5 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=20221113115002.2006026-1-brauner@kernel.org \
    --to=brauner@kernel.org \
    --cc=hubcap@omnibond.com \
    --cc=linux-fsdevel@vger.kernel.org \
    /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).