[PATCH v3] NFSv4.2: Update mode bits after ALLOCATE and DEALLOCATE

[PATCH v3] NFSv4.2: Update mode bits after ALLOCATE and DEALLOCATE

[Anna Schumaker]

From: Anna Schumaker <Anna.Schumaker@Netapp.com>

The fallocate call invalidates suid and sgid bits as part of normal
operation. We need to mark the mode bits as invalid when using fallocate
with an suid so these will be updated the next time the user looks at them.

This fixes xfstests generic/683 and generic/684.

Reported-by: Yue Cui <cuiyue-fnst@fujitsu.com>
Fixes: 913eca1aea87 ("NFS: Fallocate should use the nfs4_fattr_bitmap")
Signed-off-by: Anna Schumaker <Anna.Schumaker@Netapp.com>
---
 fs/nfs/internal.h  | 25 +++++++++++++++++++++++++
 fs/nfs/nfs42proc.c | 10 ++++++++--
 fs/nfs/write.c     | 25 -------------------------
 3 files changed, 33 insertions(+), 27 deletions(-)

diff --git a/fs/nfs/internal.h b/fs/nfs/internal.h
index 27c720d71b4e..898dd95bc7a7 100644
--- a/fs/nfs/internal.h
+++ b/fs/nfs/internal.h
@@ -606,6 +606,31 @@ static inline gfp_t nfs_io_gfp_mask(void)
 	return GFP_KERNEL;
 }
 
+/*
+ * Special version of should_remove_suid() that ignores capabilities.
+ */
+static inline int nfs_should_remove_suid(const struct inode *inode)
+{
+	umode_t mode = inode->i_mode;
+	int kill = 0;
+
+	/* suid always must be killed */
+	if (unlikely(mode & S_ISUID))
+		kill = ATTR_KILL_SUID;
+
+	/*
+	 * sgid without any exec bits is just a mandatory locking mark; leave
+	 * it alone.  If some exec bits are set, it's a real sgid; kill it.
+	 */
+	if (unlikely((mode & S_ISGID) && (mode & S_IXGRP)))
+		kill |= ATTR_KILL_SGID;
+
+	if (unlikely(kill && S_ISREG(mode)))
+		return kill;
+
+	return 0;
+}
+
 /* unlink.c */
 extern struct rpc_task *
 nfs_async_rename(struct inode *old_dir, struct inode *new_dir,
diff --git a/fs/nfs/nfs42proc.c b/fs/nfs/nfs42proc.c
index 068c45b3bc1a..85a22dfb825e 100644
--- a/fs/nfs/nfs42proc.c
+++ b/fs/nfs/nfs42proc.c
@@ -56,6 +56,7 @@ static int _nfs42_proc_fallocate(struct rpc_message *msg, struct file *filep,
 	struct nfs42_falloc_res res = {
 		.falloc_server	= server,
 	};
+	unsigned int invalid = 0;
 	int status;
 
 	msg->rpc_argp = &args;
@@ -78,10 +79,15 @@ static int _nfs42_proc_fallocate(struct rpc_message *msg, struct file *filep,
 
 	status = nfs4_call_sync(server->client, server, msg,
 				&args.seq_args, &res.seq_res, 0);
-	if (status == 0)
+	if (status == 0) {
+		if (nfs_should_remove_suid(inode)) {
+			spin_lock(&inode->i_lock);
+			nfs_set_cache_invalid(inode, NFS_INO_INVALID_MODE);
+			spin_unlock(&inode->i_lock);
+		}
 		status = nfs_post_op_update_inode_force_wcc(inode,
 							    res.falloc_fattr);
-
+	}
 	if (msg->rpc_proc == &nfs4_procedures[NFSPROC4_CLNT_ALLOCATE])
 		trace_nfs4_fallocate(inode, &args, status);
 	else
diff --git a/fs/nfs/write.c b/fs/nfs/write.c
index 1843fa235d9b..f41d24b54fd1 100644
--- a/fs/nfs/write.c
+++ b/fs/nfs/write.c
@@ -1496,31 +1496,6 @@ void nfs_commit_prepare(struct rpc_task *task, void *calldata)
 	NFS_PROTO(data->inode)->commit_rpc_prepare(task, data);
 }
 
-/*
- * Special version of should_remove_suid() that ignores capabilities.
- */
-static int nfs_should_remove_suid(const struct inode *inode)
-{
-	umode_t mode = inode->i_mode;
-	int kill = 0;
-
-	/* suid always must be killed */
-	if (unlikely(mode & S_ISUID))
-		kill = ATTR_KILL_SUID;
-
-	/*
-	 * sgid without any exec bits is just a mandatory locking mark; leave
-	 * it alone.  If some exec bits are set, it's a real sgid; kill it.
-	 */
-	if (unlikely((mode & S_ISGID) && (mode & S_IXGRP)))
-		kill |= ATTR_KILL_SGID;
-
-	if (unlikely(kill && S_ISREG(mode)))
-		return kill;
-
-	return 0;
-}
-
 static void nfs_writeback_check_extend(struct nfs_pgio_header *hdr,
 		struct nfs_fattr *fattr)
 {
-- 
2.37.2

Re: [PATCH v3] NFSv4.2: Update mode bits after ALLOCATE and DEALLOCATE

[kernel test robot]

Hi Anna,

I love your patch! Perhaps something to improve:

[auto build test WARNING on trondmy-nfs/linux-next]
[also build test WARNING on linus/master v6.0-rc3 next-20220901]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Anna-Schumaker/NFSv4-2-Update-mode-bits-after-ALLOCATE-and-DEALLOCATE/20220902-032035
base:   git://git.linux-nfs.org/projects/trondmy/linux-nfs.git linux-next
config: x86_64-allyesconfig (https://download.01.org/0day-ci/archive/20220902/202209021046.JT3dSTuK-lkp@intel.com/config)
compiler: gcc-11 (Debian 11.3.0-5) 11.3.0
reproduce (this is a W=1 build):
        # https://github.com/intel-lab-lkp/linux/commit/8a722317bde651cf6fa29c7c618ddcce4e52b4e1
        git remote add linux-review https://github.com/intel-lab-lkp/linux
        git fetch --no-tags linux-review Anna-Schumaker/NFSv4-2-Update-mode-bits-after-ALLOCATE-and-DEALLOCATE/20220902-032035
        git checkout 8a722317bde651cf6fa29c7c618ddcce4e52b4e1
        # save the config file
        mkdir build_dir && cp config build_dir/.config
        make W=1 O=build_dir ARCH=x86_64 SHELL=/bin/bash fs/

If you fix the issue, kindly add following tag where applicable
Reported-by: kernel test robot <lkp@intel.com>

All warnings (new ones prefixed by >>):

   fs/nfs/nfs42proc.c: In function '_nfs42_proc_fallocate':
>> fs/nfs/nfs42proc.c:59:22: warning: unused variable 'invalid' [-Wunused-variable]
      59 |         unsigned int invalid = 0;
         |                      ^~~~~~~


vim +/invalid +59 fs/nfs/nfs42proc.c

    43	
    44	static int _nfs42_proc_fallocate(struct rpc_message *msg, struct file *filep,
    45			struct nfs_lock_context *lock, loff_t offset, loff_t len)
    46	{
    47		struct inode *inode = file_inode(filep);
    48		struct nfs_server *server = NFS_SERVER(inode);
    49		u32 bitmask[NFS_BITMASK_SZ];
    50		struct nfs42_falloc_args args = {
    51			.falloc_fh	= NFS_FH(inode),
    52			.falloc_offset	= offset,
    53			.falloc_length	= len,
    54			.falloc_bitmask	= bitmask,
    55		};
    56		struct nfs42_falloc_res res = {
    57			.falloc_server	= server,
    58		};
  > 59		unsigned int invalid = 0;
    60		int status;
    61	
    62		msg->rpc_argp = &args;
    63		msg->rpc_resp = &res;
    64	
    65		status = nfs4_set_rw_stateid(&args.falloc_stateid, lock->open_context,
    66				lock, FMODE_WRITE);
    67		if (status) {
    68			if (status == -EAGAIN)
    69				status = -NFS4ERR_BAD_STATEID;
    70			return status;
    71		}
    72	
    73		nfs4_bitmask_set(bitmask, server->cache_consistency_bitmask, inode,
    74				 NFS_INO_INVALID_BLOCKS);
    75	
    76		res.falloc_fattr = nfs_alloc_fattr();
    77		if (!res.falloc_fattr)
    78			return -ENOMEM;
    79	
    80		status = nfs4_call_sync(server->client, server, msg,
    81					&args.seq_args, &res.seq_res, 0);
    82		if (status == 0) {
    83			if (nfs_should_remove_suid(inode)) {
    84				spin_lock(&inode->i_lock);
    85				nfs_set_cache_invalid(inode, NFS_INO_INVALID_MODE);
    86				spin_unlock(&inode->i_lock);
    87			}
    88			status = nfs_post_op_update_inode_force_wcc(inode,
    89								    res.falloc_fattr);
    90		}
    91		if (msg->rpc_proc == &nfs4_procedures[NFSPROC4_CLNT_ALLOCATE])
    92			trace_nfs4_fallocate(inode, &args, status);
    93		else
    94			trace_nfs4_deallocate(inode, &args, status);
    95		kfree(res.falloc_fattr);
    96		return status;
    97	}
    98	

-- 
0-DAY CI Kernel Test Service
https://01.org/lkp