Re: [PATCH v3 1/1] nfs42: client needs to strip file mode's suid/sgid bit after ALLOCATE op

[Jeff Layton <jlayton@kernel.org>]

On Thu, 2023-08-24 at 16:11 -0700, Dai Ngo wrote:
> The Linux NFS server strips the SUID and SGID from the file mode
> on ALLOCATE op.
> 
> Modify _nfs42_proc_fallocate to add NFS_INO_REVAL_FORCED to
> nfs_set_cache_invalid's argument to force update of the
> file mode suid/sgid bit.
> 
> Signed-off-by: Dai Ngo <dai.ngo@oracle.com>
> ---
>  fs/nfs/nfs42proc.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/fs/nfs/nfs42proc.c b/fs/nfs/nfs42proc.c
> index 63802d195556..9d2f07feeb29 100644
> --- a/fs/nfs/nfs42proc.c
> +++ b/fs/nfs/nfs42proc.c
> @@ -81,7 +81,8 @@ static int _nfs42_proc_fallocate(struct rpc_message *msg, struct file *filep,
>  	if (status == 0) {
>  		if (nfs_should_remove_suid(inode)) {
>  			spin_lock(&inode->i_lock);
> -			nfs_set_cache_invalid(inode, NFS_INO_INVALID_MODE);
> +			nfs_set_cache_invalid(inode,
> +				NFS_INO_REVAL_FORCED | NFS_INO_INVALID_MODE);
>  			spin_unlock(&inode->i_lock);
>  		}
>  		status = nfs_post_op_update_inode_force_wcc(inode,

Yeah, I think this looks like the right thing to do. IIUC,
NFS_INO_REVAL_FORCED just means "ignore the fact that I have a
delegation", which I think is what we want here.

If this turns out to be too expensive, another idea might be to only set
FORCED here if the current mode has bits that would be cleared on a
write (i.e. setuid/setgid bits with execute bits set under them). We
don't expect "stealth" mode changes unless that's the case.

Reviewed-by: Jeff Layton <jlayton@kernel.org>