On Thu, 2022-09-01 at 14:05 -0400, Anna Schumaker wrote:
> From: Anna Schumaker <Anna.Schumaker@xxxxxxxxxx>
>
> 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@xxxxxxxxxxx>
> Fixes: 913eca1aea87 ("NFS: Fallocate should use the
> nfs4_fattr_bitmap")
> Signed-off-by: Anna Schumaker <Anna.Schumaker@xxxxxxxxxx>
> ---
> fs/nfs/internal.h | 25 +++++++++++++++++++++++++
> fs/nfs/nfs42proc.c | 13 ++++++++++++-
> fs/nfs/write.c | 25 -------------------------
> 3 files changed, 37 insertions(+), 26 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..23023ddf75d1 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,20 @@ 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 (!res.falloc_fattr->valid)
> + invalid |= NFS_INO_INVALID_ATTR;
> + if (nfs_should_remove_suid(inode))
> + invalid |= NFS_INO_INVALID_MODE;
> + if (invalid) {
> + spin_lock(&inode->i_lock);
> + nfs_set_cache_invalid(inode, NFS_INO_INVALID_MODE);
> + spin_unlock(&inode->i_lock);
> + }
This all really wants to go into the 'if (status == 0)' below.
> +
> if (status == 0)
> 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)
> {
Otherwise, looks OK.
--
Trond Myklebust
Linux NFS client maintainer, Hammerspace
trond.myklebust@xxxxxxxxxxxxxxx
