Re: WARNING: locking bug in nfs_get_client

[Schumaker, Anna]

On Thu, 2019-05-09 at 07:37 -0400, Benjamin Coddington wrote:
> NetApp Security WARNING: This is an external email. Do not click links or open
> attachments unless you recognize the sender and know the content is safe.
> 
> 
> 
> 
> I think nfs_get_client and nfs_match_client could use a refactor.. but
> the
> trivial fix is:

Thanks, Ben! I'll go with this fix for now, and we can always clean up the
function later.

Anna

> 
> 8<---------------------------------
> 
>  From 4ef2fc5912c5980890e781f8c0d941330254c100 Mon Sep 17 00:00:00 2001
> Message-Id:
> <4ef2fc5912c5980890e781f8c0d941330254c100.1557401467.git.bcodding@xxxxxxxxxx>
>  From: Benjamin Coddington <bcodding@xxxxxxxxxx>
> Date: Thu, 9 May 2019 07:25:21 -0400
> Subject: [PATCH] NFS: Fix a double unlock from nfs_match,get_client
> 
> Now that nfs_match_client drops the nfs_client_lock, we should be
> careful
> to always return it in the same condition: locked.
> 
> Fixes: 950a578c6128 ("NFS: make nfs_match_client killable")
> Reported-by: syzbot+228a82b263b5da91883d@xxxxxxxxxxxxxxxxxxxxxxxxx
> Signed-off-by: Benjamin Coddington <bcodding@xxxxxxxxxx>
> ---
>   fs/nfs/client.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/fs/nfs/client.c b/fs/nfs/client.c
> index 06e8719655f0..da74c4c4a244 100644
> --- a/fs/nfs/client.c
> +++ b/fs/nfs/client.c
> @@ -299,9 +299,9 @@ static struct nfs_client *nfs_match_client(const
> struct nfs_client_initdata *dat
>                         spin_unlock(&nn->nfs_client_lock);
>                         error = nfs_wait_client_init_complete(clp);
>                         nfs_put_client(clp);
> +                       spin_lock(&nn->nfs_client_lock);
>                         if (error < 0)
>                                 return ERR_PTR(error);
> -                       spin_lock(&nn->nfs_client_lock);
>                         goto again;
>                 }
> 
> --
> 2.20.1