[syzbot] INFO: trying to register non-static key in nilfs_bmap_lookup_at_level

[syzbot]

Hello,

syzbot found the following issue on:

HEAD commit: 3800a713b607 Merge tag 'mm-hotfixes-stable-2022-09-26' of ..
git tree: upstream
console+strace: https://syzkaller.appspot.com/x/log.txt?x=12704b40880000
kernel config: https://syzkaller.appspot.com/x/.config?x=ba0d23aa7e1ffaf5
dashboard link: https://syzkaller.appspot.com/bug?extid=2b32eb36c1a825b7a74c
compiler: Debian clang version 13.0.1-++20220126092033+75e33f71c2da-1~exp1~20220126212112.63, GNU ld (GNU Binutils for Debian) 2.35.2
syz repro: https://syzkaller.appspot.com/x/repro.syz?x=14e930c0880000
C reproducer: https://syzkaller.appspot.com/x/repro.c?x=157ce604880000

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

loop0: detected capacity change from 0 to 264192
NILFS (loop0): broken superblock, retrying with spare superblock (blocksize = 1024)
INFO: trying to register non-static key.
The code is fine but needs lockdep annotation, or maybe
you didn't initialize this object before use?
turning off the locking correctness validator.
CPU: 0 PID: 3614 Comm: syz-executor231 Not tainted 6.0.0-rc7-syzkaller-00029-g3800a713b607 #0
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 08/26/2022
Call Trace:
<TASK>
__dump_stack lib/dump_stack.c:88 [inline]
dump_stack_lvl+0x1b1/0x28e lib/dump_stack.c:106
assign_lock_key+0x212/0x220 kernel/locking/lockdep.c:979
register_lock_class+0x18d/0x930 kernel/locking/lockdep.c:1292
__lock_acquire+0xe4/0x1f60 kernel/locking/lockdep.c:4932
lock_acquire+0x182/0x3c0 kernel/locking/lockdep.c:5666
down_read+0x39/0x50 kernel/locking/rwsem.c:1499
nilfs_bmap_lookup_at_level+0x88/0x350 fs/nilfs2/bmap.c:68
nilfs_bmap_lookup fs/nilfs2/bmap.h:170 [inline]
nilfs_mdt_submit_block+0x23d/0x520 fs/nilfs2/mdt.c:142
nilfs_mdt_read_block+0xa3/0x460 fs/nilfs2/mdt.c:176
nilfs_mdt_get_block+0x104/0x820 fs/nilfs2/mdt.c:251
nilfs_palloc_get_block+0x142/0x240 fs/nilfs2/alloc.c:216
nilfs_palloc_get_entry_block+0x1e8/0x2e0 fs/nilfs2/alloc.c:318
nilfs_ifile_get_inode_block+0xf0/0x1a0 fs/nilfs2/ifile.c:143
__nilfs_read_inode fs/nilfs2/inode.c:477 [inline]
nilfs_iget+0x253/0x8b0 fs/nilfs2/inode.c:603
nilfs_get_root_dentry+0x28/0x230 fs/nilfs2/super.c:904
nilfs_fill_super+0x3dd/0x5d0 fs/nilfs2/super.c:1078
nilfs_mount+0x613/0x9b0 fs/nilfs2/super.c:1317
legacy_get_tree+0xea/0x180 fs/fs_context.c:610
vfs_get_tree+0x88/0x270 fs/super.c:1530
do_new_mount+0x289/0xad0 fs/namespace.c:3040
do_mount fs/namespace.c:3383 [inline]
__do_sys_mount fs/namespace.c:3591 [inline]
__se_sys_mount+0x2d3/0x3c0 fs/namespace.c:3568
do_syscall_x64 arch/x86/entry/common.c:50 [inline]
do_syscall_64+0x3d/0xb0 arch/x86/entry/common.c:80
entry_SYSCALL_64_after_hwframe+0x63/0xcd
RIP: 0033:0x7efd97e9462a
Code: 83 c4 08 5b 5d c3 66 2e 0f 1f 84 00 00 00 00 00 c3 66 2e 0f 1f 84 00 00 00 00 00 0f 1f 44 00 00 49 89 ca b8 a5 00 00 00 0f 05 <48> 3d 01 f0 ff ff 73 01 c3 48 c7 c1 c0 ff ff ff f7 d8 64 89 01 48
RSP: 002b:00007ffc213b3aa8 EFLAGS: 00000286 ORIG_RAX: 00000000000000a5
RAX: ffffffffffffffda RBX: 00007ffc213b3b00 RCX: 00007efd97e9462a
RDX: 0000000020000000 RSI: 0000000020000100 RDI: 00007ffc213b3ac0
RBP: 00007ffc213b3ac0 R08: 00007ffc213b3b00 R09: 0000000000000000
R10: 0000000000000000 R11: 0000000000000286 R12: 0000000020000788
R13: 0000000000000003 R14: 0000000000000004 R15: 000000000000003b
</TASK>
general protection fault, probably for non-canonical address 0xdffffc0000000000: 0000 [#1] PREEMPT SMP KASAN
KASAN: null-ptr-deref in range [0x0000000000000000-0x0000000000000007]
CPU: 1 PID: 3614 Comm: syz-executor231 Not tainted 6.0.0-rc7-syzkaller-00029-g3800a713b607 #0
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 08/26/2022
RIP: 0010:nilfs_bmap_lookup_at_level+0xb9/0x350 fs/nilfs2/bmap.c:69
Code: 48 89 d8 48 c1 e8 03 42 80 3c 30 00 74 08 48 89 df e8 4b 9b 94 fe 4c 8b 33 4c 89 f0 48 c1 e8 03 48 b9 00 00 00 00 00 fc ff df <80> 3c 08 00 74 08 4c 89 f7 e8 29 9b 94 fe 4c 89 ff 4c 89 ee 44 89
RSP: 0018:ffffc9000385f440 EFLAGS: 00010246
RAX: 0000000000000000 RBX: ffff888074ff2760 RCX: dffffc0000000000
RDX: 0000000000000001 RSI: 0000000000000008 RDI: ffff888074ff26c8
RBP: ffffc9000385f4f0 R08: dffffc0000000000 R09: ffffed100e9fe4da
R10: ffffed100e9fe4da R11: 1ffff1100e9fe4d9 R12: 0000000000000001
R13: 0000000000000002 R14: 0000000000000000 R15: ffff888074ff2688
FS: 0000555555be2300(0000) GS:ffff8880b9b00000(0000) knlGS:0000000000000000
CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
CR2: 00007ffc213b4000 CR3: 00000000725e8000 CR4: 00000000003506e0
DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
Call Trace:
<TASK>
nilfs_bmap_lookup fs/nilfs2/bmap.h:170 [inline]
nilfs_mdt_submit_block+0x23d/0x520 fs/nilfs2/mdt.c:142
nilfs_mdt_read_block+0xa3/0x460 fs/nilfs2/mdt.c:176
nilfs_mdt_get_block+0x104/0x820 fs/nilfs2/mdt.c:251
nilfs_palloc_get_block+0x142/0x240 fs/nilfs2/alloc.c:216
nilfs_palloc_get_entry_block+0x1e8/0x2e0 fs/nilfs2/alloc.c:318
nilfs_ifile_get_inode_block+0xf0/0x1a0 fs/nilfs2/ifile.c:143
__nilfs_read_inode fs/nilfs2/inode.c:477 [inline]
nilfs_iget+0x253/0x8b0 fs/nilfs2/inode.c:603
nilfs_get_root_dentry+0x28/0x230 fs/nilfs2/super.c:904
nilfs_fill_super+0x3dd/0x5d0 fs/nilfs2/super.c:1078
nilfs_mount+0x613/0x9b0 fs/nilfs2/super.c:1317
legacy_get_tree+0xea/0x180 fs/fs_context.c:610
vfs_get_tree+0x88/0x270 fs/super.c:1530
do_new_mount+0x289/0xad0 fs/namespace.c:3040
do_mount fs/namespace.c:3383 [inline]
__do_sys_mount fs/namespace.c:3591 [inline]
__se_sys_mount+0x2d3/0x3c0 fs/namespace.c:3568
do_syscall_x64 arch/x86/entry/common.c:50 [inline]
do_syscall_64+0x3d/0xb0 arch/x86/entry/common.c:80
entry_SYSCALL_64_after_hwframe+0x63/0xcd
RIP: 0033:0x7efd97e9462a
Code: 83 c4 08 5b 5d c3 66 2e 0f 1f 84 00 00 00 00 00 c3 66 2e 0f 1f 84 00 00 00 00 00 0f 1f 44 00 00 49 89 ca b8 a5 00 00 00 0f 05 <48> 3d 01 f0 ff ff 73 01 c3 48 c7 c1 c0 ff ff ff f7 d8 64 89 01 48
RSP: 002b:00007ffc213b3aa8 EFLAGS: 00000286 ORIG_RAX: 00000000000000a5
RAX: ffffffffffffffda RBX: 00007ffc213b3b00 RCX: 00007efd97e9462a
RDX: 0000000020000000 RSI: 0000000020000100 RDI: 00007ffc213b3ac0
RBP: 00007ffc213b3ac0 R08: 00007ffc213b3b00 R09: 0000000000000000
R10: 0000000000000000 R11: 0000000000000286 R12: 0000000020000788
R13: 0000000000000003 R14: 0000000000000004 R15: 000000000000003b
</TASK>
Modules linked in:
---[ end trace 0000000000000000 ]---
RIP: 0010:nilfs_bmap_lookup_at_level+0xb9/0x350 fs/nilfs2/bmap.c:69
Code: 48 89 d8 48 c1 e8 03 42 80 3c 30 00 74 08 48 89 df e8 4b 9b 94 fe 4c 8b 33 4c 89 f0 48 c1 e8 03 48 b9 00 00 00 00 00 fc ff df <80> 3c 08 00 74 08 4c 89 f7 e8 29 9b 94 fe 4c 89 ff 4c 89 ee 44 89
RSP: 0018:ffffc9000385f440 EFLAGS: 00010246
RAX: 0000000000000000 RBX: ffff888074ff2760 RCX: dffffc0000000000
RDX: 0000000000000001 RSI: 0000000000000008 RDI: ffff888074ff26c8
RBP: ffffc9000385f4f0 R08: dffffc0000000000 R09: ffffed100e9fe4da
R10: ffffed100e9fe4da R11: 1ffff1100e9fe4d9 R12: 0000000000000001
R13: 0000000000000002 R14: 0000000000000000 R15: ffff888074ff2688
FS: 0000555555be2300(0000) GS:ffff8880b9a00000(0000) knlGS:0000000000000000
CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
CR2: 0000560cba9be408 CR3: 00000000725e8000 CR4: 00000000003506f0
DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
----------------
Code disassembly (best guess):
0: 48 89 d8 mov %rbx,%rax
3: 48 c1 e8 03 shr $0x3,%rax
7: 42 80 3c 30 00 cmpb $0x0,(%rax,%r14,1)
c: 74 08 je 0x16
e: 48 89 df mov %rbx,%rdi
11: e8 4b 9b 94 fe callq 0xfe949b61
16: 4c 8b 33 mov (%rbx),%r14
19: 4c 89 f0 mov %r14,%rax
1c: 48 c1 e8 03 shr $0x3,%rax
20: 48 b9 00 00 00 00 00 movabs $0xdffffc0000000000,%rcx
27: fc ff df
* 2a: 80 3c 08 00 cmpb $0x0,(%rax,%rcx,1) <-- trapping instruction
2e: 74 08 je 0x38
30: 4c 89 f7 mov %r14,%rdi
33: e8 29 9b 94 fe callq 0xfe949b61
38: 4c 89 ff mov %r15,%rdi
3b: 4c 89 ee mov %r13,%rsi
3e: 44 rex.R
3f: 89 .byte 0x89


---
This report 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 syzk...@googlegroups.com.

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

[Tetsuo Handa]

syzbot is reporting lockdep warning followed by NULL pointer dereference
at nilfs_bmap_lookup_at_level() [1], for a crafted filesystem which
contains raw_inode->i_mode == 0 is poisoning checkpoint inode at
nilfs_read_inode_common() from nilfs_ifile_read() from
nilfs_attach_checkpoint() from nilfs_fill_super() from nilfs_mount().
Check that filetype/uid/gid are valid as well as i_nlink is valid.

Link: https://syzkaller.appspot.com/bug?extid=2b32eb36c1a825b7a74c [1]
Reported-by: syzot <syzbot+2b32eb...@syzkaller.appspotmail.com>
Signed-off-by: Tetsuo Handa <penguin...@I-love.SAKURA.ne.jp>
---
This patch solves crash but I don't know whether this patch is correct.

Commit 05fe58fdc10df9eb ("nilfs2: inode operations") did

+ if (inode->i_nlink == 0 && (inode->i_mode == 0 || ii->i_dtime))

and commit 612392307cb09e49 ("nilfs2: support nanosecond timestamp") did

- if (inode->i_nlink == 0 && (inode->i_mode == 0 || ii->i_dtime))
+ if (inode->i_nlink == 0 && inode->i_mode == 0)

and commit 705304a863cc4158 ("nilfs2: fix the nilfs_iget() vs. nilfs_new_inode() races") did


- if (inode->i_nlink == 0 && inode->i_mode == 0)
+ if (inode->i_nlink == 0)

but I don't know which commit was wrong. Is inode->i_mode == 0 valid in nilfs?

fs/nilfs2/inode.c | 14 ++++++++++++++
1 file changed, 14 insertions(+)

diff --git a/fs/nilfs2/inode.c b/fs/nilfs2/inode.c
index 67f63cfeade5..798efd879642 100644
--- a/fs/nilfs2/inode.c
+++ b/fs/nilfs2/inode.c
@@ -442,6 +442,20 @@ int nilfs_read_inode_common(struct inode *inode,
inode->i_mtime.tv_nsec = le32_to_cpu(raw_inode->i_mtime_nsec);
if (inode->i_nlink == 0)
return -ESTALE; /* this inode is deleted */
+ switch (inode->i_mode & S_IFMT) {
+ case S_IFLNK:
+ case S_IFREG:
+ case S_IFDIR:
+ case S_IFCHR:
+ case S_IFBLK:
+ case S_IFIFO:
+ case S_IFSOCK:
+ break;
+ default:
+ return -EINVAL; /* Filetype is invalid */
+ }
+ if (!uid_valid(inode->i_uid) || !gid_valid(inode->i_gid))
+ return -EINVAL; /* UID/GID is invalid */

inode->i_blocks = le64_to_cpu(raw_inode->i_blocks);
ii->i_flags = le32_to_cpu(raw_inode->i_flags);
--
2.34.1

[Tetsuo Handa]

On 2022/10/01 20:24, Tetsuo Handa wrote:
> syzbot is reporting lockdep warning followed by NULL pointer dereference
> at nilfs_bmap_lookup_at_level() [1], for a crafted filesystem which
> contains raw_inode->i_mode == 0 is poisoning checkpoint inode at
> nilfs_read_inode_common() from nilfs_ifile_read() from
> nilfs_attach_checkpoint() from nilfs_fill_super() from nilfs_mount().
> Check that filetype/uid/gid are valid as well as i_nlink is valid.
>
> Link: https://syzkaller.appspot.com/bug?extid=2b32eb36c1a825b7a74c [1]
> Reported-by: syzot <syzbot+2b32eb...@syzkaller.appspotmail.com>
> Signed-off-by: Tetsuo Handa <penguin...@I-love.SAKURA.ne.jp>
> ---
> This patch solves crash but I don't know whether this patch is correct.

Hmm, since bmap->b_sem and bmap->b_ops are initialized by nilfs_bmap_read(),
and nilfs_bmap_read() is called from nilfs_read_inode_common() only if
S_ISREG(inode->i_mode) || S_ISDIR(inode->i_mode) || S_ISLNK(inode->i_mode)
is true, I guess that the caller needs to verify that the returned inode is
a regular file? But where to add that check?

[Ryusuke Konishi]

Exactly, this looks like the root cause of the issue.

For metadata files, which are internal persistent files of nilfs2 and
used to store metadata
on disk, must have a file type and must call nilfs_bmap_read(), but
that sanity check
is missing.

As you pointed out, we need to add a sanity check for i_mode of metadata files.

The simple way is to insert a conditional sanity check in
nilfs_read_inode_common()
using nilfs_is_metadata_file_inode() and S_ISREG(), for instance:

if (nilfs_is_metadata_file_inode(inode) && !S_ISREG(inode->i_mode))
return -EIO; /* On-disk inode of the metadata file is
corrupted. */

if (inode->i_nlink == 0)

return -ESTALE; /* ... */

Or, we can add the i_mode sanity checks separately in
nilfs_dat_read(), nilfs_ifile_read(), nilfs_sufile_read(), and
nilfs_cpfile_read() before they call nilfs_read_inode_common().

I think the former is fine because we don't have the idea to add
metadata file inodes other
than file type. If you make a revised patch, I will approve or
handle it. Or if you are busy,
I will take over the fix.

Thank you,
Ryusuke Konishi

[Tetsuo Handa]

On 2022/10/01 23:28, Ryusuke Konishi wrote:
> I think the former is fine because we don't have the idea to add
> metadata file inodes other
> than file type. If you make a revised patch, I will approve or
> handle it. Or if you are busy,
> I will take over the fix.

Please take over the fix. Thank you.

[Ryusuke Konishi]

All right, thank you for your help so far!

Thanks,
Ryusuke Konishi

[Ryusuke Konishi]

If the i_mode field in inode of metadata files is corrupted on disk,
it can cause the initialization of bmap structure, which should have
been called from nilfs_read_inode_common(), not to be called. This
causes a lockdep warning followed by a NULL pointer dereference at
nilfs_bmap_lookup_at_level().

This patch fixes these issues by adding a missing sanitiy check for
the i_mode field of metadata file's inode.

Signed-off-by: Ryusuke Konishi <konishi...@gmail.com>
Reported-by: syzbot+2b32eb...@syzkaller.appspotmail.com
Reported-by: Tetsuo Handa <penguin...@I-love.SAKURA.ne.jp>
Tested-by: Ryusuke Konishi <konishi...@gmail.com>
Cc: sta...@vger.kernel.org
---
fs/nilfs2/inode.c | 2 ++
1 file changed, 2 insertions(+)

diff --git a/fs/nilfs2/inode.c b/fs/nilfs2/inode.c
index 67f63cfeade5..d4e653d51869 100644
--- a/fs/nilfs2/inode.c
+++ b/fs/nilfs2/inode.c
@@ -440,6 +440,8 @@ int nilfs_read_inode_common(struct inode *inode,
inode->i_atime.tv_nsec = le32_to_cpu(raw_inode->i_mtime_nsec);
inode->i_ctime.tv_nsec = le32_to_cpu(raw_inode->i_ctime_nsec);

inode->i_mtime.tv_nsec = le32_to_cpu(raw_inode->i_mtime_nsec);

+ if (nilfs_is_metadata_file_inode(inode) && !S_ISREG(inode->i_mode))
+ return -EIO; /* this inode is for metadata and corrupted */

if (inode->i_nlink == 0)

return -ESTALE; /* this inode is deleted */

--
2.34.1