fs/nilfs2/inode.c | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-)
syzbot reported a WARNING in nilfs_rmdir. [1]
The inode is used twice by the same task to unmount and remove directories
".nilfs" and "file0", it trigger warning in nilfs_rmdir.
Avoid to this issue, check i_size and i_nlink in nilfs_iget(), if they are
both 0, it means that this inode has been removed, and iput is executed to
reclaim it.
[1]
WARNING: CPU: 1 PID: 5824 at fs/inode.c:407 drop_nlink+0xc4/0x110 fs/inode.c:407
Modules linked in:
CPU: 1 UID: 0 PID: 5824 Comm: syz-executor223 Not tainted 6.12.0-syzkaller-12113-gbcc8eda6d349 #0
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 09/13/2024
RIP: 0010:drop_nlink+0xc4/0x110 fs/inode.c:407
Code: bb 70 07 00 00 be 08 00 00 00 e8 57 0b e6 ff f0 48 ff 83 70 07 00 00 5b 41 5c 41 5e 41 5f 5d c3 cc cc cc cc e8 9d 4c 7e ff 90 <0f> 0b 90 eb 83 44 89 e1 80 e1 07 80 c1 03 38 c1 0f 8c 5c ff ff ff
RSP: 0018:ffffc900037f7c70 EFLAGS: 00010293
RAX: ffffffff822124a3 RBX: 1ffff1100e7ae034 RCX: ffff88807cf53c00
RDX: 0000000000000000 RSI: 0000000000000000 RDI: 0000000000000000
RBP: 0000000000000000 R08: ffffffff82212423 R09: 1ffff1100f8ba8ee
R10: dffffc0000000000 R11: ffffed100f8ba8ef R12: ffff888073d701a0
R13: 1ffff1100e79f5c4 R14: ffff888073d70158 R15: dffffc0000000000
FS: 0000555558d1e480(0000) GS:ffff8880b8700000(0000) knlGS:0000000000000000
CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
CR2: 0000555558d37878 CR3: 000000007d920000 CR4: 00000000003526f0
DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
Call Trace:
<TASK>
nilfs_rmdir+0x1b0/0x250 fs/nilfs2/namei.c:342
vfs_rmdir+0x3a3/0x510 fs/namei.c:4394
do_rmdir+0x3b5/0x580 fs/namei.c:4453
__do_sys_rmdir fs/namei.c:4472 [inline]
__se_sys_rmdir fs/namei.c:4470 [inline]
__x64_sys_rmdir+0x47/0x50 fs/namei.c:4470
do_syscall_x64 arch/x86/entry/common.c:52 [inline]
do_syscall_64+0xf3/0x230 arch/x86/entry/common.c:83
entry_SYSCALL_64_after_hwframe+0x77/0x7f
Reported-and-tested-by: syzbot+9260555647a5132edd48@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=9260555647a5132edd48
Signed-off-by: Edward Adam Davis <eadavis@qq.com>
---
fs/nilfs2/inode.c | 9 ++++++++-
1 file changed, 8 insertions(+), 1 deletion(-)
diff --git a/fs/nilfs2/inode.c b/fs/nilfs2/inode.c
index cf9ba481ae37..254a5e46f8ea 100644
--- a/fs/nilfs2/inode.c
+++ b/fs/nilfs2/inode.c
@@ -544,8 +544,15 @@ struct inode *nilfs_iget(struct super_block *sb, struct nilfs_root *root,
inode = nilfs_iget_locked(sb, root, ino);
if (unlikely(!inode))
return ERR_PTR(-ENOMEM);
- if (!(inode->i_state & I_NEW))
+
+ if (!(inode->i_state & I_NEW)) {
+ if (!inode->i_size && !inode->i_nlink) {
+ make_bad_inode(inode);
+ iput(inode);
+ return ERR_PTR(-EIO);
+ }
return inode;
+ }
err = __nilfs_read_inode(sb, root, ino, inode);
if (unlikely(err)) {
--
2.47.0On Thu, Dec 5, 2024 at 9:26 PM Edward Adam Davis wrote:
>
> syzbot reported a WARNING in nilfs_rmdir. [1]
>
> The inode is used twice by the same task to unmount and remove directories
> ".nilfs" and "file0", it trigger warning in nilfs_rmdir.
>
> Avoid to this issue, check i_size and i_nlink in nilfs_iget(), if they are
> both 0, it means that this inode has been removed, and iput is executed to
> reclaim it.
>
> [1]
> WARNING: CPU: 1 PID: 5824 at fs/inode.c:407 drop_nlink+0xc4/0x110 fs/inode.c:407
> Modules linked in:
> CPU: 1 UID: 0 PID: 5824 Comm: syz-executor223 Not tainted 6.12.0-syzkaller-12113-gbcc8eda6d349 #0
> Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 09/13/2024
> RIP: 0010:drop_nlink+0xc4/0x110 fs/inode.c:407
> Code: bb 70 07 00 00 be 08 00 00 00 e8 57 0b e6 ff f0 48 ff 83 70 07 00 00 5b 41 5c 41 5e 41 5f 5d c3 cc cc cc cc e8 9d 4c 7e ff 90 <0f> 0b 90 eb 83 44 89 e1 80 e1 07 80 c1 03 38 c1 0f 8c 5c ff ff ff
> RSP: 0018:ffffc900037f7c70 EFLAGS: 00010293
> RAX: ffffffff822124a3 RBX: 1ffff1100e7ae034 RCX: ffff88807cf53c00
> RDX: 0000000000000000 RSI: 0000000000000000 RDI: 0000000000000000
> RBP: 0000000000000000 R08: ffffffff82212423 R09: 1ffff1100f8ba8ee
> R10: dffffc0000000000 R11: ffffed100f8ba8ef R12: ffff888073d701a0
> R13: 1ffff1100e79f5c4 R14: ffff888073d70158 R15: dffffc0000000000
> FS: 0000555558d1e480(0000) GS:ffff8880b8700000(0000) knlGS:0000000000000000
> CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> CR2: 0000555558d37878 CR3: 000000007d920000 CR4: 00000000003526f0
> DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
> DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
> Call Trace:
> <TASK>
> nilfs_rmdir+0x1b0/0x250 fs/nilfs2/namei.c:342
> vfs_rmdir+0x3a3/0x510 fs/namei.c:4394
> do_rmdir+0x3b5/0x580 fs/namei.c:4453
> __do_sys_rmdir fs/namei.c:4472 [inline]
> __se_sys_rmdir fs/namei.c:4470 [inline]
> __x64_sys_rmdir+0x47/0x50 fs/namei.c:4470
> do_syscall_x64 arch/x86/entry/common.c:52 [inline]
> do_syscall_64+0xf3/0x230 arch/x86/entry/common.c:83
> entry_SYSCALL_64_after_hwframe+0x77/0x7f
>
> Reported-and-tested-by: syzbot+9260555647a5132edd48@syzkaller.appspotmail.com
> Closes: https://syzkaller.appspot.com/bug?extid=9260555647a5132edd48
> Signed-off-by: Edward Adam Davis <eadavis@qq.com>
> ---
> fs/nilfs2/inode.c | 9 ++++++++-
> 1 file changed, 8 insertions(+), 1 deletion(-)
>
> diff --git a/fs/nilfs2/inode.c b/fs/nilfs2/inode.c
> index cf9ba481ae37..254a5e46f8ea 100644
> --- a/fs/nilfs2/inode.c
> +++ b/fs/nilfs2/inode.c
> @@ -544,8 +544,15 @@ struct inode *nilfs_iget(struct super_block *sb, struct nilfs_root *root,
> inode = nilfs_iget_locked(sb, root, ino);
> if (unlikely(!inode))
> return ERR_PTR(-ENOMEM);
> - if (!(inode->i_state & I_NEW))
> +
> + if (!(inode->i_state & I_NEW)) {
> + if (!inode->i_size && !inode->i_nlink) {
> + make_bad_inode(inode);
> + iput(inode);
> + return ERR_PTR(-EIO);
> + }
> return inode;
> + }
>
> err = __nilfs_read_inode(sb, root, ino, inode);
> if (unlikely(err)) {
> --
> 2.47.0
Thank you Edward.
This fix seems good except for the i_size check, but I think we need
to look into what's going on a bit more.
I was unable to work for a while due to machine trouble, so I'd like
to know if you have made any progress on your investigation.
First, is this caused by a corrupted filesystem image? Or is it that
the directories or files with the same inode number were generated
during the namespace operations (due to a timing issue or something),
and could this problem occur even if the original filesystem image is
normal?
When I mounted the mount_0 image as read-only, the filesystem looked
normal without such inode duplication.
At least, nilfs_read_inode_common(), which reads inodes from block
devices, is implemented to return an error with -ESTALE if i_nlink ==
0. So it seems that nilfs_iget() picked up this inode with i_nlilnk
== 0 because it hit an inode being deleted in the inode cache. Why is
that happening?
Also, why do you put the i_size check as an AND condition?
i_size is independent of i_nlink and the inode lifecycles. If i_size
is also broken, this check will not work properly.
If something is not working and you have included it as a workaround,
I would like to know about it.
Thanks,
Ryusuke Konishi
On Fri, 6 Dec 2024 01:04:23 +0900, Ryusuke Konishi wrote:
> > syzbot reported a WARNING in nilfs_rmdir. [1]
> >
> > The inode is used twice by the same task to unmount and remove directories
> > ".nilfs" and "file0", it trigger warning in nilfs_rmdir.
> >
> > Avoid to this issue, check i_size and i_nlink in nilfs_iget(), if they are
> > both 0, it means that this inode has been removed, and iput is executed to
> > reclaim it.
> >
> > [1]
> > WARNING: CPU: 1 PID: 5824 at fs/inode.c:407 drop_nlink+0xc4/0x110 fs/inode.c:407
> > Modules linked in:
> > CPU: 1 UID: 0 PID: 5824 Comm: syz-executor223 Not tainted 6.12.0-syzkaller-12113-gbcc8eda6d349 #0
> > Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 09/13/2024
> > RIP: 0010:drop_nlink+0xc4/0x110 fs/inode.c:407
> > Code: bb 70 07 00 00 be 08 00 00 00 e8 57 0b e6 ff f0 48 ff 83 70 07 00 00 5b 41 5c 41 5e 41 5f 5d c3 cc cc cc cc e8 9d 4c 7e ff 90 <0f> 0b 90 eb 83 44 89 e1 80 e1 07 80 c1 03 38 c1 0f 8c 5c ff ff ff
> > RSP: 0018:ffffc900037f7c70 EFLAGS: 00010293
> > RAX: ffffffff822124a3 RBX: 1ffff1100e7ae034 RCX: ffff88807cf53c00
> > RDX: 0000000000000000 RSI: 0000000000000000 RDI: 0000000000000000
> > RBP: 0000000000000000 R08: ffffffff82212423 R09: 1ffff1100f8ba8ee
> > R10: dffffc0000000000 R11: ffffed100f8ba8ef R12: ffff888073d701a0
> > R13: 1ffff1100e79f5c4 R14: ffff888073d70158 R15: dffffc0000000000
> > FS: 0000555558d1e480(0000) GS:ffff8880b8700000(0000) knlGS:0000000000000000
> > CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> > CR2: 0000555558d37878 CR3: 000000007d920000 CR4: 00000000003526f0
> > DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
> > DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
> > Call Trace:
> > <TASK>
> > nilfs_rmdir+0x1b0/0x250 fs/nilfs2/namei.c:342
> > vfs_rmdir+0x3a3/0x510 fs/namei.c:4394
> > do_rmdir+0x3b5/0x580 fs/namei.c:4453
> > __do_sys_rmdir fs/namei.c:4472 [inline]
> > __se_sys_rmdir fs/namei.c:4470 [inline]
> > __x64_sys_rmdir+0x47/0x50 fs/namei.c:4470
> > do_syscall_x64 arch/x86/entry/common.c:52 [inline]
> > do_syscall_64+0xf3/0x230 arch/x86/entry/common.c:83
> > entry_SYSCALL_64_after_hwframe+0x77/0x7f
> >
> > Reported-and-tested-by: syzbot+9260555647a5132edd48@syzkaller.appspotmail.com
> > Closes: https://syzkaller.appspot.com/bug?extid=9260555647a5132edd48
> > Signed-off-by: Edward Adam Davis <eadavis@qq.com>
> > ---
> > fs/nilfs2/inode.c | 9 ++++++++-
> > 1 file changed, 8 insertions(+), 1 deletion(-)
> >
> > diff --git a/fs/nilfs2/inode.c b/fs/nilfs2/inode.c
> > index cf9ba481ae37..254a5e46f8ea 100644
> > --- a/fs/nilfs2/inode.c
> > +++ b/fs/nilfs2/inode.c
> > @@ -544,8 +544,15 @@ struct inode *nilfs_iget(struct super_block *sb, struct nilfs_root *root,
> > inode = nilfs_iget_locked(sb, root, ino);
> > if (unlikely(!inode))
> > return ERR_PTR(-ENOMEM);
> > - if (!(inode->i_state & I_NEW))
> > +
> > + if (!(inode->i_state & I_NEW)) {
> > + if (!inode->i_size && !inode->i_nlink) {
> > + make_bad_inode(inode);
> > + iput(inode);
> > + return ERR_PTR(-EIO);
> > + }
> > return inode;
> > + }
> >
> > err = __nilfs_read_inode(sb, root, ino, inode);
> > if (unlikely(err)) {
> > --
> > 2.47.0
>
> Thank you Edward.
>
> This fix seems good except for the i_size check, but I think we need
> to look into what's going on a bit more.
>
> I was unable to work for a while due to machine trouble, so I'd like
> to know if you have made any progress on your investigation.
>
> First, is this caused by a corrupted filesystem image? Or is it that
> the directories or files with the same inode number were generated
> during the namespace operations (due to a timing issue or something),
> and could this problem occur even if the original filesystem image is
> normal?
According to the log when I reproduced the problem, I analyzed that the
problem occurred like this:
CPU0 CPU1
==== ====
nilfs_mkdir // file0
nilfs_new_inode // ino is 11
mount // mount file0
umount // .nilfs, ino is 11
nilfs_rmdir // ino is 11, i_size = 0, i_nlink = 0
umount // file0, ino is 11
nilfs_rmdir // ino 11, i_size = 0, i_nlink = 0, trigger warning
>
> When I mounted the mount_0 image as read-only, the filesystem looked
> normal without such inode duplication.
>
> At least, nilfs_read_inode_common(), which reads inodes from block
> devices, is implemented to return an error with -ESTALE if i_nlink ==
> 0. So it seems that nilfs_iget() picked up this inode with i_nlilnk
> == 0 because it hit an inode being deleted in the inode cache. Why is
> that happening?
Are you talking about the following call trace?
If so, then because the value of inode->i_state is I_DIRTY (set in nilfs_mkdir)
it will not enter __nilfs_read_inode().
nilfs_iget()->
__nilfs_read_inode()->
nilfs_read_inode_common()
>
> Also, why do you put the i_size check as an AND condition?
i_size will set to 0 in nilfs_rmdir(), so check it too.
> i_size is independent of i_nlink and the inode lifecycles. If i_size
> is also broken, this check will not work properly.
> If something is not working and you have included it as a workaround,
> I would like to know about it.
BR,
Edward
On Fri, Dec 6, 2024 at 10:14 PM Edward Adam Davis wrote:
>
> On Fri, 6 Dec 2024 01:04:23 +0900, Ryusuke Konishi wrote:
> > > syzbot reported a WARNING in nilfs_rmdir. [1]
> > >
> > > The inode is used twice by the same task to unmount and remove directories
> > > ".nilfs" and "file0", it trigger warning in nilfs_rmdir.
> > >
> > > Avoid to this issue, check i_size and i_nlink in nilfs_iget(), if they are
> > > both 0, it means that this inode has been removed, and iput is executed to
> > > reclaim it.
> > >
> > > [1]
> > > WARNING: CPU: 1 PID: 5824 at fs/inode.c:407 drop_nlink+0xc4/0x110 fs/inode.c:407
> > > Modules linked in:
> > > CPU: 1 UID: 0 PID: 5824 Comm: syz-executor223 Not tainted 6.12.0-syzkaller-12113-gbcc8eda6d349 #0
> > > Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 09/13/2024
> > > RIP: 0010:drop_nlink+0xc4/0x110 fs/inode.c:407
> > > Code: bb 70 07 00 00 be 08 00 00 00 e8 57 0b e6 ff f0 48 ff 83 70 07 00 00 5b 41 5c 41 5e 41 5f 5d c3 cc cc cc cc e8 9d 4c 7e ff 90 <0f> 0b 90 eb 83 44 89 e1 80 e1 07 80 c1 03 38 c1 0f 8c 5c ff ff ff
> > > RSP: 0018:ffffc900037f7c70 EFLAGS: 00010293
> > > RAX: ffffffff822124a3 RBX: 1ffff1100e7ae034 RCX: ffff88807cf53c00
> > > RDX: 0000000000000000 RSI: 0000000000000000 RDI: 0000000000000000
> > > RBP: 0000000000000000 R08: ffffffff82212423 R09: 1ffff1100f8ba8ee
> > > R10: dffffc0000000000 R11: ffffed100f8ba8ef R12: ffff888073d701a0
> > > R13: 1ffff1100e79f5c4 R14: ffff888073d70158 R15: dffffc0000000000
> > > FS: 0000555558d1e480(0000) GS:ffff8880b8700000(0000) knlGS:0000000000000000
> > > CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> > > CR2: 0000555558d37878 CR3: 000000007d920000 CR4: 00000000003526f0
> > > DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
> > > DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
> > > Call Trace:
> > > <TASK>
> > > nilfs_rmdir+0x1b0/0x250 fs/nilfs2/namei.c:342
> > > vfs_rmdir+0x3a3/0x510 fs/namei.c:4394
> > > do_rmdir+0x3b5/0x580 fs/namei.c:4453
> > > __do_sys_rmdir fs/namei.c:4472 [inline]
> > > __se_sys_rmdir fs/namei.c:4470 [inline]
> > > __x64_sys_rmdir+0x47/0x50 fs/namei.c:4470
> > > do_syscall_x64 arch/x86/entry/common.c:52 [inline]
> > > do_syscall_64+0xf3/0x230 arch/x86/entry/common.c:83
> > > entry_SYSCALL_64_after_hwframe+0x77/0x7f
> > >
> > > Reported-and-tested-by: syzbot+9260555647a5132edd48@syzkaller.appspotmail.com
> > > Closes: https://syzkaller.appspot.com/bug?extid=9260555647a5132edd48
> > > Signed-off-by: Edward Adam Davis <eadavis@qq.com>
> > > ---
> > > fs/nilfs2/inode.c | 9 ++++++++-
> > > 1 file changed, 8 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/fs/nilfs2/inode.c b/fs/nilfs2/inode.c
> > > index cf9ba481ae37..254a5e46f8ea 100644
> > > --- a/fs/nilfs2/inode.c
> > > +++ b/fs/nilfs2/inode.c
> > > @@ -544,8 +544,15 @@ struct inode *nilfs_iget(struct super_block *sb, struct nilfs_root *root,
> > > inode = nilfs_iget_locked(sb, root, ino);
> > > if (unlikely(!inode))
> > > return ERR_PTR(-ENOMEM);
> > > - if (!(inode->i_state & I_NEW))
> > > +
> > > + if (!(inode->i_state & I_NEW)) {
> > > + if (!inode->i_size && !inode->i_nlink) {
> > > + make_bad_inode(inode);
> > > + iput(inode);
> > > + return ERR_PTR(-EIO);
> > > + }
> > > return inode;
> > > + }
> > >
> > > err = __nilfs_read_inode(sb, root, ino, inode);
> > > if (unlikely(err)) {
> > > --
> > > 2.47.0
> >
> > Thank you Edward.
> >
> > This fix seems good except for the i_size check, but I think we need
> > to look into what's going on a bit more.
> >
> > I was unable to work for a while due to machine trouble, so I'd like
> > to know if you have made any progress on your investigation.
> >
> > First, is this caused by a corrupted filesystem image? Or is it that
> > the directories or files with the same inode number were generated
> > during the namespace operations (due to a timing issue or something),
> > and could this problem occur even if the original filesystem image is
> > normal?
> According to the log when I reproduced the problem, I analyzed that the
> problem occurred like this:
>
> CPU0 CPU1
> ==== ====
> nilfs_mkdir // file0
> nilfs_new_inode // ino is 11
> mount // mount file0
> umount // .nilfs, ino is 11
> nilfs_rmdir // ino is 11, i_size = 0, i_nlink = 0
> umount // file0, ino is 11
> nilfs_rmdir // ino 11, i_size = 0, i_nlink = 0, trigger warning
>
Thank you for explaining the situation. I understand what's going on.
In the end, going back to my question, this is caused by file system corruption.
Because the inode bitmap is corrupted, an inode with an inode number
that should exist as a ".nilfs" file was reassigned by nilfs_mkdir for
"file0", causing an inode duplication during execution.
And this causes an underflow of i_nlink in rmdir operations.
After considering the issue, I came to the conclusion that although
there is an approach that strengthens checks for bitmap
inconsistencies and inode type inconsistencies to detect errors in
advance, these approaches are difficult to fundamentally solve.
This is caused by a corrupted inode bitmap, but checking is difficult
in the first place when there are multiple directories with the same
inode number in the name space.
Therefore, the appropriate solution is to either check for i_nlink
underflow during rmdir, or to detect i_nlink == 0 in the call path of
nilfs_lookup() --> nilfs_iget(), and I think the latter approach is
better, as you have chosen.
> >
> > When I mounted the mount_0 image as read-only, the filesystem looked
> > normal without such inode duplication.
> >
> > At least, nilfs_read_inode_common(), which reads inodes from block
> > devices, is implemented to return an error with -ESTALE if i_nlink ==
> > 0. So it seems that nilfs_iget() picked up this inode with i_nlilnk
> > == 0 because it hit an inode being deleted in the inode cache. Why is
> > that happening?
> Are you talking about the following call trace?
> If so, then because the value of inode->i_state is I_DIRTY (set in nilfs_mkdir)
> it will not enter __nilfs_read_inode().
>
> nilfs_iget()->
> __nilfs_read_inode()->
> nilfs_read_inode_common()
Yes, in this case __nifls_read_inode() is not called. As mentioned
above, the conclusion is that i_nlink abnormalities can occur in other
FS corruption patterns such as inode bitmap corruption and directory
inconsistency.
> >
> > Also, why do you put the i_size check as an AND condition?
> i_size will set to 0 in nilfs_rmdir(), so check it too.
> > i_size is independent of i_nlink and the inode lifecycles. If i_size
> > is also broken, this check will not work properly.
> > If something is not working and you have included it as a workaround,
> > I would like to know about it.
To fix the problem, please modify the patch as follows:
(1) In nilfs_iget(), if the inode without I_NEW obtained by
nilfs_iget_locked() has i_nlink == 0, call iput() and return
ERR_PTR(-ESTALE). Do not call make_bad_inode(). Also do not check for
i_size == 0 (it is not a good idea to check this to identify the case
of rmdir).
(2) In nilfs_lookup(), if the return value of nilfs_iget() is
ERR_PTR(-ESTALE), output a message with nilfs_error() indicating that
file system corruption has been detected , and returns ERR_PTR(-EIO).
Please refer to ext2_lookup() for the concrete implementation.
The reason for not calling make_bad_inode() is to prevent interference
from the side when i_nlink is set to 0 and nilfs_evict() is trying to
delete the inode. The VFS layer guarantees that inode acquisition and
evict are mutually exclusive, but it is possible to grab it before
evict(), and the result of interference in that case is unpredictable.
Instead, as mentioned above, I think it is safer to call nilfs_error()
at the nilfs_lookup() level, where namespace operations are involved.
Also, could you please explain in the commit log that inode
duplication has occurred due to file system corruption (in this case,
inode bitmap corruption), and that when inode duplication occurs due
to file system inconsistencies like this, a link count underflow can
occur during an rmdir operation, so that a link count check is
necessary at runtime?
Thank you in advance.
Ryusuke Konishi
syzbot reported a WARNING in nilfs_rmdir. [1]
Because the inode bitmap is corrupted, an inode with an inode number
that should exist as a ".nilfs" file was reassigned by nilfs_mkdir for
"file0", causing an inode duplication during execution.
And this causes an underflow of i_nlink in rmdir operations.
Avoid to this issue, check i_nlink in nilfs_iget(), if it is 0, it means
that this inode has been deleted, and iput is executed to reclaim it.
[1]
WARNING: CPU: 1 PID: 5824 at fs/inode.c:407 drop_nlink+0xc4/0x110 fs/inode.c:407
Modules linked in:
CPU: 1 UID: 0 PID: 5824 Comm: syz-executor223 Not tainted 6.12.0-syzkaller-12113-gbcc8eda6d349 #0
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 09/13/2024
RIP: 0010:drop_nlink+0xc4/0x110 fs/inode.c:407
Code: bb 70 07 00 00 be 08 00 00 00 e8 57 0b e6 ff f0 48 ff 83 70 07 00 00 5b 41 5c 41 5e 41 5f 5d c3 cc cc cc cc e8 9d 4c 7e ff 90 <0f> 0b 90 eb 83 44 89 e1 80 e1 07 80 c1 03 38 c1 0f 8c 5c ff ff ff
RSP: 0018:ffffc900037f7c70 EFLAGS: 00010293
RAX: ffffffff822124a3 RBX: 1ffff1100e7ae034 RCX: ffff88807cf53c00
RDX: 0000000000000000 RSI: 0000000000000000 RDI: 0000000000000000
RBP: 0000000000000000 R08: ffffffff82212423 R09: 1ffff1100f8ba8ee
R10: dffffc0000000000 R11: ffffed100f8ba8ef R12: ffff888073d701a0
R13: 1ffff1100e79f5c4 R14: ffff888073d70158 R15: dffffc0000000000
FS: 0000555558d1e480(0000) GS:ffff8880b8700000(0000) knlGS:0000000000000000
CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
CR2: 0000555558d37878 CR3: 000000007d920000 CR4: 00000000003526f0
DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
Call Trace:
<TASK>
nilfs_rmdir+0x1b0/0x250 fs/nilfs2/namei.c:342
vfs_rmdir+0x3a3/0x510 fs/namei.c:4394
do_rmdir+0x3b5/0x580 fs/namei.c:4453
__do_sys_rmdir fs/namei.c:4472 [inline]
__se_sys_rmdir fs/namei.c:4470 [inline]
__x64_sys_rmdir+0x47/0x50 fs/namei.c:4470
do_syscall_x64 arch/x86/entry/common.c:52 [inline]
do_syscall_64+0xf3/0x230 arch/x86/entry/common.c:83
entry_SYSCALL_64_after_hwframe+0x77/0x7f
Reported-and-tested-by: syzbot+9260555647a5132edd48@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=9260555647a5132edd48
Signed-off-by: Edward Adam Davis <eadavis@qq.com>
---
V1 -> V2: Adjust the patch as suggested by Ryusuke Konishi
fs/nilfs2/inode.c | 8 +++++++-
fs/nilfs2/namei.c | 6 ++++++
2 files changed, 13 insertions(+), 1 deletion(-)
diff --git a/fs/nilfs2/inode.c b/fs/nilfs2/inode.c
index cf9ba481ae37..b7d4105f37bf 100644
--- a/fs/nilfs2/inode.c
+++ b/fs/nilfs2/inode.c
@@ -544,8 +544,14 @@ struct inode *nilfs_iget(struct super_block *sb, struct nilfs_root *root,
inode = nilfs_iget_locked(sb, root, ino);
if (unlikely(!inode))
return ERR_PTR(-ENOMEM);
- if (!(inode->i_state & I_NEW))
+
+ if (!(inode->i_state & I_NEW)) {
+ if (!inode->i_nlink) {
+ iput(inode);
+ return ERR_PTR(-ESTALE);
+ }
return inode;
+ }
err = __nilfs_read_inode(sb, root, ino, inode);
if (unlikely(err)) {
diff --git a/fs/nilfs2/namei.c b/fs/nilfs2/namei.c
index 9b108052d9f7..7037f47c454f 100644
--- a/fs/nilfs2/namei.c
+++ b/fs/nilfs2/namei.c
@@ -67,6 +67,12 @@ nilfs_lookup(struct inode *dir, struct dentry *dentry, unsigned int flags)
inode = NULL;
} else {
inode = nilfs_iget(dir->i_sb, NILFS_I(dir)->i_root, ino);
+ if (inode == ERR_PTR(-ESTALE)) {
+ nilfs_error(dir->i_sb, __func__,
+ "deleted inode referenced: %lu",
+ (unsigned long) ino);
+ return ERR_PTR(-EIO);
+ }
}
return d_splice_alias(inode, dentry);
--
2.47.0On Sun, Dec 8, 2024 at 12:24 PM Edward Adam Davis wrote:
>
> syzbot reported a WARNING in nilfs_rmdir. [1]
>
> Because the inode bitmap is corrupted, an inode with an inode number
> that should exist as a ".nilfs" file was reassigned by nilfs_mkdir for
> "file0", causing an inode duplication during execution.
> And this causes an underflow of i_nlink in rmdir operations.
>
> Avoid to this issue, check i_nlink in nilfs_iget(), if it is 0, it means
> that this inode has been deleted, and iput is executed to reclaim it.
>
> [1]
> WARNING: CPU: 1 PID: 5824 at fs/inode.c:407 drop_nlink+0xc4/0x110 fs/inode.c:407
> Modules linked in:
> CPU: 1 UID: 0 PID: 5824 Comm: syz-executor223 Not tainted 6.12.0-syzkaller-12113-gbcc8eda6d349 #0
> Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 09/13/2024
> RIP: 0010:drop_nlink+0xc4/0x110 fs/inode.c:407
> Code: bb 70 07 00 00 be 08 00 00 00 e8 57 0b e6 ff f0 48 ff 83 70 07 00 00 5b 41 5c 41 5e 41 5f 5d c3 cc cc cc cc e8 9d 4c 7e ff 90 <0f> 0b 90 eb 83 44 89 e1 80 e1 07 80 c1 03 38 c1 0f 8c 5c ff ff ff
> RSP: 0018:ffffc900037f7c70 EFLAGS: 00010293
> RAX: ffffffff822124a3 RBX: 1ffff1100e7ae034 RCX: ffff88807cf53c00
> RDX: 0000000000000000 RSI: 0000000000000000 RDI: 0000000000000000
> RBP: 0000000000000000 R08: ffffffff82212423 R09: 1ffff1100f8ba8ee
> R10: dffffc0000000000 R11: ffffed100f8ba8ef R12: ffff888073d701a0
> R13: 1ffff1100e79f5c4 R14: ffff888073d70158 R15: dffffc0000000000
> FS: 0000555558d1e480(0000) GS:ffff8880b8700000(0000) knlGS:0000000000000000
> CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> CR2: 0000555558d37878 CR3: 000000007d920000 CR4: 00000000003526f0
> DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
> DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
> Call Trace:
> <TASK>
> nilfs_rmdir+0x1b0/0x250 fs/nilfs2/namei.c:342
> vfs_rmdir+0x3a3/0x510 fs/namei.c:4394
> do_rmdir+0x3b5/0x580 fs/namei.c:4453
> __do_sys_rmdir fs/namei.c:4472 [inline]
> __se_sys_rmdir fs/namei.c:4470 [inline]
> __x64_sys_rmdir+0x47/0x50 fs/namei.c:4470
> do_syscall_x64 arch/x86/entry/common.c:52 [inline]
> do_syscall_64+0xf3/0x230 arch/x86/entry/common.c:83
> entry_SYSCALL_64_after_hwframe+0x77/0x7f
>
I'll make a few comments.
> Reported-and-tested-by: syzbot+9260555647a5132edd48@syzkaller.appspotmail.com
First please separate "Reported-and-tested-by" into two tags.
Although it is still seen occasionally, it causes warnings for the
checkpatch script. (It has already been explicitly deprecated in some
subtrees, because it just complicates automatic tag extraction.)
> Closes: https://syzkaller.appspot.com/bug?extid=9260555647a5132edd48
> Signed-off-by: Edward Adam Davis <eadavis@qq.com>
> ---
> V1 -> V2: Adjust the patch as suggested by Ryusuke Konishi
>
> fs/nilfs2/inode.c | 8 +++++++-
> fs/nilfs2/namei.c | 6 ++++++
> 2 files changed, 13 insertions(+), 1 deletion(-)
>
> diff --git a/fs/nilfs2/inode.c b/fs/nilfs2/inode.c
> index cf9ba481ae37..b7d4105f37bf 100644
> --- a/fs/nilfs2/inode.c
> +++ b/fs/nilfs2/inode.c
> @@ -544,8 +544,14 @@ struct inode *nilfs_iget(struct super_block *sb, struct nilfs_root *root,
> inode = nilfs_iget_locked(sb, root, ino);
> if (unlikely(!inode))
> return ERR_PTR(-ENOMEM);
> - if (!(inode->i_state & I_NEW))
> +
> + if (!(inode->i_state & I_NEW)) {
> + if (!inode->i_nlink) {
> + iput(inode);
> + return ERR_PTR(-ESTALE);
> + }
> return inode;
> + }
>
> err = __nilfs_read_inode(sb, root, ino, inode);
> if (unlikely(err)) {
> diff --git a/fs/nilfs2/namei.c b/fs/nilfs2/namei.c
> index 9b108052d9f7..7037f47c454f 100644
> --- a/fs/nilfs2/namei.c
> +++ b/fs/nilfs2/namei.c
> @@ -67,6 +67,12 @@ nilfs_lookup(struct inode *dir, struct dentry *dentry, unsigned int flags)
> inode = NULL;
> } else {
> inode = nilfs_iget(dir->i_sb, NILFS_I(dir)->i_root, ino);
> + if (inode == ERR_PTR(-ESTALE)) {
> + nilfs_error(dir->i_sb, __func__,
> + "deleted inode referenced: %lu",
> + (unsigned long) ino);
Unlink ext2_error(), nilfs_error() does not require __func__, to be
passed as an argument.
nilfs_error() is a wrapper macro for the actual error output function
__nilfs_error(), which hides __func__ there.
(I should have mentioned the difference, sorry.)
Another comment: "ino" is of type "ino_t", which is of type "unsigned
long", so the typecast to "unsigned long" for the argument "ino" is
not necessary.
I don't know why the ext2 implementation does it, but even if this
patch is backported to stable trees, this typecast is not necessary.
Thanks,
Ryusuke Konishi
> + return ERR_PTR(-EIO);
> + }
> }
>
> return d_splice_alias(inode, dentry);
> --
> 2.47.0
>
syzbot reported a WARNING in nilfs_rmdir. [1]
Because the inode bitmap is corrupted, an inode with an inode number
that should exist as a ".nilfs" file was reassigned by nilfs_mkdir for
"file0", causing an inode duplication during execution.
And this causes an underflow of i_nlink in rmdir operations.
The inode is used twice by the same task to unmount and remove directories
".nilfs" and "file0", it trigger warning in nilfs_rmdir.
Avoid to this issue, check i_nlink in nilfs_iget(), if it is 0, it means
that this inode has been deleted, and iput is executed to reclaim it.
[1]
WARNING: CPU: 1 PID: 5824 at fs/inode.c:407 drop_nlink+0xc4/0x110 fs/inode.c:407
Modules linked in:
CPU: 1 UID: 0 PID: 5824 Comm: syz-executor223 Not tainted 6.12.0-syzkaller-12113-gbcc8eda6d349 #0
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 09/13/2024
RIP: 0010:drop_nlink+0xc4/0x110 fs/inode.c:407
Code: bb 70 07 00 00 be 08 00 00 00 e8 57 0b e6 ff f0 48 ff 83 70 07 00 00 5b 41 5c 41 5e 41 5f 5d c3 cc cc cc cc e8 9d 4c 7e ff 90 <0f> 0b 90 eb 83 44 89 e1 80 e1 07 80 c1 03 38 c1 0f 8c 5c ff ff ff
RSP: 0018:ffffc900037f7c70 EFLAGS: 00010293
RAX: ffffffff822124a3 RBX: 1ffff1100e7ae034 RCX: ffff88807cf53c00
RDX: 0000000000000000 RSI: 0000000000000000 RDI: 0000000000000000
RBP: 0000000000000000 R08: ffffffff82212423 R09: 1ffff1100f8ba8ee
R10: dffffc0000000000 R11: ffffed100f8ba8ef R12: ffff888073d701a0
R13: 1ffff1100e79f5c4 R14: ffff888073d70158 R15: dffffc0000000000
FS: 0000555558d1e480(0000) GS:ffff8880b8700000(0000) knlGS:0000000000000000
CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
CR2: 0000555558d37878 CR3: 000000007d920000 CR4: 00000000003526f0
DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
Call Trace:
<TASK>
nilfs_rmdir+0x1b0/0x250 fs/nilfs2/namei.c:342
vfs_rmdir+0x3a3/0x510 fs/namei.c:4394
do_rmdir+0x3b5/0x580 fs/namei.c:4453
__do_sys_rmdir fs/namei.c:4472 [inline]
__se_sys_rmdir fs/namei.c:4470 [inline]
__x64_sys_rmdir+0x47/0x50 fs/namei.c:4470
do_syscall_x64 arch/x86/entry/common.c:52 [inline]
do_syscall_64+0xf3/0x230 arch/x86/entry/common.c:83
entry_SYSCALL_64_after_hwframe+0x77/0x7f
Reported-by: syzbot+9260555647a5132edd48@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=9260555647a5132edd48
Tested-by: syzbot+9260555647a5132edd48@syzkaller.appspotmail.com
Signed-off-by: Edward Adam Davis <eadavis@qq.com>
---
V1 -> V2: Adjust the patch as suggested by Ryusuke Konishi
V2 -> V3: Modify the input parameters of nilfs_error and split Reported-and-tested_by
fs/nilfs2/inode.c | 8 +++++++-
fs/nilfs2/namei.c | 5 +++++
2 files changed, 12 insertions(+), 1 deletion(-)
diff --git a/fs/nilfs2/inode.c b/fs/nilfs2/inode.c
index cf9ba481ae37..b7d4105f37bf 100644
--- a/fs/nilfs2/inode.c
+++ b/fs/nilfs2/inode.c
@@ -544,8 +544,14 @@ struct inode *nilfs_iget(struct super_block *sb, struct nilfs_root *root,
inode = nilfs_iget_locked(sb, root, ino);
if (unlikely(!inode))
return ERR_PTR(-ENOMEM);
- if (!(inode->i_state & I_NEW))
+
+ if (!(inode->i_state & I_NEW)) {
+ if (!inode->i_nlink) {
+ iput(inode);
+ return ERR_PTR(-ESTALE);
+ }
return inode;
+ }
err = __nilfs_read_inode(sb, root, ino, inode);
if (unlikely(err)) {
diff --git a/fs/nilfs2/namei.c b/fs/nilfs2/namei.c
index 9b108052d9f7..1d836a5540f3 100644
--- a/fs/nilfs2/namei.c
+++ b/fs/nilfs2/namei.c
@@ -67,6 +67,11 @@ nilfs_lookup(struct inode *dir, struct dentry *dentry, unsigned int flags)
inode = NULL;
} else {
inode = nilfs_iget(dir->i_sb, NILFS_I(dir)->i_root, ino);
+ if (inode == ERR_PTR(-ESTALE)) {
+ nilfs_error(dir->i_sb,
+ "deleted inode referenced: %lu", ino);
+ return ERR_PTR(-EIO);
+ }
}
return d_splice_alias(inode, dentry);
--
2.47.0On Sun, Dec 8, 2024 at 3:00 PM Edward Adam Davis wrote:
>
> syzbot reported a WARNING in nilfs_rmdir. [1]
>
> Because the inode bitmap is corrupted, an inode with an inode number
> that should exist as a ".nilfs" file was reassigned by nilfs_mkdir for
> "file0", causing an inode duplication during execution.
> And this causes an underflow of i_nlink in rmdir operations.
>
> The inode is used twice by the same task to unmount and remove directories
> ".nilfs" and "file0", it trigger warning in nilfs_rmdir.
>
> Avoid to this issue, check i_nlink in nilfs_iget(), if it is 0, it means
> that this inode has been deleted, and iput is executed to reclaim it.
>
> [1]
> WARNING: CPU: 1 PID: 5824 at fs/inode.c:407 drop_nlink+0xc4/0x110 fs/inode.c:407
> Modules linked in:
> CPU: 1 UID: 0 PID: 5824 Comm: syz-executor223 Not tainted 6.12.0-syzkaller-12113-gbcc8eda6d349 #0
> Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 09/13/2024
> RIP: 0010:drop_nlink+0xc4/0x110 fs/inode.c:407
> Code: bb 70 07 00 00 be 08 00 00 00 e8 57 0b e6 ff f0 48 ff 83 70 07 00 00 5b 41 5c 41 5e 41 5f 5d c3 cc cc cc cc e8 9d 4c 7e ff 90 <0f> 0b 90 eb 83 44 89 e1 80 e1 07 80 c1 03 38 c1 0f 8c 5c ff ff ff
> RSP: 0018:ffffc900037f7c70 EFLAGS: 00010293
> RAX: ffffffff822124a3 RBX: 1ffff1100e7ae034 RCX: ffff88807cf53c00
> RDX: 0000000000000000 RSI: 0000000000000000 RDI: 0000000000000000
> RBP: 0000000000000000 R08: ffffffff82212423 R09: 1ffff1100f8ba8ee
> R10: dffffc0000000000 R11: ffffed100f8ba8ef R12: ffff888073d701a0
> R13: 1ffff1100e79f5c4 R14: ffff888073d70158 R15: dffffc0000000000
> FS: 0000555558d1e480(0000) GS:ffff8880b8700000(0000) knlGS:0000000000000000
> CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> CR2: 0000555558d37878 CR3: 000000007d920000 CR4: 00000000003526f0
> DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
> DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
> Call Trace:
> <TASK>
> nilfs_rmdir+0x1b0/0x250 fs/nilfs2/namei.c:342
> vfs_rmdir+0x3a3/0x510 fs/namei.c:4394
> do_rmdir+0x3b5/0x580 fs/namei.c:4453
> __do_sys_rmdir fs/namei.c:4472 [inline]
> __se_sys_rmdir fs/namei.c:4470 [inline]
> __x64_sys_rmdir+0x47/0x50 fs/namei.c:4470
> do_syscall_x64 arch/x86/entry/common.c:52 [inline]
> do_syscall_64+0xf3/0x230 arch/x86/entry/common.c:83
> entry_SYSCALL_64_after_hwframe+0x77/0x7f
>
> Reported-by: syzbot+9260555647a5132edd48@syzkaller.appspotmail.com
> Closes: https://syzkaller.appspot.com/bug?extid=9260555647a5132edd48
> Tested-by: syzbot+9260555647a5132edd48@syzkaller.appspotmail.com
> Signed-off-by: Edward Adam Davis <eadavis@qq.com>
> ---
> V1 -> V2: Adjust the patch as suggested by Ryusuke Konishi
> V2 -> V3: Modify the input parameters of nilfs_error and split Reported-and-tested_by
>
> fs/nilfs2/inode.c | 8 +++++++-
> fs/nilfs2/namei.c | 5 +++++
> 2 files changed, 12 insertions(+), 1 deletion(-)
Thank you for your help. I'll adopt this and send it upstream after
running tests that include more than just the reproducer.
Thanks,
Ryusuke Konishi
>
> diff --git a/fs/nilfs2/inode.c b/fs/nilfs2/inode.c
> index cf9ba481ae37..b7d4105f37bf 100644
> --- a/fs/nilfs2/inode.c
> +++ b/fs/nilfs2/inode.c
> @@ -544,8 +544,14 @@ struct inode *nilfs_iget(struct super_block *sb, struct nilfs_root *root,
> inode = nilfs_iget_locked(sb, root, ino);
> if (unlikely(!inode))
> return ERR_PTR(-ENOMEM);
> - if (!(inode->i_state & I_NEW))
> +
> + if (!(inode->i_state & I_NEW)) {
> + if (!inode->i_nlink) {
> + iput(inode);
> + return ERR_PTR(-ESTALE);
> + }
> return inode;
> + }
>
> err = __nilfs_read_inode(sb, root, ino, inode);
> if (unlikely(err)) {
> diff --git a/fs/nilfs2/namei.c b/fs/nilfs2/namei.c
> index 9b108052d9f7..1d836a5540f3 100644
> --- a/fs/nilfs2/namei.c
> +++ b/fs/nilfs2/namei.c
> @@ -67,6 +67,11 @@ nilfs_lookup(struct inode *dir, struct dentry *dentry, unsigned int flags)
> inode = NULL;
> } else {
> inode = nilfs_iget(dir->i_sb, NILFS_I(dir)->i_root, ino);
> + if (inode == ERR_PTR(-ESTALE)) {
> + nilfs_error(dir->i_sb,
> + "deleted inode referenced: %lu", ino);
> + return ERR_PTR(-EIO);
> + }
> }
>
> return d_splice_alias(inode, dentry);
> --
> 2.47.0
>
© 2016 - 2025 Red Hat, Inc.