* [syzbot] [jfs?] possible deadlock in diFree (2)
@ 2024-11-21 12:18 syzbot
2024-12-19 2:56 ` [syzbot] " syzbot
` (3 more replies)
0 siblings, 4 replies; 13+ messages in thread
From: syzbot @ 2024-11-21 12:18 UTC (permalink / raw)
To: jfs-discussion, linux-kernel, shaggy, syzkaller-bugs
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+355da3b3a74881008e8f@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 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
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [syzbot] Re: [syzbot] [jfs?] possible deadlock in diFree (2)
2024-11-21 12:18 [syzbot] [jfs?] possible deadlock in diFree (2) syzbot
@ 2024-12-19 2:56 ` syzbot
2024-12-19 8:24 ` syzbot
` (2 subsequent siblings)
3 siblings, 0 replies; 13+ messages in thread
From: syzbot @ 2024-12-19 2:56 UTC (permalink / raw)
To: linux-kernel
For archival purposes, forwarding an incoming command email to
linux-kernel@vger.kernel.org.
***
Subject: Re: [syzbot] [jfs?] possible deadlock in diFree (2)
Author: lizhi.xu@windriver.com
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;
^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [syzbot] Re: [syzbot] [jfs?] possible deadlock in diFree (2)
2024-11-21 12:18 [syzbot] [jfs?] possible deadlock in diFree (2) syzbot
2024-12-19 2:56 ` [syzbot] " syzbot
@ 2024-12-19 8:24 ` syzbot
2024-12-19 8:52 ` syzbot
2024-12-21 7:33 ` [PATCH] jfs: Prevent setting of nlink with value 0 from disk inode Edward Adam Davis
3 siblings, 0 replies; 13+ messages in thread
From: syzbot @ 2024-12-19 8:24 UTC (permalink / raw)
To: linux-kernel
For archival purposes, forwarding an incoming command email to
linux-kernel@vger.kernel.org.
***
Subject: Re: [syzbot] [jfs?] possible deadlock in diFree (2)
Author: lizhi.xu@windriver.com
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);
/* update the next available iag number */
imap->im_nextiag += 1;
^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [syzbot] Re: [syzbot] [jfs?] possible deadlock in diFree (2)
2024-11-21 12:18 [syzbot] [jfs?] possible deadlock in diFree (2) syzbot
2024-12-19 2:56 ` [syzbot] " syzbot
2024-12-19 8:24 ` syzbot
@ 2024-12-19 8:52 ` syzbot
2024-12-21 7:33 ` [PATCH] jfs: Prevent setting of nlink with value 0 from disk inode Edward Adam Davis
3 siblings, 0 replies; 13+ messages in thread
From: syzbot @ 2024-12-19 8:52 UTC (permalink / raw)
To: linux-kernel
For archival purposes, forwarding an incoming command email to
linux-kernel@vger.kernel.org.
***
Subject: Re: [syzbot] [jfs?] possible deadlock in diFree (2)
Author: lizhi.xu@windriver.com
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);
+ IWRITE_LOCK(ipimap, RDWRLOCK_IMAP);
/* update the next available iag number */
imap->im_nextiag += 1;
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH] jfs: Prevent setting of nlink with value 0 from disk inode
2024-11-21 12:18 [syzbot] [jfs?] possible deadlock in diFree (2) syzbot
` (2 preceding siblings ...)
2024-12-19 8:52 ` syzbot
@ 2024-12-21 7:33 ` Edward Adam Davis
2024-12-24 8:17 ` [PATCH V2] jfs: Prevent copying " Edward Adam Davis
3 siblings, 1 reply; 13+ messages in thread
From: Edward Adam Davis @ 2024-12-21 7:33 UTC (permalink / raw)
To: syzbot+355da3b3a74881008e8f
Cc: jfs-discussion, linux-kernel, shaggy, syzkaller-bugs
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]
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
Reported-by: syzbot+355da3b3a74881008e8f@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=355da3b3a74881008e8f
Signed-off-by: Edward Adam Davis <eadavis@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
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH V2] jfs: Prevent copying of nlink with value 0 from disk inode
2024-12-21 7:33 ` [PATCH] jfs: Prevent setting of nlink with value 0 from disk inode Edward Adam Davis
@ 2024-12-24 8:17 ` Edward Adam Davis
2025-02-19 16:38 ` [Jfs-discussion] " Dave Kleikamp
0 siblings, 1 reply; 13+ messages in thread
From: Edward Adam Davis @ 2024-12-24 8:17 UTC (permalink / raw)
To: eadavis
Cc: jfs-discussion, linux-kernel, shaggy, syzbot+355da3b3a74881008e8f,
syzkaller-bugs
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.
[1]
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
Reported-by: syzbot+355da3b3a74881008e8f@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=355da3b3a74881008e8f
Signed-off-by: Edward Adam Davis <eadavis@qq.com>
---
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);
jfs_ip->saved_uid = make_kuid(&init_user_ns, le32_to_cpu(dip->di_uid));
if (!uid_valid(sbi->uid))
--
2.47.0
^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [Jfs-discussion] [PATCH V2] jfs: Prevent copying of nlink with value 0 from disk inode
2024-12-24 8:17 ` [PATCH V2] jfs: Prevent copying " Edward Adam Davis
@ 2025-02-19 16:38 ` Dave Kleikamp
2025-02-20 11:13 ` [PATCH V3] " Edward Adam Davis
0 siblings, 1 reply; 13+ messages in thread
From: Dave Kleikamp @ 2025-02-19 16:38 UTC (permalink / raw)
To: Edward Adam Davis
Cc: jfs-discussion, syzkaller-bugs, linux-kernel,
syzbot+355da3b3a74881008e8f
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
>
> To avoid this, first check the nlink value of dinode before setting iag inode.
>
> [1]
> 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
>
> Reported-by: syzbot+355da3b3a74881008e8f@syzkaller.appspotmail.com
> Closes: https://syzkaller.appspot.com/bug?extid=355da3b3a74881008e8f
> Signed-off-by: Edward Adam Davis <eadavis@qq.com>
> ---
> 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);
>
> jfs_ip->saved_uid = make_kuid(&init_user_ns, le32_to_cpu(dip->di_uid));
> if (!uid_valid(sbi->uid))
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH V3] jfs: Prevent copying of nlink with value 0 from disk inode
2025-02-19 16:38 ` [Jfs-discussion] " Dave Kleikamp
@ 2025-02-20 11:13 ` Edward Adam Davis
2025-02-20 16:15 ` Dave Kleikamp
0 siblings, 1 reply; 13+ messages in thread
From: Edward Adam Davis @ 2025-02-20 11:13 UTC (permalink / raw)
To: dave.kleikamp
Cc: jfs-discussion, linux-kernel, shaggy, syzbot+355da3b3a74881008e8f,
syzkaller-bugs
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.
[1]
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
Reported-by: syzbot+355da3b3a74881008e8f@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=355da3b3a74881008e8f
Signed-off-by: Edward Adam Davis <eadavis@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);
--
2.43.0
^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [PATCH V3] jfs: Prevent copying of nlink with value 0 from disk inode
2025-02-20 11:13 ` [PATCH V3] " Edward Adam Davis
@ 2025-02-20 16:15 ` Dave Kleikamp
2025-02-20 23:22 ` Edward Adam Davis
0 siblings, 1 reply; 13+ messages in thread
From: Dave Kleikamp @ 2025-02-20 16:15 UTC (permalink / raw)
To: Edward Adam Davis
Cc: jfs-discussion, linux-kernel, syzbot+355da3b3a74881008e8f,
syzkaller-bugs
On 2/20/25 5:13AM, Edward Adam Davis 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().
>
> To avoid this, first check the nlink value of dinode before setting iag inode.
>
> [1]
> 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
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+355da3b3a74881008e8f@syzkaller.appspotmail.com
> Closes: https://syzkaller.appspot.com/bug?extid=355da3b3a74881008e8f
> Signed-off-by: Edward Adam Davis <eadavis@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)) {
/* handle bad return by returning NULL for ip */
set_nlink(ip, 1); /* Don't want iput() deleting it */
iput(ip);
^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [PATCH V3] jfs: Prevent copying of nlink with value 0 from disk inode
2025-02-20 16:15 ` Dave Kleikamp
@ 2025-02-20 23:22 ` Edward Adam Davis
2025-02-20 23:28 ` Dave Kleikamp
0 siblings, 1 reply; 13+ messages in thread
From: Edward Adam Davis @ 2025-02-20 23:22 UTC (permalink / raw)
To: dave.kleikamp
Cc: eadavis, jfs-discussion, linux-kernel,
syzbot+355da3b3a74881008e8f, syzkaller-bugs
On Thu, 20 Feb 2025 10:15:04 -0600, Dave Kleikamp 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().
> >
> > To avoid this, first check the nlink value of dinode before setting iag inode.
> >
> > [1]
> > 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
>
> 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+355da3b3a74881008e8f@syzkaller.appspotmail.com
> > Closes: https://syzkaller.appspot.com/bug?extid=355da3b3a74881008e8f
> > Signed-off-by: Edward Adam Davis <eadavis@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)) {
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
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH V3] jfs: Prevent copying of nlink with value 0 from disk inode
2025-02-20 23:22 ` Edward Adam Davis
@ 2025-02-20 23:28 ` Dave Kleikamp
2025-02-22 0:16 ` Edward Adam Davis
0 siblings, 1 reply; 13+ messages in thread
From: Dave Kleikamp @ 2025-02-20 23:28 UTC (permalink / raw)
To: Edward Adam Davis
Cc: jfs-discussion, linux-kernel, syzbot+355da3b3a74881008e8f,
syzkaller-bugs
On 2/20/25 5:22PM, Edward Adam Davis wrote:
> On Thu, 20 Feb 2025 10:15:04 -0600, Dave Kleikamp 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().
>>>
>>> To avoid this, first check the nlink value of dinode before setting iag inode.
>>>
>>> [1]
>>> 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
>>
>> 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+355da3b3a74881008e8f@syzkaller.appspotmail.com
>>> Closes: https://syzkaller.appspot.com/bug?extid=355da3b3a74881008e8f
>>> Signed-off-by: Edward Adam Davis <eadavis@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)) {
> 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 */
It will get set to 1 right here ^^^
>> iput(ip);
> BR,
> Edward
>
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH V3] jfs: Prevent copying of nlink with value 0 from disk inode
2025-02-20 23:28 ` Dave Kleikamp
@ 2025-02-22 0:16 ` Edward Adam Davis
2025-02-24 14:56 ` Dave Kleikamp
0 siblings, 1 reply; 13+ messages in thread
From: Edward Adam Davis @ 2025-02-22 0:16 UTC (permalink / raw)
To: dave.kleikamp
Cc: eadavis, jfs-discussion, linux-kernel,
syzbot+355da3b3a74881008e8f, syzkaller-bugs
On Thu, 20 Feb 2025 17:28:49 -0600, Dave Kleikamp wrote:
>On 2/20/25 5:22PM, Edward Adam Davis wrote:
>> On Thu, 20 Feb 2025 10:15:04 -0600, Dave Kleikamp 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().
>>>>
>>>> To avoid this, first check the nlink value of dinode before setting iag inode.
>>>>
>>>> [1]
>>>> 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
>>>
>>> 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+355da3b3a74881008e8f@syzkaller.appspotmail.com
>>>> Closes: https://syzkaller.appspot.com/bug?extid=355da3b3a74881008e8f
>>>> Signed-off-by: Edward Adam Davis <eadavis@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)) {
>> 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 */
>
>It will get set to 1 right here ^^^
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
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH V3] jfs: Prevent copying of nlink with value 0 from disk inode
2025-02-22 0:16 ` Edward Adam Davis
@ 2025-02-24 14:56 ` Dave Kleikamp
0 siblings, 0 replies; 13+ messages in thread
From: Dave Kleikamp @ 2025-02-24 14:56 UTC (permalink / raw)
To: Edward Adam Davis
Cc: jfs-discussion, linux-kernel, syzbot+355da3b3a74881008e8f,
syzkaller-bugs
On 2/21/25 6:16PM, Edward Adam Davis wrote:
> On Thu, 20 Feb 2025 17:28:49 -0600, Dave Kleikamp wrote:
>> On 2/20/25 5:22PM, Edward Adam Davis wrote:
>>> On Thu, 20 Feb 2025 10:15:04 -0600, Dave Kleikamp 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().
>>>>>
>>>>> To avoid this, first check the nlink value of dinode before setting iag inode.
>>>>>
>>>>> [1]
>>>>> 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
>>>>
>>>> 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+355da3b3a74881008e8f@syzkaller.appspotmail.com
>>>>> Closes: https://syzkaller.appspot.com/bug?extid=355da3b3a74881008e8f
>>>>> Signed-off-by: Edward Adam Davis <eadavis@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)) {
>>> 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 */
>>
>> It will get set to 1 right here ^^^
> 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?
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
>
^ permalink raw reply [flat|nested] 13+ messages in thread
end of thread, other threads:[~2025-02-24 14:56 UTC | newest]
Thread overview: 13+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-11-21 12:18 [syzbot] [jfs?] possible deadlock in diFree (2) syzbot
2024-12-19 2:56 ` [syzbot] " syzbot
2024-12-19 8:24 ` syzbot
2024-12-19 8:52 ` syzbot
2024-12-21 7:33 ` [PATCH] jfs: Prevent setting of nlink with value 0 from disk inode Edward Adam Davis
2024-12-24 8:17 ` [PATCH V2] jfs: Prevent copying " Edward Adam Davis
2025-02-19 16:38 ` [Jfs-discussion] " Dave Kleikamp
2025-02-20 11:13 ` [PATCH V3] " Edward Adam Davis
2025-02-20 16:15 ` Dave Kleikamp
2025-02-20 23:22 ` Edward Adam Davis
2025-02-20 23:28 ` Dave Kleikamp
2025-02-22 0:16 ` Edward Adam Davis
2025-02-24 14:56 ` Dave Kleikamp
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.