> On Aug 24, 2023, at 12:01 PM, Trond Myklebust <trondmy@xxxxxxxxxxxxxxx> wrote:
>
> On Thu, 2023-08-24 at 11:42 -0700, dai.ngo@xxxxxxxxxx wrote:
>>
>>> On 8/24/23 9:38 AM, dai.ngo@xxxxxxxxxx wrote:
>>>
>>> On 8/24/23 9:34 AM, Trond Myklebust wrote:
>>>> On Thu, 2023-08-24 at 09:12 -0700, dai.ngo@xxxxxxxxxx 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@xxxxxxxxxx>
>>>>>>> ---
>>>>>>> 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.
>>
>> This is the new patch:
>>
>> diff --git a/fs/nfs/nfs42proc.c b/fs/nfs/nfs42proc.c
>> index 63802d195556..ea1991e393e2 100644
>> --- a/fs/nfs/nfs42proc.c
>> +++ b/fs/nfs/nfs42proc.c
>> @@ -81,7 +81,7 @@ 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);
>
> No. The above needs to add NFS_INO_REVAL_FORCED.
>
> IOW:
> nfs_set_cache_invalid(inode, NFS_INO_INVALID_MODE | NFS_INO_REVAL_FORCED);
Ok, I’ll try again.
Thanks,
-Dai
>
>> spin_unlock(&inode->i_lock);
>> }
>> status = nfs_post_op_update_inode_force_wcc(inode,
>
> --
> Trond Myklebust
> Linux NFS client maintainer, Hammerspace
> trond.myklebust@xxxxxxxxxxxxxxx
>
>
