All of lore.kernel.org
 help / color / mirror / Atom feed
* BUG: Dentry still in use [unmount of nfsd nfsd]
@ 2019-08-12  2:07 syzbot
  2019-08-12  2:16 ` [PATCH] nfsd: fix dentry leak upon mkdir failure Tetsuo Handa
  0 siblings, 1 reply; 6+ messages in thread
From: syzbot @ 2019-08-12  2:07 UTC (permalink / raw)
  To: linux-fsdevel, linux-kernel, syzkaller-bugs, viro

Hello,

syzbot found the following crash on:

HEAD commit:    f4eb1423 Merge branch 'for-linus' of git://git.kernel.org/..
git tree:       upstream
console output: https://syzkaller.appspot.com/x/log.txt?x=11ab1c74600000
kernel config:  https://syzkaller.appspot.com/x/.config?x=a4c9e9f08e9e8960
dashboard link: https://syzkaller.appspot.com/bug?extid=2c95195d5d433f6ed6cb
compiler:       gcc (GCC) 9.0.0 20181231 (experimental)

Unfortunately, I don't have any reproducer for this crash yet.

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

RBP: 000000000075bf20 R08: 0000000000000000 R09: 0000000000000000
R10: 0000000000000000 R11: 0000000000000246 R12: 00007fa0af7a86d4
R13: 00000000004c018f R14: 00000000004d2228 R15: 0000000000000004
BUG: Dentry 00000000117d3c54{i=0,n=clients}  still in use (1) [unmount of  
nfsd nfsd]
------------[ cut here ]------------
WARNING: CPU: 1 PID: 18817 at fs/dcache.c:1595 umount_check  
fs/dcache.c:1595 [inline]
WARNING: CPU: 1 PID: 18817 at fs/dcache.c:1595 umount_check.cold+0xf5/0x116  
fs/dcache.c:1576
Kernel panic - not syncing: panic_on_warn set ...
CPU: 1 PID: 18817 Comm: syz-executor.3 Not tainted 5.3.0-rc3+ #100
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+0x2dc/0x755 kernel/panic.c:219
  __warn.cold+0x20/0x4c kernel/panic.c:576
  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+0x23/0x30 arch/x86/entry/entry_64.S:1028
RIP: 0010:umount_check fs/dcache.c:1595 [inline]
RIP: 0010:umount_check.cold+0xf5/0x116 fs/dcache.c:1576
Code: 00 00 45 89 e8 4c 89 e1 53 4d 8b 0f 4c 89 f2 4c 89 e6 48 c7 c7 80 97  
96 87 e8 61 a6 9f ff 48 c7 c7 00 98 96 87 e8 55 a6 9f ff <0f> 0b 58 e9 b1  
15 ff ff e8 24 1f f0 ff e9 1d ff ff ff 45 31 f6 e9
RSP: 0018:ffff888060187b88 EFLAGS: 00010282
RAX: 0000000000000024 RBX: ffff8880624f8bb8 RCX: 0000000000000000
RDX: 0000000000000000 RSI: ffffffff815c3ba6 RDI: ffffed100c030f63
RBP: ffff888060187bb8 R08: 0000000000000024 R09: fffffbfff11b42c5
R10: fffffbfff11b42c4 R11: ffffffff88da1623 R12: ffff88803f50fa20
R13: 0000000000000001 R14: 0000000000000000 R15: ffffffff88f75220
  d_walk+0x283/0x950 fs/dcache.c:1305
  do_one_tree+0x28/0x40 fs/dcache.c:1602
  shrink_dcache_for_umount+0x72/0x170 fs/dcache.c:1618
  generic_shutdown_super+0x6d/0x370 fs/super.c:443
  kill_anon_super+0x3e/0x60 fs/super.c:1102
  kill_litter_super+0x50/0x60 fs/super.c:1111
  nfsd_umount+0x3f/0x90 fs/nfsd/nfsctl.c:1416
  deactivate_locked_super+0x95/0x100 fs/super.c:331
  vfs_get_super+0x210/0x270 fs/super.c:1185
  nfsd_fs_get_tree+0x7a/0x90 fs/nfsd/nfsctl.c:1390
  vfs_get_tree+0x8e/0x390 fs/super.c:1413
  vfs_fsconfig_locked+0x236/0x3d0 fs/fsopen.c:232
  __do_sys_fsconfig fs/fsopen.c:445 [inline]
  __se_sys_fsconfig fs/fsopen.c:314 [inline]
  __x64_sys_fsconfig+0x8e0/0xa40 fs/fsopen.c:314
  do_syscall_64+0xfd/0x6a0 arch/x86/entry/common.c:296
  entry_SYSCALL_64_after_hwframe+0x49/0xbe
RIP: 0033:0x459829
Code: fd b7 fb ff c3 66 2e 0f 1f 84 00 00 00 00 00 66 90 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 cb b7 fb ff c3 66 2e 0f 1f 84 00 00 00 00
RSP: 002b:00007fa0af7a7c78 EFLAGS: 00000246 ORIG_RAX: 00000000000001af
RAX: ffffffffffffffda RBX: 00007fa0af7a7c90 RCX: 0000000000459829
RDX: 0000000000000000 RSI: 0000000000000006 RDI: 0000000000000003
RBP: 000000000075bf20 R08: 0000000000000000 R09: 0000000000000000
R10: 0000000000000000 R11: 0000000000000246 R12: 00007fa0af7a86d4
R13: 00000000004c018f R14: 00000000004d2228 R15: 0000000000000004
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.

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH] nfsd: fix dentry leak upon mkdir failure.
  2019-08-12  2:07 BUG: Dentry still in use [unmount of nfsd nfsd] syzbot
@ 2019-08-12  2:16 ` Tetsuo Handa
  2019-08-12  3:03   ` Al Viro
  2019-08-15 19:59   ` J. Bruce Fields
  0 siblings, 2 replies; 6+ messages in thread
From: Tetsuo Handa @ 2019-08-12  2:16 UTC (permalink / raw)
  To: viro; +Cc: linux-fsdevel, Tetsuo Handa, J. Bruce Fields, syzbot

syzbot is reporting that nfsd_mkdir() forgot to remove dentry created by
d_alloc_name() when __nfsd_mkdir() failed (due to memory allocation fault
injection) [1].

[1] https://syzkaller.appspot.com/bug?id=ce41a1f769ea4637ebffedf004a803e8405b4674

Signed-off-by: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>
Reported-by: syzbot <syzbot+2c95195d5d433f6ed6cb@syzkaller.appspotmail.com>
Fixes: e8a79fb14f6b76b5 ("nfsd: add nfsd/clients directory")
Cc: J. Bruce Fields <bfields@redhat.com>
---
 fs/nfsd/nfsctl.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/fs/nfsd/nfsctl.c b/fs/nfsd/nfsctl.c
index 13c5487..e32dc1c 100644
--- a/fs/nfsd/nfsctl.c
+++ b/fs/nfsd/nfsctl.c
@@ -1176,8 +1176,10 @@ static int __nfsd_mkdir(struct inode *dir, struct dentry *dentry, umode_t mode)
 	struct inode *inode;
 
 	inode = nfsd_get_inode(dir->i_sb, mode);
-	if (!inode)
+	if (!inode) {
+		dput(dentry);
 		return -ENOMEM;
+	}
 	d_add(dentry, inode);
 	inc_nlink(dir);
 	fsnotify_mkdir(dir, dentry);
-- 
1.8.3.1


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH] nfsd: fix dentry leak upon mkdir failure.
  2019-08-12  2:16 ` [PATCH] nfsd: fix dentry leak upon mkdir failure Tetsuo Handa
@ 2019-08-12  3:03   ` Al Viro
  2019-08-15 20:23     ` J. Bruce Fields
  2019-08-15 19:59   ` J. Bruce Fields
  1 sibling, 1 reply; 6+ messages in thread
From: Al Viro @ 2019-08-12  3:03 UTC (permalink / raw)
  To: Tetsuo Handa; +Cc: linux-fsdevel, J. Bruce Fields, syzbot

On Mon, Aug 12, 2019 at 11:16:11AM +0900, Tetsuo Handa wrote:
> syzbot is reporting that nfsd_mkdir() forgot to remove dentry created by
> d_alloc_name() when __nfsd_mkdir() failed (due to memory allocation fault
> injection) [1].

That's not the only problem I see there.
        ret = __nfsd_mkdir(d_inode(parent), dentry, S_IFDIR | 0600);
        if (ret)
                goto out_err;
        if (ncl) {
                d_inode(dentry)->i_private = ncl;
                kref_get(&ncl->cl_ref);
        }
and we are doing that to inode *after* it has become visible via dcache -
__nfsd_mkdir() has already done d_add() (and pinged f-snotify, at that).
Looks fishy...

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] nfsd: fix dentry leak upon mkdir failure.
  2019-08-12  2:16 ` [PATCH] nfsd: fix dentry leak upon mkdir failure Tetsuo Handa
  2019-08-12  3:03   ` Al Viro
@ 2019-08-15 19:59   ` J. Bruce Fields
  2019-08-15 20:53     ` Tetsuo Handa
  1 sibling, 1 reply; 6+ messages in thread
From: J. Bruce Fields @ 2019-08-15 19:59 UTC (permalink / raw)
  To: Tetsuo Handa; +Cc: viro, linux-fsdevel, J. Bruce Fields, syzbot

On Mon, Aug 12, 2019 at 11:16:11AM +0900, Tetsuo Handa wrote:
> syzbot is reporting that nfsd_mkdir() forgot to remove dentry created by
> d_alloc_name() when __nfsd_mkdir() failed (due to memory allocation fault
> injection) [1].

Thanks!  But it might be clearer to do this in the caller, in the same
place the dentry was allocated?

--b.

commit d6846bfbeeac
Author: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>
Date:   Mon Aug 12 11:16:11 2019 +0900

    nfsd: fix dentry leak upon mkdir failure.
    
    syzbot is reporting that nfsd_mkdir() forgot to remove dentry created by
    d_alloc_name() when __nfsd_mkdir() failed (due to memory allocation fault
    injection) [1].
    
    [1] https://syzkaller.appspot.com/bug?id=ce41a1f769ea4637ebffedf004a803e8405b4674
    
    Signed-off-by: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>
    Reported-by: syzbot <syzbot+2c95195d5d433f6ed6cb@syzkaller.appspotmail.com>
    Fixes: e8a79fb14f6b76b5 ("nfsd: add nfsd/clients directory")
    [bfields: clean up in nfsd_mkdir instead of __nfsd_mkdir]
    Signed-off-by: J. Bruce Fields <bfields@redhat.com>

diff --git a/fs/nfsd/nfsctl.c b/fs/nfsd/nfsctl.c
index 13c548733860..928a0b2c05dc 100644
--- a/fs/nfsd/nfsctl.c
+++ b/fs/nfsd/nfsctl.c
@@ -1205,6 +1205,7 @@ static struct dentry *nfsd_mkdir(struct dentry *parent, struct nfsdfs_client *nc
 	inode_unlock(dir);
 	return dentry;
 out_err:
+	dput(dentry);
 	dentry = ERR_PTR(ret);
 	goto out;
 }

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH] nfsd: fix dentry leak upon mkdir failure.
  2019-08-12  3:03   ` Al Viro
@ 2019-08-15 20:23     ` J. Bruce Fields
  0 siblings, 0 replies; 6+ messages in thread
From: J. Bruce Fields @ 2019-08-15 20:23 UTC (permalink / raw)
  To: Al Viro; +Cc: Tetsuo Handa, linux-fsdevel, J. Bruce Fields, syzbot

On Mon, Aug 12, 2019 at 04:03:54AM +0100, Al Viro wrote:
> On Mon, Aug 12, 2019 at 11:16:11AM +0900, Tetsuo Handa wrote:
> > syzbot is reporting that nfsd_mkdir() forgot to remove dentry created by
> > d_alloc_name() when __nfsd_mkdir() failed (due to memory allocation fault
> > injection) [1].
> 
> That's not the only problem I see there.
>         ret = __nfsd_mkdir(d_inode(parent), dentry, S_IFDIR | 0600);
>         if (ret)
>                 goto out_err;
>         if (ncl) {
>                 d_inode(dentry)->i_private = ncl;
>                 kref_get(&ncl->cl_ref);
>         }
> and we are doing that to inode *after* it has become visible via dcache -
> __nfsd_mkdir() has already done d_add() (and pinged f-snotify, at that).
> Looks fishy...

Whoops, thanks.  Considering the following (untested).

--b.

commit 930f7dd94cc2
Author: J. Bruce Fields <bfields@redhat.com>
Date:   Thu Aug 15 16:18:26 2019 -0400

    nfsd: initialize i_private before d_add
    
    A process could race in an open and attempt to read one of these files
    before i_private is initialized, and get a spurious error.
    
    Reported-by: Al Viro <viro@zeniv.linux.org.uk>
    Signed-off-by: J. Bruce Fields <bfields@redhat.com>

diff --git a/fs/nfsd/nfsctl.c b/fs/nfsd/nfsctl.c
index b14f825c62fe..3cf4f6aa48d6 100644
--- a/fs/nfsd/nfsctl.c
+++ b/fs/nfsd/nfsctl.c
@@ -1171,13 +1171,17 @@ static struct inode *nfsd_get_inode(struct super_block *sb, umode_t mode)
 	return inode;
 }
 
-static int __nfsd_mkdir(struct inode *dir, struct dentry *dentry, umode_t mode)
+static int __nfsd_mkdir(struct inode *dir, struct dentry *dentry, umode_t mode, struct nfsdfs_client *ncl)
 {
 	struct inode *inode;
 
 	inode = nfsd_get_inode(dir->i_sb, mode);
 	if (!inode)
 		return -ENOMEM;
+	if (ncl) {
+		inode->i_private = ncl;
+		kref_get(&ncl->cl_ref);
+	}
 	d_add(dentry, inode);
 	inc_nlink(dir);
 	fsnotify_mkdir(dir, dentry);
@@ -1194,13 +1198,9 @@ static struct dentry *nfsd_mkdir(struct dentry *parent, struct nfsdfs_client *nc
 	dentry = d_alloc_name(parent, name);
 	if (!dentry)
 		goto out_err;
-	ret = __nfsd_mkdir(d_inode(parent), dentry, S_IFDIR | 0600);
+	ret = __nfsd_mkdir(d_inode(parent), dentry, S_IFDIR | 0600, ncl);
 	if (ret)
 		goto out_err;
-	if (ncl) {
-		d_inode(dentry)->i_private = ncl;
-		kref_get(&ncl->cl_ref);
-	}
 out:
 	inode_unlock(dir);
 	return dentry;

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH] nfsd: fix dentry leak upon mkdir failure.
  2019-08-15 19:59   ` J. Bruce Fields
@ 2019-08-15 20:53     ` Tetsuo Handa
  0 siblings, 0 replies; 6+ messages in thread
From: Tetsuo Handa @ 2019-08-15 20:53 UTC (permalink / raw)
  To: bfields; +Cc: viro, linux-fsdevel, J. Bruce Fields, syzbot

J. Bruce Fields wrote:
> On Mon, Aug 12, 2019 at 11:16:11AM +0900, Tetsuo Handa wrote:
> > syzbot is reporting that nfsd_mkdir() forgot to remove dentry created by
> > d_alloc_name() when __nfsd_mkdir() failed (due to memory allocation fault
> > injection) [1].
> 
> Thanks!  But it might be clearer to do this in the caller, in the same
> place the dentry was allocated?

I'm fine with that.

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2019-08-15 20:54 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-08-12  2:07 BUG: Dentry still in use [unmount of nfsd nfsd] syzbot
2019-08-12  2:16 ` [PATCH] nfsd: fix dentry leak upon mkdir failure Tetsuo Handa
2019-08-12  3:03   ` Al Viro
2019-08-15 20:23     ` J. Bruce Fields
2019-08-15 19:59   ` J. Bruce Fields
2019-08-15 20:53     ` Tetsuo Handa

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.