linux-kernel - Re: WARNING: locking bug in nfs_get

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Date:   Thu, 9 May 2019 12:06:44 +0000
From:   "Schumaker, Anna" <Anna.Schumaker@...app.com>
To:     "bcodding@...hat.com" <bcodding@...hat.com>
CC:     "syzkaller-bugs@...glegroups.com" <syzkaller-bugs@...glegroups.com>,
        "linux-nfs@...r.kernel.org" <linux-nfs@...r.kernel.org>,
        "syzbot+228a82b263b5da91883d@...kaller.appspotmail.com" 
        <syzbot+228a82b263b5da91883d@...kaller.appspotmail.com>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "trond.myklebust@...merspace.com" <trond.myklebust@...merspace.com>,
        "rbergant@...hat.com" <rbergant@...hat.com>
Subject: Re: WARNING: locking bug in nfs_get_client

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@...hat.com>
>  From: Benjamin Coddington <bcodding@...hat.com>
> 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@...kaller.appspotmail.com
> Signed-off-by: Benjamin Coddington <bcodding@...hat.com>
> ---
>   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