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

[Trond Myklebust <trondmy@hammerspace.com>]

On Tue, 2022-08-30 at 16:58 -0400, Anna Schumaker wrote:
> 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
> 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/nfs42proc.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/fs/nfs/nfs42proc.c b/fs/nfs/nfs42proc.c
> index 068c45b3bc1a..a1b264b1a09f 100644
> --- a/fs/nfs/nfs42proc.c
> +++ b/fs/nfs/nfs42proc.c
> @@ -70,7 +70,7 @@ static int _nfs42_proc_fallocate(struct rpc_message
> *msg, struct file *filep,
>         }
>  
>         nfs4_bitmask_set(bitmask, server->cache_consistency_bitmask,
> inode,
> -                        NFS_INO_INVALID_BLOCKS);
> +                        NFS_INO_INVALID_BLOCKS |
> NFS_INO_INVALID_MODE);

Hmm... Could we please set NFS_INO_INVALID_MODE only in the case where
we see that either the S_ISUID or S_ISGID bits are set, just like we do
in nfs_writeback_done()?

>  
>         res.falloc_fattr = nfs_alloc_fattr();
>         if (!res.falloc_fattr)

Note also that you really should (conditionally!) call
nfs_set_cache_invalid() at some point between the RPC call completing,
and the call to nfs_post_op_update_inode_force_wcc(), just in case the
server fails to return the attributes.

-- 
Trond Myklebust
Linux NFS client maintainer, Hammerspace
trond.myklebust@hammerspace.com