kernel BUG in btrfs_get_ordered_extents_for_logging
syzbot
syzbotHello,
syzbot found the following issue on:
HEAD commit: 5be63fc19fca Linux 6.11-rc5 git tree: upstream console output: https://syzkaller.appspot.com/x/log.txt?x=11f0800d980000 kernel config: https://syzkaller.appspot.com/x/.config?x=a0455552d0b27491 dashboard link: https://syzkaller.appspot.com/bug?extid=4704b3cc972bd76024f1 compiler: Debian clang version 15.0.6, GNU ld (GNU Binutils for Debian) 2.40 syz repro: https://syzkaller.appspot.com/x/repro.syz?x=16692a29980000
Downloadable assets:
disk image: https://storage.googleapis.com/syzbot-assets/cad8c335ccde/disk-5be63fc1.raw.xz
vmlinux: https://storage.googleapis.com/syzbot-assets/c4d2dd818c33/vmlinux-5be63fc1.xz
kernel image: https://storage.googleapis.com/syzbot-assets/ab5cbe08133f/bzImage-5be63fc1.xz
mounted in repro #1: https://storage.googleapis.com/syzbot-assets/9b6630601ecf/mount_0.gz
mounted in repro #2: https://storage.googleapis.com/syzbot-assets/1eee9b57c8d5/mount_3.gz
mounted in repro #3: https://storage.googleapis.com/syzbot-assets/ba6ddfb704dc/mount_4.gz
mounted in repro #4: https://storage.googleapis.com/syzbot-assets/e681ae9a9992/mount_9.gz
IMPORTANT: if you fix the issue, please add the following tag to the commit:
------------[ cut here ]------------ kernel BUG at fs/btrfs/ordered-data.c:1018! Oops: invalid opcode: 0000 [#1] PREEMPT SMP KASAN PTI CPU: 1 UID: 0 PID: 6479 Comm: syz.4.61 Not tainted 6.11.0-rc5-syzkaller #0 Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 08/06/2024 RIP: 0010:btrfs_get_ordered_extents_for_logging+0x4fd/0x500 fs/btrfs/ordered-data.c:1018 Code: f7 07 90 0f 0b e8 23 f1 df fd 48 c7 c7 40 8a 2c 8c 48 c7 c6 e0 8c 2c 8c 48 c7 c2 20 8a 2c 8c b9 fa 03 00 00 e8 e4 ad f7 07 90 <0f> 0b 90 90 90 90 90 90 90 90 90 90 90 90 90 90 90 90 90 66 0f 1f RSP: 0018:ffffc90009627938 EFLAGS: 00010246 RAX: 0000000000000055 RBX: 0000000000000000 RCX: 3784e5b0ceb58c00 RDX: 0000000000000000 RSI: 0000000080000000 RDI: 0000000000000000 RBP: ffff88806597d690 R08: ffffffff817402ac R09: 1ffff920012c4ec8 R10: dffffc0000000000 R11: fffff520012c4ec9 R12: 0000000000000000 R13: dffffc0000000000 R14: ffffc90009627ae0 R15: ffff88806597d690 FS: 00007f60c8eec6c0(0000) GS:ffff8880b9300000(0000) knlGS:0000000000000000 CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 CR2: 00007fa599c4220a CR3: 000000007953c000 CR4: 00000000003506f0 DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000 DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400 Call Trace: <TASK> btrfs_sync_file+0x929/0x10f0 fs/btrfs/file.c:1712 generic_write_sync include/linux/fs.h:2821 [inline] btrfs_do_write_iter+0x5e2/0x760 fs/btrfs/file.c:1515 new_sync_write fs/read_write.c:497 [inline] vfs_write+0xa72/0xc90 fs/read_write.c:590 ksys_pwrite64 fs/read_write.c:705 [inline] __do_sys_pwrite64 fs/read_write.c:715 [inline] __se_sys_pwrite64 fs/read_write.c:712 [inline] __x64_sys_pwrite64+0x1aa/0x230 fs/read_write.c:712 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 RIP: 0033:0x7f60c8179e79 Code: ff ff c3 66 2e 0f 1f 84 00 00 00 00 00 0f 1f 40 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 73 01 c3 48 c7 c1 a8 ff ff ff f7 d8 64 89 01 48 RSP: 002b:00007f60c8eec038 EFLAGS: 00000246 ORIG_RAX: 0000000000000012 RAX: ffffffffffffffda RBX: 00007f60c8316130 RCX: 00007f60c8179e79 RDX: 000000000000003d RSI: 00000000200001c0 RDI: 0000000000000007 RBP: 00007f60c81e793e R08: 0000000000000000 R09: 0000000000000000 R10: 0000000000000000 R11: 0000000000000246 R12: 0000000000000000 R13: 0000000000000001 R14: 00007f60c8316130 R15: 00007ffd4fce3d08 </TASK> Modules linked in: ---[ end trace 0000000000000000 ]--- RIP: 0010:btrfs_get_ordered_extents_for_logging+0x4fd/0x500 fs/btrfs/ordered-data.c:1018 Code: f7 07 90 0f 0b e8 23 f1 df fd 48 c7 c7 40 8a 2c 8c 48 c7 c6 e0 8c 2c 8c 48 c7 c2 20 8a 2c 8c b9 fa 03 00 00 e8 e4 ad f7 07 90 <0f> 0b 90 90 90 90 90 90 90 90 90 90 90 90 90 90 90 90 90 66 0f 1f RSP: 0018:ffffc90009627938 EFLAGS: 00010246 RAX: 0000000000000055 RBX: 0000000000000000 RCX: 3784e5b0ceb58c00 RDX: 0000000000000000 RSI: 0000000080000000 RDI: 0000000000000000 RBP: ffff88806597d690 R08: ffffffff817402ac R09: 1ffff920012c4ec8 R10: dffffc0000000000 R11: fffff520012c4ec9 R12: 0000000000000000 R13: dffffc0000000000 R14: ffffc90009627ae0 R15: ffff88806597d690 FS: 00007f60c8eec6c0(0000) GS:ffff8880b9300000(0000) knlGS:0000000000000000 CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 CR2: 00007f449fcc2723 CR3: 000000007953c000 CR4: 00000000003506f0 DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000 DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
---
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 [email protected].
syzbot will keep track of this issue. See:
https://goo.gl/tpsmEJ#status for how to communicate with syzbot.
If the report is already addressed, let syzbot know by replying with:
#syz fix: exact-commit-title
If you want syzbot to run the reproducer, reply with:
#syz test: git://repo/address.git branch-or-commit-hash
If you attach or paste a git patch, syzbot will apply it before testing.
If you want to overwrite report's subsystems, reply with:
#syz set subsystems: new-subsystem
(See the list of subsystem names on the web dashboard)
If the report is a duplicate of another one, reply with:
#syz dup: exact-subject-of-another-report
If you want to undo deduplication, reply with:
#syz undup
syzbot
syzbotFor archival purposes, forwarding an incoming command email to [email protected].
For archival purposes, forwarding an incoming command email to [email protected].
***
Subject: Re: [btrfs?] kernel BUG in btrfs_get_ordered_extents_for_logging
Author: [email protected]
or i_mmap_lock ?
#syz test
fs/btrfs/ordered-data.c +2 -1
@@ -1015,7 +1015,8 @@ void btrfs_get_ordered_extents_for_logging(struct btrfs_inode *inode, {struct rb_node *n; -ASSERT (inode_is_locked (&inode->vfs_inode)); +ASSERT (inode_is_locked (&inode->vfs_inode) || +rwsem_is_locked (&inode->i_mmap_lock));spin_lock_irq (&inode->ordered_tree_lock);for (n =rb_first (&inode->ordered_tree); n; n =rb_next (n)) {
PATCH 1/1 btrfs: Add assert or condition
In reply to syzbotWhen the value of fsync_skip_inode_lock is true, i_mmap_lock is used, so add it or condition in the ASSERT.
--- fs/btrfs/ordered-data.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
fs/btrfs/ordered-data.c +2 -1
@@ -1015,7 +1015,8 @@ void btrfs_get_ordered_extents_for_logging(struct btrfs_inode *inode, {struct rb_node *n; -ASSERT (inode_is_locked (&inode->vfs_inode)); +ASSERT (inode_is_locked (&inode->vfs_inode) || +rwsem_is_locked (&inode->i_mmap_lock));spin_lock_irq (&inode->ordered_tree_lock);for (n =rb_first (&inode->ordered_tree); n; n =rb_next (n)) {
Signature · 1 line
2.43.0
Re: [PATCH] btrfs: Add assert or condition
Quoted 16 lines · Lizhi Xu
When the value of
fsync_skip_inode_lockis true,i_mmap_lockis used, so add it or condition in the ASSERT.Reported-and-tested-by [email protected]Signed-off-by Lizhi Xu <[email protected]>--- fs/btrfs/ordered-data.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)fs/btrfs/ordered-data.c +0
-0
fs/btrfs/ordered-data.c void btrfs_get_ordered_extents_for_logging(struct btrfs_inode *inode, · line 1019
Quoted 6 lines
fs/btrfs/ordered-data.c +2
-1@@ -1015,7 +1015,8 @@ void btrfs_get_ordered_extents_for_logging(struct btrfs_inode *inode, {struct rb_node *n; -ASSERT (inode_is_locked (&inode->vfs_inode)); +ASSERT (inode_is_locked (&inode->vfs_inode) || +rwsem_is_locked (&inode->i_mmap_lock));
This definitely fixes the syzbot report, in the sense the assertion won't fail anymore.
But it's wrong, very, very, very, very wrong.
The inode must be locked during the course of the fsync, that's why the assertion is there.
Why do you think it's ok to not have the inode locked? Have you done any analysis about that?
You mention "fsync_skip_inode_lock is true" in the changelog, but have you checked where and why it's set to true?
Where we set it to true, at btrfs_direct_write(), there's a comment which explains it's to avoid a deadlock on the inode lock at btrfs_sync_file().
This is a perfect example of trying a patch not only without having any idea how the code works but also being very lazy,
as there's a very explicit comment in the code about why the variable is set to true, and even much more detailed in the
change log of the commit that introduced it.
And btw, there's already a patch to fix this issue:
Quoted 7 lines
fs/btrfs/ordered-data.c +0
-1spin_lock_irq (&inode->ordered_tree_lock);for (n =rb_first (&inode->ordered_tree); n; n =rb_next (n)) { --2.43.0
Re: [PATCH] btrfs: Add assert or condition
Quoted 27 lines · Filipe Manana
fs/btrfs/ordered-data.c void btrfs_get_ordered_extents_for_logging(struct btrfs_inode *inode, · line 1019
Quoted 3 lines
fs/btrfs/ordered-data.c +2
-1-ASSERT (inode_is_locked (&inode->vfs_inode)); +ASSERT (inode_is_locked (&inode->vfs_inode) || +rwsem_is_locked (&inode->i_mmap_lock));This definitely fixes the syzbot report, in the sense the assertion won't fail anymore.
But it's wrong, very, very, very, very wrong.The inode must be locked during the course of the fsync, that's why the assertion is there.
Why do you think it's ok to not have the inode locked? Have you done any analysis about that?
You mention "
fsync_skip_inode_lockis true" in the changelog, but have you checked where and why it's set to true?Where we set it to true, at
btrfs_direct_write(), there's a comment which explains it's to avoid a deadlock on the inode lock atbtrfs_sync_file().This is a perfect example of trying a patch not only without having any idea how the code works but also being very lazy,
as there's a very explicit comment in the code about why the variable is set to true, and even much more detailed in the
change log of the commit that introduced it.
Quoted 3 lines
And btw, there's already a patch to fix this issue:
In your patch, I get what the mean of fsync_skip_inode_lock.
Thanks.