[syzbot] [ext4?] [ocfs2?] KASAN: null-ptr-deref Write in jbd2_journal_update_sb_log_tail

[syzbot] [ext4?] [ocfs2?] KASAN: null-ptr-deref Write in jbd2_journal_update_sb_log_tail

[syzbot]

Hello,

syzbot found the following issue on:

HEAD commit:    c3f2d783a459 Merge tag 'mm-hotfixes-stable-2024-08-17-19-3..
git tree:       upstream
console output: https://syzkaller.appspot.com/x/log.txt?x=13736c29980000
kernel config:  https://syzkaller.appspot.com/x/.config?x=7229118d88b4a71b
dashboard link: https://syzkaller.appspot.com/bug?extid=05b9b39d8bdfe1a0861f
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=15f1b191980000
C reproducer:   https://syzkaller.appspot.com/x/repro.c?x=1042525b980000

Downloadable assets:
disk image (non-bootable): https://storage.googleapis.com/syzbot-assets/7bc7510fe41f/non_bootable_disk-c3f2d783.raw.xz
vmlinux: https://storage.googleapis.com/syzbot-assets/4d927f7c3cfd/vmlinux-c3f2d783.xz
kernel image: https://storage.googleapis.com/syzbot-assets/ea54bdfad24b/bzImage-c3f2d783.xz
mounted in repro: https://storage.googleapis.com/syzbot-assets/562379f73e38/mount_0.gz

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

(syz-executor198,5100,0):ocfs2_check_volume:2481 ERROR: status = -22
(syz-executor198,5100,0):ocfs2_mount_volume:1821 ERROR: status = -22
==================================================================
BUG: KASAN: null-ptr-deref in instrument_atomic_read_write include/linux/instrumented.h:96 [inline]
BUG: KASAN: null-ptr-deref in test_and_set_bit_lock include/asm-generic/bitops/instrumented-lock.h:57 [inline]
BUG: KASAN: null-ptr-deref in trylock_buffer include/linux/buffer_head.h:420 [inline]
BUG: KASAN: null-ptr-deref in lock_buffer include/linux/buffer_head.h:426 [inline]
BUG: KASAN: null-ptr-deref in jbd2_journal_update_sb_log_tail+0x19b/0x360 fs/jbd2/journal.c:1889
Write of size 8 at addr 0000000000000000 by task syz-executor198/5100

CPU: 0 UID: 0 PID: 5100 Comm: syz-executor198 Not tainted 6.11.0-rc3-syzkaller-00338-gc3f2d783a459 #0
Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS 1.16.3-debian-1.16.3-2~bpo12+1 04/01/2014
Call Trace:
 <TASK>
 __dump_stack lib/dump_stack.c:93 [inline]
 dump_stack_lvl+0x241/0x360 lib/dump_stack.c:119
 print_report+0xe8/0x550 mm/kasan/report.c:491
 kasan_report+0x143/0x180 mm/kasan/report.c:601
 kasan_check_range+0x282/0x290 mm/kasan/generic.c:189
 instrument_atomic_read_write include/linux/instrumented.h:96 [inline]
 test_and_set_bit_lock include/asm-generic/bitops/instrumented-lock.h:57 [inline]
 trylock_buffer include/linux/buffer_head.h:420 [inline]
 lock_buffer include/linux/buffer_head.h:426 [inline]
 jbd2_journal_update_sb_log_tail+0x19b/0x360 fs/jbd2/journal.c:1889
 __jbd2_update_log_tail+0x48/0x3f0 fs/jbd2/journal.c:1079
 jbd2_cleanup_journal_tail+0x230/0x2d0 fs/jbd2/checkpoint.c:334
 jbd2_journal_flush+0x290/0xc10 fs/jbd2/journal.c:2479
 ocfs2_journal_shutdown+0x443/0xbe0 fs/ocfs2/journal.c:1081
 ocfs2_mount_volume+0x169f/0x1940 fs/ocfs2/super.c:1842
 ocfs2_fill_super+0x483b/0x5880 fs/ocfs2/super.c:1084
 mount_bdev+0x20a/0x2d0 fs/super.c:1679
 legacy_get_tree+0xee/0x190 fs/fs_context.c:662
 vfs_get_tree+0x90/0x2a0 fs/super.c:1800
 do_new_mount+0x2be/0xb40 fs/namespace.c:3472
 do_mount fs/namespace.c:3812 [inline]
 __do_sys_mount fs/namespace.c:4020 [inline]
 __se_sys_mount+0x2d6/0x3c0 fs/namespace.c:3997
 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:0x7f69037ad16a
Code: d8 64 89 02 48 c7 c0 ff ff ff ff eb a6 e8 5e 04 00 00 66 2e 0f 1f 84 00 00 00 00 00 0f 1f 40 00 49 89 ca b8 a5 00 00 00 0f 05 <48> 3d 01 f0 ff ff 73 01 c3 48 c7 c1 b8 ff ff ff f7 d8 64 89 01 48
RSP: 002b:00007ffed646ff58 EFLAGS: 00000282 ORIG_RAX: 00000000000000a5
RAX: ffffffffffffffda RBX: 00007ffed646ff70 RCX: 00007f69037ad16a
RDX: 0000000020004480 RSI: 00000000200044c0 RDI: 00007ffed646ff70
RBP: 0000000000000004 R08: 00007ffed646ffb0 R09: 0000000000004470
R10: 0000000000000000 R11: 0000000000000282 R12: 0000000000000000
R13: 00007ffed646ffb0 R14: 0000000000000003 R15: 0000000001000000
 </TASK>
==================================================================


---
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 syzkaller@googlegroups.com.

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

[PATCH] ocfs2: Fix null-ptr-deref in jbd2_journal_update_sb_log_tail

[Edward Adam Davis]

Journal too short will cause ocfs2_check_volume failed, and will set
journal->j_sb_buffer to NULL in journal_fail_superblock before running
journal shutdown.

Reported-and-tested-by: syzbot+05b9b39d8bdfe1a0861f@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=05b9b39d8bdfe1a0861f
Signed-off-by: Edward Adam Davis <eadavis@qq.com>
---
 fs/ocfs2/journal.c | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/fs/ocfs2/journal.c b/fs/ocfs2/journal.c
index 530fba34f6d3..25821077b855 100644
--- a/fs/ocfs2/journal.c
+++ b/fs/ocfs2/journal.c
@@ -1077,9 +1077,11 @@ void ocfs2_journal_shutdown(struct ocfs2_super *osb)
 	BUG_ON(atomic_read(&(osb->journal->j_num_trans)) != 0);
 
 	if (ocfs2_mount_local(osb)) {
-		jbd2_journal_lock_updates(journal->j_journal);
-		status = jbd2_journal_flush(journal->j_journal, 0);
-		jbd2_journal_unlock_updates(journal->j_journal);
+		if (journal->j_journal->j_sb_buffer) {
+			jbd2_journal_lock_updates(journal->j_journal);
+			status = jbd2_journal_flush(journal->j_journal, 0);
+			jbd2_journal_unlock_updates(journal->j_journal);
+		}
 		if (status < 0)
 			mlog_errno(status);
 	}
-- 
2.43.0

Re: [PATCH] ocfs2: Fix null-ptr-deref in jbd2_journal_update_sb_log_tail

[Julian Sun]

Well,In my modest opinion, j_sb_buffer is an internal variable of
jbd2. Directly accessing internal variables from other modules can
degrade the maintainability of the code (for instance, this variable
might be removed at some point in the future). I have just sent
another patch to address this issue [1].

[1]: https://lore.kernel.org/ocfs2-devel/20240819131120.746077-1-sunjunchao2870@gmail.com/

Edward Adam Davis <eadavis@qq.com> 于2024年8月19日周一 21:41写道：
>
> Journal too short will cause ocfs2_check_volume failed, and will set
> journal->j_sb_buffer to NULL in journal_fail_superblock before running
> journal shutdown.
>
> Reported-and-tested-by: syzbot+05b9b39d8bdfe1a0861f@syzkaller.appspotmail.com
> Closes: https://syzkaller.appspot.com/bug?extid=05b9b39d8bdfe1a0861f
> Signed-off-by: Edward Adam Davis <eadavis@qq.com>
> ---
>  fs/ocfs2/journal.c | 8 +++++---
>  1 file changed, 5 insertions(+), 3 deletions(-)
>
> diff --git a/fs/ocfs2/journal.c b/fs/ocfs2/journal.c
> index 530fba34f6d3..25821077b855 100644
> --- a/fs/ocfs2/journal.c
> +++ b/fs/ocfs2/journal.c
> @@ -1077,9 +1077,11 @@ void ocfs2_journal_shutdown(struct ocfs2_super *osb)
>         BUG_ON(atomic_read(&(osb->journal->j_num_trans)) != 0);
>
>         if (ocfs2_mount_local(osb)) {
> -               jbd2_journal_lock_updates(journal->j_journal);
> -               status = jbd2_journal_flush(journal->j_journal, 0);
> -               jbd2_journal_unlock_updates(journal->j_journal);
> +               if (journal->j_journal->j_sb_buffer) {
> +                       jbd2_journal_lock_updates(journal->j_journal);
> +                       status = jbd2_journal_flush(journal->j_journal, 0);
> +                       jbd2_journal_unlock_updates(journal->j_journal);
> +               }
>                 if (status < 0)
>                         mlog_errno(status);
>         }
> --
> 2.43.0
>
>

Thanks,
-- 
Julian Sun <sunjunchao2870@gmail.com>