Re: [PATCH v2 1/1] nfs42: client needs to update file mode after ALLOCATE op

[dai.ngo@oracle.com]

On 8/24/23 9:01 AM, Trond Myklebust wrote:
> On Thu, 2023-08-24 at 08:53 -0700, Dai Ngo wrote:
>> The Linux NFS server strips the SUID and SGID from the file mode
>> on ALLOCATE op. The GETATTR op in the ALLOCATE compound needs to
>> request the file mode from the server to update its file mode in
>> case the SUID/SGUI bit were stripped.
>>
>> Signed-off-by: Dai Ngo <dai.ngo@oracle.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 63802d195556..d3d050171822 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);
>>   
>>          res.falloc_fattr = nfs_alloc_fattr();
>>          if (!res.falloc_fattr)
> Actually... Wait... Why isn't the existing code sufficient?
>
>          status = nfs4_call_sync(server->client, server, msg,
>                                  &args.seq_args, &res.seq_res, 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);
>          }
>
> We explicitly check for SUID bits, and invalidate the mode if they are
> set.

nfs_set_cache_invalid checks for delegation and clears the NFS_INO_INVALID_MODE.

-Dai

>