* 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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).