On Wed, 2021-06-02 at 14:31 -0400, schumaker.anna@xxxxxxxxx wrote:
> From: Anna Schumaker <Anna.Schumaker@xxxxxxxxxx>
>
> 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@xxxxxxxxxxxxxxx # 4.13+
> Signed-off-by: Anna Schumaker <Anna.Schumaker@xxxxxxxxxx>
> ---
> 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())?
> return old;
>
> error:
--
Trond Myklebust
Linux NFS client maintainer, Hammerspace
trond.myklebust@xxxxxxxxxxxxxxx
