[syzbot] [jfs?] possible deadlock in diFree (2)

[syzbot]

Hello,

syzbot found the following issue on:

HEAD commit: 4a5df3796467 Merge tag 'mm-hotfixes-stable-2024-11-16-15-3..
git tree: upstream
console output: https://syzkaller.appspot.com/x/log.txt?x=17315378580000
kernel config: https://syzkaller.appspot.com/x/.config?x=95b7d4b29182ed62
dashboard link: https://syzkaller.appspot.com/bug?extid=355da3b3a74881008e8f
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=15d4db5f980000
C reproducer: https://syzkaller.appspot.com/x/repro.c?x=10bef130580000

Downloadable assets:
disk image (non-bootable): https://storage.googleapis.com/syzbot-assets/7feb34a89c2a/non_bootable_disk-4a5df379.raw.xz
vmlinux: https://storage.googleapis.com/syzbot-assets/cb509a18129b/vmlinux-4a5df379.xz
kernel image: https://storage.googleapis.com/syzbot-assets/4a4deaeedf54/bzImage-4a5df379.xz
mounted in repro: https://storage.googleapis.com/syzbot-assets/041539d17a26/mount_0.gz

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

ERROR: (device loop0): remounting filesystem as read-only
ERROR: (device loop0): duplicateIXtree:
============================================
WARNING: possible recursive locking detected
6.12.0-rc7-syzkaller-00212-g4a5df3796467 #0 Not tainted
--------------------------------------------
syz-executor301/5309 is trying to acquire lock:
ffff888044548920 (&(imap->im_aglock[index])){+.+.}-{3:3}, at: diFree+0x37c/0x2fb0 fs/jfs/jfs_imap.c:889

but task is already holding lock:
ffff888044548920 (&(imap->im_aglock[index])){+.+.}-{3:3}, at: diAlloc+0x1b6/0x1630

other info that might help us debug this:
Possible unsafe locking scenario:

CPU0
----
lock(&(imap->im_aglock[index]));
lock(&(imap->im_aglock[index]));

*** DEADLOCK ***

May be due to missing lock nesting notation

5 locks held by syz-executor301/5309:
#0: ffff8880422a4420 (sb_writers#9){.+.+}-{0:0}, at: mnt_want_write+0x3f/0x90 fs/namespace.c:515
#1: ffff88804755b390 (&type->i_mutex_dir_key#6/1){+.+.}-{3:3}, at: inode_lock_nested include/linux/fs.h:850 [inline]
#1: ffff88804755b390 (&type->i_mutex_dir_key#6/1){+.+.}-{3:3}, at: filename_create+0x260/0x540 fs/namei.c:4026
#2: ffff888044548920 (&(imap->im_aglock[index])){+.+.}-{3:3}, at: diAlloc+0x1b6/0x1630
#3: ffff888044548890 (&imap->im_freelock){+.+.}-{3:3}, at: diNewIAG fs/jfs/jfs_imap.c:2460 [inline]
#3: ffff888044548890 (&imap->im_freelock){+.+.}-{3:3}, at: diAllocExt fs/jfs/jfs_imap.c:1905 [inline]
#3: ffff888044548890 (&imap->im_freelock){+.+.}-{3:3}, at: diAllocAG+0x4b7/0x1e50 fs/jfs/jfs_imap.c:1669
#4: ffff88804755a618 (&jfs_ip->rdwrlock/1){++++}-{3:3}, at: diNewIAG fs/jfs/jfs_imap.c:2477 [inline]
#4: ffff88804755a618 (&jfs_ip->rdwrlock/1){++++}-{3:3}, at: diAllocExt fs/jfs/jfs_imap.c:1905 [inline]
#4: ffff88804755a618 (&jfs_ip->rdwrlock/1){++++}-{3:3}, at: diAllocAG+0x869/0x1e50 fs/jfs/jfs_imap.c:1669

stack backtrace:
CPU: 0 UID: 0 PID: 5309 Comm: syz-executor301 Not tainted 6.12.0-rc7-syzkaller-00212-g4a5df3796467 #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:94 [inline]
dump_stack_lvl+0x241/0x360 lib/dump_stack.c:120
print_deadlock_bug+0x483/0x620 kernel/locking/lockdep.c:3037
check_deadlock kernel/locking/lockdep.c:3089 [inline]
validate_chain+0x15e2/0x5920 kernel/locking/lockdep.c:3891
__lock_acquire+0x1384/0x2050 kernel/locking/lockdep.c:5202
lock_acquire+0x1ed/0x550 kernel/locking/lockdep.c:5825
__mutex_lock_common kernel/locking/mutex.c:608 [inline]
__mutex_lock+0x136/0xd70 kernel/locking/mutex.c:752
diFree+0x37c/0x2fb0 fs/jfs/jfs_imap.c:889
jfs_evict_inode+0x32d/0x440 fs/jfs/inode.c:156
evict+0x4e8/0x9b0 fs/inode.c:725
diFreeSpecial fs/jfs/jfs_imap.c:552 [inline]
duplicateIXtree+0x3c6/0x550 fs/jfs/jfs_imap.c:3022
diNewIAG fs/jfs/jfs_imap.c:2597 [inline]
diAllocExt fs/jfs/jfs_imap.c:1905 [inline]
diAllocAG+0x17dc/0x1e50 fs/jfs/jfs_imap.c:1669
diAlloc+0x1d2/0x1630 fs/jfs/jfs_imap.c:1590
ialloc+0x8f/0x900 fs/jfs/jfs_inode.c:56
jfs_mkdir+0x1c5/0xba0 fs/jfs/namei.c:225
vfs_mkdir+0x2f9/0x4f0 fs/namei.c:4257
do_mkdirat+0x264/0x3a0 fs/namei.c:4280
__do_sys_mkdirat fs/namei.c:4295 [inline]
__se_sys_mkdirat fs/namei.c:4293 [inline]
__x64_sys_mkdirat+0x87/0xa0 fs/namei.c:4293
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:0x7fc96b45f879
Code: 28 00 00 00 75 05 48 83 c4 28 c3 e8 61 17 00 00 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 73 01 c3 48 c7 c1 b8 ff ff ff f7 d8 64 89 01 48
RSP: 002b:00007ffca87793b8 EFLAGS: 00000246 ORIG_RAX: 0000000000000102
RAX: ffffffffffffffda RBX: 00007ffca8779588 RCX: 00007fc96b45f879
RDX: 0000000000000000 RSI: 00000000200005c0 RDI: 00000000ffffff9c
RBP: 00007fc96b4d9610 R08: 00007ffca8779588 R09: 00007ffca8779588
R10: 0000000000000000 R11: 0000000000000246 R12: 0000000000000001
R13: 00007ffca8779578 R14: 0000000000000001 R15: 0000000000000001
</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 syzk...@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

[Lizhi Xu]

In following calltrace, diAlloc and diFree need to hold same ag lock,
so before calling duplicateIXtree(), we maybe unlock it, and relock it after.

diAlloc()->
diAllocAG()->
diAllocExt()->
diNewIAG()->
duplicateIXtree()->
diFreeSpecial()->
evict()->
jfs_evict_inode()->
diFree()

#syz test

diff --git a/fs/jfs/jfs_imap.c b/fs/jfs/jfs_imap.c
index a360b24ed320..1f47c6e5456b 100644
--- a/fs/jfs/jfs_imap.c
+++ b/fs/jfs/jfs_imap.c
@@ -2594,7 +2594,9 @@ diNewIAG(struct inomap * imap, int *iagnop, int agno, struct metapage ** mpp)
txEnd(tid);
mutex_unlock(&JFS_IP(ipimap)->commit_mutex);

+ AG_UNLOCK(imap, agno);
duplicateIXtree(sb, blkno, xlen, &xaddr);
+ AG_LOCK(imap, agno);

/* update the next available iag number */
imap->im_nextiag += 1;

[syzbot]

Hello,

syzbot has tested the proposed patch but the reproducer is still triggering an issue:
possible deadlock in diFree

ERROR: (device loop0): remounting filesystem as read-only
ERROR: (device loop0): duplicateIXtree:

======================================================
WARNING: possible circular locking dependency detected
6.13.0-rc3-syzkaller-geabcdba3ad40-dirty #0 Not tainted
------------------------------------------------------
syz.0.15/5791 is trying to acquire lock:
ffff88804f3e0920 (&(imap->im_aglock[index])){+.+.}-{4:4}, at: diFree+0x37c/0x2fb0 fs/jfs/jfs_imap.c:889

but task is already holding lock:

ffff8880420f2618 (&jfs_ip->rdwrlock/1){++++}-{4:4}, at: diNewIAG fs/jfs/jfs_imap.c:2477 [inline]
ffff8880420f2618 (&jfs_ip->rdwrlock/1){++++}-{4:4}, at: diAllocExt fs/jfs/jfs_imap.c:1905 [inline]
ffff8880420f2618 (&jfs_ip->rdwrlock/1){++++}-{4:4}, at: diAllocAG+0x866/0x1ee0 fs/jfs/jfs_imap.c:1669

which lock already depends on the new lock.


the existing dependency chain (in reverse order) is:

-> #1 (&jfs_ip->rdwrlock/1){++++}-{4:4}:
lock_acquire+0x1ed/0x550 kernel/locking/lockdep.c:5849
down_read_nested+0xb5/0xa50 kernel/locking/rwsem.c:1649
diAlloc+0x775/0x1630 fs/jfs/jfs_imap.c:1388
ialloc+0x8f/0x900 fs/jfs/jfs_inode.c:56
jfs_create+0x1be/0xbb0 fs/jfs/namei.c:92
lookup_open fs/namei.c:3649 [inline]
open_last_lookups fs/namei.c:3748 [inline]
path_openat+0x1c03/0x3590 fs/namei.c:3984
do_filp_open+0x27f/0x4e0 fs/namei.c:4014
do_sys_openat2+0x13e/0x1d0 fs/open.c:1402
do_sys_open fs/open.c:1417 [inline]
__do_sys_openat fs/open.c:1433 [inline]
__se_sys_openat fs/open.c:1428 [inline]
__x64_sys_openat+0x247/0x2a0 fs/open.c:1428

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

-> #0 (&(imap->im_aglock[index])){+.+.}-{4:4}:
check_prev_add kernel/locking/lockdep.c:3161 [inline]
check_prevs_add kernel/locking/lockdep.c:3280 [inline]
validate_chain+0x18ef/0x5920 kernel/locking/lockdep.c:3904
__lock_acquire+0x1397/0x2100 kernel/locking/lockdep.c:5226
lock_acquire+0x1ed/0x550 kernel/locking/lockdep.c:5849
__mutex_lock_common kernel/locking/mutex.c:585 [inline]
__mutex_lock+0x1ac/0xee0 kernel/locking/mutex.c:735

diFree+0x37c/0x2fb0 fs/jfs/jfs_imap.c:889
jfs_evict_inode+0x32d/0x440 fs/jfs/inode.c:156

evict+0x4e8/0x9a0 fs/inode.c:796
diFreeSpecial fs/jfs/jfs_imap.c:552 [inline]
duplicateIXtree+0x3c6/0x550 fs/jfs/jfs_imap.c:3024
diNewIAG fs/jfs/jfs_imap.c:2598 [inline]
diAllocExt fs/jfs/jfs_imap.c:1905 [inline]
diAllocAG+0x1805/0x1ee0 fs/jfs/jfs_imap.c:1669

diAlloc+0x1d2/0x1630 fs/jfs/jfs_imap.c:1590
ialloc+0x8f/0x900 fs/jfs/jfs_inode.c:56
jfs_mkdir+0x1c5/0xba0 fs/jfs/namei.c:225

vfs_mkdir+0x2f9/0x4f0 fs/namei.c:4311
do_mkdirat+0x264/0x3a0 fs/namei.c:4334
__do_sys_mkdirat fs/namei.c:4349 [inline]
__se_sys_mkdirat fs/namei.c:4347 [inline]
__x64_sys_mkdirat+0x87/0xa0 fs/namei.c:4347

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

other info that might help us debug this:

Possible unsafe locking scenario:

CPU0 CPU1
---- ----
lock(&jfs_ip->rdwrlock/1);
lock(&(imap->im_aglock[index]));
lock(&jfs_ip->rdwrlock/1);

lock(&(imap->im_aglock[index]));

*** DEADLOCK ***

4 locks held by syz.0.15/5791:
#0: ffff888000c70420 (sb_writers#13){.+.+}-{0:0}, at: mnt_want_write+0x3f/0x90 fs/namespace.c:515
#1: ffff8880420f3390 (&type->i_mutex_dir_key#8/1){+.+.}-{4:4}, at: inode_lock_nested include/linux/fs.h:853 [inline]
#1: ffff8880420f3390 (&type->i_mutex_dir_key#8/1){+.+.}-{4:4}, at: filename_create+0x260/0x540 fs/namei.c:4080
#2: ffff88804f3e0890 (&imap->im_freelock){+.+.}-{4:4}, at: diNewIAG fs/jfs/jfs_imap.c:2460 [inline]
#2: ffff88804f3e0890 (&imap->im_freelock){+.+.}-{4:4}, at: diAllocExt fs/jfs/jfs_imap.c:1905 [inline]
#2: ffff88804f3e0890 (&imap->im_freelock){+.+.}-{4:4}, at: diAllocAG+0x49b/0x1ee0 fs/jfs/jfs_imap.c:1669
#3: ffff8880420f2618 (&jfs_ip->rdwrlock/1){++++}-{4:4}, at: diNewIAG fs/jfs/jfs_imap.c:2477 [inline]
#3: ffff8880420f2618 (&jfs_ip->rdwrlock/1){++++}-{4:4}, at: diAllocExt fs/jfs/jfs_imap.c:1905 [inline]
#3: ffff8880420f2618 (&jfs_ip->rdwrlock/1){++++}-{4:4}, at: diAllocAG+0x866/0x1ee0 fs/jfs/jfs_imap.c:1669

stack backtrace:
CPU: 0 UID: 0 PID: 5791 Comm: syz.0.15 Not tainted 6.13.0-rc3-syzkaller-geabcdba3ad40-dirty #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:94 [inline]
dump_stack_lvl+0x241/0x360 lib/dump_stack.c:120

print_circular_bug+0x13a/0x1b0 kernel/locking/lockdep.c:2074
check_noncircular+0x36a/0x4a0 kernel/locking/lockdep.c:2206
check_prev_add kernel/locking/lockdep.c:3161 [inline]
check_prevs_add kernel/locking/lockdep.c:3280 [inline]
validate_chain+0x18ef/0x5920 kernel/locking/lockdep.c:3904
__lock_acquire+0x1397/0x2100 kernel/locking/lockdep.c:5226
lock_acquire+0x1ed/0x550 kernel/locking/lockdep.c:5849
__mutex_lock_common kernel/locking/mutex.c:585 [inline]
__mutex_lock+0x1ac/0xee0 kernel/locking/mutex.c:735

diFree+0x37c/0x2fb0 fs/jfs/jfs_imap.c:889
jfs_evict_inode+0x32d/0x440 fs/jfs/inode.c:156

evict+0x4e8/0x9a0 fs/inode.c:796
diFreeSpecial fs/jfs/jfs_imap.c:552 [inline]
duplicateIXtree+0x3c6/0x550 fs/jfs/jfs_imap.c:3024
diNewIAG fs/jfs/jfs_imap.c:2598 [inline]
diAllocExt fs/jfs/jfs_imap.c:1905 [inline]
diAllocAG+0x1805/0x1ee0 fs/jfs/jfs_imap.c:1669

diAlloc+0x1d2/0x1630 fs/jfs/jfs_imap.c:1590
ialloc+0x8f/0x900 fs/jfs/jfs_inode.c:56
jfs_mkdir+0x1c5/0xba0 fs/jfs/namei.c:225

vfs_mkdir+0x2f9/0x4f0 fs/namei.c:4311
do_mkdirat+0x264/0x3a0 fs/namei.c:4334
__do_sys_mkdirat fs/namei.c:4349 [inline]
__se_sys_mkdirat fs/namei.c:4347 [inline]
__x64_sys_mkdirat+0x87/0xa0 fs/namei.c:4347

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:0x7f849617e719
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:00007f8496ecc038 EFLAGS: 00000246 ORIG_RAX: 0000000000000102
RAX: ffffffffffffffda RBX: 00007f8496335f80 RCX: 00007f849617e719
RDX: 0000000000000000 RSI: 00000000200005c0 RDI: ffffffffffffff9c
RBP: 00007f84961f175e R08: 0000000000000000 R09: 0000000000000000
R10: 0000000000000000 R11: 0000000000000246 R12: 0000000000000000
R13: 0000000000000000 R14: 00007f8496335f80 R15: 00007ffec77ff6a8
</TASK>


Tested on:

commit: eabcdba3 Merge tag 'for-6.13-rc3-tag' of git://git.ker..
git tree: upstream
console output: https://syzkaller.appspot.com/x/log.txt?x=12b5b7e8580000
kernel config: https://syzkaller.appspot.com/x/.config?x=f339dc38531e69a7

dashboard link: https://syzkaller.appspot.com/bug?extid=355da3b3a74881008e8f
compiler: Debian clang version 15.0.6, GNU ld (GNU Binutils for Debian) 2.40

patch: https://syzkaller.appspot.com/x/patch.diff?x=15e5b7e8580000

[Lizhi Xu]

In following calltrace, diAlloc and diFree need to hold same ag lock,
so before calling duplicateIXtree(), we maybe unlock it, and relock it after.

diAlloc()->
diAllocAG()->
diAllocExt()->
diNewIAG()->
duplicateIXtree()->
diFreeSpecial()->
evict()->
jfs_evict_inode()->
diFree()

#syz test

diff --git a/fs/jfs/jfs_imap.c b/fs/jfs/jfs_imap.c

index a360b24ed320..ca25ff57c48a 100644
--- a/fs/jfs/jfs_imap.c
+++ b/fs/jfs/jfs_imap.c
@@ -2594,7 +2594,11 @@ diNewIAG(struct inomap * imap, int *iagnop, int agno, struct metapage ** mpp)
txEnd(tid);
mutex_unlock(&JFS_IP(ipimap)->commit_mutex);

+ IWRITE_UNLOCK(ipimap);

+ AG_UNLOCK(imap, agno);
duplicateIXtree(sb, blkno, xlen, &xaddr);
+ AG_LOCK(imap, agno);

+ IWRITE_LOCK(ipimap, RDWRLOCK_IMAP);

[syzbot]

Hello,

syzbot has tested the proposed patch but the reproducer is still triggering an issue:
possible deadlock in diFree

ERROR: (device loop0): remounting filesystem as read-only
ERROR: (device loop0): duplicateIXtree:
======================================================
WARNING: possible circular locking dependency detected
6.13.0-rc3-syzkaller-geabcdba3ad40-dirty #0 Not tainted
------------------------------------------------------

syz.0.15/5813 is trying to acquire lock:
ffff8880397c0920 (&(imap->im_aglock[index])){+.+.}-{4:4}, at: diFree+0x37c/0x2fb0 fs/jfs/jfs_imap.c:889

but task is already holding lock:

ffff8880397c0890 (&imap->im_freelock){+.+.}-{4:4}, at: diNewIAG fs/jfs/jfs_imap.c:2460 [inline]
ffff8880397c0890 (&imap->im_freelock){+.+.}-{4:4}, at: diAllocExt fs/jfs/jfs_imap.c:1905 [inline]
ffff8880397c0890 (&imap->im_freelock){+.+.}-{4:4}, at: diAllocAG+0x49b/0x1f00 fs/jfs/jfs_imap.c:1669

which lock already depends on the new lock.


the existing dependency chain (in reverse order) is:

-> #1 (&imap->im_freelock){+.+.}-{4:4}:

lock_acquire+0x1ed/0x550 kernel/locking/lockdep.c:5849
__mutex_lock_common kernel/locking/mutex.c:585 [inline]
__mutex_lock+0x1ac/0xee0 kernel/locking/mutex.c:735

diNewIAG fs/jfs/jfs_imap.c:2460 [inline]
diAllocExt fs/jfs/jfs_imap.c:1905 [inline]
diAllocAG+0x49b/0x1f00 fs/jfs/jfs_imap.c:1669

diAlloc+0x1d2/0x1630 fs/jfs/jfs_imap.c:1590
ialloc+0x8f/0x900 fs/jfs/jfs_inode.c:56
jfs_mkdir+0x1c5/0xba0 fs/jfs/namei.c:225
vfs_mkdir+0x2f9/0x4f0 fs/namei.c:4311
do_mkdirat+0x264/0x3a0 fs/namei.c:4334
__do_sys_mkdirat fs/namei.c:4349 [inline]
__se_sys_mkdirat fs/namei.c:4347 [inline]
__x64_sys_mkdirat+0x87/0xa0 fs/namei.c:4347
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

-> #0 (&(imap->im_aglock[index])){+.+.}-{4:4}:
check_prev_add kernel/locking/lockdep.c:3161 [inline]
check_prevs_add kernel/locking/lockdep.c:3280 [inline]
validate_chain+0x18ef/0x5920 kernel/locking/lockdep.c:3904
__lock_acquire+0x1397/0x2100 kernel/locking/lockdep.c:5226
lock_acquire+0x1ed/0x550 kernel/locking/lockdep.c:5849
__mutex_lock_common kernel/locking/mutex.c:585 [inline]
__mutex_lock+0x1ac/0xee0 kernel/locking/mutex.c:735
diFree+0x37c/0x2fb0 fs/jfs/jfs_imap.c:889
jfs_evict_inode+0x32d/0x440 fs/jfs/inode.c:156
evict+0x4e8/0x9a0 fs/inode.c:796
diFreeSpecial fs/jfs/jfs_imap.c:552 [inline]

duplicateIXtree+0x3c6/0x550 fs/jfs/jfs_imap.c:3026
diNewIAG fs/jfs/jfs_imap.c:2599 [inline]
diAllocExt fs/jfs/jfs_imap.c:1905 [inline]
diAllocAG+0x180f/0x1f00 fs/jfs/jfs_imap.c:1669

diAlloc+0x1d2/0x1630 fs/jfs/jfs_imap.c:1590
ialloc+0x8f/0x900 fs/jfs/jfs_inode.c:56
jfs_mkdir+0x1c5/0xba0 fs/jfs/namei.c:225
vfs_mkdir+0x2f9/0x4f0 fs/namei.c:4311
do_mkdirat+0x264/0x3a0 fs/namei.c:4334
__do_sys_mkdirat fs/namei.c:4349 [inline]
__se_sys_mkdirat fs/namei.c:4347 [inline]
__x64_sys_mkdirat+0x87/0xa0 fs/namei.c:4347
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

other info that might help us debug this:

Possible unsafe locking scenario:

CPU0 CPU1
---- ----

lock(&imap->im_freelock);
lock(&(imap->im_aglock[index]));
lock(&imap->im_freelock);

lock(&(imap->im_aglock[index]));

*** DEADLOCK ***

3 locks held by syz.0.15/5813:
#0: ffff88801e2fa420 (sb_writers#13){.+.+}-{0:0}, at: mnt_want_write+0x3f/0x90 fs/namespace.c:515
#1: ffff888043c1b390 (&type->i_mutex_dir_key#8/1){+.+.}-{4:4}, at: inode_lock_nested include/linux/fs.h:853 [inline]
#1: ffff888043c1b390 (&type->i_mutex_dir_key#8/1){+.+.}-{4:4}, at: filename_create+0x260/0x540 fs/namei.c:4080
#2: ffff8880397c0890 (&imap->im_freelock){+.+.}-{4:4}, at: diNewIAG fs/jfs/jfs_imap.c:2460 [inline]
#2: ffff8880397c0890 (&imap->im_freelock){+.+.}-{4:4}, at: diAllocExt fs/jfs/jfs_imap.c:1905 [inline]
#2: ffff8880397c0890 (&imap->im_freelock){+.+.}-{4:4}, at: diAllocAG+0x49b/0x1f00 fs/jfs/jfs_imap.c:1669

stack backtrace:
CPU: 0 UID: 0 PID: 5813 Comm: syz.0.15 Not tainted 6.13.0-rc3-syzkaller-geabcdba3ad40-dirty #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:94 [inline]
dump_stack_lvl+0x241/0x360 lib/dump_stack.c:120
print_circular_bug+0x13a/0x1b0 kernel/locking/lockdep.c:2074
check_noncircular+0x36a/0x4a0 kernel/locking/lockdep.c:2206
check_prev_add kernel/locking/lockdep.c:3161 [inline]
check_prevs_add kernel/locking/lockdep.c:3280 [inline]
validate_chain+0x18ef/0x5920 kernel/locking/lockdep.c:3904
__lock_acquire+0x1397/0x2100 kernel/locking/lockdep.c:5226
lock_acquire+0x1ed/0x550 kernel/locking/lockdep.c:5849
__mutex_lock_common kernel/locking/mutex.c:585 [inline]
__mutex_lock+0x1ac/0xee0 kernel/locking/mutex.c:735
diFree+0x37c/0x2fb0 fs/jfs/jfs_imap.c:889
jfs_evict_inode+0x32d/0x440 fs/jfs/inode.c:156
evict+0x4e8/0x9a0 fs/inode.c:796
diFreeSpecial fs/jfs/jfs_imap.c:552 [inline]

duplicateIXtree+0x3c6/0x550 fs/jfs/jfs_imap.c:3026
diNewIAG fs/jfs/jfs_imap.c:2599 [inline]
diAllocExt fs/jfs/jfs_imap.c:1905 [inline]
diAllocAG+0x180f/0x1f00 fs/jfs/jfs_imap.c:1669

diAlloc+0x1d2/0x1630 fs/jfs/jfs_imap.c:1590
ialloc+0x8f/0x900 fs/jfs/jfs_inode.c:56
jfs_mkdir+0x1c5/0xba0 fs/jfs/namei.c:225
vfs_mkdir+0x2f9/0x4f0 fs/namei.c:4311
do_mkdirat+0x264/0x3a0 fs/namei.c:4334
__do_sys_mkdirat fs/namei.c:4349 [inline]
__se_sys_mkdirat fs/namei.c:4347 [inline]
__x64_sys_mkdirat+0x87/0xa0 fs/namei.c:4347
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:0x7f35b537e719

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:00007f35b6113038 EFLAGS: 00000246 ORIG_RAX: 0000000000000102
RAX: ffffffffffffffda RBX: 00007f35b5535f80 RCX: 00007f35b537e719

RDX: 0000000000000000 RSI: 00000000200005c0 RDI: ffffffffffffff9c

RBP: 00007f35b53f175e R08: 0000000000000000 R09: 0000000000000000

R10: 0000000000000000 R11: 0000000000000246 R12: 0000000000000000

R13: 0000000000000000 R14: 00007f35b5535f80 R15: 00007ffe3b78dd48
</TASK>
ERROR: (device loop0): diFree: wmap shows inode already free

Tested on:

commit: eabcdba3 Merge tag 'for-6.13-rc3-tag' of git://git.ker..
git tree: upstream

console output: https://syzkaller.appspot.com/x/log.txt?x=17ea77e8580000

kernel config: https://syzkaller.appspot.com/x/.config?x=f339dc38531e69a7
dashboard link: https://syzkaller.appspot.com/bug?extid=355da3b3a74881008e8f
compiler: Debian clang version 15.0.6, GNU ld (GNU Binutils for Debian) 2.40

patch: https://syzkaller.appspot.com/x/patch.diff?x=17054cf8580000

[Lizhi Xu]

In following calltrace, diAlloc and diFree need to hold same ag lock,
so before calling duplicateIXtree(), we maybe unlock it, and relock it after.

diAlloc()->
diAllocAG()->
diAllocExt()->
diNewIAG()->
duplicateIXtree()->
diFreeSpecial()->
evict()->
jfs_evict_inode()->
diFree()

#syz test

diff --git a/fs/jfs/jfs_imap.c b/fs/jfs/jfs_imap.c

index a360b24ed320..9f105c748447 100644
--- a/fs/jfs/jfs_imap.c
+++ b/fs/jfs/jfs_imap.c
@@ -2594,7 +2594,13 @@ diNewIAG(struct inomap * imap, int *iagnop, int agno, struct metapage ** mpp)

txEnd(tid);
mutex_unlock(&JFS_IP(ipimap)->commit_mutex);

+ IWRITE_UNLOCK(ipimap);

+ IAGFREE_UNLOCK(imap);

+ AG_UNLOCK(imap, agno);
duplicateIXtree(sb, blkno, xlen, &xaddr);
+ AG_LOCK(imap, agno);

+ IAGFREE_LOCK(imap);

[syzbot]

Hello,

syzbot has tested the proposed patch and the reproducer did not trigger any issue:

Reported-by: syzbot+355da3...@syzkaller.appspotmail.com
Tested-by: syzbot+355da3...@syzkaller.appspotmail.com

Tested on:

commit: eabcdba3 Merge tag 'for-6.13-rc3-tag' of git://git.ker..
git tree: upstream

console output: https://syzkaller.appspot.com/x/log.txt?x=11922e0f980000

kernel config: https://syzkaller.appspot.com/x/.config?x=f339dc38531e69a7
dashboard link: https://syzkaller.appspot.com/bug?extid=355da3b3a74881008e8f
compiler: Debian clang version 15.0.6, GNU ld (GNU Binutils for Debian) 2.40

patch: https://syzkaller.appspot.com/x/patch.diff?x=1493ef44580000

Note: testing is done by a robot and is best-effort only.

[Edward Adam Davis]

syzbot report a deadlock in diFree. [1]

When calling "ioctl$LOOP_SET_STATUS64", the offset value passed in is 4,
which does not match the mounted loop device, causing the mapping of the
mounted loop device to be invalidated.

When creating the directory and creating the inode of iag in diReadSpecial(),
read the page of fixed disk inode (AIT) in raw mode in read_metapage(), the
metapage data it returns is corrupted, which causes the nlink value of 0 to be
assigned to the iag inode when executing copy_from_dinode(), which ultimately
causes a deadlock when entering diFree().

To avoid this, first check the nlink value of dinode before setting iag inode,
if the value is 0, set it to 1.

[1]

Reported-by: syzbot+355da3...@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=355da3b3a74881008e8f
Signed-off-by: Edward Adam Davis <ead...@qq.com>
---
fs/jfs/jfs_imap.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/fs/jfs/jfs_imap.c b/fs/jfs/jfs_imap.c
index a360b24ed320..78892d252159 100644
--- a/fs/jfs/jfs_imap.c
+++ b/fs/jfs/jfs_imap.c
@@ -3035,6 +3035,7 @@ static int copy_from_dinode(struct dinode * dip, struct inode *ip)
{
struct jfs_inode_info *jfs_ip = JFS_IP(ip);
struct jfs_sb_info *sbi = JFS_SBI(ip->i_sb);
+ u32 di_nlink;

jfs_ip->fileset = le32_to_cpu(dip->di_fileset);
jfs_ip->mode2 = le32_to_cpu(dip->di_mode);
@@ -3053,7 +3054,9 @@ static int copy_from_dinode(struct dinode * dip, struct inode *ip)
ip->i_mode |= 0001;
}
}
- set_nlink(ip, le32_to_cpu(dip->di_nlink));
+
+ di_nlink = le32_to_cpu(dip->di_nlink);
+ set_nlink(ip, di_nlink > 0 ? di_nlink : 1);

jfs_ip->saved_uid = make_kuid(&init_user_ns, le32_to_cpu(dip->di_uid));
if (!uid_valid(sbi->uid))
--
2.47.0

[Edward Adam Davis]

syzbot report a deadlock in diFree. [1]

When calling "ioctl$LOOP_SET_STATUS64", the offset value passed in is 4,
which does not match the mounted loop device, causing the mapping of the
mounted loop device to be invalidated.

When creating the directory and creating the inode of iag in diReadSpecial(),
read the page of fixed disk inode (AIT) in raw mode in read_metapage(), the
metapage data it returns is corrupted, which causes the nlink value of 0 to be
assigned to the iag inode when executing copy_from_dinode(), which ultimately
causes a deadlock when entering diFree().

To avoid this, first check the nlink value of dinode before setting iag inode.

V1 -> V2: if the nlink of disk inode is 0 return -EIO

fs/jfs/jfs_imap.c | 8 +++++++-
1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/fs/jfs/jfs_imap.c b/fs/jfs/jfs_imap.c
index a360b24ed320..b3146e335782 100644

--- a/fs/jfs/jfs_imap.c
+++ b/fs/jfs/jfs_imap.c
@@ -3035,6 +3035,7 @@ static int copy_from_dinode(struct dinode * dip, struct inode *ip)
{
struct jfs_inode_info *jfs_ip = JFS_IP(ip);
struct jfs_sb_info *sbi = JFS_SBI(ip->i_sb);
+ u32 di_nlink;

jfs_ip->fileset = le32_to_cpu(dip->di_fileset);
jfs_ip->mode2 = le32_to_cpu(dip->di_mode);

@@ -3053,7 +3054,12 @@ static int copy_from_dinode(struct dinode * dip, struct inode *ip)

ip->i_mode |= 0001;
}
}
- set_nlink(ip, le32_to_cpu(dip->di_nlink));
+
+ di_nlink = le32_to_cpu(dip->di_nlink);

+ if (!di_nlink)
+ return -EIO;
+
+ set_nlink(ip, di_nlink);

[Dave Kleikamp]

I'm catching up on some long-ignored emails and have a concern about
this patch.

On 12/24/24 2:17AM, Edward Adam Davis via Jfs-discussion wrote:
> syzbot report a deadlock in diFree. [1]
>
> When calling "ioctl$LOOP_SET_STATUS64", the offset value passed in is 4,
> which does not match the mounted loop device, causing the mapping of the
> mounted loop device to be invalidated.
>
> When creating the directory and creating the inode of iag in diReadSpecial(),
> read the page of fixed disk inode (AIT) in raw mode in read_metapage(), the
> metapage data it returns is corrupted, which causes the nlink value of 0 to be
> assigned to the iag inode when executing copy_from_dinode(), which ultimately
> causes a deadlock when entering diFree().

I'm not quite sure if it's always a problem to read an inode with
i_nlink = 0, so I think it may be safer to move this check from
copy_from_dinode to diReadSpecial.


Shaggy

[Edward Adam Davis]

syzbot report a deadlock in diFree. [1]

When calling "ioctl$LOOP_SET_STATUS64", the offset value passed in is 4,
which does not match the mounted loop device, causing the mapping of the
mounted loop device to be invalidated.

When creating the directory and creating the inode of iag in diReadSpecial(),
read the page of fixed disk inode (AIT) in raw mode in read_metapage(), the
metapage data it returns is corrupted, which causes the nlink value of 0 to be
assigned to the iag inode when executing copy_from_dinode(), which ultimately
causes a deadlock when entering diFree().

V2 -> V3: move the checking to diReadSpecial

---
fs/jfs/jfs_imap.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fs/jfs/jfs_imap.c b/fs/jfs/jfs_imap.c
index 0cedaccb7218..25bb3485da3b 100644
--- a/fs/jfs/jfs_imap.c
+++ b/fs/jfs/jfs_imap.c
@@ -460,7 +460,7 @@ struct inode *diReadSpecial(struct super_block *sb, ino_t inum, int secondary)
dp += inum % 8; /* 8 inodes per 4K page */

/* copy on-disk inode to in-memory inode */
- if ((copy_from_dinode(dp, ip)) != 0) {
+ if (!le32_to_cpu(dp->di_nlink) || (copy_from_dinode(dp, ip)) != 0) {
/* handle bad return by returning NULL for ip */
set_nlink(ip, 1); /* Don't want iput() deleting it */
iput(ip);
--
2.43.0

[Dave Kleikamp]

I'm taking this patch, but making a change. It's a little cleaner to check ip->i_nlink after calling copy_from_dinode.

>
> Reported-by: syzbot+355da3...@syzkaller.appspotmail.com
> Closes: https://syzkaller.appspot.com/bug?extid=355da3b3a74881008e8f
> Signed-off-by: Edward Adam Davis <ead...@qq.com>
> ---
> V1 -> V2: if the nlink of disk inode is 0 return -EIO
> V2 -> V3: move the checking to diReadSpecial
>
> ---
> fs/jfs/jfs_imap.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/fs/jfs/jfs_imap.c b/fs/jfs/jfs_imap.c
> index 0cedaccb7218..25bb3485da3b 100644
> --- a/fs/jfs/jfs_imap.c
> +++ b/fs/jfs/jfs_imap.c
> @@ -460,7 +460,7 @@ struct inode *diReadSpecial(struct super_block *sb, ino_t inum, int secondary)
> dp += inum % 8; /* 8 inodes per 4K page */
>
> /* copy on-disk inode to in-memory inode */
> - if ((copy_from_dinode(dp, ip)) != 0) {
> + if (!le32_to_cpu(dp->di_nlink) || (copy_from_dinode(dp, ip)) != 0) {
> /* handle bad return by returning NULL for ip */
> set_nlink(ip, 1); /* Don't want iput() deleting it */
> iput(ip);

My change:

diff --git a/fs/jfs/jfs_imap.c b/fs/jfs/jfs_imap.c
index 298445f6d3d4..ecb8e05b8b84 100644
--- a/fs/jfs/jfs_imap.c
+++ b/fs/jfs/jfs_imap.c
@@ -456,7 +456,7 @@ struct inode *diReadSpecial(struct super_block *sb, ino_t inum, int secondary)

dp += inum % 8; /* 8 inodes per 4K page */

/* copy on-disk inode to in-memory inode */
- if ((copy_from_dinode(dp, ip)) != 0) {

+ if ((copy_from_dinode(dp, ip) != 0) || (ip->i_nlink == 0)) {

[Edward Adam Davis]

This is incorrect. The purpose of adding this check is to prevent copy_from_dinode()
from using dip->i_nlink with a value of 0 to assign to ip.

> /* handle bad return by returning NULL for ip */
> set_nlink(ip, 1); /* Don't want iput() deleting it */
> iput(ip);

BR,
Edward

[Dave Kleikamp]

It will get set to 1 right here ^^^

>> iput(ip);
> BR,
> Edward
>

[Edward Adam Davis]

Things that can be determined by "di_nlink" before executing copy_from_dinode(),
Why let the CPU run copy_from_dinode() for an extra time before checking?
Isn't this a waste of CPU?

BR,
Edward

[Dave Kleikamp]

It's an exceptional case. It's very, very unlikely to fail, so the extra
cpu cycles that are executed in the common case are not a concern.

>
> BR,
> Edward
>