Re: [PATCH v2] NFS: Fix use-after-free in nfs4_init_client()

[Chuck Lever III <chuck.lever@oracle.com>]

> On Jun 2, 2021, at 2:52 PM, Trond Myklebust <trondmy@hammerspace.com> wrote:
> 
> On Wed, 2021-06-02 at 14:31 -0400, schumaker.anna@gmail.com wrote:
>> From: Anna Schumaker <Anna.Schumaker@Netapp.com>
>> 
>> KASAN reports a use-after-free when attempting to mount two different
>> exports through two different NICs that belong to the same server.
>> 
>> Olga was able to hit this with kernels starting somewhere between 5.7
>> and 5.10, but I traced the patch that introduced the clear_bit() call
>> to
>> 4.13. So something must have changed in the refcounting of the clp
>> pointer to make this call to nfs_put_client() the very last one.
>> 
>> Fixes: 8dcbec6d20 ("NFSv41: Handle EXCHID4_FLAG_CONFIRMED_R during
>> NFSv4.1 migration")
>> Cc: stable@vger.kernel.org # 4.13+
>> Signed-off-by: Anna Schumaker <Anna.Schumaker@Netapp.com>
>> ---
>> v2: No changes except adding the fixes tag that I initially forgot
>> ---
>>  fs/nfs/nfs4client.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>> 
>> diff --git a/fs/nfs/nfs4client.c b/fs/nfs/nfs4client.c
>> index 889a9f4c0310..42719384e25f 100644
>> --- a/fs/nfs/nfs4client.c
>> +++ b/fs/nfs/nfs4client.c
>> @@ -435,8 +435,8 @@ struct nfs_client *nfs4_init_client(struct
>> nfs_client *clp,
>>                  */
>>                 nfs_mark_client_ready(clp, -EPERM);
>>         }
>> -       nfs_put_client(clp);
>>         clear_bit(NFS_CS_TSM_POSSIBLE, &clp->cl_flags);
>> +       nfs_put_client(clp);
> 
> OK. I'm reading this, and it is not making sense to me. Why are we
> changing a flag on an object that is about to be destroyed by the
> nfs_put_client() anyway? Let's go back to the author of commit
> 8dcbec6d20 and ask him.
> 
> Chuck, is it possible that you were actually intending to clear
> NFS_CS_TSM_POSSIBLE on &old->cl_flags (i.e. on the object that is
> actually returned by the call to nfs4_init_client())?

Yes, that's plausible.


>>         return old;
>>  
>>  error:
> 
> -- 
> Trond Myklebust
> Linux NFS client maintainer, Hammerspace
> trond.myklebust@hammerspace.com
> 
> 

--
Chuck Lever