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

[dai.ngo@oracle.com]

On 8/24/23 9:34 AM, Trond Myklebust wrote:
> On Thu, 2023-08-24 at 09:12 -0700, dai.ngo@oracle.com wrote:
>> 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.
>>
> Oh. That just means we need to add NFS_INO_REVAL_FORCED, so let's
> rather do that.

ok, I'll create a new patch and test it.

-Dai

>