WARNING: locking bug in nfs_get_client

WARNING: locking bug in nfs_get_client

[syzbot]

Hello,

syzbot found the following crash on:

HEAD commit:    31ccad9b Add linux-next specific files for 20190508
git tree:       linux-next
console output: https://syzkaller.appspot.com/x/log.txt?x=1082d2aca00000
kernel config:  https://syzkaller.appspot.com/x/.config?x=63cd766601c6c9fc
dashboard link: https://syzkaller.appspot.com/bug?extid=228a82b263b5da91883d
compiler:       gcc (GCC) 9.0.0 20181231 (experimental)
syz repro:      https://syzkaller.appspot.com/x/repro.syz?x=140fdce8a00000
C reproducer:   https://syzkaller.appspot.com/x/repro.c?x=134dce02a00000

IMPORTANT: if you fix the bug, please add the following tag to the commit:
Reported-by: syzbot+228a82b263b5da91883d@syzkaller.appspotmail.com

------------[ cut here ]------------
DEBUG_LOCKS_WARN_ON(depth <= 0)
WARNING: CPU: 1 PID: 7948 at kernel/locking/lockdep.c:4052 __lock_release  
kernel/locking/lockdep.c:4052 [inline]
WARNING: CPU: 1 PID: 7948 at kernel/locking/lockdep.c:4052  
lock_release+0x667/0xa00 kernel/locking/lockdep.c:4321
Kernel panic - not syncing: panic_on_warn set ...
CPU: 1 PID: 7948 Comm: syz-executor561 Not tainted 5.1.0-next-20190508 #3
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS  
Google 01/01/2011
Call Trace:
  __dump_stack lib/dump_stack.c:77 [inline]
  dump_stack+0x172/0x1f0 lib/dump_stack.c:113
  panic+0x2cb/0x75a kernel/panic.c:218
  __warn.cold+0x20/0x47 kernel/panic.c:575
  report_bug+0x263/0x2b0 lib/bug.c:186
  fixup_bug arch/x86/kernel/traps.c:179 [inline]
  fixup_bug arch/x86/kernel/traps.c:174 [inline]
  do_error_trap+0x11b/0x200 arch/x86/kernel/traps.c:272
  do_invalid_op+0x37/0x50 arch/x86/kernel/traps.c:291
  invalid_op+0x14/0x20 arch/x86/entry/entry_64.S:972
RIP: 0010:__lock_release kernel/locking/lockdep.c:4052 [inline]
RIP: 0010:lock_release+0x667/0xa00 kernel/locking/lockdep.c:4321
Code: 0f 85 a0 03 00 00 8b 35 a7 02 09 08 85 f6 75 23 48 c7 c6 e0 57 6b 87  
48 c7 c7 80 27 6b 87 4c 89 85 70 ff ff ff e8 37 7c eb ff <0f> 0b 4c 8b 85  
70 ff ff ff 4c 89 ea 4c 89 e6 4c 89 c7 e8 52 63 ff
RSP: 0018:ffff888095867730 EFLAGS: 00010082
RAX: 0000000000000000 RBX: 1ffff11012b0ceec RCX: 0000000000000000
RDX: 0000000000000000 RSI: ffffffff815b20c6 RDI: ffffed1012b0ced8
RBP: ffff8880958677e8 R08: ffff88809a5b83c0 R09: fffffbfff1133071
R10: fffffbfff1133070 R11: ffffffff88998383 R12: ffff888219eeb700
R13: ffffffff821668ab R14: ffffffff8a45bd40 R15: ffff8880958677c0
  __raw_spin_unlock include/linux/spinlock_api_smp.h:150 [inline]
  _raw_spin_unlock+0x1b/0x50 kernel/locking/spinlock.c:183
  spin_unlock include/linux/spinlock.h:378 [inline]
  nfs_get_client+0xc1b/0x1300 fs/nfs/client.c:412
  nfs_init_server+0x26f/0xea0 fs/nfs/client.c:675
  nfs_create_server+0x12b/0x6d0 fs/nfs/client.c:962
  nfs_try_mount+0x15a/0x910 fs/nfs/super.c:1882
  nfs_fs_mount+0x184a/0x2690 fs/nfs/super.c:2718
  legacy_get_tree+0x10a/0x220 fs/fs_context.c:665
  vfs_get_tree+0x93/0x3a0 fs/super.c:1476
  do_new_mount fs/namespace.c:2790 [inline]
  do_mount+0x138c/0x1c00 fs/namespace.c:3110
  ksys_mount+0xdb/0x150 fs/namespace.c:3319
  __do_sys_mount fs/namespace.c:3333 [inline]
  __se_sys_mount fs/namespace.c:3330 [inline]
  __x64_sys_mount+0xbe/0x150 fs/namespace.c:3330
  do_syscall_64+0x103/0x670 arch/x86/entry/common.c:298
  entry_SYSCALL_64_after_hwframe+0x49/0xbe
RIP: 0033:0x4467d9
Code: e8 5c b3 02 00 48 83 c4 18 c3 0f 1f 80 00 00 00 00 48 89 f8 48 89 f7  
48 89 d6 48 89 ca 4d 89 c2 4d 89 c8 4c 8b 4c 24 08 0f 05 <48> 3d 01 f0 ff  
ff 0f 83 0b 08 fc ff c3 66 2e 0f 1f 84 00 00 00 00
RSP: 002b:00007f54c53e0db8 EFLAGS: 00000246 ORIG_RAX: 00000000000000a5
RAX: ffffffffffffffda RBX: 00000000006dbc38 RCX: 00000000004467d9
RDX: 0000000020fb5ffc RSI: 0000000020343ff8 RDI: 0000000000000000
RBP: 00000000006dbc30 R08: 000000002000a000 R09: 0000000000000000
R10: 0000000000000000 R11: 0000000000000246 R12: 00000000006dbc3c
R13: 00007ffe22037e2f R14: 00007f54c53e19c0 R15: 20c49ba5e353f7cf
Kernel Offset: disabled
Rebooting in 86400 seconds..


---
This bug is generated by a bot. It may contain errors.
See https://goo.gl/tpsmEJ for more information about syzbot.
syzbot engineers can be reached at syzkaller@googlegroups.com.

syzbot will keep track of this bug report. See:
https://goo.gl/tpsmEJ#status for how to communicate with syzbot.
syzbot can test patches for this bug, for details see:
https://goo.gl/tpsmEJ#testing-patches

Re: WARNING: locking bug in nfs_get_client

[syzbot]

syzbot has bisected this bug to:

commit 950a578c6128c2886e295b9c7ecb0b6b22fcc92b
Author: Roberto Bergantinos Corpas <rbergant@redhat.com>
Date:   Thu Apr 25 13:36:51 2019 +0000

     NFS: make nfs_match_client killable

bisection log:  https://syzkaller.appspot.com/x/bisect.txt?x=11e40aaca00000
start commit:   31ccad9b Add linux-next specific files for 20190508
git tree:       linux-next
final crash:    https://syzkaller.appspot.com/x/report.txt?x=13e40aaca00000
console output: https://syzkaller.appspot.com/x/log.txt?x=15e40aaca00000
kernel config:  https://syzkaller.appspot.com/x/.config?x=63cd766601c6c9fc
dashboard link: https://syzkaller.appspot.com/bug?extid=228a82b263b5da91883d
syz repro:      https://syzkaller.appspot.com/x/repro.syz?x=140fdce8a00000
C reproducer:   https://syzkaller.appspot.com/x/repro.c?x=134dce02a00000

Reported-by: syzbot+228a82b263b5da91883d@syzkaller.appspotmail.com
Fixes: 950a578c6128 ("NFS: make nfs_match_client killable")

For information about bisection process see: https://goo.gl/tpsmEJ#bisection

Re: WARNING: locking bug in nfs_get_client

[Benjamin Coddington]

We need to return from nfs_match_client() while still holding the 
nfs_client_lock, or maybe fixed with ISERR_OR_NULL in nfs_match 
client().

Anna, would you like a patch to fix it, or can that commit be pulled out 
and fixed up at this point?

Ben


On 8 May 2019, at 20:17, syzbot wrote:

> Hello,
>
> syzbot found the following crash on:
>
> HEAD commit:    31ccad9b Add linux-next specific files for 20190508
> git tree:       linux-next
> console output: 
> https://syzkaller.appspot.com/x/log.txt?x=1082d2aca00000
> kernel config:  
> https://syzkaller.appspot.com/x/.config?x=63cd766601c6c9fc
> dashboard link: 
> https://syzkaller.appspot.com/bug?extid=228a82b263b5da91883d
> compiler:       gcc (GCC) 9.0.0 20181231 (experimental)
> syz repro:      
> https://syzkaller.appspot.com/x/repro.syz?x=140fdce8a00000
> C reproducer:   
> https://syzkaller.appspot.com/x/repro.c?x=134dce02a00000
>
> IMPORTANT: if you fix the bug, please add the following tag to the 
> commit:
> Reported-by: syzbot+228a82b263b5da91883d@syzkaller.appspotmail.com
>
> ------------[ cut here ]------------
> DEBUG_LOCKS_WARN_ON(depth <= 0)
> WARNING: CPU: 1 PID: 7948 at kernel/locking/lockdep.c:4052 
> __lock_release kernel/locking/lockdep.c:4052 [inline]
> WARNING: CPU: 1 PID: 7948 at kernel/locking/lockdep.c:4052 
> lock_release+0x667/0xa00 kernel/locking/lockdep.c:4321
> Kernel panic - not syncing: panic_on_warn set ...
> CPU: 1 PID: 7948 Comm: syz-executor561 Not tainted 5.1.0-next-20190508 
> #3
> Hardware name: Google Google Compute Engine/Google Compute Engine, 
> BIOS Google 01/01/2011
> Call Trace:
>  __dump_stack lib/dump_stack.c:77 [inline]
>  dump_stack+0x172/0x1f0 lib/dump_stack.c:113
>  panic+0x2cb/0x75a kernel/panic.c:218
>  __warn.cold+0x20/0x47 kernel/panic.c:575
>  report_bug+0x263/0x2b0 lib/bug.c:186
>  fixup_bug arch/x86/kernel/traps.c:179 [inline]
>  fixup_bug arch/x86/kernel/traps.c:174 [inline]
>  do_error_trap+0x11b/0x200 arch/x86/kernel/traps.c:272
>  do_invalid_op+0x37/0x50 arch/x86/kernel/traps.c:291
>  invalid_op+0x14/0x20 arch/x86/entry/entry_64.S:972
> RIP: 0010:__lock_release kernel/locking/lockdep.c:4052 [inline]
> RIP: 0010:lock_release+0x667/0xa00 kernel/locking/lockdep.c:4321
> Code: 0f 85 a0 03 00 00 8b 35 a7 02 09 08 85 f6 75 23 48 c7 c6 e0 57 
> 6b 87 48 c7 c7 80 27 6b 87 4c 89 85 70 ff ff ff e8 37 7c eb ff <0f> 0b 
> 4c 8b 85 70 ff ff ff 4c 89 ea 4c 89 e6 4c 89 c7 e8 52 63 ff
> RSP: 0018:ffff888095867730 EFLAGS: 00010082
> RAX: 0000000000000000 RBX: 1ffff11012b0ceec RCX: 0000000000000000
> RDX: 0000000000000000 RSI: ffffffff815b20c6 RDI: ffffed1012b0ced8
> RBP: ffff8880958677e8 R08: ffff88809a5b83c0 R09: fffffbfff1133071
> R10: fffffbfff1133070 R11: ffffffff88998383 R12: ffff888219eeb700
> R13: ffffffff821668ab R14: ffffffff8a45bd40 R15: ffff8880958677c0
>  __raw_spin_unlock include/linux/spinlock_api_smp.h:150 [inline]
>  _raw_spin_unlock+0x1b/0x50 kernel/locking/spinlock.c:183
>  spin_unlock include/linux/spinlock.h:378 [inline]
>  nfs_get_client+0xc1b/0x1300 fs/nfs/client.c:412
>  nfs_init_server+0x26f/0xea0 fs/nfs/client.c:675
>  nfs_create_server+0x12b/0x6d0 fs/nfs/client.c:962
>  nfs_try_mount+0x15a/0x910 fs/nfs/super.c:1882
>  nfs_fs_mount+0x184a/0x2690 fs/nfs/super.c:2718
>  legacy_get_tree+0x10a/0x220 fs/fs_context.c:665
>  vfs_get_tree+0x93/0x3a0 fs/super.c:1476
>  do_new_mount fs/namespace.c:2790 [inline]
>  do_mount+0x138c/0x1c00 fs/namespace.c:3110
>  ksys_mount+0xdb/0x150 fs/namespace.c:3319
>  __do_sys_mount fs/namespace.c:3333 [inline]
>  __se_sys_mount fs/namespace.c:3330 [inline]
>  __x64_sys_mount+0xbe/0x150 fs/namespace.c:3330
>  do_syscall_64+0x103/0x670 arch/x86/entry/common.c:298
>  entry_SYSCALL_64_after_hwframe+0x49/0xbe
> RIP: 0033:0x4467d9
> Code: e8 5c b3 02 00 48 83 c4 18 c3 0f 1f 80 00 00 00 00 48 89 f8 48 
> 89 f7 48 89 d6 48 89 ca 4d 89 c2 4d 89 c8 4c 8b 4c 24 08 0f 05 <48> 3d 
> 01 f0 ff ff 0f 83 0b 08 fc ff c3 66 2e 0f 1f 84 00 00 00 00
> RSP: 002b:00007f54c53e0db8 EFLAGS: 00000246 ORIG_RAX: 00000000000000a5
> RAX: ffffffffffffffda RBX: 00000000006dbc38 RCX: 00000000004467d9
> RDX: 0000000020fb5ffc RSI: 0000000020343ff8 RDI: 0000000000000000
> RBP: 00000000006dbc30 R08: 000000002000a000 R09: 0000000000000000
> R10: 0000000000000000 R11: 0000000000000246 R12: 00000000006dbc3c
> R13: 00007ffe22037e2f R14: 00007f54c53e19c0 R15: 20c49ba5e353f7cf
> Kernel Offset: disabled
> Rebooting in 86400 seconds..
>
>
> ---
> This bug is generated by a bot. It may contain errors.
> See https://goo.gl/tpsmEJ for more information about syzbot.
> syzbot engineers can be reached at syzkaller@googlegroups.com.
>
> syzbot will keep track of this bug report. See:
> https://goo.gl/tpsmEJ#status for how to communicate with syzbot.
> syzbot can test patches for this bug, for details see:
> https://goo.gl/tpsmEJ#testing-patches

Re: WARNING: locking bug in nfs_get_client

[Benjamin Coddington]

I think nfs_get_client and nfs_match_client could use a refactor.. but 
the
trivial fix is:

8<---------------------------------

 From 4ef2fc5912c5980890e781f8c0d941330254c100 Mon Sep 17 00:00:00 2001
Message-Id: 
<4ef2fc5912c5980890e781f8c0d941330254c100.1557401467.git.bcodding@redhat.com>
 From: Benjamin Coddington <bcodding@redhat.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@syzkaller.appspotmail.com
Signed-off-by: Benjamin Coddington <bcodding@redhat.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

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