Re: [PATCH 1/2] NFSD: Fix use-after-free warning when doing inter-server copy

[dai.ngo@oracle.com]

On 2/19/21 7:20 PM, J. Bruce Fields wrote:
> On Fri, Feb 19, 2021 at 05:31:58PM -0800, dai.ngo@oracle.com wrote:
>> If this is the cause why we don't drop the mount after the copy
>> then I can restore the patch and look into this problem. Unfortunately,
>> all my test machines are down for maintenance until Sunday/Monday.
> I think we can take some time to figure out what's actually going on
> here before reverting anything.

Thanks Bruce, I will look into this.

-Dai

>
> --b.
>
>> -Dai
>>
>> On 2/19/21 5:09 PM, J. Bruce Fields wrote:
>>> Dai, do you have a copy of the original use-after-free warning?
>>>
>>> --b.
>>>
>>> On Fri, Feb 19, 2021 at 07:18:53PM -0500, Olga Kornievskaia wrote:
>>>> Hi Dai (Bruce),
>>>>
>>>> This patch is what broke the mount that's now left behind between the
>>>> source server and the destination server. We are no longer dropping
>>>> the necessary reference on the mount to go away. I haven't been paying
>>>> as much attention as I should have been to the changes. The original
>>>> code called fput(src) so a simple refcount of the file. Then things
>>>> got complicated and moved to nfsd_file_put(). So I don't understand
>>>> complexity. But we need to do some kind of put to decrement the needed
>>>> reference on the superblock. Bruce any ideas? Can we go back to
>>>> fput()?
>>>>
>>>> On Thu, Oct 29, 2020 at 3:08 PM Dai Ngo <dai.ngo@oracle.com> wrote:
>>>>> The source file nfsd_file is not constructed the same as other
>>>>> nfsd_file's via nfsd_file_alloc. nfsd_file_put should not be
>>>>> called to free the object; nfsd_file_put is not the inverse of
>>>>> kzalloc, instead kfree is called by nfsd4_do_async_copy when done.
>>>>>
>>>>> Fixes: ce0887ac96d3 ("NFSD add nfs4 inter ssc to nfsd4_copy")
>>>>> Signed-off-by: Dai Ngo <dai.ngo@oracle.com>
>>>>> ---
>>>>>   fs/nfsd/nfs4proc.c | 2 +-
>>>>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>>>>
>>>>> diff --git a/fs/nfsd/nfs4proc.c b/fs/nfsd/nfs4proc.c
>>>>> index ad2fa1a8e7ad..9c43cad7e408 100644
>>>>> --- a/fs/nfsd/nfs4proc.c
>>>>> +++ b/fs/nfsd/nfs4proc.c
>>>>> @@ -1299,7 +1299,7 @@ nfsd4_cleanup_inter_ssc(struct vfsmount *ss_mnt, struct nfsd_file *src,
>>>>>                          struct nfsd_file *dst)
>>>>>   {
>>>>>          nfs42_ssc_close(src->nf_file);
>>>>> -       nfsd_file_put(src);
>>>>> +       /* 'src' is freed by nfsd4_do_async_copy */
>>>>>          nfsd_file_put(dst);
>>>>>          mntput(ss_mnt);
>>>>>   }
>>>>> --
>>>>> 2.20.1.1226.g1595ea5.dirty
>>>>>